Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like contentSystemAuthors often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use contentSystemAuthors, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class contentSystemAuthors extends AdministrationPage |
||
14 | { |
||
15 | public $_Author; |
||
16 | public $_errors = array(); |
||
17 | |||
18 | public function sort(&$sort, &$order, $params) |
||
30 | } |
||
31 | |||
32 | public function isRemoteLoginActionChecked() |
||
37 | } |
||
38 | |||
39 | public function __viewIndex() |
||
40 | { |
||
41 | $this->setPageType('table'); |
||
42 | $this->setTitle(__('%1$s – %2$s', array(__('Authors'), __('Symphony')))); |
||
43 | |||
44 | if (Symphony::Author()->isDeveloper() || Symphony::Author()->isManager()) { |
||
45 | $this->appendSubheading(__('Authors'), Widget::Anchor(__('Create New'), Administration::instance()->getCurrentPageURL().'new/', __('Create a new author'), 'create button', null, array('accesskey' => 'c'))); |
||
46 | } else { |
||
47 | $this->appendSubheading(__('Authors')); |
||
48 | } |
||
49 | |||
50 | Sortable::initialize($this, $authors, $sort, $order); |
||
51 | |||
52 | $columns = array( |
||
53 | array( |
||
54 | 'label' => __('Name'), |
||
55 | 'sortable' => true, |
||
56 | 'handle' => 'name' |
||
57 | ), |
||
58 | array( |
||
59 | 'label' => __('Email Address'), |
||
60 | 'sortable' => true, |
||
61 | 'handle' => 'email' |
||
62 | ), |
||
63 | array( |
||
64 | 'label' => __('Last Seen'), |
||
65 | 'sortable' => true, |
||
66 | 'handle' => 'last_seen' |
||
67 | ) |
||
68 | ); |
||
69 | |||
70 | if (Symphony::Author()->isDeveloper() || Symphony::Author()->isManager()) { |
||
71 | $columns = array_merge($columns, array( |
||
72 | array( |
||
73 | 'label' => __('User Type'), |
||
74 | 'sortable' => true, |
||
75 | 'handle' => 'user_type' |
||
76 | ), |
||
77 | array( |
||
78 | 'label' => __('Language'), |
||
79 | 'sortable' => true, |
||
80 | 'handle' => 'language' |
||
81 | ) |
||
82 | )); |
||
83 | } |
||
84 | |||
85 | /** |
||
86 | * Allows the creation of custom table columns for each author. Called |
||
87 | * after all the table headers columns have been added. |
||
88 | * |
||
89 | * @delegate AddCustomAuthorColumn |
||
90 | * @since Symphony 2.7.0 |
||
91 | * @param string $context |
||
92 | * '/system/authors/' |
||
93 | * @param array $columns |
||
94 | * An array of the current columns, passed by reference |
||
95 | * @param string $sort |
||
96 | * @since Symphony 3.0.0 |
||
97 | * The sort field |
||
98 | * @param string $order |
||
99 | * @since Symphony 3.0.0 |
||
100 | * The sort order |
||
101 | */ |
||
102 | Symphony::ExtensionManager()->notifyMembers('AddCustomAuthorColumn', '/system/authors/', array( |
||
103 | 'columns' => &$columns, |
||
104 | 'sort' => $sort, |
||
105 | 'order' => $order, |
||
106 | )); |
||
107 | |||
108 | $aTableHead = Sortable::buildTableHeaders($columns, $sort, $order, (isset($_REQUEST['filter']) ? '&filter=' . $_REQUEST['filter'] : '')); |
||
109 | |||
110 | $aTableBody = array(); |
||
111 | |||
112 | if (!is_array($authors) || empty($authors)) { |
||
113 | $aTableBody = array( |
||
114 | Widget::TableRow(array(Widget::TableData(__('None found.'), 'inactive', null, count($aTableHead))), 'odd') |
||
115 | ); |
||
116 | } else { |
||
117 | foreach ($authors as $a) { |
||
118 | // Setup each cell |
||
119 | if ( |
||
120 | (Symphony::Author()->isDeveloper() || (Symphony::Author()->isManager() && !$a->isDeveloper() && !$a->isManager())) |
||
121 | || Symphony::Author()->get('id') == $a->get('id') |
||
122 | ) { |
||
123 | $td1 = Widget::TableData( |
||
124 | Widget::Anchor($a->getFullName(), Administration::instance()->getCurrentPageURL() . 'edit/' . $a->get('id') . '/', $a->get('username'), 'author') |
||
125 | ); |
||
126 | } else { |
||
127 | $td1 = Widget::TableData($a->getFullName(), 'inactive'); |
||
128 | } |
||
129 | |||
130 | // Can this Author be edited by the current Author? |
||
131 | if (Symphony::Author()->isDeveloper() || Symphony::Author()->isManager()) { |
||
132 | if ($a->get('id') != Symphony::Author()->get('id')) { |
||
133 | $td1->appendChild(Widget::Label(__('Select Author %s', array($a->getFullName())), null, 'accessible', null, array( |
||
134 | 'for' => 'author-' . $a->get('id') |
||
135 | ))); |
||
136 | $td1->appendChild(Widget::Input('items['.$a->get('id').']', 'on', 'checkbox', array( |
||
137 | 'id' => 'author-' . $a->get('id') |
||
138 | ))); |
||
139 | } |
||
140 | } |
||
141 | |||
142 | $td2 = Widget::TableData(Widget::Anchor($a->get('email'), 'mailto:'.$a->get('email'), __('Email this author'))); |
||
143 | |||
144 | if (!is_null($a->get('last_seen'))) { |
||
145 | $td3 = Widget::TableData( |
||
146 | DateTimeObj::format($a->get('last_seen'), __SYM_DATETIME_FORMAT__) |
||
147 | ); |
||
148 | } else { |
||
149 | $td3 = Widget::TableData(__('Unknown'), 'inactive'); |
||
150 | } |
||
151 | |||
152 | if ($a->isDeveloper()) { |
||
153 | $type = 'Developer'; |
||
154 | } elseif ($a->isManager()) { |
||
155 | $type = 'Manager'; |
||
156 | } else { |
||
157 | $type = 'Author'; |
||
158 | } |
||
159 | |||
160 | $td4 = Widget::TableData(__($type)); |
||
161 | |||
162 | $languages = Lang::getAvailableLanguages(); |
||
163 | |||
164 | $td5 = Widget::TableData($a->get("language") == null ? __("System Default") : $languages[$a->get("language")]); |
||
165 | |||
166 | $tableData = array(); |
||
167 | // Add a row to the body array, assigning each cell to the row |
||
168 | if (Symphony::Author()->isDeveloper() || Symphony::Author()->isManager()) { |
||
169 | $tableData = array($td1, $td2, $td3, $td4, $td5); |
||
170 | } else { |
||
171 | $tableData = array($td1, $td2, $td3); |
||
172 | } |
||
173 | |||
174 | /** |
||
175 | * Allows Extensions to inject custom table data for each Author |
||
176 | * into the Authors Index |
||
177 | * |
||
178 | * @delegate AddCustomAuthorColumnData |
||
179 | * @since Symphony 2.7.0 |
||
180 | * @param string $context |
||
181 | * '/system/authors/' |
||
182 | * @param array $tableData |
||
183 | * An array of `Widget::TableData`, passed by reference |
||
184 | * @param array $columns |
||
185 | * An array of the current columns |
||
186 | * @param Author $author |
||
187 | * The Author object. |
||
188 | */ |
||
189 | Symphony::ExtensionManager()->notifyMembers('AddCustomAuthorColumnData', '/system/authors/', array( |
||
190 | 'tableData' => &$tableData, |
||
191 | 'columns' => $columns, |
||
192 | 'author' => $a, |
||
193 | )); |
||
194 | |||
195 | $aTableBody[] = Widget::TableRow($tableData); |
||
196 | } |
||
197 | } |
||
198 | |||
199 | $table = Widget::Table( |
||
200 | Widget::TableHead($aTableHead), |
||
201 | null, |
||
202 | Widget::TableBody($aTableBody), |
||
203 | 'selectable', |
||
204 | null, |
||
205 | array('role' => 'directory', 'aria-labelledby' => 'symphony-subheading') |
||
206 | ); |
||
207 | |||
208 | $this->Form->appendChild($table); |
||
209 | |||
210 | $version = new XMLElement('p', 'Symphony ' . Symphony::Configuration()->get('version', 'symphony'), array( |
||
211 | 'id' => 'version' |
||
212 | )); |
||
213 | |||
214 | $this->Form->appendChild($version); |
||
215 | } |
||
216 | |||
217 | // Both the Edit and New pages need the same form |
||
218 | public function __viewNew() |
||
219 | { |
||
220 | $this->__form(); |
||
221 | } |
||
222 | |||
223 | public function __viewEdit() |
||
224 | { |
||
225 | $this->__form(); |
||
226 | } |
||
227 | |||
228 | public function __form() |
||
594 | )); |
||
595 | } |
||
596 | |||
597 | public function __actionNew() |
||
700 | ); |
||
701 | } |
||
702 | } |
||
703 | } |
||
704 | |||
705 | public function __actionEdit() |
||
706 | { |
||
707 | if (!$author_id = (int)$this->_context[1]) { |
||
708 | redirect(SYMPHONY_URL . '/system/authors/'); |
||
709 | } |
||
710 | |||
711 | $isOwner = ($author_id == Symphony::Author()->get('id')); |
||
712 | $fields = $_POST['fields']; |
||
713 | $this->_Author = AuthorManager::fetchByID($author_id); |
||
714 | |||
715 | $canEdit = // Managers can edit all Authors, and their own. |
||
716 | (Symphony::Author()->isManager() && $this->_Author->isAuthor()) |
||
717 | // Primary account can edit all accounts. |
||
718 | || Symphony::Author()->isPrimaryAccount() |
||
719 | // Developers can edit all developers, managers and authors, and their own, |
||
720 | // but not the primary account |
||
721 | || (Symphony::Author()->isDeveloper() && $this->_Author->isPrimaryAccount() === false); |
||
722 | |||
723 | if (!$isOwner && !$canEdit) { |
||
724 | Administration::instance()->throwCustomError( |
||
725 | __('You are not authorised to modify this author.'), |
||
726 | __('Access Denied'), |
||
727 | Page::HTTP_STATUS_UNAUTHORIZED |
||
728 | ); |
||
729 | } |
||
730 | |||
731 | if (is_array($_POST['action']) && array_key_exists('save', $_POST['action'])) { |
||
732 | $authenticated = $changing_password = $changing_email = false; |
||
733 | |||
734 | if ($fields['email'] != $this->_Author->get('email')) { |
||
735 | $changing_email = true; |
||
736 | } |
||
737 | |||
738 | // Check the old password was correct |
||
739 | if (isset($fields['old-password']) && strlen(trim($fields['old-password'])) > 0 && Cryptography::compare(trim($fields['old-password']), $this->_Author->get('password'))) { |
||
740 | $authenticated = true; |
||
741 | |||
742 | // Developers don't need to specify the old password, unless it's their own account |
||
743 | } elseif ( |
||
744 | // All accounts can edit their own |
||
745 | $isOwner || |
||
746 | // Is allowed to edit? |
||
747 | $canEdit |
||
748 | ) { |
||
749 | $authenticated = true; |
||
750 | } |
||
751 | |||
752 | $this->_Author->set('id', $author_id); |
||
753 | |||
754 | if ($this->_Author->isPrimaryAccount() || ($isOwner && Symphony::Author()->isDeveloper())) { |
||
755 | $this->_Author->set('user_type', 'developer'); // Primary accounts are always developer, Developers can't lower their level |
||
756 | } elseif (Symphony::Author()->isManager() && isset($fields['user_type'])) { // Manager can only change user type for author and managers |
||
757 | if ($fields['user_type'] !== 'author' && $fields['user_type'] !== 'manager') { |
||
758 | $this->_errors['user_type'] = __('The user type is invalid. You can only create Authors.'); |
||
759 | } else { |
||
760 | $this->_Author->set('user_type', $fields['user_type']); |
||
761 | } |
||
762 | } elseif (Symphony::Author()->isDeveloper() && isset($fields['user_type'])) { |
||
763 | $this->_Author->set('user_type', $fields['user_type']); // Only developer can change user type |
||
764 | } |
||
765 | |||
766 | $this->_Author->set('email', $fields['email']); |
||
767 | $this->_Author->set('username', General::sanitize($fields['username'])); |
||
768 | $this->_Author->set('first_name', General::sanitize($fields['first_name'])); |
||
769 | $this->_Author->set('last_name', General::sanitize($fields['last_name'])); |
||
770 | $this->_Author->set('language', isset($fields['language']) ? $fields['language'] : null); |
||
771 | |||
772 | if (trim($fields['password']) != '') { |
||
773 | $this->_Author->set('password', Cryptography::hash($fields['password'])); |
||
774 | $changing_password = true; |
||
775 | } |
||
776 | |||
777 | // Don't allow authors to set the Section Index as a default area |
||
778 | // If they had it previously set, just save `null` which will redirect |
||
779 | // the Author (when logging in) to their own Author record |
||
780 | if ( |
||
781 | $this->_Author->get('user_type') == 'author' |
||
782 | && $fields['default_area'] == '/blueprints/sections/' |
||
783 | ) { |
||
784 | $this->_Author->set('default_area', null); |
||
785 | } else { |
||
786 | $this->_Author->set('default_area', $fields['default_area']); |
||
787 | } |
||
788 | |||
789 | if ($authenticated && $this->isRemoteLoginActionChecked() && !$this->_Author->isTokenActive()) { |
||
790 | $this->_Author->set('auth_token', Cryptography::randomBytes()); |
||
791 | } elseif (!$this->isRemoteLoginActionChecked()) { |
||
792 | $this->_Author->set('auth_token', null); |
||
793 | } |
||
794 | |||
795 | /** |
||
796 | * Before editing an author, provided with the Author object |
||
797 | * |
||
798 | * @delegate AuthorPreEdit |
||
799 | * @since Symphony 2.7.0 |
||
800 | * @param string $context |
||
801 | * '/system/authors/' |
||
802 | * @param Author $author |
||
803 | * An Author object not yet committed, nor validated |
||
804 | * @param array $fields |
||
805 | * The POST fields |
||
806 | * @param array $errors |
||
807 | * The error array used to validate the Author, passed by reference. |
||
808 | * Extension should append to this array if they detect validation problems. |
||
809 | * @param bool $changing_email |
||
810 | * @since Symphony 3.0.0 |
||
811 | * The changing email flag, so extension can act only if the email changes. |
||
812 | * @param bool $changing_password |
||
813 | * @since Symphony 3.0.0 |
||
814 | * The changing password flag, so extension can act only if the password changes. |
||
815 | */ |
||
816 | Symphony::ExtensionManager()->notifyMembers('AuthorPreEdit', '/system/authors/', array( |
||
817 | 'author' => $this->_Author, |
||
818 | 'field' => $fields, |
||
819 | 'errors' => &$this->_errors, |
||
820 | 'changing_email' => $changing_email, |
||
821 | 'changing_password' => $changing_password, |
||
822 | )); |
||
823 | |||
824 | if (empty($this->_errors) && $this->_Author->validate($this->_errors)) { |
||
825 | // Admin changing another profile |
||
826 | if (!$isOwner) { |
||
827 | $entered_password = $fields['confirm-change-password']; |
||
828 | |||
829 | if (!isset($fields['confirm-change-password']) || empty($fields['confirm-change-password'])) { |
||
830 | $this->_errors['confirm-change-password'] = __('Please provide your own password to make changes to this author.'); |
||
831 | } elseif (Cryptography::compare($entered_password, Symphony::Author()->get('password')) !== true) { |
||
832 | $this->_errors['confirm-change-password'] = __('Wrong password, please enter your own password to make changes to this author.'); |
||
833 | } |
||
834 | } |
||
835 | |||
836 | // Author is changing their password |
||
837 | if (!$authenticated && ($changing_password || $changing_email)) { |
||
838 | if ($changing_password) { |
||
839 | $this->_errors['old-password'] = __('Wrong password. Enter old password to change it.'); |
||
840 | } elseif ($changing_email) { |
||
841 | $this->_errors['old-password'] = __('Wrong password. Enter old one to change email address.'); |
||
842 | } |
||
843 | |||
844 | // Passwords provided, but doesn't match. |
||
845 | } elseif (($fields['password'] != '' || $fields['password-confirmation'] != '') && $fields['password'] != $fields['password-confirmation']) { |
||
846 | $this->_errors['password'] = $this->_errors['password-confirmation'] = __('Passwords did not match'); |
||
847 | } |
||
848 | |||
849 | // All good, let's save the Author |
||
850 | if (is_array($this->_errors) && empty($this->_errors) && $this->_Author->commit()) { |
||
851 | Symphony::Database() |
||
852 | ->delete('tbl_forgotpass') |
||
853 | ->where(['or' => [ |
||
854 | 'expiry' => ['<' => DateTimeObj::getGMT('c')], |
||
855 | 'author_id' => $author_id, |
||
856 | ]]) |
||
857 | ->execute(); |
||
858 | |||
859 | if ($isOwner) { |
||
860 | Administration::instance()->login($this->_Author->get('username'), $this->_Author->get('password'), true); |
||
861 | } |
||
862 | |||
863 | /** |
||
864 | * After editing an author, provided with the Author object |
||
865 | * |
||
866 | * @delegate AuthorPostEdit |
||
867 | * @since Symphony 2.2 |
||
868 | * @param string $context |
||
869 | * '/system/authors/' |
||
870 | * @param Author $author |
||
871 | * An Author object |
||
872 | * @param array $fields |
||
873 | * The POST fields |
||
874 | * This parameter is available @since Symphony 2.7.0 |
||
875 | * @param array $errors |
||
876 | * The error array used to validate the Author, passed by reference. |
||
877 | * Extension should append to this array if they detect saving problems. |
||
878 | * This parameter is available @since Symphony 2.7.0 |
||
879 | * @param bool $changing_email |
||
880 | * @since Symphony 3.0.0 |
||
881 | * The changing email flag, so extension can act only if the email changes. |
||
882 | * @param bool $changing_password |
||
883 | * @since Symphony 3.0.0 |
||
884 | * The changing password flag, so extension can act only if the password changes. |
||
885 | */ |
||
886 | Symphony::ExtensionManager()->notifyMembers('AuthorPostEdit', '/system/authors/', array( |
||
887 | 'author' => $this->_Author, |
||
888 | 'field' => $fields, |
||
889 | 'errors' => &$this->_errors, |
||
890 | 'changing_email' => $changing_email, |
||
891 | 'changing_password' => $changing_password, |
||
892 | )); |
||
893 | |||
894 | if (empty($this->_errors)) { |
||
895 | redirect(SYMPHONY_URL . '/system/authors/edit/' . $author_id . '/saved/'); |
||
896 | } |
||
897 | |||
898 | // Problems. |
||
899 | } else { |
||
900 | $this->pageAlert( |
||
901 | __('Unknown errors occurred while attempting to save.') |
||
902 | . '<a href="' . SYMPHONY_URL . '/system/log/">' |
||
903 | . __('Check your activity log') |
||
904 | . '</a>.', |
||
905 | Alert::ERROR |
||
906 | ); |
||
907 | } |
||
908 | } |
||
909 | |||
910 | // Author doesn't have valid data, throw back. |
||
911 | if (is_array($this->_errors) && !empty($this->_errors)) { |
||
912 | $this->pageAlert(__('There were some problems while attempting to save. Please check below for problem fields.'), Alert::ERROR); |
||
913 | } |
||
914 | } elseif (is_array($_POST['action']) && array_key_exists('delete', $_POST['action'])) { |
||
915 | // Validate rights |
||
916 | if (!$canEdit) { |
||
917 | $this->pageAlert(__('You are not allowed to delete this author.'), Alert::ERROR); |
||
918 | return; |
||
919 | } |
||
920 | // Admin changing another profile |
||
921 | if (!$isOwner) { |
||
922 | $entered_password = $fields['confirm-change-password']; |
||
923 | |||
924 | if (!isset($fields['confirm-change-password']) || empty($fields['confirm-change-password'])) { |
||
925 | $this->_errors['confirm-change-password'] = __('Please provide your own password to make changes to this author.'); |
||
926 | } elseif (Cryptography::compare($entered_password, Symphony::Author()->get('password')) !== true) { |
||
927 | $this->_errors['confirm-change-password'] = __('Wrong password, please enter your own password to make changes to this author.'); |
||
928 | } |
||
929 | } |
||
930 | if (is_array($this->_errors) && !empty($this->_errors)) { |
||
931 | $this->pageAlert(__('There were some problems while attempting to save. Please check below for problem fields.'), Alert::ERROR); |
||
985 |