Paths

Table of Contentst

Differential D25968

Mail Notifications: Do not CC disabled user accounts
Needs RevisionPublic
Actions

Authored by aklapper on Apr 21 2025, 19:51.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16033: "To:" and "CC:" Recipient Hint footers in mail notifications list disabled user accounts

Summary

Do not list disabled user accounts in mail footer (when metamta.recipients.show-hints is enabled) and do not add them to the X-Phabricator-Cc header, as they are not supposed to receive mail anyway per https://we.phorge.it/source/phorge/browse/master/src/applications/metamta/query/PhabricatorMetaMTAActor.php;07723b46274c6dbf062af070e5c19101537e0d91$130-131.

Closes T16033

Test Plan

Likely trigger notifications on a task or changeset with disabled user accounts being subscribed or members/watchers of a project. Not tested though as I do not have a local mail setup.

Diff Detail

Repository

rP Phorge

Branch

T16033disabledRecipients (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1880
Build 1880: arc lint + arc unit

Event Timeline

aklapper created this revision.Apr 21 2025, 19:51

Owners added a reviewer: O1: Blessed Committers.Apr 21 2025, 19:51

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptApr 21 2025, 19:51

aklapper requested review of this revision.Apr 21 2025, 19:51

Harbormaster completed remote builds in B1878: Diff 2892.Apr 21 2025, 19:51

Clean my local dir from other experiments

Harbormaster completed remote builds in B1879: Diff 2893.Apr 21 2025, 19:55

Don't check twice for account status; already covered by query parameter

Harbormaster completed remote builds in B1880: Diff 2894.Apr 21 2025, 19:57

I guess I should add a big This is untested banner here

valerio.bozzolan added inline comments.Apr 23 2025, 07:12

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
3459	Can we really assume that `$email_cc` always has the keys as this? Probably instead of just unset by key, it's necessary to search by value -> unset by its key.

Mark as "some comments"

This revision now requires changes to proceed.Sat, May 3, 16:36

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

applications/

transactions/

editor/

PhabricatorApplicationTransactionEditor.php

12 lines

O1: Blessed Committers

Diff 2894

View Options

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

Mail Notifications: Do not CC disabled user accountsNeeds RevisionPublicActions