silverstripe /
silverstripe-contentreview
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 SiteTreeContentReview 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 SiteTreeContentReview, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 17 | class SiteTreeContentReview extends DataExtension implements PermissionProvider |
||
|
|
|||
| 18 | { |
||
| 19 | /** |
||
| 20 | * @var array |
||
| 21 | */ |
||
| 22 | private static $db = array( |
||
| 23 | "ContentReviewType" => "Enum('Inherit, Disabled, Custom', 'Inherit')", |
||
| 24 | "ReviewPeriodDays" => "Int", |
||
| 25 | "NextReviewDate" => "Date", |
||
| 26 | "LastEditedByName" => "Varchar(255)", |
||
| 27 | "OwnerNames" => "Varchar(255)", |
||
| 28 | ); |
||
| 29 | |||
| 30 | /** |
||
| 31 | * @var array |
||
| 32 | */ |
||
| 33 | private static $defaults = array( |
||
| 34 | "ContentReviewType" => "Inherit", |
||
| 35 | ); |
||
| 36 | |||
| 37 | /** |
||
| 38 | * @var array |
||
| 39 | */ |
||
| 40 | private static $has_many = array( |
||
| 41 | "ReviewLogs" => "ContentReviewLog", |
||
| 42 | ); |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var array |
||
| 46 | */ |
||
| 47 | private static $belongs_many_many = array( |
||
| 48 | "ContentReviewGroups" => "Group", |
||
| 49 | "ContentReviewUsers" => "Member", |
||
| 50 | ); |
||
| 51 | |||
| 52 | /** |
||
| 53 | * @var array |
||
| 54 | */ |
||
| 55 | private static $schedule = array( |
||
| 56 | 0 => "No automatic review date", |
||
| 57 | 1 => "1 day", |
||
| 58 | 7 => "1 week", |
||
| 59 | 30 => "1 month", |
||
| 60 | 60 => "2 months", |
||
| 61 | 91 => "3 months", |
||
| 62 | 121 => "4 months", |
||
| 63 | 152 => "5 months", |
||
| 64 | 183 => "6 months", |
||
| 65 | 365 => "12 months", |
||
| 66 | ); |
||
| 67 | |||
| 68 | /** |
||
| 69 | * @return array |
||
| 70 | */ |
||
| 71 | public static function get_schedule() |
||
| 75 | |||
| 76 | /** |
||
| 77 | * Takes a list of groups and members and return a list of unique member. |
||
| 78 | * |
||
| 79 | * @param SS_List $groups |
||
| 80 | * @param SS_List $members |
||
| 81 | * |
||
| 82 | * @return ArrayList |
||
| 83 | */ |
||
| 84 | public static function merge_owners(SS_List $groups, SS_List $members) |
||
| 85 | { |
||
| 86 | $contentReviewOwners = new ArrayList(); |
||
| 87 | |||
| 88 | if ($groups->count()) { |
||
| 89 | $groupIDs = array(); |
||
| 90 | |||
| 91 | foreach ($groups as $group) { |
||
| 92 | $familyIDs = $group->collateFamilyIDs(); |
||
| 93 | |||
| 94 | if (is_array($familyIDs)) { |
||
| 95 | $groupIDs = array_merge($groupIDs, array_values($familyIDs)); |
||
| 96 | } |
||
| 97 | } |
||
| 98 | |||
| 99 | array_unique($groupIDs); |
||
| 100 | |||
| 101 | if (count($groupIDs)) { |
||
| 102 | $groupMembers = DataObject::get("Member")->where("\"Group\".\"ID\" IN (" . implode(",", $groupIDs) . ")") |
||
| 103 | ->leftJoin("Group_Members", "\"Member\".\"ID\" = \"Group_Members\".\"MemberID\"") |
||
| 104 | ->leftJoin("Group", "\"Group_Members\".\"GroupID\" = \"Group\".\"ID\""); |
||
| 105 | |||
| 106 | $contentReviewOwners->merge($groupMembers); |
||
| 107 | } |
||
| 108 | } |
||
| 109 | |||
| 110 | $contentReviewOwners->merge($members); |
||
| 111 | $contentReviewOwners->removeDuplicates(); |
||
| 112 | |||
| 113 | return $contentReviewOwners; |
||
| 114 | } |
||
| 115 | |||
| 116 | /** |
||
| 117 | * @param FieldList $actions |
||
| 118 | */ |
||
| 119 | public function updateCMSActions(FieldList $actions) |
||
| 120 | { |
||
| 121 | if ($this->canBeReviewedBy(Member::currentUser())) { |
||
| 122 | Requirements::css("contentreview/css/contentreview.css"); |
||
| 123 | |||
| 124 | $reviewTitle = LiteralField::create( |
||
| 125 | "ReviewContentNotesLabel", |
||
| 126 | "<label class=\"left\" for=\"Form_EditForm_ReviewNotes\">" . _t("ContentReview.CONTENTREVIEW", "Content due for review") . "</label>" |
||
| 127 | ); |
||
| 128 | |||
| 129 | $ReviewNotes = LiteralField::create("ReviewNotes", "<textarea class=\"no-change-track\" id=\"Form_EditForm_ReviewNotes\" name=\"ReviewNotes\" placeholder=\"" . _t("ContentReview.COMMENTS", "(optional) Add comments...") . "\" class=\"text\"></textarea>"); |
||
| 130 | |||
| 131 | $quickReviewAction = FormAction::create("savereview", _t("ContentReview.MARKREVIEWED", "Mark as reviewed")) |
||
| 132 | ->setAttribute("data-icon", "pencil") |
||
| 133 | ->setAttribute("data-text-alternate", _t("ContentReview.MARKREVIEWED", "Mark as reviewed")); |
||
| 134 | |||
| 135 | $allFields = CompositeField::create($reviewTitle, $ReviewNotes, $quickReviewAction) |
||
| 136 | ->addExtraClass('review-notes field'); |
||
| 137 | |||
| 138 | $reviewTab = Tab::create('ReviewContent', $allFields); |
||
| 139 | $reviewTab->addExtraClass('contentreview-tab'); |
||
| 140 | |||
| 141 | $actions->fieldByName('ActionMenus')->insertBefore($reviewTab, 'MoreOptions'); |
||
| 142 | } |
||
| 143 | } |
||
| 144 | |||
| 145 | /** |
||
| 146 | * Returns false if the content review have disabled. |
||
| 147 | * |
||
| 148 | * @param SiteTree $page |
||
| 149 | * |
||
| 150 | * @return bool|Date |
||
| 151 | */ |
||
| 152 | public function getReviewDate(SiteTree $page = null) |
||
| 153 | { |
||
| 154 | if ($page === null) { |
||
| 155 | $page = $this->owner; |
||
| 156 | } |
||
| 157 | |||
| 158 | if ($page->obj("NextReviewDate")->exists()) { |
||
| 159 | return $page->obj("NextReviewDate"); |
||
| 160 | } |
||
| 161 | |||
| 162 | $options = $this->owner->getOptions(); |
||
| 163 | |||
| 164 | if (!$options) { |
||
| 165 | return false; |
||
| 166 | } |
||
| 167 | |||
| 168 | if (!$options->ReviewPeriodDays) { |
||
| 169 | return false; |
||
| 170 | } |
||
| 171 | |||
| 172 | // Failover to check on ReviewPeriodDays + LastEdited |
||
| 173 | $nextReviewUnixSec = strtotime(" + " . $options->ReviewPeriodDays . " days", SS_Datetime::now()->format("U")); |
||
| 174 | $date = Date::create("NextReviewDate"); |
||
| 175 | $date->setValue(date("Y-m-d H:i:s", $nextReviewUnixSec)); |
||
| 176 | |||
| 177 | return $date; |
||
| 178 | } |
||
| 179 | |||
| 180 | /** |
||
| 181 | * Get the object that have the information about the content review settings. Either: |
||
| 182 | * |
||
| 183 | * - a SiteTreeContentReview decorated object |
||
| 184 | * - the default SiteTree config |
||
| 185 | * - false if this page have it's content review disabled |
||
| 186 | * |
||
| 187 | * Will go through parents and root pages will use the site config if their setting is Inherit. |
||
| 188 | * |
||
| 189 | * @return bool|DataObject |
||
| 190 | * |
||
| 191 | * @throws Exception |
||
| 192 | */ |
||
| 193 | public function getOptions() |
||
| 194 | { |
||
| 195 | if ($this->owner->ContentReviewType == "Custom") { |
||
| 196 | return $this->owner; |
||
| 197 | } |
||
| 198 | |||
| 199 | if ($this->owner->ContentReviewType == "Disabled") { |
||
| 200 | return false; |
||
| 201 | } |
||
| 202 | |||
| 203 | $page = $this->owner; |
||
| 204 | |||
| 205 | // $page is inheriting it's settings from it's parent, find |
||
| 206 | // the first valid parent with a valid setting |
||
| 207 | while ($parent = $page->Parent()) { |
||
| 208 | |||
| 209 | // Root page, use site config |
||
| 210 | if (!$parent->exists()) { |
||
| 211 | return SiteConfig::current_site_config(); |
||
| 212 | } |
||
| 213 | |||
| 214 | if ($parent->ContentReviewType == "Custom") { |
||
| 215 | return $parent; |
||
| 216 | } |
||
| 217 | |||
| 218 | if ($parent->ContentReviewType == "Disabled") { |
||
| 219 | return false; |
||
| 220 | } |
||
| 221 | |||
| 222 | $page = $parent; |
||
| 223 | } |
||
| 224 | |||
| 225 | throw new Exception("This shouldn't really happen, as per usual developer logic."); |
||
| 226 | } |
||
| 227 | |||
| 228 | /** |
||
| 229 | * @return string |
||
| 230 | */ |
||
| 231 | public function getOwnerNames() |
||
| 251 | |||
| 252 | /** |
||
| 253 | * @return null|string |
||
| 254 | */ |
||
| 255 | public function getEditorName() |
||
| 256 | { |
||
| 257 | $member = Member::currentUser(); |
||
| 258 | |||
| 259 | if ($member) { |
||
| 260 | return $member->FirstName . " " . $member->Surname; |
||
| 261 | } |
||
| 262 | |||
| 263 | return null; |
||
| 264 | } |
||
| 265 | |||
| 266 | /** |
||
| 267 | * Get all Members that are Content Owners to this page. This includes checking group |
||
| 268 | * hierarchy and adding any direct users. |
||
| 269 | * |
||
| 270 | * @return ArrayList |
||
| 271 | */ |
||
| 272 | public function ContentReviewOwners() |
||
| 273 | { |
||
| 274 | return SiteTreeContentReview::merge_owners( |
||
| 275 | $this->OwnerGroups(), |
||
| 276 | $this->OwnerUsers() |
||
| 277 | ); |
||
| 278 | } |
||
| 279 | |||
| 280 | /** |
||
| 281 | * @return ManyManyList |
||
| 282 | */ |
||
| 283 | public function OwnerGroups() |
||
| 287 | |||
| 288 | /** |
||
| 289 | * @return ManyManyList |
||
| 290 | */ |
||
| 291 | public function OwnerUsers() |
||
| 295 | |||
| 296 | /** |
||
| 297 | * @param FieldList $fields |
||
| 298 | */ |
||
| 299 | public function updateSettingsFields(FieldList $fields) |
||
| 300 | { |
||
| 301 | Requirements::javascript("contentreview/javascript/contentreview.js"); |
||
| 401 | |||
| 402 | /** |
||
| 403 | * Creates a ContentReviewLog and connects it to this Page. |
||
| 404 | * |
||
| 405 | * @param Member $reviewer |
||
| 406 | * @param string $message |
||
| 407 | */ |
||
| 408 | public function addReviewNote(Member $reviewer, $message) |
||
| 415 | |||
| 416 | /** |
||
| 417 | * Advance review date to the next date based on review period or set it to null |
||
| 418 | * if there is no schedule. Returns true if date was required and false is content |
||
| 419 | * review is 'off'. |
||
| 420 | * |
||
| 421 | * @return bool |
||
| 422 | */ |
||
| 423 | public function advanceReviewDate() |
||
| 440 | |||
| 441 | /** |
||
| 442 | * Check if a review is due by a member for this owner. |
||
| 443 | * |
||
| 444 | * @param Member $member |
||
| 445 | * |
||
| 446 | * @return bool |
||
| 447 | */ |
||
| 448 | public function canBeReviewedBy(Member $member = null) |
||
| 482 | |||
| 483 | /** |
||
| 484 | * Set the review data from the review period, if set. |
||
| 485 | */ |
||
| 486 | public function onBeforeWrite() |
||
| 521 | |||
| 522 | private function setDefaultReviewDateForDisabled() |
||
| 526 | |||
| 527 | protected function setDefaultReviewDateForCustom() |
||
| 543 | |||
| 544 | protected function setDefaultReviewDateForInherited() |
||
| 566 | |||
| 567 | /** |
||
| 568 | * Provide permissions to the CMS. |
||
| 569 | * |
||
| 570 | * @return array |
||
| 571 | */ |
||
| 572 | public function providePermissions() |
||
| 582 | |||
| 583 | /** |
||
| 584 | * If the queued jobs module is installed, queue up the first job for 9am tomorrow morning |
||
| 585 | * (by default). |
||
| 586 | */ |
||
| 587 | public function requireDefaultRecords() |
||
| 607 | } |
||
| 608 |
Loading history...
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.