Page MenuHomePhabricator
Differential D8565

Reload subscribers before sending mail, etc
ClosedPublic
Actions

Authored by epriestley on Mar 18 2014, 10:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 2:07 AM
Unknown Object (File)
Sun, Apr 21, 2:07 AM
Unknown Object (File)
Sun, Apr 21, 1:43 AM
Unknown Object (File)
Sun, Apr 21, 1:22 AM
Unknown Object (File)
Thu, Apr 11, 10:12 AM
Unknown Object (File)
Wed, Apr 10, 6:56 AM
Unknown Object (File)
Thu, Mar 28, 2:11 PM
Unknown Object (File)
Mar 5 2024, 4:41 AM
View All Files
Subscribers
epriestley
sophiebits

Details

Reviewers
btrahan
Maniphest Tasks
T4629: Herald: Not receiving email on diff creation
Commits
Restricted Diffusion Commit
rP833b8a78d89f: Reload subscribers before sending mail, etc
Summary

Fixes T4629. CCs added by Herald don't get added to the cached subscriber list. Just reload subscribers before sending mail to pick up effects.

Test Plan

Created an "always add X as CC" Herald rule for revisions, created a revision, saw them get initial mail.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley updated this revision to Diff 20330.Mar 18 2014, 10:51 PM
epriestley retitled this revision from to Reload subscribers before sending mail, etc.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley added a task: T4629: Herald: Not receiving email on diff creation.
Herald added a subscriber: epriestley.Mar 18 2014, 10:51 PM
btrahan accepted this revision.Mar 18 2014, 10:57 PM
btrahan edited edge metadata.

I was worried about doing this but I can't think of any cases where subscribers are added and *should not* be notified at the time they are added...? Maybe some folks might report getting emails when they subscribe to stuff and they weren't before or something.

This revision is now accepted and ready to land.Mar 18 2014, 10:57 PM
epriestley added a comment.Mar 18 2014, 11:01 PM

I think it has always been intended to work like this in all cases, it just got a bit mucked up with T2222 and the Herald / sub-editor stuff interacting with the local cache on the Editor.

epriestley closed this revision.Mar 18 2014, 11:01 PM
epriestley updated this revision to Diff 20331.

Closed by commit rP833b8a78d89f (authored by @epriestley).

Revision Contents
Changeset List

PathSize
src/
applications/
transactions/
editor/
5 lines

Diff 20331

View Options

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

Loading...
Phabricator · Built by Phacility