Skip to content

feat: Emit UserShareAccessUpdatedEvent #2289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
provokateurin merged 2 commits into from
Jan 22, 2026
Merged

feat: Emit UserShareAccessUpdatedEvent #2289

provokateurin merged 2 commits into from
Jan 22, 2026
+457 −811

Conversation

@provokateurin
Copy link
Member

@provokateurin provokateurin commented Jan 15, 2026

No description provided.

@provokateurin provokateurin added this to the Nextcloud 33 milestone Jan 15, 2026
@provokateurin provokateurin self-assigned this Jan 15, 2026
ArtificialOwl
ArtificialOwl approved these changes Jan 15, 2026
View reviewed changes
Copy link
Member

@ArtificialOwl ArtificialOwl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listening on CircleMemberAddedEvent and CircleMemberRemovedEvent might be better

@provokateurin
Copy link
Member Author

provokateurin commented Jan 15, 2026

Needs adjustment for nextcloud/server#57564

@provokateurin provokateurin marked this pull request as draft January 15, 2026 09:44
provokateurin
provokateurin commented Jan 15, 2026
View reviewed changes
@icewind1991
Copy link
Member

icewind1991 commented Jan 15, 2026 •
edited
Loading

I'm not seeing any sign of the event being emitted in my testing.

Either adding or removing a user from a circle doesn't trigger the event listener

@icewind1991 icewind1991 mentioned this pull request Jan 15, 2026
Merged
5 tasks
@provokateurin
test(stubs): Update
ed36ce2 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin provokateurin force-pushed the feat/share-access-update-event branch from 045a4e5 to 4087a9e Compare January 21, 2026 14:53
@provokateurin provokateurin marked this pull request as ready for review January 21, 2026 14:54
@provokateurin
Copy link
Member Author

provokateurin commented Jan 21, 2026

I'm not seeing any sign of the event being emitted in my testing.

Fixed

@artonge artonge mentioned this pull request Jan 21, 2026
Open
icewind1991
icewind1991 reviewed Jan 21, 2026
View reviewed changes
icewind1991
icewind1991 reviewed Jan 21, 2026
View reviewed changes
icewind1991
icewind1991 requested changes Jan 21, 2026
View reviewed changes
Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the two changes it seems to work as expected

@provokateurin provokateurin force-pushed the feat/share-access-update-event branch from 4087a9e to 30492cd Compare January 21, 2026 15:51
@icewind1991
Copy link
Member

icewind1991 commented Jan 21, 2026

Nope, I was wrong $member->getCircle() is the circle it's a member of, not the circle that is a member.

$this->circleRequest->getCircle($member->getSingleId()); is the correct one.

icewind1991
icewind1991 reviewed Jan 21, 2026
View reviewed changes
@provokateurin
feat: Emit UserShareAccessUpdatedEvent
81eeebe 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin provokateurin force-pushed the feat/share-access-update-event branch from 4c8f4b5 to 81eeebe Compare January 22, 2026 07:28
@provokateurin provokateurin merged commit c2b8926 into master Jan 22, 2026
44 checks passed
@provokateurin provokateurin deleted the feat/share-access-update-event branch January 22, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ArtificialOwl ArtificialOwl ArtificialOwl approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

Assignees

@provokateurin provokateurin

Labels

3. to review enhancement

Projects

None yet

Milestone

Nextcloud 33

Development

Successfully merging this pull request may close these issues.

4 participants

@provokateurin @icewind1991 @ArtificialOwl