Page MenuHomePhabricator

Implement "Resign" action against ApplicationTransactions
ClosedPublic

Authored by epriestley on Feb 25 2014, 2:27 AM.
Tags
None
Referenced Files
F13087430: D8328.diff
Thu, Apr 25, 12:58 AM
Unknown Object (File)
Sat, Apr 20, 6:29 PM
Unknown Object (File)
Thu, Apr 11, 10:30 AM
Unknown Object (File)
Mon, Apr 8, 9:23 AM
Unknown Object (File)
Sat, Mar 30, 6:09 PM
Unknown Object (File)
Feb 17 2024, 7:23 PM
Unknown Object (File)
Feb 10 2024, 10:26 AM
Unknown Object (File)
Feb 10 2024, 10:10 AM
Subscribers

Details

Summary

Ref T2222. This introduces two small new concepts:

  • expandTransactions(): allows a transaction to expand into several transactions. For example, "resign" adds a "remove reviewers" transaction.
    • We have some other cases which could use this, but currently hard-code things outside of the Editor.
      • One example is that in Maniphest, closing a task implies claiming it if it is unowned.
  • setIgnoreOnNoEffect(): The whole Editor can be set to continue or stop if any transactions have no effect, but this allows the behavior to be refined at the individual transaction level. This is primarily to make the UX less confusing, so the user gets only a single relevant error instead of one for each expanded transaction.

Otherwise, this is pretty straightforward.

Test Plan

Rigged comment form to use SavePro controller, enabled resign action, then tried to resign from a bunch of stuff.

{F117743}

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I am probably missing something with my inline.

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
873

no array_mergev or anything? the array of transactions becomes an array of arrays of transactions so i figure eg combineTransactions isn't working since it used the earlier format?

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
868

We iterate over the array of return values here, so $expanded is the individual transactions.

We could array_mergev() this instead but this seemed easier to read when I was writing it. :P

Let me rewrite this slightly for clarity, I was confused by this too in the cold light of morning when I saw your comment.