mirror of
https://github.com/wekan/wekan.git
synced 2026-11-04 17:10:08 -05:00
Honor scoped notification subscriptions and settings.
Keep muted board assignments silent while honoring explicit list and card subscriptions. Resolve notification controls against their owning settings scope and show the selected option. Thanks to Nissulya and xet7 ! Fixes #6658,
This commit is contained in:
1 parent
959f15cbe0
commit
03e8a6bf66
10 files changed
+120
-24
No files matched your search
@@ -19,19 +19,19 @@ template(name="notificationSettingsPopup")
|
||||
| {{_ labelKey}}
|
||||
.notification-settings-popup-options
|
||||
if isAdminScope
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="true" class="{{#if onSelected}}is-checked{{/if}}")
|
||||
.materialCheckBox
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="true")
|
||||
.materialCheckBox(class="{{#if onSelected}}is-checked{{/if}}")
|
||||
span {{_ 'yes'}}
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="false" class="{{#if offSelected}}is-checked{{/if}}")
|
||||
.materialCheckBox
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="false")
|
||||
.materialCheckBox(class="{{#if offSelected}}is-checked{{/if}}")
|
||||
span {{_ 'no'}}
|
||||
else
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="" class="{{#if inheritSelected}}is-checked{{/if}}")
|
||||
.materialCheckBox
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="")
|
||||
.materialCheckBox(class="{{#if inheritSelected}}is-checked{{/if}}")
|
||||
span {{_ 'default'}}
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="true" class="{{#if onSelected}}is-checked{{/if}}")
|
||||
.materialCheckBox
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="true")
|
||||
.materialCheckBox(class="{{#if onSelected}}is-checked{{/if}}")
|
||||
span {{_ 'yes'}}
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="false" class="{{#if offSelected}}is-checked{{/if}}")
|
||||
.materialCheckBox
|
||||
a.flex.js-notify-option(data-service="{{service}}" data-value="false")
|
||||
.materialCheckBox(class="{{#if offSelected}}is-checked{{/if}}")
|
||||
span {{_ 'no'}}
|
||||
@@ -35,10 +35,10 @@ function currentValue(scope, service) {
|
||||
|
||||
Template.notificationSettingsPopup.helpers({
|
||||
isAdminScope() {
|
||||
return Template.currentData().scope === 'admin';
|
||||
return Template.instance().data.scope === 'admin';
|
||||
},
|
||||
notifyServiceRows() {
|
||||
const scope = Template.currentData().scope;
|
||||
const scope = Template.instance().data.scope;
|
||||
return Object.keys(NOTIFICATION_SERVICES).map(service => {
|
||||
const value = currentValue(scope, service);
|
||||
return {
|
||||
@@ -60,7 +60,7 @@ Template.notificationSettingsPopup.events({
|
||||
const service = target.dataset.service;
|
||||
const rawValue = target.dataset.value;
|
||||
const value = rawValue === 'true' ? true : rawValue === 'false' ? false : null;
|
||||
const scope = Template.currentData().scope;
|
||||
const scope = instance.data.scope;
|
||||
|
||||
if (scope === 'admin') {
|
||||
// Inherit is not offered at admin scope - it IS the base default - so
|
||||
|
||||
@@ -529,7 +529,7 @@ Template.boardMenuPopup.events({
|
||||
'click .js-change-background-image': Popup.open('boardChangeBackgroundImage'),
|
||||
'click .js-open-notification-settings': Popup.open('notificationSettings', {
|
||||
titleKey: 'notifications',
|
||||
scope: 'board',
|
||||
dataContext: { scope: 'board' },
|
||||
}),
|
||||
'click .js-board-info-on-my-boards': Popup.open('boardInfoOnMyBoards'),
|
||||
'click .js-change-language': Popup.open('changeLanguage'),
|
||||
|
||||
@@ -93,7 +93,7 @@ Template.memberMenuPopup.events({
|
||||
'click .js-change-color': Popup.open('changeColor'),
|
||||
'click .js-open-notification-settings': Popup.open('notificationSettings', {
|
||||
titleKey: 'notifications',
|
||||
scope: 'member',
|
||||
dataContext: { scope: 'member' },
|
||||
}),
|
||||
'click .js-change-font': Popup.open('changeFont'),
|
||||
'click .js-change-avatar': Popup.open('changeAvatar'),
|
||||
|
||||
+1
-1
@@ -116,7 +116,7 @@ window.Popup = new (class {
|
||||
title: self._getTitle(popupName, openOptions.titleKey),
|
||||
depth: self._stack.length,
|
||||
offset: self._getOffset(openerElement, popupName),
|
||||
dataContext: (this && this.currentData && this.currentData()) || (options && options.dataContextIfCurrentDataIsUndefined) || this,
|
||||
dataContext: openOptions.dataContext || (this && this.currentData && this.currentData()) || (options && options.dataContextIfCurrentDataIsUndefined) || this,
|
||||
});
|
||||
|
||||
const $contentWrapper = $('.content-wrapper')
|
||||
|
||||
@@ -4,11 +4,11 @@ const NOTIFYING_LEVELS = new Set(['watching', 'tracking']);
|
||||
|
||||
/**
|
||||
* Keep notification candidates who are active board members and whose board
|
||||
* watch level permits notifications. A missing watcher entry has the board's
|
||||
* default `muted` meaning and must not be bypassed by assignment, mentions,
|
||||
* card/list watching or BIGEVENTS_PATTERN.
|
||||
* watch level permits notifications. Explicit list/card subscriptions opt in
|
||||
* to that narrower scope even when the board itself is muted. Assignment,
|
||||
* mentions and BIGEVENTS_PATTERN alone must not bypass a muted board.
|
||||
*/
|
||||
function boardNotificationRecipients(candidates, members, boardWatchers) {
|
||||
function boardNotificationRecipients(candidates, members, boardWatchers, scopedWatchers = []) {
|
||||
const activeMemberIds = new Set(
|
||||
(members || []).filter(member => member.isActive === true).map(member => member.userId),
|
||||
);
|
||||
@@ -17,6 +17,7 @@ function boardNotificationRecipients(candidates, members, boardWatchers) {
|
||||
.filter(watcher => NOTIFYING_LEVELS.has(watcher.level))
|
||||
.map(watcher => watcher.userId),
|
||||
);
|
||||
scopedWatchers.forEach(userId => notifyingUserIds.add(userId));
|
||||
|
||||
return [...new Set(candidates || [])].filter(
|
||||
userId => activeMemberIds.has(userId) && notifyingUserIds.has(userId),
|
||||
|
||||
@@ -68,6 +68,7 @@ Activities.after.insert(async (userId, doc) => {
|
||||
const activity = Activities._transform(doc);
|
||||
let participants = [];
|
||||
let watchers = [];
|
||||
const scopedWatchers = new Set();
|
||||
let title = 'act-activity-notify';
|
||||
const board = activity.boardId
|
||||
? (await ReactiveCache.getBoard(activity.boardId)) || (await Boards.findOneAsync(activity.boardId))
|
||||
@@ -124,6 +125,7 @@ Activities.after.insert(async (userId, doc) => {
|
||||
const list = await activity.list();
|
||||
if (list) {
|
||||
if (list.watchers !== undefined) {
|
||||
(list.watchers || []).forEach(id => scopedWatchers.add(id));
|
||||
watchers = [...new Set([...watchers, ...(list.watchers || [])])];
|
||||
}
|
||||
params.list = normalizeActivityText(list.title);
|
||||
@@ -134,6 +136,7 @@ Activities.after.insert(async (userId, doc) => {
|
||||
if (activity.oldListId) {
|
||||
const oldList = await activity.oldList();
|
||||
if (oldList) {
|
||||
(oldList.watchers || []).forEach(id => scopedWatchers.add(id));
|
||||
watchers = [...new Set([...watchers, ...(oldList.watchers || [])])];
|
||||
params.oldList = normalizeActivityText(oldList.title);
|
||||
params.oldListId = activity.oldListId;
|
||||
@@ -163,6 +166,7 @@ Activities.after.insert(async (userId, doc) => {
|
||||
...(card.assignees || []),
|
||||
])];
|
||||
watchers = [...new Set([...watchers, ...(card.watchers || [])])];
|
||||
(card.watchers || []).forEach(id => scopedWatchers.add(id));
|
||||
params.card = normalizeActivityText(card.title);
|
||||
title = ACTIVITY_NOTIFICATION_TITLE.CARD;
|
||||
if (typeof card.absoluteUrl === 'function') {
|
||||
@@ -396,14 +400,14 @@ Activities.after.insert(async (userId, doc) => {
|
||||
}
|
||||
}
|
||||
|
||||
// #6658: Muted is the board's final notification boundary. Direct
|
||||
// assignment (#5833), mentions, list/card watchers and BIGEVENTS_PATTERN
|
||||
// may nominate a recipient, but none may override that recipient's board
|
||||
// watch level. No watcher entry also means the default `muted` level.
|
||||
// #6658: mute board-wide notifications, while honoring an explicit
|
||||
// subscription to this list/card. Assignment and mentions alone do not
|
||||
// opt a muted board member back in.
|
||||
watchers = boardNotificationRecipients(
|
||||
watchers,
|
||||
board.members,
|
||||
board.watchers,
|
||||
[...scopedWatchers],
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -39,6 +39,23 @@ assert.deepEqual(
|
||||
'Muted/default-muted, inactive and non-member candidates stay silent',
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
boardNotificationRecipients(
|
||||
['muted', 'default-muted', 'inactive', 'not-a-member', 'tracking'],
|
||||
members,
|
||||
boardWatchers,
|
||||
['muted', 'default-muted', 'inactive', 'not-a-member'],
|
||||
),
|
||||
['muted', 'default-muted', 'tracking'],
|
||||
'Explicit list/card subscriptions survive board mute but never bypass membership',
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
boardNotificationRecipients(['muted'], members, boardWatchers, ['default-muted']),
|
||||
[],
|
||||
'An unrelated subscription does not nominate a recipient or unmute assignments',
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
boardNotificationRecipients(undefined, undefined, undefined),
|
||||
[],
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
'use strict';
|
||||
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const vm = require('node:vm');
|
||||
const source = fs.readFileSync('client/components/settings/notificationSettingsPopup.js', 'utf8');
|
||||
let helpers, events, instance;
|
||||
const calls = [];
|
||||
const board = { setNotifyOverride: (...args) => calls.push(['board', ...args]) };
|
||||
const user = { profile: {}, setNotifyOverride: (...args) => calls.push(['member', ...args]) };
|
||||
vm.runInNewContext(source.replace(/^import .*;\n/gm, ''), {
|
||||
Template: {
|
||||
notificationSettingsPopup: {
|
||||
helpers: value => { helpers = value; },
|
||||
events: value => { events = value; },
|
||||
},
|
||||
instance: () => instance,
|
||||
// Inside #each this is the service row, not the panel scope.
|
||||
currentData: () => ({ service: 'email' }),
|
||||
},
|
||||
ReactiveCache: {
|
||||
getCurrentSetting: () => ({ notifyDefaultEmail: false }),
|
||||
getBoard: () => board,
|
||||
getCurrentUser: () => user,
|
||||
},
|
||||
Utils: { getCurrentBoardId: () => 'board-id' },
|
||||
NOTIFICATION_SERVICES: { email: {}, tray: {} },
|
||||
Meteor: { call: (...args) => calls.push(args) },
|
||||
});
|
||||
for (const scope of ['admin', 'board', 'member']) {
|
||||
instance = { data: { scope } };
|
||||
assert.equal(helpers.isAdminScope(), scope === 'admin');
|
||||
for (const [raw, value] of [['true', true], ['false', false], ['', null]]) {
|
||||
if (scope === 'admin' && value === null) continue;
|
||||
calls.length = 0;
|
||||
events['click .js-notify-option']({
|
||||
preventDefault() {}, currentTarget: { dataset: { service: 'email', value: raw } },
|
||||
}, instance);
|
||||
assert.deepEqual(calls, [[scope === 'admin' ? 'setAdminNotifyDefault' : scope, 'email', value]]);
|
||||
}
|
||||
}
|
||||
for (const [file, scope] of [['users/userHeader', 'member'], ['sidebar/sidebar', 'board']]) {
|
||||
assert.ok(fs.readFileSync(`client/components/${file}.js`, 'utf8').includes(`dataContext: { scope: '${scope}' }`));
|
||||
}
|
||||
assert.match(fs.readFileSync('client/lib/popup.js', 'utf8'), /dataContext: openOptions\.dataContext \|\|/);
|
||||
const jade = fs.readFileSync('client/components/settings/notificationSettingsPopup.jade', 'utf8');
|
||||
assert.doesNotMatch(jade, /a\.flex\.js-notify-option[^\n]*is-checked/);
|
||||
assert.match(jade, /\.materialCheckBox\(class="\{\{#if onSelected/);
|
||||
console.log('Notification settings route each row to the owning panel scope');
|
||||
@@ -0,0 +1,25 @@
|
||||
'use strict';
|
||||
const { test, expect } = require('../fixtures');
|
||||
const db = require('../helpers/db');
|
||||
|
||||
test('#6658 notification options persist in the member and board scopes', async ({ boardPage: page, user, board }) => {
|
||||
for (const scope of ['member', 'board']) {
|
||||
await page.evaluate(scope => {
|
||||
Popup.close();
|
||||
const opener = document.body;
|
||||
Popup.open('notificationSettings', { dataContext: { scope } })({
|
||||
currentTarget: opener, target: opener, preventDefault() {},
|
||||
});
|
||||
}, scope);
|
||||
const popup = page.locator('.notification-settings-popup');
|
||||
const option = popup.locator('[data-service="email"][data-value="false"]');
|
||||
await option.click();
|
||||
await expect(option.locator('.materialCheckBox')).toHaveClass(/is-checked/);
|
||||
const readValue = () => scope === 'member'
|
||||
? db.findOne('users', { _id: user.id }).profile.notifyOverrideEmail
|
||||
: db.findOne('boards', { _id: board.boardId }).notifyOverrideEmail;
|
||||
await expect.poll(readValue).toBe(false);
|
||||
await popup.locator('[data-service="email"][data-value=""]').click();
|
||||
await expect.poll(readValue).toBeUndefined();
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user