feat: allow users to get notified about changes in a collective#2541
feat: allow users to get notified about changes in a collective#2541mejo- wants to merge 3 commits into
Conversation
2e07f05 to
aa099f0
Compare
marcoambrosini
left a comment
There was a problem hiding this comment.
Hi Jonas, I think that the location you've chosen makes sense, I would maybe do it like in Talk though, using a sub-action-menu, so we have more space to be clear with labels and icons.
Screen.Recording.2026-05-29.at.16.51.45.mov
On top of that, this could be indeed put in the collective settings dialog. Just like in Talk, the dialog could be exposed to all users, and have the conditional display logic (owner/user) on settings section instead. Templates could also go in here.
db59a0f to
7b22b1e
Compare
Great idea @marcoambrosini. Implemented now, see the updated screenshots. |
That might be worthy activity information, but I am doubtful whether they are worth to create a notification. I would keep it as is for now. (Tagging might probably also need to discriminate between tag types and user privileges if it was implemented 🤔) |
Sends notifications if pages got updated or deleted. Doesn't send notifications whne a page got created on purpose as usually pages get renamed straight after they got created. Fixes: #317 Signed-off-by: Jonas <jonas@freesources.org>
Requires to install composer in Docker container as notifications app has run-time PHP dependencies. Signed-off-by: Jonas <jonas@freesources.org>
Adds a submenu to NcActionsCollectivesActions for selecting the notification level, like it's done in Nextcloud Talk. Signed-off-by: Jonas <jonas@freesources.org>
max-nextcloud
left a comment
There was a problem hiding this comment.
All in all looks good. Just a few minor comments.
Have not tested it yet though.
| export function setCollectiveUserSettingShowMembers(collectiveId, showMembers) { | ||
| return axios.put( | ||
| collectiveUserSettingsApiUrl(collectiveId, 'showMembers'), | ||
| collectiveUserSettingsApiUrl(collectiveId, ['showMembers']), |
There was a problem hiding this comment.
The 'showMembers' part should not need to be an array.
I think collectiveUserSettingsApiUrl is missing ...parts in the call to apiUrl. Also the type anotation for apiUrl might be off.
| // Collect direct user member IDs | ||
| $memberUserIds = []; | ||
| foreach ($circle->getMembers() as $member) { | ||
| // TODO: also notify indirect circle/team members | ||
| if ($member->getUserType() === Member::TYPE_USER) { | ||
| $memberUserIds[] = $member->getUserId(); | ||
| } | ||
| } | ||
|
|
||
| if (empty($memberUserIds)) { | ||
| return; | ||
| } | ||
|
|
||
| // Fetch all user settings for this collective in one query | ||
| $allSettings = $this->settingsMapper->findByCollectiveId($collective->getId()); | ||
| $notifyUserIds = []; | ||
| foreach ($allSettings as $setting) { | ||
| if ($setting->getSetting('notify') === CollectiveUserSettings::NOTIFY_ALL) { | ||
| $notifyUserIds[] = $setting->getUserId(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we could address the indirect members right away:
| // Collect direct user member IDs | |
| $memberUserIds = []; | |
| foreach ($circle->getMembers() as $member) { | |
| // TODO: also notify indirect circle/team members | |
| if ($member->getUserType() === Member::TYPE_USER) { | |
| $memberUserIds[] = $member->getUserId(); | |
| } | |
| } | |
| if (empty($memberUserIds)) { | |
| return; | |
| } | |
| // Fetch all user settings for this collective in one query | |
| $allSettings = $this->settingsMapper->findByCollectiveId($collective->getId()); | |
| $notifyUserIds = []; | |
| foreach ($allSettings as $setting) { | |
| if ($setting->getSetting('notify') === CollectiveUserSettings::NOTIFY_ALL) { | |
| $notifyUserIds[] = $setting->getUserId(); | |
| } | |
| } | |
| // Collect direct user member IDs in one query | |
| $memberUserIds = []; | |
| foreach ($circle->getMembers() as $member) { | |
| if ($member->getUserType() === Member::TYPE_USER) { | |
| $memberUserIds[] = $member->getUserId(); | |
| } | |
| } | |
| // Fetch all user settings for this collective in one query | |
| $allSettings = $this->settingsMapper->findByCollectiveId($collective->getId()); | |
| $notifyUserIds = []; | |
| foreach ($allSettings as $setting) { | |
| if ($setting->getSetting('notify') === CollectiveUserSettings::NOTIFY_ALL) { | |
| $userId = $setting->getUserId(); | |
| // Skip acting user | |
| if ($userId === $actingUserId) { | |
| continue; | |
| } | |
| if (in_array($userId, $memberUserIds, true)) { | |
| $notifyUserIds[] = $setting->getUserId(); | |
| continue; | |
| } | |
| // Check for indirect membership | |
| $circles = $this->circleHelper->getCircles($userId); | |
| $circleIds = array_map(static fn ($circle) => $circle->getSingleId(), $circles); | |
| if (in_array($collective->getCircleId(), $circleIds, true)) { | |
| $notifyUserIds[] = $setting->getUserId(); | |
| } | |
| } | |
| } |
| foreach ($memberUserIds as $userId) { | ||
| // Skip acting and non-notify users | ||
| if ($userId === $actingUserId) { | ||
| continue; | ||
| } | ||
| if (!in_array($userId, $notifyUserIds, true)) { | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
With the change proposed above we only need to iterate over $notifyUserIds as group membership is already accounted for.
| foreach ($memberUserIds as $userId) { | |
| // Skip acting and non-notify users | |
| if ($userId === $actingUserId) { | |
| continue; | |
| } | |
| if (!in_array($userId, $notifyUserIds, true)) { | |
| continue; | |
| } | |
| foreach ($notifyUserIds as $userId) { | |
Sends notifications if pages got updated or deleted. Doesn't send notifications when a page got created on purpose as usually pages get renamed straight after they got created.
Also allows to turn notifications off completely for a collective, which also silences notifications on being mentioned in a page.
Fixes: #317
🖼️ Screenshots
🚧 TODO
So far it only sends notifications on page updates (content updated, page renamed/moved/deleted, emoji changed). I wonder whether adding/removing members shall trigger a notification as well. And how about tagging/untagging pages etc.?
Also, so far there's only three states: "all changes", "@-mention only" (the default) and "off" (no notifications at all). I wonder whether we want to make this more fine-grained, but maybe that's something for a follow-up later on.
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)🤖 AI (if applicable)