apply remove share for users/teams/groups#2728
Conversation
Signed-off-by: samin-z <samin.zavarkesh@gmail.com>
enjeck
left a comment
enjeck
left a comment
There was a problem hiding this comment.
not sure if ready for review yet, but I have some preliminary comments
| class ReceiverCleanupListener implements IEventListener { | ||
| public function __construct( | ||
| private ShareMapper $shareMapper, | ||
| private CirclesManager $circlesManager, |
There was a problem hiding this comment.
CirclesManager is a hard constructor dependency, but Circles is an optional app. When Circles is not available, this might crash?. Should use CircleHelper.php instead
There was a problem hiding this comment.
ok i see your point, circles is optional in tables and hard injecting CirclesManager does not match our existing CircleHelper pattern. i'll update it.
|
|
||
| public function handle(Event $event): void { | ||
| if ($event instanceof UserDeletedEvent) { | ||
| $this->cleanupByParticipant('user', $event->getUser()->getUID()); |
There was a problem hiding this comment.
instead of hardcoding user, group, circle, use the constants e.g ShareReceiverType::USER
| return; | ||
| } | ||
|
|
||
| $this->cleanupByParticipant('group', $singleId); |
There was a problem hiding this comment.
should this be 'circle' instead?
There was a problem hiding this comment.
if a group is added to table, in tables_shares table for receiver_type we have 'group'
| } | ||
|
|
||
| private function cleanupByParticipant(string $type, string $participant): void { | ||
| $this->shareMapper->deleteByReceiver($participant, $type); |
There was a problem hiding this comment.
could throw an Exception, and nothing here catches it
Signed-off-by: samin-z <samin.zavarkesh@gmail.com>
|
@enjeck i decided to simplify the logic and do a follow-up ticket on the case for checking groups before adding and also removing them if exist. |
| $context->registerEventListener(TableOwnershipTransferredEvent::class, WhenTableTransferredAuditLogListener::class); | ||
| $context->registerEventListener(AddMissingIndicesEvent::class, AddMissingIndicesListener::class); | ||
| $context->registerEventListener(UserDeletedEvent::class, ReceiverCleanupListener::class); | ||
| $context->registerEventListener(BeforeGroupDeletedEvent::class, ReceiverCleanupListener::class); |
There was a problem hiding this comment.
Group deletion may still be aborted at this stage. Like with the other events, GroupDeletedEvent should be used here instead, right?
3 participants
IF user/team/groups is removed, all the connections with tables share is also removed.