Page MenuHomePhabricator

Prepare TransactionEditor for silent transactions via bulk edit
ClosedPublic

Authored by epriestley on Jan 19 2018, 5:48 PM.
Tags
None
Referenced Files
F14480110: D18882.id45276.diff
Sun, Dec 29, 5:55 PM
Unknown Object (File)
Sat, Dec 28, 7:14 AM
Unknown Object (File)
Thu, Dec 26, 5:16 PM
Unknown Object (File)
Thu, Dec 26, 6:08 AM
Unknown Object (File)
Fri, Dec 13, 5:07 AM
Unknown Object (File)
Tue, Dec 10, 8:38 AM
Unknown Object (File)
Fri, Dec 6, 6:46 PM
Unknown Object (File)
Fri, Dec 6, 12:48 PM
Subscribers
None

Details

Summary

Ref T13042. This adds a "silent" edit mechanism which suppresses feed stories, email, and notifications.

The other behaviors here are:

  • The transactions are marked as "silent" so we can render a hint in the UI in the future to make it clear to users that they aren't missing email.
  • If the editor uses Herald, mail rules are suppressed so they don't fire incorrectly (this mostly affects "the first time this rule matches, send me an email" rules: without this, they'd match "the first time" on the bulk edit, not send email, then never match again since they already matched).
  • If the edit queues additional edits, those are applied silently too.

This doesn't (or, at least, shouldn't) actually change any behavior since you can't apply silent edits yet.

Test Plan

Somewhat theoretical, since this isn't reachable yet. Should get meaningful testing in an upcoming change.

Did a bit of var_dump() / debug poking to attempt to verify that nothing too crazy is happening.

Viewed and edited objects, no changes in behavior.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/applications/herald/state/HeraldCoreStateReasons.php
12

"is being applied"?

src/applications/transactions/storage/PhabricatorApplicationTransaction.php
162

Should this be HeraldCoreStateReasons:: REASON_SILENT?

166

Same as above.

This revision is now accepted and ready to land.Jan 19 2018, 8:01 PM
  • Juggle tense/grammar on Herald messaging.
  • core.silent on transactions and the Herald reason aren't technically the same semantically so I left them as-is for the moment. We could extract the transactions strings somewhere else, but nothing else should ever be using them directly anyway.