Fix Last Posted Discussion, Improve DiscussionCount (#78)

The calculation/caching of the last posted discsussion is currently not very good. This PR adds fixes for:

Ensuring that private and hidden discussions aren't returned
Hiding and restoring discussions (hidden discussions shouldn't be returned)
Editing tags on a discussion (previously the discussion wasn't removed from the old tags).

extensions/tags/src/Listener/UpdateTagMetadata.php

@@ -9,12 +9,15 @@
 
 namespace Flarum\Tags\Listener;
 
+use Flarum\Discussion\Discussion;
 use Flarum\Discussion\Event\Deleted;
+use Flarum\Discussion\Event\Hidden;
+use Flarum\Discussion\Event\Restored;
 use Flarum\Discussion\Event\Started;
 use Flarum\Post\Event\Deleted as PostDeleted;
-use Flarum\Post\Event\Hidden;
+use Flarum\Post\Event\Hidden as PostHidden;
 use Flarum\Post\Event\Posted;
-use Flarum\Post\Event\Restored;
+use Flarum\Post\Event\Restored as PostRestored;
 use Flarum\Tags\Event\DiscussionWasTagged;
 use Flarum\Tags\Tag;
 use Illuminate\Contracts\Events\Dispatcher;
@@ -29,11 +32,13 @@ class UpdateTagMetadata
         $events->listen(Started::class, [$this, 'whenDiscussionIsStarted']);
         $events->listen(DiscussionWasTagged::class, [$this, 'whenDiscussionWasTagged']);
         $events->listen(Deleted::class, [$this, 'whenDiscussionIsDeleted']);
+        $events->listen(Hidden::class, [$this, 'whenDiscussionIsHidden']);
+        $events->listen(Restored::class, [$this, 'whenDiscussionIsRestored']);
 
         $events->listen(Posted::class, [$this, 'whenPostIsPosted']);
         $events->listen(PostDeleted::class, [$this, 'whenPostIsDeleted']);
-        $events->listen(Hidden::class, [$this, 'whenPostIsHidden']);
-        $events->listen(Restored::class, [$this, 'whenPostIsRestored']);
+        $events->listen(PostHidden::class, [$this, 'whenPostIsHidden']);
+        $events->listen(PostRestored::class, [$this, 'whenPostIsRestored']);
     }
 
     /**
@@ -49,7 +54,7 @@ class UpdateTagMetadata
      */
     public function whenDiscussionWasTagged(DiscussionWasTagged $event)
     {
-        $oldTags = Tag::whereIn('id', array_pluck($event->oldTags, 'id'));
+        $oldTags = Tag::whereIn('id', array_pluck($event->oldTags, 'id'))->get();
 
         $this->updateTags($event->discussion, -1, $oldTags);
         $this->updateTags($event->discussion, 1);
@@ -65,6 +70,22 @@ class UpdateTagMetadata
         $event->discussion->tags()->detach();
     }
 
+    /**
+     * @param Hidden $event
+     */
+    public function whenDiscussionIsHidden(Hidden $event)
+    {
+        $this->updateTags($event->discussion, -1);
+    }
+
+    /**
+     * @param Restored $event
+     */
+    public function whenDiscussionIsRestored(Restored $event)
+    {
+        $this->updateTags($event->discussion, 1);
+    }
+
     /**
      * @param Posted $event
      */
@@ -74,7 +95,7 @@ class UpdateTagMetadata
     }
 
     /**
-     * @param Deleted $event
+     * @param PostDeleted $event
      */
     public function whenPostIsDeleted(PostDeleted $event)
     {
@@ -82,47 +103,57 @@ class UpdateTagMetadata
     }
 
     /**
-     * @param Hidden $event
+     * @param PostHidden $event
      */
-    public function whenPostIsHidden(Hidden $event)
+    public function whenPostIsHidden(PostHidden $event)
     {
-        $this->updateTags($event->post->discussion);
+        $this->updateTags($event->post->discussion, 0, null, $event->post);
     }
 
     /**
-     * @param Restored $event
+     * @param PostRestored $event
      */
-    public function whenPostIsRestored(Restored $event)
+    public function whenPostIsRestored(PostRestored $event)
     {
-        $this->updateTags($event->post->discussion);
+        $this->updateTags($event->post->discussion, 0, null, $event->post);
     }
 
     /**
      * @param \Flarum\Discussion\Discussion $discussion
      * @param int $delta
      * @param Tag[]|null $tags
+     * @param Post $post: This is only used when a post has been hidden
      */
-    protected function updateTags($discussion, $delta = 0, $tags = null)
+    protected function updateTags(Discussion $discussion, $delta = 0, $tags = null, $post = null)
     {
-        if (! $discussion) {
-            return;
-        }
-
-        // We do not count private discussions in tags
-        if ($discussion->is_private) {
-            return;
-        }
-
         if (! $tags) {
             $tags = $discussion->tags;
         }
 
-        foreach ($tags as $tag) {
-            $tag->discussion_count += $delta;
+        // If we've just hidden or restored a post, the discussion's last posted at metadata might not have updated yet.
+        // Therefore, we must refresh the last post, even though that might be repeated in the future.
+        if ($post) {
+            $discussion->refreshLastPost();
+        }
 
-            if ($discussion->last_posted_at > $tag->last_posted_at) {
+        foreach ($tags as $tag) {
+            // We do not count private discussions or hidden discussions in tags
+            if (! $discussion->is_private) {
+                $tag->discussion_count += $delta;
+            }
+
+            // If this is a new / restored discussion, it isn't private, it isn't null,
+            // and it's more recent than what we have now, set it as last posted discussion.
+            if ($delta >= 0 && ! $discussion->is_private && $discussion->hidden_at == null && ($discussion->last_posted_at >= $tag->last_posted_at)) {
                 $tag->setLastPostedDiscussion($discussion);
             } elseif ($discussion->id == $tag->last_posted_discussion_id) {
+                // This is to persist refreshLastPost above. It is here instead of there so that
+                // if it's not necessary, we save a DB query.
+                if ($post) {
+                    $discussion->save();
+                }
+                // This discussion is currently the last posted discussion, but since it didn't qualify for the above check,
+                // it should not be the last posted discussion. Therefore, we should refresh the last posted discussion..
                 $tag->refreshLastPostedDiscussion();
             }
 

extensions/tags/src/Tag.php

@@ -110,18 +110,20 @@ class Tag extends AbstractModel
 
     public function refreshLastPostedDiscussion()
     {
-        if ($lastPostedDiscussion = $this->discussions()->latest('last_posted_at')->first()) {
+        if ($lastPostedDiscussion = $this->discussions()->where('is_private', false)->whereNull('hidden_at')->latest('last_posted_at')->first()) {
             $this->setLastPostedDiscussion($lastPostedDiscussion);
+        } else {
+            $this->setLastPostedDiscussion(null);
         }
 
         return $this;
     }
 
-    public function setLastPostedDiscussion(Discussion $discussion)
+    public function setLastPostedDiscussion(Discussion $discussion = null)
     {
-        $this->last_posted_at = $discussion->last_posted_at;
-        $this->last_posted_discussion_id = $discussion->id;
-        $this->last_posted_user_id = $discussion->last_posted_user_id;
+        $this->last_posted_at = optional($discussion)->last_posted_at;
+        $this->last_posted_discussion_id = optional($discussion)->id;
+        $this->last_posted_user_id = optional($discussion)->last_posted_user_id;
 
         return $this;
     }