BUG Prevent non-real draft changes forcing record to appear as changed when saved

code/extensions/SiteTreeContentReview.php

@@ -120,24 +120,24 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
     {
         if ($this->canBeReviewedBy(Member::currentUser())) {
             Requirements::css("contentreview/css/contentreview.css");
-            
+
             $reviewTitle = LiteralField::create(
                 "ReviewContentNotesLabel",
                 "<label class=\"left\" for=\"Form_EditForm_ReviewNotes\">" . _t("ContentReview.CONTENTREVIEW", "Content due for review") . "</label>"
             );
-            
+
             $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>");
-                                
+
             $quickReviewAction = FormAction::create("savereview", _t("ContentReview.MARKREVIEWED", "Mark as reviewed"))
                 ->setAttribute("data-icon", "pencil")
                 ->setAttribute("data-text-alternate", _t("ContentReview.MARKREVIEWED", "Mark as reviewed"));
-            
+
             $allFields = CompositeField::create($reviewTitle, $ReviewNotes, $quickReviewAction)
                 ->addExtraClass('review-notes field');
-            
+
             $reviewTab = Tab::create('ReviewContent', $allFields);
             $reviewTab->addExtraClass('contentreview-tab');
-            
+
             $actions->fieldByName('ActionMenus')->insertBefore($reviewTab, 'MoreOptions');
         }
     }
@@ -257,7 +257,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
         $member = Member::currentUser();
 
         if ($member) {
-            return $member->FirstName . " " . $member->Surname;
+            return $member->getTitle();
         }
 
         return null;
@@ -478,8 +478,12 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
      */
     public function onBeforeWrite()
     {
-        $this->owner->LastEditedByName = $this->owner->getEditorName();
+        // Only update if DB fields have been changed
-        $this->owner->OwnerNames = $this->owner->getOwnerNames();
+        $changedFields = $this->owner->getChangedFields(true, 2);
+        if($changedFields) {
+            $this->owner->LastEditedByName = $this->owner->getEditorName();
+            $this->owner->OwnerNames = $this->owner->getOwnerNames();
+        }
 
         // If the user changed the type, we need to recalculate the review date.
         if ($this->owner->isChanged("ContentReviewType", 2)) {
@@ -504,7 +508,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
             return;
         }
 
-        // parent page change it's review period 
+        // parent page change its review period
         // && !$this->owner->isChanged('ContentReviewType', 2)
         if ($this->owner->isChanged("ReviewPeriodDays", 2)) {
             $nextReviewUnixSec = strtotime(" + " . $this->owner->ReviewPeriodDays . " days", SS_Datetime::now()->format("U"));

tests/SiteTreeContentReviewTest.php

@@ -244,6 +244,38 @@ class SiteTreeContentReviewTest extends ContentReviewBaseTest
         SS_Datetime::clear_mock_now();
     }
 
+    public function testUnModifiedPagesDontChangeEditor() {
+        SS_Datetime::set_mock_now("2013-03-01 12:00:00");
+
+        /** @var Member $author */
+        $author = $this->objFromFixture("Member", "author");
+        $this->logInAs($author);
+
+        // Page which is un-modified doesn't advance version of have an editor assigned
+        $contactPage = $this->objFromFixture("Page", "contact");
+        $contactPageVersion = $contactPage->Version;
+        $contactPage->write();
+        $this->assertEmpty($contactPage->LastEditedByName);
+        $this->assertEquals(
+            $contactPageVersion,
+            Versioned::get_versionnumber_by_stage('SiteTree', 'Stage', $contactPage->ID, false)
+        );
+
+        // Page with modifications gets marked
+        $homePage = $this->objFromFixture("Page", "home");
+        $homePageVersion = $homePage->Version;
+        $homePage->Content = '<p>Welcome!</p>';
+        $homePage->write();
+        $this->assertNotEmpty($homePage->LastEditedByName);
+        $this->assertEquals($author->getTitle(), $homePage->LastEditedByName);
+        $this->assertGreaterThan(
+            $homePageVersion,
+            Versioned::get_versionnumber_by_stage('SiteTree', 'Stage', $homePage->ID, false)
+        );
+
+        SS_Datetime::clear_mock_now();
+    }
+
     public function testReviewActionVisibleForAuthor()
     {
         SS_Datetime::set_mock_now("2020-03-01 12:00:00");