feat: Digest mail notifications - eXIP7.3.0.22 - integration into feature/mips

[azayati]

eXIP 7.3.0.22 — Digest mail notifications · integration into feature/mips

The tasks category (assignment, coworker, mention) declared in Kernel XML, its email line plugin, and the removal of the legacy makeDigest overrides and of the superseded translation keys.

The 3 commit(s) replayed from feature/experience

• 4bb3c0bd feat: Declare the Tasks digest mail notifications category - EXO-89484 (feat: Declare the Tasks digest mail notifications category - EXO-89484 - eXIP7.3.0.22 #640)
• 5fd77e64 feat: Digest email lines for task notifications - EXO-89486_EXO-89487 (feat: Digest email lines for task notifications - EXO-89486_EXO-89487 - eXIP7.3.0.22 #642)
• d66e427f fix: Remove the legacy digest bodies and keys of the task notifications - EXO-90072 (fix: Remove the legacy digest bodies and keys of the task notifications - EXO-90072 - eXIP7.3.0.22 #644)

47 files changed, 339 insertions(+), 1341 deletions(-)

How this branch was built

exip-7.3.0.22-mips starts from origin/feature/mips and replays only the commits tagged eXIP7.3.0.22 (git cherry-pick -x, the original reference is in every message). The FB version-bump commit (Task-87990) and the commits of the other eXIPs present on feature/experience are deliberately left out — no pom.xml is touched.

Merge order

makeDigest is still an abstract method of commons-api on feature/mips; the other 18 PRs remove their overrides, so they only compile once commons is merged: commons#786 first (wait for the Nexus snapshot), then social#6093, then the 17 addons in any order.

Classification

N1 for the whole eXIP (Liquibase schema and JPA entities, the commons notification dispatcher, new REST endpoints, mass email sending). Approver ≠ author: this PR must be approved by an Architect / Senior Developer who knows it is N1, not on the AI review alone.

Already validated on feature/experience

Every US of the board (project 8372) is "Tested & Validated" by the PO, the legacy engine cleanup (EXO-90072, 19 repositories) included. Full functional test plan: capture, daily and weekly contents, timezones, catch-up at startup, safety cleanup, and non-regression of the instant notifications.

Knowledge: TODO — eng-standards PR to open (/domain-doc commons + social) before leaving draft

🤖 Generated with Claude Code

[azayati]

AI review — Round #1 — eXIP 7.3.0.22 Digest mail notifications (task category)

Reviewed as one delivery with the 18 sibling PRs, against Tech Spec note 50469 and board 8372, at head d66e427f.

🟢 Nit — notification-configuration.xml:389: the category <description> says "assignments, due dates, comments and mentions" while the category covers TaskAssignPlugin, TaskCoworkerPlugin, TaskMentionedPlugin only. Fix: "assignments, coworkers and mentions".

Verified conform: category tasks, order 40, the three spec plugin ids (matching the real ID constants); TaskDigestLinePlugin reads taskId/creator/taskUrl — the very keys the plugins write — returns no line on a vanished task or a bad id, and produces an absolute URL (stored http link or the permanent-link format); digest.category.tasks and the three digest.line.* keys in TaskNotification_en.properties, text only, wording == board US06/US07; the legacy Notification.digest.* block and the makeDigest/buildDigestMsg bodies and their tests are removed; 4 line-plugin tests.

Classification: N1 by aggregation with the eXIP (the diff itself touches no REST/DAO/schema/ACL surface). Compiles only once commons#786 is on Nexus. Knowledge: line still TODO.

🤖 Generated with Claude Code

[boubaker]

AI review — Round #2 (independent review, Architects Lead's reviewer)

Independent pass at 87b9ef36, as part of the 19-PR delivery reviewed against Tech Spec note 50469 and board 8372 (delivery summary on commons#786). The round-1 nit (category description) is ✅ fixed at head.

🟡 Medium — the task notifications carry no from, so the digest capture cannot leave the actor out; a self-mention reaches the digest

AbstractNotificationPlugin.java:86-105 — untouched by this PR, hence reported here rather than inline.

    return NotificationInfo.instance()
                           .to(new LinkedList<>(receivers))
                           ...
                           .with(NotificationUtils.CREATOR.getKey(), notificationCreator)   // the actor is a parameter, never `from`
                           .key(getKey()).end();

The capture excludes the actor only through StringUtils.equals(recipient, notification.getFrom()) (commons DigestServiceImpl.java:149) — spec §4 Capture: "the actor of the notification (its from) is never a recipient of his own item". TaskAssignPlugin and TaskCoworkerPlugin come out right by accident of getReceiver (receivers.remove(ctx.value(CREATOR)), L131-133); TaskMentionPlugin.getReceiver (TaskMentionPlugin.java:66-68) returns the raw MENTIONED set, so a user who @-mentions themself in a comment reads "John Smith mentioned you in …" about their own comment in their recap. Agenda received the same change in this eXIP for that reason (AgendaNotificationPlugin.java:89-94).

Fix: .setFrom(notificationCreator) in AbstractNotificationPlugin.makeNotification — the value is a username (TaskLoggingListener.java:195, TaskCommentNotificationListener.java:67); nothing in the on-site or mail lifecycles reads from, so the instant behaviours do not move.

🟢 Nit — notification-configuration.xml:76 and 5 more daily/weekly defaultConfig values, dead since UserSetting.FREQUENCY keeps INSTANTLY only (same nit on social#6093). Knowledge: still TODO.

Verified conform: category tasks (order 40) covers exactly the three spec plugin ids; TaskDigestLinePlugin reads taskId/creator/taskUrl as AbstractNotificationPlugin.java:89-95 writes them (taskId numeric, creator a username), guards Long.parseLong, gives no line on a vanished task and an absolute URL (stored http link or TaskPermanentLinkPlugin.URL_FORMAT); TaskService.getTask(long) has no ACL, acceptable here since the item was captured for that very recipient; bundle locale.notification.TaskNotification equals the PluginConfig.bundlePath and the three wordings equal board US06/US07; the legacy makeDigest/buildDigestMsg bodies, their tests and the Notification.digest.* keys are removed, no java.io.Writer left; 4 line-plugin tests.

Classification: N1 by aggregation with the eXIP (the diff itself touches no REST/DAO/schema/ACL surface). Compiles once commons#786 is on Nexus. Approver ≠ author, never on the AI review alone; Knowledge: line required before merge.

🤖 Generated with Claude Code

[azayati]

Thanks for the pass. Answers to both points, at head 2f675cf8.

🟡 No from on the task notifications — out of scope of this PR

AbstractNotificationPlugin.java and TaskMentionPlugin.java are not part of this eXIP's diff. git diff origin/feature/mips HEAD on either file returns nothing: both are identical to the merge target. The Java files this PR touches are TaskDigestLinePlugin, the two template providers and their tests. Nothing here was introduced or changed by the delivery, so the point does not belong to this review; it is not fixed here.

For the record, the behaviour it describes does not occur either. The actor never reaches the recipients of any of the three plugins the tasks category covers, and the capture iterates getSendToUserIds():

• TaskAssignPlugin and TaskCoworkerPlugin go through AbstractNotificationPlugin.getReceiver, which does receivers.remove(ctx.value(NotificationUtils.CREATOR));
• TaskMentionPlugin.getReceiver returns the MENTIONED set, and the only place that fills it removes the author first — TaskCommentNotificationListener line 111:

    Set<String> mentioned = comment.getMentionedUsers();
    if (mentioned == null) {
      mentioned = new HashSet<>();
    }
    mentioned.remove(creator);
    ...
    ctx.append(NotificationUtils.MENTIONED, mentioned);

git grep "append(NotificationUtils.MENTIONED" returns that single call site, so a user who @-mentions themself is not a recipient and cannot read it in their own recap. The spec rule holds here through the recipient lists rather than through from. Agenda received the from change in this eXIP because its recipients do contain the actor, which is what spec §3 asks of that addon; task's do not.

If you want every addon to declare the actor the same way regardless, that is a change of its own: a separate PR against its own task, since AbstractNotificationPlugin serves the whole task notification family and not only the three digest types.

🟢 Dead daily / weekly defaultConfig values — fixed

2f675cf8 removes the five daily values and the one weekly value. Both readers of that collection — NotificationPluginContainer.getDefaultActivePlugins and JPAUserSettingServiceImpl.getDefaultSettings — compare each value to UserSetting.FREQUENCY.INSTANTLY, and FREQUENCY holds INSTANTLY alone since the legacy digest engine was removed, so getFrequecy("daily") returns null and the value is silently ignored. Removing it changes no default activation: an ignored value and an absent value give the same empty result in both readers. The defaultConfig field of TaskCompletedPlugin held weekly alone, so it is dropped whole rather than left as an empty collection — an absent field and an empty list are the same thing for PluginConfig, whose defaultConfig initialises to an empty ArrayList. The file still parses (checked with a DOM parser after the edit) and every collection that carried Instantly keeps it.

The Knowledge: line is still to be filled on the 19 descriptions, with the eng-standards PR refreshing the domain docs.

[boubaker]

AI review — Round #3 (follow-up, final for the code)

Independent review, Architects Lead's reviewer, at head 2f675cf8 (diff 87b9ef36..2f675cf8: 22 lines removed in notification-configuration.xml, nothing else).

Status of the previous rounds

Round	Finding	Status
1 🟢	Category description names notifications the category does not cover	✅ fixed at 87b9ef36
2 🟡	No from on the task notifications; a self-mention reaches the digest	➖ Withdrawn — reviewer's error. The only producer of NotificationUtils.MENTIONED is TaskCommentNotificationListener.java:66,107-117: it takes creator from the current identity and does mentioned.remove(creator) before appending the set, so TaskMentionPlugin.getReceiver never contains the actor; TaskAssignPlugin and TaskCoworkerPlugin go through AbstractNotificationPlugin.getReceiver, which removes the creator too. The capture iterates getSendToUserIds(), so the spec rule holds through the recipient lists and from is not needed. Both files are byte-identical to feature/mips. The finding was taken from the plugin alone without following the listener that fills the set. No separate setFrom PR is needed.
2 🟢	Dead daily / weekly defaultConfig values	✅ fixed in 2f675cf8 — the 5 daily and 1 weekly values are gone, the defaultConfig field of TaskCompletedPlugin that held weekly alone is dropped whole (PluginConfig.defaultConfig initialises to an empty list), the 6 Instantly values are untouched, the file parses
2 🟢	Knowledge: line TODO	❌ still open — a delivery-level field of the 19 bodies, tracked on commons#786

New findings

None. The only hunk since the last round is the configuration cleanup.

Verified conform (unchanged from Round #2, re-checked at head): category tasks (order 40) covers exactly the three spec plugin ids; TaskDigestLinePlugin reads taskId / creator / taskUrl as AbstractNotificationPlugin writes them, guards Long.parseLong, gives no line on a vanished task and an absolute URL; bundle locale.notification.TaskNotification equals the PluginConfig.bundlePath, wordings equal board US06/US07; the legacy makeDigest / buildDigestMsg bodies, their tests and the Notification.digest.* keys are removed; 4 line-plugin tests.

All findings from the previous rounds are resolved or withdrawn on this PR; nothing outstanding from the AI review side on the code. The Knowledge: body line remains a delivery-level prerequisite before the feature/mips merge.

Classification: N1 by aggregation with the eXIP (the diff itself touches no REST/DAO/schema/ACL surface). Compiles once commons#786 is on Nexus. Approver ≠ author, never on the AI review alone.

🤖 Generated with Claude Code