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
F14476650: D18882.id45293.diff
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
Unknown Object (File)
Nov 26 2024, 7:14 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
Branch
bulk17
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 19060
Build 25725: Run Core Tests
Build 25724: arc lint + arc unit

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.