[COMPONENT][Conversation] Refactor and fix Conversation component

components/Conversation/Controller/Conversation.php

@@ -27,11 +27,17 @@ declare(strict_types = 1);
 
 namespace Component\Conversation\Controller;
 
+use App\Core\Cache;
 use App\Core\DB\DB;
 use App\Core\Form;
 use function App\Core\I18n\_m;
+use App\Entity\Note;
 use App\Util\Common;
+use App\Util\Exception\ClientException;
+use App\Util\Exception\NoLoggedInUser;
+use App\Util\Exception\NoSuchNoteException;
 use App\Util\Exception\RedirectException;
+use App\Util\Exception\ServerException;
 use Component\Collection\Util\Controller\FeedController;
 use Component\Conversation\Entity\ConversationMute;
 use Symfony\Component\Form\Extension\Core\Type\SubmitType;
@@ -50,17 +56,35 @@ class Conversation extends FeedController
      */
     public function showConversation(Request $request, int $conversation_id): array
     {
-        $data  = $this->query(query: "note-conversation:{$conversation_id}");
-        $notes = $data['notes'];
-
         return [
             '_template'     => 'collection/notes.html.twig',
-            'notes'         => $notes,
+            'notes'         => $this->query(query: "note-conversation:{$conversation_id}")['notes'] ?? [],
             'should_format' => false,
             'page_title'    => _m('Conversation'),
         ];
     }
 
+    /**
+     * Controller for the note reply non-JS page
+     *
+     * Leverages the `PostingModifyData` event to add the `reply_to_id` field from the GET variable 'reply_to_id'
+     *
+     * @throws ClientException
+     * @throws NoLoggedInUser
+     * @throws NoSuchNoteException
+     * @throws ServerException
+     *
+     * @return array
+     */
+    public function addReply(Request $request)
+    {
+        $user            = Common::ensureLoggedIn();
+        $note_id         = $this->int('reply_to_id', new ClientException(_m('Malformed query.')));
+        $note            = Note::ensureCanInteract(Note::getByPK($note_id), $user);
+        $conversation_id = $note->getConversationId();
+        return $this->showConversation($request, $conversation_id);
+    }
+
     /**
      * Creates form view for Muting Conversation extra action.
      *
@@ -72,17 +96,23 @@ class Conversation extends FeedController
      *
      * @return array Array containing templating where the form is to be rendered, and the form itself
      */
-    public function muteConversation(Request $request, int $conversation_id): array
+    public function muteConversation(Request $request, int $conversation_id)
     {
-        $user = Common::ensureLoggedIn();
-        $form = Form::create([
-            ['mute_conversation', SubmitType::class, ['label' => _m('Mute conversation')]],
+        $user     = Common::ensureLoggedIn();
+        $is_muted = ConversationMute::isMuted($conversation_id, $user);
+        $form     = Form::create([
+            ['mute_conversation', SubmitType::class, ['label' => $is_muted ? _m('Mute conversation') : _m('Unmute conversation')]],
         ]);
 
         $form->handleRequest($request);
         if ($form->isSubmitted() && $form->isValid()) {
-            DB::persist(ConversationMute::create(['conversation_id' => $conversation_id, 'actor_id' => $user->getId()]));
+            if ($is_muted) {
+                DB::persist(ConversationMute::create(['conversation_id' => $conversation_id, 'actor_id' => $user->getId()]));
+            } else {
+                DB::removeBy('conversation_mute', ['conversation_id' => $conversation_id, 'actor_id' => $user->getId()]);
+            }
             DB::flush();
+            Cache::delete(ConversationMute::cacheKeys($conversation_id, $user->getId())['mute']);
             throw new RedirectException();
         }
 

components/Conversation/Controller/Reply.php

@@ -1,71 +0,0 @@
-<?php
-
-declare(strict_types = 1);
-// {{{ License
-// This file is part of GNU social - https://www.gnu.org/software/social
-//
-// GNU social is free software: you can redistribute it and/or modify
-// it under the terms of the GNU Affero General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// GNU social is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU Affero General Public License for more details.
-//
-// You should have received a copy of the GNU Affero General Public License
-// along with GNU social.  If not, see <http://www.gnu.org/licenses/>.
-// }}}
-
-/**
- * @author    Hugo Sales <hugo@hsal.es>
- * @copyright 2021 Free Software Foundation, Inc http://www.fsf.org
- * @license   https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
- */
-
-namespace Component\Conversation\Controller;
-
-use App\Entity\Note;
-use App\Util\Common;
-use App\Util\Exception\ClientException;
-use App\Util\Exception\NoLoggedInUser;
-use App\Util\Exception\NoSuchNoteException;
-use App\Util\Exception\ServerException;
-use Component\Collection\Util\Controller\FeedController;
-use Component\Feed\Feed;
-use Symfony\Component\HttpFoundation\Request;
-use function App\Core\I18n\_m;
-
-class Reply extends FeedController
-{
-    /**
-     * Controller for the note reply non-JS page
-     *
-     * @throws ClientException
-     * @throws NoLoggedInUser
-     * @throws NoSuchNoteException
-     * @throws ServerException
-     *
-     * @return array
-     */
-    public function addReply(Request $request, int $note_id)
-    {
-        $user = Common::ensureLoggedIn();
-
-        $note = Note::getByPK($note_id);
-        if (\is_null($note) || !$note->isVisibleTo($user)) {
-            throw new NoSuchNoteException();
-        }
-
-        $conversation_id = $note->getConversationId();
-        $data            = $this->query(query: "note-conversation:{$conversation_id}");
-        $notes           = $data['notes'];
-        return [
-            '_template'     => 'collection/notes.html.twig',
-            'notes'         => $notes,
-            'should_format' => false,
-            'page_title'    => _m('Conversation'),
-        ];
-    }
-}

components/Conversation/Conversation.php

@@ -34,12 +34,22 @@ use App\Entity\Activity;
 use App\Entity\Actor;
 use App\Entity\Note;
 use App\Util\Common;
-use Component\Conversation\Controller\Reply as ReplyController;
 use Component\Conversation\Entity\Conversation as ConversationEntity;
+use Component\Conversation\Entity\ConversationMute;
+use Functional as F;
 use Symfony\Component\HttpFoundation\Request;
 
 class Conversation extends Component
 {
+    public function onAddRoute(RouteLoader $r): bool
+    {
+        $r->connect('conversation', '/conversation/{conversation_id<\d+>}', [Controller\Conversation::class, 'showConversation']);
+        $r->connect('conversation_mute', '/conversation/{conversation_id<\d+>}/mute', [Controller\Conversation::class, 'muteConversation']);
+        $r->connect('conversation_reply_to', '/conversation/reply', [Controller\Conversation::class, 'addReply']);
+
+        return Event::next;
+    }
+
     /**
      * **Assigns** the given local Note it's corresponding **Conversation**.
      *
@@ -96,14 +106,18 @@ class Conversation extends Component
             return Event::next;
         }
 
-        // Generating URL for reply action route
-        $args             = ['note_id' => $note->getId()];
-        $type             = Router::ABSOLUTE_PATH;
-        $reply_action_url = Router::url('conversation_reply_to', $args, $type);
+        $from = $request->query->has('from')
+              ? $request->query->get('from')
+              : $request->getPathInfo();
 
-        $query_string = $request->getQueryString();
-        // Concatenating get parameter to redirect the user to where he came from
-        $reply_action_url .= '?from=' . urlencode($request->getRequestUri()) . '#note-anchor-' . $note->getId();
+        $reply_action_url = Router::url(
+            'conversation_reply_to',
+            [
+                'reply_to_id' => $note->getId(),
+                'from'        => $from . '#note-anchor-' . $note->getId(),
+            ],
+            Router::ABSOLUTE_PATH,
+        );
 
         $reply_action = [
             'url'     => $reply_action_url,
@@ -117,10 +131,18 @@ class Conversation extends Component
         return Event::next;
     }
 
-    public function onAddExtraArgsToNoteContent(Request $request, Actor $actor, array $data, array &$extra_args): bool
+    /**
+     * Posting event to add extra info to a note
+     */
+    public function onPostingModifyData(Request $request, Actor $actor, array &$data): bool
     {
-        $extra_args['reply_to'] = 'conversation_reply_to' === $request->get('_route') ? (int) $request->get('note_id') : null;
+        $data['reply_to_id'] = $request->get('_route') === 'conversation_reply_to' && $request->query->has('reply_to_id')
+                             ? $request->query->getInt('reply_to_id')
+                             : null;
 
+        if (!\is_null($data['reply_to_id'])) {
+            Note::ensureCanInteract(Note::getById($data['reply_to_id']), $actor);
+        }
         return Event::next;
     }
 
@@ -132,23 +154,17 @@ class Conversation extends Component
      */
     public function onAppendCardNote(array $vars, array &$result): bool
     {
-        // If note is the original and user isn't the one who repeated, append on end "user repeated this"
-        // If user is the one who repeated, append on end "you repeated this, remove repeat?"
-        $check_user = !\is_null(Common::user());
-
         // The current Note being rendered
         $note = $vars['note'];
 
         // Will have actors array, and action string
         // Actors are the subjects, action is the verb (in the final phrase)
-        $reply_actors = [];
-        $note_replies = $note->getReplies();
+        $reply_actors = F\map(
+            $note->getReplies(),
+            fn (Note $reply) => Actor::getByPK($reply->getActorId()),
+        );
 
-        // Get actors who repeated the note
-        foreach ($note_replies as $reply) {
-            $reply_actors[] = Actor::getByPK($reply->getActorId());
-        }
-        if (\count($reply_actors) < 1) {
+        if (empty($reply_actors)) {
             return Event::next;
         }
 
@@ -159,20 +175,6 @@ class Conversation extends Component
         return Event::next;
     }
 
-    /**
-     * Connects the various Conversation related routes to their respective controllers.
-     *
-     * @return bool EventHook
-     */
-    public function onAddRoute(RouteLoader $r): bool
-    {
-        $r->connect('conversation_reply_to', '/conversation/reply?reply_to_note={note_id<\d+>}', [ReplyController::class, 'addReply']);
-        $r->connect('conversation', '/conversation/{conversation_id<\d+>}', [Controller\Conversation::class, 'showConversation']);
-        $r->connect('conversation_mute', '/conversation/{conversation_id<\d+>}/mute', [Controller\Conversation::class, 'muteConversation']);
-
-        return Event::next;
-    }
-
     /**
      * Informs **\App\Component\Posting::onAppendRightPostingBlock**, of the **current page context** in which the given
      * Actor is in. This is valuable when posting within a group route, allowing \App\Component\Posting to create a
@@ -181,15 +183,14 @@ class Conversation extends Component
      * @param \App\Entity\Actor      $actor         The Actor currently attempting to post a Note
      * @param null|\App\Entity\Actor $context_actor The 'owner' of the current route (e.g. Group or Actor), used to target it
      */
-    public function onPostingGetContextActor(Request $request, Actor $actor, ?Actor &$context_actor): bool
+    public function onPostingGetContextActor(Request $request, Actor $actor, ?Actor &$context_actor)
     {
-        // TODO: check if actor is posting in group, changing the context actor to that group
-        /*$to_query = $request->get('actor_id');
-        if (!\is_null($to_query)) {
+        $to_note_id = $request->query->get('reply_to_id');
+        if (!\is_null($to_note_id)) {
             // Getting the actor itself
-            $context_actor = Actor::getById((int) $to_query);
+            $context_actor = Actor::getById(Note::getById((int) $to_note_id)->getActorId());
             return Event::stop;
-        }*/
+        }
         return Event::next;
     }
 
@@ -202,14 +203,10 @@ class Conversation extends Component
      */
     public function onNoteDeleteRelated(Note &$note, Actor $actor): bool
     {
-        Cache::delete("note-replies-{$note->getId()}");
-        DB::wrapInTransaction(function () use ($note) {
-            foreach ($note->getReplies() as $reply) {
-                $reply->setReplyTo(null);
-            }
-        });
-        Cache::delete("note-replies-{$note->getId()}");
-
+        // Ensure we have the most up to date replies
+        Cache::delete(Note::cacheKeys($note->getId())['replies']);
+        DB::wrapInTransaction(fn () => F\each($note->getReplies(), fn (Note $note) => $note->setReplyTo(null)));
+        Cache::delete(Note::cacheKeys($note->getId())['replies']);
         return Event::next;
     }
 
@@ -225,12 +222,12 @@ class Conversation extends Component
      */
     public function onAddExtraNoteActions(Request $request, Note $note, array &$actions)
     {
-        if (\is_null($actor = Common::actor())) {
+        if (\is_null($user = Common::user())) {
             return Event::next;
         }
 
         $actions[] = [
-            'title'   => _m('Mute conversation'),
+            'title'   => ConversationMute::isMuted($note, $user) ? _m('Mute conversation') : _m('Unmute conversation'),
             'classes' => '',
             'url'     => Router::url('conversation_mute', ['conversation_id' => $note->getConversationId()]),
         ];
@@ -240,21 +237,9 @@ class Conversation extends Component
 
     public function onNewNotificationShould(Activity $activity, Actor $actor)
     {
-        if ('note' === $activity->getObjectType()) {
-            $is_blocked = !empty(DB::dql(
-                <<<'EOQ'
-                    SELECT 1
-                    FROM note AS n
-                    JOIN conversation_mute AS cm WITH n.conversation_id = cm.conversation_id
-                    WHERE n.id = :object_id
-                    EOQ,
-                ['object_id' => $activity->getObjectId()],
-            ));
-            if ($is_blocked) {
-                return Event::stop;
-            } else {
-                return Event::next;
-            }
+        if ($activity->getObjectType() === 'note' && ConversationMute::isMuted($activity, $actor)) {
+            return Event::stop;
         }
+        return Event::next;
     }
 }

components/Conversation/Entity/ConversationMute.php

@@ -23,7 +23,13 @@ declare(strict_types = 1);
 
 namespace Component\Conversation\Entity;
 
+use App\Core\Cache;
+use App\Core\DB\DB;
 use App\Core\Entity;
+use App\Entity\Activity;
+use App\Entity\Actor;
+use App\Entity\LocalUser;
+use App\Entity\Note;
 use DateTimeInterface;
 
 /**
@@ -80,6 +86,33 @@ class ConversationMute extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
+    public static function cacheKeys(int $conversation_id, int $actor_id): array
+    {
+        return [
+            'mute' => "conversation-mute-{$conversation_id}-{$actor_id}",
+        ];
+    }
+
+    /**
+     * Check if a conversation referenced by $object is muted form $actor
+     */
+    public static function isMuted(Activity|Note|int $object, Actor|LocalUser $actor): bool
+    {
+        $conversation_id = null;
+        if (\is_int($object)) {
+            $conversation_id = $object;
+        } elseif ($object instanceof Note) {
+            $conversation_id = $object->getConversationId();
+        } elseif ($object instanceof Activity) {
+            $conversation_id = Note::getById($object->getObjectId())->getConversationId();
+        }
+
+        return Cache::get(
+            self::cacheKeys($conversation_id, $actor->getId())['mute'],
+            fn () => (bool) DB::count('conversation_mute', ['conversation_id' => $conversation_id, 'actor_id' => $actor->getId()]),
+        );
+    }
+
     public static function schemaDef(): array
     {
         return [