Merge pull request #20 from creative-commoners/pulls/2.1/nullable-ori…

…ginal FIX Handle nullable $original object argument in onAfterPublish
silverstripe · Mar 25, 2018 · d632113 · d632113
2 parents 7352502 + 7e9f6ce
commit d632113
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/code/AuditHook.php b/code/AuditHook.php
@@ -200,29 +200,24 @@ public static function handle_manipulation($manipulation)
      */
     public function onAfterPublish(&$original)
     {
-        $member = Security::getcurrentUser();
+        $member = Security::getCurrentUser();
         if (!$member || !$member->exists()) {
             return false;
         }
 
         $effectiveViewerGroups = '';
         if ($this->owner->CanViewType === 'OnlyTheseUsers') {
-            $effectiveViewerGroups = implode(
-                ', ',
-                array_values($original->ViewerGroups()->map('ID', 'Title')->toArray())
-            );
+            $originalViewerGroups = $original ? $original->ViewerGroups()->map('ID', 'Title')->toArray() : [];
+            $effectiveViewerGroups = implode(', ', array_values($originalViewerGroups));
         }
         if (!$effectiveViewerGroups) {
             $effectiveViewerGroups = $this->owner->CanViewType;
         }
 
         $effectiveEditorGroups = '';
-        if ($this->owner->CanEditType === 'OnlyTheseUsers' && $original->EditorGroups()->exists()) {
-            $groups = [];
-            foreach ($original->EditorGroups() as $group) {
-                $groups[$group->ID] = $group->Title;
-            }
-            $effectiveEditorGroups = implode(', ', array_values($groups));
+        if ($this->owner->CanEditType === 'OnlyTheseUsers') {
+            $originalEditorGroups =  $original ? $original->EditorGroups()->map('ID', 'Title')->toArray() : [];
+            $effectiveEditorGroups = implode(', ', array_values($originalEditorGroups));
         }
         if (!$effectiveEditorGroups) {
             $effectiveEditorGroups = $this->owner->CanEditType;

diff --git a/tests/AuditHookTest.php b/tests/AuditHookTest.php
@@ -159,6 +159,29 @@ public function testRemoveMemberFromGroupUsingMemberGroupsRelation()
         $this->assertContains('from Group "My group"', $message);
     }
 
+    public function testAddViewerGroupToPage()
+    {
+        if (!class_exists(Page::class)) {
+            $this->markTestSkipped('This test requires the CMS module installed.');
+        }
+
+        $this->logInWithPermission('ADMIN');
+
+        $group = new Group();
+        $group->Title = 'Test group';
+        $group->write();
+
+        $page = new Page();
+        $page->CanViewType = 'OnlyTheseUsers';
+        $page->ViewerGroups()->add($group);
+        $page->write();
+        $page->publishSingle();
+
+        $message = $this->writer->getLastMessage();
+        $this->assertContains('Effective ViewerGroups', $message);
+        $this->assertContains('OnlyTheseUsers', $message);
+    }
+
     public function testPublishPage()
     {
         if (!class_exists(Page::class)) {