From 16f51e5143fe8bd5ce16369daffbe9965a32ecf6 Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Mon, 28 Mar 2022 20:52:12 +0100 Subject: [PATCH] [COMPONENT][Notification] ->getSubscribers() should not be pre-included Notification bug fix on Subscription component Correct docblock --- components/Notification/Notification.php | 12 ++++-------- components/Subscription/Subscription.php | 5 ++--- plugins/Favourite/Controller/Favourite.php | 18 ++++++++++++++++-- plugins/RepeatNote/Controller/Repeat.php | 22 ++++++++++++++++++---- plugins/RepeatNote/RepeatNote.php | 4 ++-- src/Entity/Actor.php | 8 ++++---- 6 files changed, 46 insertions(+), 23 deletions(-) diff --git a/components/Notification/Notification.php b/components/Notification/Notification.php index 0e83cfa496..eb9f582ba5 100644 --- a/components/Notification/Notification.php +++ b/components/Notification/Notification.php @@ -65,25 +65,21 @@ class Notification extends Component * Enqueues a notification for an Actor (such as person or group) which means * it shows up in their home feed and such. * WARNING: It's highly advisable to have flushed any relevant objects before triggering this event. - * OBSERVATION: $sender->getSubscribers() will always be pre-included, thus why $targets=[] is normal * * $targets should be of the shape: - * ['source' => (int|Actor)[]] // Prefer Actor whenever possible + * (int|Actor)[] // Prefer Actor whenever possible * Example of $targets: - * [[42, $actor_alice, $actor_bob]] // Avoid repeating actors or ids + * [42, $actor_alice, $actor_bob] // Avoid repeating actors or ids * * @param Actor $sender The one responsible for this activity, take care not to include it in targets * @param Activity $activity The activity responsible for the object being given to known to targets - * @param array $targets attentions, Mentions, any other source + * @param array $targets Attentions, Mentions, any other source. Should never be empty, you usually want to register an attention to every $sender->getSubscribers() * @param null|string $reason An optional reason explaining why this notification exists */ - public function onNewNotification(Actor $sender, Activity $activity, array $targets = [], ?string $reason = null): bool + public function onNewNotification(Actor $sender, Activity $activity, array $targets, ?string $reason = null): bool { // Ensure targets are all actor objects and unique $effective_targets = []; - foreach ($sender->getSubscribers() as $subscriber) { - $effective_targets[$subscriber->getId()] = $subscriber; - } foreach ($targets as $target) { if (\is_int($target)) { $target_id = $target; diff --git a/components/Subscription/Subscription.php b/components/Subscription/Subscription.php index 0b40189f45..50314ea0d7 100644 --- a/components/Subscription/Subscription.php +++ b/components/Subscription/Subscription.php @@ -40,7 +40,6 @@ use App\Util\Exception\ServerException; use Component\Notification\Entity\Attention; use Component\Subscription\Controller\Subscribers as SubscribersController; use Component\Subscription\Controller\Subscriptions as SubscriptionsController; - use Symfony\Component\HttpFoundation\Request; class Subscription extends Component @@ -99,7 +98,7 @@ class Subscription extends Component $activity = null; if (\is_null($subscription)) { DB::persist($subscription = Entity\ActorSubscription::create($opts)); - $activity = Activity::create([ + $activity = Activity::create([ 'actor_id' => $subscriber_id, 'verb' => 'subscribe', 'object_type' => Actor::schemaName(), @@ -163,7 +162,7 @@ class Subscription extends Component Event::handle('NewNotification', [ \is_int($subject) ? $subject : Actor::getById($subscriber_id), $activity, - [], + [$subscribed_id], _m('{subject} unsubscribed from {object}.', ['{subject}' => $activity->getActorId(), '{object}' => $previous_follow_activity->getObjectId()]), ]); } diff --git a/plugins/Favourite/Controller/Favourite.php b/plugins/Favourite/Controller/Favourite.php index c4abb9f985..e1caa6f6d8 100644 --- a/plugins/Favourite/Controller/Favourite.php +++ b/plugins/Favourite/Controller/Favourite.php @@ -29,6 +29,7 @@ use App\Core\Form; use function App\Core\I18n\_m; use App\Core\Log; use App\Core\Router\Router; +use App\Entity\Activity; use App\Entity\Actor; use App\Entity\LocalUser; use App\Entity\Note; @@ -40,6 +41,7 @@ use App\Util\Exception\NoSuchNoteException; use App\Util\Exception\RedirectException; use App\Util\Exception\ServerException; use Component\Collection\Util\Controller\FeedController; +use Component\Notification\Entity\Attention; use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\HttpFoundation\Request; @@ -77,8 +79,14 @@ class Favourite extends FeedController if ($form_add_to_favourite->isSubmitted()) { if (!\is_null($activity = \Plugin\Favourite\Favourite::favourNote(note_id: $id, actor_id: $actor_id))) { + $actor = Actor::getById($actor_id); + foreach ($actor->getSubscribers() as $subscriber) { + $target_id = $subscriber->getId(); + DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $target_id])); + $effective_attentions[$target_id] = $subscriber; + } DB::flush(); - Event::handle('NewNotification', [$actor = Actor::getById($actor_id), $activity, $activity->getAttentionTargets(), _m('{actor_id} favoured note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $activity->getObjectId()])]); + Event::handle('NewNotification', [$actor, $activity, $activity->getAttentionTargets(), _m('{actor_id} favoured note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $activity->getObjectId()])]); } else { throw new ClientException(_m('Note already favoured!')); } @@ -137,8 +145,14 @@ class Favourite extends FeedController $form_remove_favourite->handleRequest($request); if ($form_remove_favourite->isSubmitted()) { if (!\is_null($activity = \Plugin\Favourite\Favourite::unfavourNote(note_id: $id, actor_id: $actor_id))) { + $actor = Actor::getById($actor_id); + foreach ($actor->getSubscribers() as $subscriber) { + $target_id = $subscriber->getId(); + DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $target_id])); + $effective_attentions[$target_id] = $subscriber; + } DB::flush(); - Event::handle('NewNotification', [$actor = Actor::getById($actor_id), $activity, $activity->getAttentionTargets(), _m('{actor_id} unfavoured note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $activity->getObjectId()])]); + Event::handle('NewNotification', [$actor, $activity, $activity->getAttentionTargets(), _m('{actor_id} unfavoured note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $activity->getObjectId()])]); } else { throw new ClientException(_m('Note already unfavoured!')); } diff --git a/plugins/RepeatNote/Controller/Repeat.php b/plugins/RepeatNote/Controller/Repeat.php index 56e4c93e7f..f57532cd4e 100644 --- a/plugins/RepeatNote/Controller/Repeat.php +++ b/plugins/RepeatNote/Controller/Repeat.php @@ -30,6 +30,7 @@ use App\Core\Form; use function App\Core\I18n\_m; use App\Core\Log; use App\Core\Router\Router; +use App\Entity\Activity; use App\Entity\Actor; use App\Entity\Note; use App\Util\Common; @@ -37,6 +38,7 @@ use App\Util\Exception\ClientException; use App\Util\Exception\NoLoggedInUser; use App\Util\Exception\RedirectException; use App\Util\Exception\ServerException; +use Component\Notification\Entity\Attention; use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\HttpFoundation\Request; @@ -73,9 +75,15 @@ class Repeat extends Controller $form_add_to_repeat->handleRequest($request); if ($form_add_to_repeat->isSubmitted()) { - $repeat_activity = \Plugin\RepeatNote\RepeatNote::repeatNote(note: $note, actor_id: $actor_id); + $activity = \Plugin\RepeatNote\RepeatNote::repeatNote(note: $note, actor_id: $actor_id); + $actor = Actor::getById($actor_id); + foreach ($actor->getSubscribers() as $subscriber) { + $target_id = $subscriber->getId(); + DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $target_id])); + $effective_attentions[$target_id] = $subscriber; + } DB::flush(); - Event::handle('NewNotification', [$actor = Actor::getById($actor_id), $repeat_activity, $repeat_activity->getAttentionTargets(), _m('{actor_id} repeated note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $repeat_activity->getObjectId()])]); + Event::handle('NewNotification', [$actor, $activity, $activity->getAttentionTargets(), _m('{actor_id} repeated note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $activity->getObjectId()])]); // Redirect user to where they came from // Prevent open redirect @@ -130,9 +138,15 @@ class Repeat extends Controller $form_remove_repeat->handleRequest($request); if ($form_remove_repeat->isSubmitted()) { - if (!\is_null($undo_repeat_activity = \Plugin\RepeatNote\RepeatNote::unrepeatNote(note_id: $note_id, actor_id: $actor_id))) { + if (!\is_null($activity = \Plugin\RepeatNote\RepeatNote::unrepeatNote(note_id: $note_id, actor_id: $actor_id))) { + $actor = Actor::getById($actor_id); + foreach ($actor->getSubscribers() as $subscriber) { + $target_id = $subscriber->getId(); + DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $target_id])); + $effective_attentions[$target_id] = $subscriber; + } DB::flush(); - Event::handle('NewNotification', [$actor = Actor::getById($actor_id), $undo_repeat_activity, $undo_repeat_activity->getAttentionTargets(), _m('{actor_id} unrepeated note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $note_id])]); + Event::handle('NewNotification', [$actor, $activity, $activity->getAttentionTargets(), _m('{actor_id} unrepeated note {note_id}.', ['{actor_id}' => $actor->getId(), '{note_id}' => $note_id])]); } else { throw new ClientException(_m('Note wasn\'t repeated!')); } diff --git a/plugins/RepeatNote/RepeatNote.php b/plugins/RepeatNote/RepeatNote.php index c02651e2a6..ff4c3d112b 100644 --- a/plugins/RepeatNote/RepeatNote.php +++ b/plugins/RepeatNote/RepeatNote.php @@ -81,7 +81,7 @@ class RepeatNote extends NoteHandlerPlugin $original_note_id = $note->getId(); // Create a new note with the same content as the original - [, $repeat, ] = Posting::storeLocalNote( + [, $repeat, ] = Posting::storeLocalNote( actor: $actor = Actor::getById($actor_id), content: $note->getContent(), content_type: $note->getContentType(), @@ -379,7 +379,7 @@ class RepeatNote extends NoteHandlerPlugin } if ($type_activity->get('type') === 'Announce') { // Repeat if ($type_object instanceof \ActivityPhp\Type\AbstractObject) { - if ($type_object->get('type') === 'Note' || $type_object->get('type') === 'ChatMessage' || $type_object->get('type') === 'Page') { + if ($type_object->get('type') === 'Note' || $type_object->get('type') === 'ChatMessage' || $type_object->get('type') === 'Article' || $type_object->get('type') === 'Page') { $note = \Plugin\ActivityPub\Util\Model\Note::fromJson($type_object); $note_id = $note->getId(); } else { diff --git a/src/Entity/Actor.php b/src/Entity/Actor.php index 67a508f729..a2897362c9 100644 --- a/src/Entity/Actor.php +++ b/src/Entity/Actor.php @@ -385,14 +385,14 @@ class Actor extends Entity EOF, ['self' => $this->getId()]); } - public function getSubscriptionsUrl(): string + public function getSubscriptionsUrl(int $type = Router::ABSOLUTE_PATH): string { - return Router::url('actor_subscriptions_id', ['id' => $this->getId()]); + return Router::url(id: 'actor_subscriptions_id', args: ['id' => $this->getId()], type: $type); } - public function getSubscribersUrl(): string + public function getSubscribersUrl(int $type = Router::ABSOLUTE_PATH): string { - return Router::url('actor_subscribers_id', ['id' => $this->getId()]); + return Router::url(id: 'actor_subscribers_id', args: ['id' => $this->getId()], type: $type); } /**