[COMPONENT][Notification] ->getSubscribers() should not be pre-included

Notification bug fix on Subscription component
Correct docblock
This commit is contained in:
Diogo Peralta Cordeiro 2022-03-28 20:52:12 +01:00
parent ba4230447e
commit 16f51e5143
Signed by: diogo
GPG Key ID: 18D2D35001FBFAB0
6 changed files with 46 additions and 23 deletions

View File

@ -65,25 +65,21 @@ class Notification extends Component
* Enqueues a notification for an Actor (such as person or group) which means * Enqueues a notification for an Actor (such as person or group) which means
* it shows up in their home feed and such. * it shows up in their home feed and such.
* WARNING: It's highly advisable to have flushed any relevant objects before triggering this event. * 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: * $targets should be of the shape:
* ['source' => (int|Actor)[]] // Prefer Actor whenever possible * (int|Actor)[] // Prefer Actor whenever possible
* Example of $targets: * 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 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 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 * @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 // Ensure targets are all actor objects and unique
$effective_targets = []; $effective_targets = [];
foreach ($sender->getSubscribers() as $subscriber) {
$effective_targets[$subscriber->getId()] = $subscriber;
}
foreach ($targets as $target) { foreach ($targets as $target) {
if (\is_int($target)) { if (\is_int($target)) {
$target_id = $target; $target_id = $target;

View File

@ -40,7 +40,6 @@ use App\Util\Exception\ServerException;
use Component\Notification\Entity\Attention; use Component\Notification\Entity\Attention;
use Component\Subscription\Controller\Subscribers as SubscribersController; use Component\Subscription\Controller\Subscribers as SubscribersController;
use Component\Subscription\Controller\Subscriptions as SubscriptionsController; use Component\Subscription\Controller\Subscriptions as SubscriptionsController;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
class Subscription extends Component class Subscription extends Component
@ -99,7 +98,7 @@ class Subscription extends Component
$activity = null; $activity = null;
if (\is_null($subscription)) { if (\is_null($subscription)) {
DB::persist($subscription = Entity\ActorSubscription::create($opts)); DB::persist($subscription = Entity\ActorSubscription::create($opts));
$activity = Activity::create([ $activity = Activity::create([
'actor_id' => $subscriber_id, 'actor_id' => $subscriber_id,
'verb' => 'subscribe', 'verb' => 'subscribe',
'object_type' => Actor::schemaName(), 'object_type' => Actor::schemaName(),
@ -163,7 +162,7 @@ class Subscription extends Component
Event::handle('NewNotification', [ Event::handle('NewNotification', [
\is_int($subject) ? $subject : Actor::getById($subscriber_id), \is_int($subject) ? $subject : Actor::getById($subscriber_id),
$activity, $activity,
[], [$subscribed_id],
_m('{subject} unsubscribed from {object}.', ['{subject}' => $activity->getActorId(), '{object}' => $previous_follow_activity->getObjectId()]), _m('{subject} unsubscribed from {object}.', ['{subject}' => $activity->getActorId(), '{object}' => $previous_follow_activity->getObjectId()]),
]); ]);
} }

View File

@ -29,6 +29,7 @@ use App\Core\Form;
use function App\Core\I18n\_m; use function App\Core\I18n\_m;
use App\Core\Log; use App\Core\Log;
use App\Core\Router\Router; use App\Core\Router\Router;
use App\Entity\Activity;
use App\Entity\Actor; use App\Entity\Actor;
use App\Entity\LocalUser; use App\Entity\LocalUser;
use App\Entity\Note; use App\Entity\Note;
@ -40,6 +41,7 @@ use App\Util\Exception\NoSuchNoteException;
use App\Util\Exception\RedirectException; use App\Util\Exception\RedirectException;
use App\Util\Exception\ServerException; use App\Util\Exception\ServerException;
use Component\Collection\Util\Controller\FeedController; use Component\Collection\Util\Controller\FeedController;
use Component\Notification\Entity\Attention;
use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\Form\Extension\Core\Type\SubmitType;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
@ -77,8 +79,14 @@ class Favourite extends FeedController
if ($form_add_to_favourite->isSubmitted()) { if ($form_add_to_favourite->isSubmitted()) {
if (!\is_null($activity = \Plugin\Favourite\Favourite::favourNote(note_id: $id, actor_id: $actor_id))) { 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(); 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 { } else {
throw new ClientException(_m('Note already favoured!')); throw new ClientException(_m('Note already favoured!'));
} }
@ -137,8 +145,14 @@ class Favourite extends FeedController
$form_remove_favourite->handleRequest($request); $form_remove_favourite->handleRequest($request);
if ($form_remove_favourite->isSubmitted()) { if ($form_remove_favourite->isSubmitted()) {
if (!\is_null($activity = \Plugin\Favourite\Favourite::unfavourNote(note_id: $id, actor_id: $actor_id))) { 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(); 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 { } else {
throw new ClientException(_m('Note already unfavoured!')); throw new ClientException(_m('Note already unfavoured!'));
} }

View File

@ -30,6 +30,7 @@ use App\Core\Form;
use function App\Core\I18n\_m; use function App\Core\I18n\_m;
use App\Core\Log; use App\Core\Log;
use App\Core\Router\Router; use App\Core\Router\Router;
use App\Entity\Activity;
use App\Entity\Actor; use App\Entity\Actor;
use App\Entity\Note; use App\Entity\Note;
use App\Util\Common; use App\Util\Common;
@ -37,6 +38,7 @@ use App\Util\Exception\ClientException;
use App\Util\Exception\NoLoggedInUser; use App\Util\Exception\NoLoggedInUser;
use App\Util\Exception\RedirectException; use App\Util\Exception\RedirectException;
use App\Util\Exception\ServerException; use App\Util\Exception\ServerException;
use Component\Notification\Entity\Attention;
use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\Form\Extension\Core\Type\SubmitType;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
@ -73,9 +75,15 @@ class Repeat extends Controller
$form_add_to_repeat->handleRequest($request); $form_add_to_repeat->handleRequest($request);
if ($form_add_to_repeat->isSubmitted()) { 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(); 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 // Redirect user to where they came from
// Prevent open redirect // Prevent open redirect
@ -130,9 +138,15 @@ class Repeat extends Controller
$form_remove_repeat->handleRequest($request); $form_remove_repeat->handleRequest($request);
if ($form_remove_repeat->isSubmitted()) { 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(); 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 { } else {
throw new ClientException(_m('Note wasn\'t repeated!')); throw new ClientException(_m('Note wasn\'t repeated!'));
} }

View File

@ -81,7 +81,7 @@ class RepeatNote extends NoteHandlerPlugin
$original_note_id = $note->getId(); $original_note_id = $note->getId();
// Create a new note with the same content as the original // Create a new note with the same content as the original
[, $repeat, ] = Posting::storeLocalNote( [, $repeat, ] = Posting::storeLocalNote(
actor: $actor = Actor::getById($actor_id), actor: $actor = Actor::getById($actor_id),
content: $note->getContent(), content: $note->getContent(),
content_type: $note->getContentType(), content_type: $note->getContentType(),
@ -379,7 +379,7 @@ class RepeatNote extends NoteHandlerPlugin
} }
if ($type_activity->get('type') === 'Announce') { // Repeat if ($type_activity->get('type') === 'Announce') { // Repeat
if ($type_object instanceof \ActivityPhp\Type\AbstractObject) { 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 = \Plugin\ActivityPub\Util\Model\Note::fromJson($type_object);
$note_id = $note->getId(); $note_id = $note->getId();
} else { } else {

View File

@ -385,14 +385,14 @@ class Actor extends Entity
EOF, ['self' => $this->getId()]); 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);
} }
/** /**