diff --git a/src/Controller/ActivityPub/Magazine/MagazineModeratorsController.php b/src/Controller/ActivityPub/Magazine/MagazineModeratorsController.php index 7305242f54..2a7b0ce45f 100644 --- a/src/Controller/ActivityPub/Magazine/MagazineModeratorsController.php +++ b/src/Controller/ActivityPub/Magazine/MagazineModeratorsController.php @@ -6,12 +6,14 @@ use App\Entity\Contracts\ActivityPubActivityInterface; use App\Entity\Magazine; +use App\Entity\Moderator; use App\Repository\MagazineRepository; use App\Service\ActivityPubManager; use JetBrains\PhpStorm\ArrayShape; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Uid\Uuid; class MagazineModeratorsController { @@ -40,23 +42,21 @@ public function __invoke(Magazine $magazine, Request $request): JsonResponse ])] private function getCollectionItems(Magazine $magazine): array { - $moderatorsCount = $this->magazineRepository->findModerators($magazine)->count(); - $moderators = $this->magazineRepository->findModerators($magazine, perPage: $moderatorsCount); - $actors = array_map(fn ($mod) => $mod->user, iterator_to_array($moderators->getCurrentPageResults())); + $moderators = $this->magazineRepository->findModerators($magazine, perPage: $magazine->moderators->count()); $items = []; - foreach ($actors as $actor) { + foreach ($moderators->getCurrentPageResults() as /* @var Moderator $mod */ $mod) { + $actor = $mod->user; $items[] = $this->manager->getActorProfileId($actor); } - $routeName = 'ap_magazine_moderators'; - $routeParams = ['name' => $magazine->name]; + $id = Uuid::v4()->toRfc4122(); return [ '@context' => ActivityPubActivityInterface::CONTEXT_URL, 'type' => 'OrderedCollection', - 'id' => $this->urlGenerator->generate($routeName, $routeParams, UrlGeneratorInterface::ABSOLUTE_URL), - 'totalItems' => $moderatorsCount, + 'id' => $this->urlGenerator->generate('ap_object', ['id' => $id], UrlGeneratorInterface::ABSOLUTE_URL), + 'totalItems' => \sizeof($items), 'orderedItems' => $items, ]; } diff --git a/src/Controller/Magazine/MagazineModController.php b/src/Controller/Magazine/MagazineModController.php index 72e88e67a8..187692cfbf 100644 --- a/src/Controller/Magazine/MagazineModController.php +++ b/src/Controller/Magazine/MagazineModController.php @@ -6,7 +6,6 @@ use App\Controller\AbstractController; use App\Entity\Magazine; -use App\Entity\Moderator; use App\Repository\MagazineRepository; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -15,19 +14,13 @@ class MagazineModController extends AbstractController { public function __invoke(Magazine $magazine, MagazineRepository $repository, Request $request): Response { - $moderatorsWithoutOwner = []; - foreach ($repository->findModerators($magazine, $this->getPageNb($request)) as /* @var $mod Moderator */ $mod) { - // only include the owner if it is a local magazine, for remote magazines the owner is always the admin - if (!$mod->isOwner or null === $magazine->apId) { - $moderatorsWithoutOwner[] = $mod; - } - } + $moderators = $repository->findModerators($magazine, $this->getPageNb($request)); return $this->render( 'magazine/moderators.html.twig', [ 'magazine' => $magazine, - 'moderators' => $moderatorsWithoutOwner, + 'moderators' => $moderators, ] ); } diff --git a/src/Entity/Magazine.php b/src/Entity/Magazine.php index eb83cf5b5e..16145d5637 100644 --- a/src/Entity/Magazine.php +++ b/src/Entity/Magazine.php @@ -191,20 +191,6 @@ public function getUserAsModeratorOrNull(User $user): ?Moderator return null; } - public function removeUserAsModerator(User $user): void - { - $user->moderatorTokens->get(-1); - - $criteria = Criteria::create() - ->where(Criteria::expr()->eq('magazine', $this)); - - foreach ($user->moderatorTokens->matching($criteria) as $item) { - /** @var Moderator $mod */ - $mod = $item; - $this->moderators->remove($mod->getId()); - } - } - public function userIsOwner(User $user): bool { $user->moderatorTokens->get(-1); @@ -232,6 +218,14 @@ public function getOwner(): User return $this->moderators->matching($criteria)->first()->user; } + public function getModeratorCount(): int + { + $criteria = Criteria::create() + ->where(Criteria::expr()->eq('isOwner', false)); + + return $this->moderators->matching($criteria)->count(); + } + public function addEntry(Entry $entry): self { if (!$this->entries->contains($entry)) { @@ -330,7 +324,7 @@ public function updateSubscriptionsCount(): void { if (null !== $this->apFollowersCount) { $criteria = Criteria::create() - ->where(Criteria::expr()->gt('createdAt', $this->apFetchedAt)); + ->where(Criteria::expr()->gt('createdAt', \DateTimeImmutable::createFromMutable($this->apFetchedAt))); $newSubscribers = $this->subscriptions->matching($criteria)->count(); $this->subscriptionsCount = $this->apFollowersCount + $newSubscribers; diff --git a/src/Entity/User.php b/src/Entity/User.php index 5ae9877de2..2bad2944fc 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -465,8 +465,10 @@ public function isFollowing(User $user): bool public function updateFollowCounts(): void { if (null !== $this->apFollowersCount) { - $criteria = Criteria::create() - ->where(Criteria::expr()->gt('createdAt', $this->apFetchedAt)); + $criteria = Criteria::create(); + if ($this->apFetchedAt) { + $criteria->where(Criteria::expr()->gt('createdAt', \DateTimeImmutable::createFromMutable($this->apFetchedAt))); + } $newFollowers = $this->followers->matching($criteria)->count(); $this->followersCount = $this->apFollowersCount + $newFollowers; diff --git a/src/EventSubscriber/ActivityPub/MagazineModeratorAddedRemovedSubscriber.php b/src/EventSubscriber/ActivityPub/MagazineModeratorAddedRemovedSubscriber.php index 489ecb4451..cfd99ac7d7 100644 --- a/src/EventSubscriber/ActivityPub/MagazineModeratorAddedRemovedSubscriber.php +++ b/src/EventSubscriber/ActivityPub/MagazineModeratorAddedRemovedSubscriber.php @@ -20,16 +20,16 @@ public function __construct( public function onModeratorAdded(MagazineModeratorAddedEvent $event): void { - // if the magazine is local then we have authority over it, otherwise the addedBy user has to be defined - if (!$event->magazine->apId or null !== $event->addedBy) { + // if the magazine is local then we have authority over it, otherwise the addedBy user has to be a local user + if (!$event->magazine->apId or (null !== $event->addedBy and !$event->addedBy->apId)) { $this->bus->dispatch(new AddMessage($event->addedBy->getId(), $event->magazine->getId(), $event->user->getId())); } } public function onModeratorRemoved(MagazineModeratorRemovedEvent $event): void { - // if the magazine is local then we have authority over it, otherwise the removedBy user has to be defined - if (!$event->magazine->apId or null !== $event->removedBy) { + // if the magazine is local then we have authority over it, otherwise the removedBy user has to be a local user + if (!$event->magazine->apId or (null !== $event->removedBy and !$event->removedBy->apId)) { $this->bus->dispatch(new RemoveMessage($event->removedBy->getId(), $event->magazine->getId(), $event->user->getId())); } } diff --git a/src/MessageHandler/ActivityPub/Inbox/AddHandler.php b/src/MessageHandler/ActivityPub/Inbox/AddHandler.php index 88a613978e..35e84e2c09 100644 --- a/src/MessageHandler/ActivityPub/Inbox/AddHandler.php +++ b/src/MessageHandler/ActivityPub/Inbox/AddHandler.php @@ -5,7 +5,6 @@ namespace App\MessageHandler\ActivityPub\Inbox; use App\DTO\ModeratorDto; -use App\Entity\Moderator; use App\Message\ActivityPub\Inbox\AddMessage; use App\Repository\MagazineRepository; use App\Service\ActivityPubManager; @@ -26,28 +25,24 @@ public function __construct( public function __invoke(AddMessage $message): void { - $actor = $this->activityPubManager->findUserActorOrCreateOrThrow($message['actor']); - $targetMag = $this->magazineRepository->getMagazineFromModeratorsUrl($message['target']); + $payload = $message->payload; + $actor = $this->activityPubManager->findUserActorOrCreateOrThrow($payload['actor']); + $targetMag = $this->magazineRepository->getMagazineFromModeratorsUrl($payload['target']); if (!$targetMag) { - throw new \LogicException("could not find a magazine with moderators url like: '{$message['target']}'"); + throw new \LogicException("could not find a magazine with moderators url like: '{$payload['target']}'"); } if (!$targetMag->userIsModerator($actor) and !$targetMag->hasSameHostAsUser($actor)) { throw new \LogicException("the user '$actor->username' ({$actor->getId()}) is not a moderator of $targetMag->name ({$targetMag->getId()}) and is not from the same instance. He can therefore not add moderators"); } - $object = $this->activityPubManager->findUserActorOrCreateOrThrow($message['object']); + $object = $this->activityPubManager->findUserActorOrCreateOrThrow($payload['object']); if ($targetMag->userIsModerator($object)) { - $this->logger->warning("the user '$object->username' ({$object->getId()}) already is a moderator of $targetMag->name ({$targetMag->getId()}. Discarding message"); - } - $this->logger->info("'$actor->username' ({$actor->getId()}) added '$object->username' ({$object->getId()}) as moderator to '$targetMag->name' ({$targetMag->getId()}"); - if ($targetMag->apId) { - // don't federate the added moderator if the magazine is not local - $targetMag->addModerator(new Moderator($targetMag, $object, $actor, false, true)); - } else { - // use the manager to trigger outbox federation for the new moderator - // use case for this: a remote moderator added a new moderator - $this->magazineManager->addModerator(new ModeratorDto($targetMag, $object, $actor)); + $this->logger->warning("the user '$object->username' ({$object->getId()}) already is a moderator of $targetMag->name ({$targetMag->getId()}). Discarding message"); + + return; } + $this->logger->info("'$actor->username' ({$actor->getId()}) added '$object->username' ({$object->getId()}) as moderator to '$targetMag->name' ({$targetMag->getId()})"); + $this->magazineManager->addModerator(new ModeratorDto($targetMag, $object, $actor)); } } diff --git a/src/MessageHandler/ActivityPub/Inbox/RemoveHandler.php b/src/MessageHandler/ActivityPub/Inbox/RemoveHandler.php index f46277b9da..89de5e528f 100644 --- a/src/MessageHandler/ActivityPub/Inbox/RemoveHandler.php +++ b/src/MessageHandler/ActivityPub/Inbox/RemoveHandler.php @@ -24,29 +24,23 @@ public function __construct( public function __invoke(RemoveMessage $message): void { - $actor = $this->activityPubManager->findUserActorOrCreateOrThrow($message['actor']); - $targetMag = $this->magazineRepository->getMagazineFromModeratorsUrl($message['target']); + $payload = $message->payload; + $actor = $this->activityPubManager->findUserActorOrCreateOrThrow($payload['actor']); + $targetMag = $this->magazineRepository->getMagazineFromModeratorsUrl($payload['target']); if (!$targetMag) { - throw new \LogicException("could not find a magazine with moderators url like: '{$message['target']}'"); + throw new \LogicException("could not find a magazine with moderators url like: '{$payload['target']}'"); } if (!$targetMag->userIsModerator($actor) and !$targetMag->hasSameHostAsUser($actor)) { throw new \LogicException("the user '$actor->username' ({$actor->getId()}) is not a moderator of $targetMag->name ({$targetMag->getId()}) and is not from the same instance. He can therefore not remove moderators"); } - $object = $this->activityPubManager->findUserActorOrCreateOrThrow($message['object']); + $object = $this->activityPubManager->findUserActorOrCreateOrThrow($payload['object']); $objectMod = $targetMag->getUserAsModeratorOrNull($object); if (null === $objectMod) { - $this->logger->warning("the user '$object->username' ({$object->getId()}) is not a moderator of $targetMag->name ({$targetMag->getId()} and can therefore not be removed as one. Discarding message"); - } - $this->logger->info("'$actor->username' ({$actor->getId()}) removed '$object->username' ({$object->getId()}) as moderator from '$targetMag->name' ({$targetMag->getId()}"); - if ($targetMag->apId) { - // don't federate the added moderator if the magazine is not local - $targetMag->removeUserAsModerator($object); - } else { - // use the manager to trigger outbox federation for the new moderator - // use case for this: a remote moderator added a new moderator - $this->magazineManager->removeModerator($objectMod); + $this->logger->warning("the user '$object->username' ({$object->getId()}) is not a moderator of $targetMag->name ({$targetMag->getId()}) and can therefore not be removed as one. Discarding message"); } + $this->logger->info("'$actor->username' ({$actor->getId()}) removed '$object->username' ({$object->getId()}) as moderator from '$targetMag->name' ({$targetMag->getId()})"); + $this->magazineManager->removeModerator($objectMod, $actor); } } diff --git a/src/Repository/MagazineRepository.php b/src/Repository/MagazineRepository.php index 2a76ac0b9a..b3f7b3a681 100644 --- a/src/Repository/MagazineRepository.php +++ b/src/Repository/MagazineRepository.php @@ -162,6 +162,10 @@ public function findModerators( ): PagerfantaInterface { $criteria = Criteria::create()->orderBy(['createdAt' => 'ASC']); + if ($magazine->apId) { + $criteria->where(Criteria::expr()->eq('isOwner', false)); + } + $moderators = new Pagerfanta(new SelectableAdapter($magazine->moderators, $criteria)); try { $moderators->setMaxPerPage($perPage); diff --git a/src/Service/ActivityPubManager.php b/src/Service/ActivityPubManager.php index 94bc747122..4d914aace3 100644 --- a/src/Service/ActivityPubManager.php +++ b/src/Service/ActivityPubManager.php @@ -26,6 +26,7 @@ use App\Service\ActivityPub\ApHttpClient; use App\Service\ActivityPub\Webfinger\WebFinger; use App\Service\ActivityPub\Webfinger\WebFingerFactory; +use Doctrine\Common\Collections\Criteria; use Doctrine\ORM\EntityManagerInterface; use League\HTMLToMarkdown\HtmlConverter; use Psr\Log\LoggerInterface; @@ -428,6 +429,8 @@ public function updateMagazine(string $actorUrl): ?Magazine $items = $attributedObj['orderedItems']; } + $this->logger->debug("got moderator items for magazine '$magazine->name': ".json_encode($attributedObj)); + if (null !== $items) { $moderatorsToRemove = []; foreach ($magazine->moderators as /* @var $mod Moderator */ $mod) { @@ -448,6 +451,7 @@ public function updateMagazine(string $actorUrl): ?Magazine } } if (!$magazine->userIsModerator($user)) { + $this->logger->info("adding '$user->username' as moderator in '$magazine->name' because they are a mod upstream, but not locally"); $this->magazineManager->addModerator(new ModeratorDto($magazine, $user, null)); } } @@ -462,7 +466,10 @@ public function updateMagazine(string $actorUrl): ?Magazine if (null === $modToRemove) { continue; } - $magazine->removeUserAsModerator($modToRemove); + $criteria = Criteria::create()->where(Criteria::expr()->eq('magazine', $magazine)); + $modObject = $modToRemove->moderatorTokens->matching($criteria)->first(); + $this->logger->info("removing '$modToRemove->username' from '$magazine->name' as mod locally because they are no longer mod upstream"); + $this->magazineManager->removeModerator($modObject, null); } } else { $this->logger->warning("could not update the moderators of $actorUrl, the response doesn't have a 'items' or 'orderedItems' property or it is not an array"); diff --git a/src/Service/MagazineManager.php b/src/Service/MagazineManager.php index 3aa6653f70..e143f10568 100644 --- a/src/Service/MagazineManager.php +++ b/src/Service/MagazineManager.php @@ -240,7 +240,7 @@ private function clearCommentsCache(User $user) ]); } - public function removeModerator(Moderator $moderator): void + public function removeModerator(Moderator $moderator, ?User $removedBy): void { $user = $moderator->user; @@ -248,7 +248,7 @@ public function removeModerator(Moderator $moderator): void $this->entityManager->flush(); $this->clearCommentsCache($user); - $this->dispatcher->dispatch(new MagazineModeratorRemovedEvent($moderator->magazine, $moderator->user, $moderator->addedByUser)); + $this->dispatcher->dispatch(new MagazineModeratorRemovedEvent($moderator->magazine, $moderator->user, $removedBy)); } public function changeTheme(MagazineThemeDto $dto): Magazine @@ -329,7 +329,7 @@ public function toggleOwnershipRequest(Magazine $magazine, User $user): void public function acceptOwnershipRequest(Magazine $magazine, User $user, ?User $addedBy): void { - $this->removeModerator($magazine->getOwnerModerator()); + $this->removeModerator($magazine->getOwnerModerator(), $addedBy); $this->addModerator(new ModeratorDto($magazine, $user, $addedBy), true); diff --git a/templates/components/magazine_box.html.twig b/templates/components/magazine_box.html.twig index 775149f969..0bc2e3b30c 100644 --- a/templates/components/magazine_box.html.twig +++ b/templates/components/magazine_box.html.twig @@ -48,7 +48,7 @@ {{ _self.meta_item( 'comments'|trans, path('magazine_entry_comments', {'name': computed.magazine.name}), computed.magazine.entryCommentCount) }} {{ _self.meta_item( 'posts'|trans, path('magazine_posts', {'name': computed.magazine.name}), computed.magazine.postCount) }} {{ _self.meta_item( 'replies'|trans, path('magazine_posts', {'name': computed.magazine.name}), computed.magazine.postCommentCount) }} - {{ _self.meta_item( 'moderators'|trans, path('magazine_moderators', {'name': computed.magazine.name}), computed.magazine.moderators|length) }} + {{ _self.meta_item( 'moderators'|trans, path('magazine_moderators', {'name': computed.magazine.name}), computed.magazine.getModeratorCount()) }} {{ _self.meta_item( 'mod_log'|trans, path('magazine_modlog', {'name': computed.magazine.name}), computed.magazine.logs|length) }} {% endif %}