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 |