Differential D14669

Make "Assign / Claim" stacked action work properly in Maniphest
ClosedPublic
Actions

Authored by epriestley on Dec 4 2015, 6:17 PM.

Details

Reviewers

chad

Maniphest Tasks

T9132: Build an ApplicationEditor abstraction

Commits

Restricted Diffusion Commit
rPf9e84d1a88af: Make "Assign / Claim" stacked action work properly in Maniphest

Summary

Ref T9132. This is kind of a mess because the tokenizer rewrite left rendering tokenizers in Javascript a little rough. This causes bugs like icons not showing up on tokens in the "Policy" dialog, which there's a task for somewhere I think.

I think I've fixed it enough that the beahavior is now correct (i.e., icons show up properly), but some of the code is a bit iffy. I'll eventually clean this up properly, but it's fairly well contained for now.

Test Plan

Reassigned a task.
Put a task up for grabs.
No reassign on closed tasks.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 35475.Dec 4 2015, 6:17 PM

epriestley retitled this revision from to Make "Assign / Claim" stacked action work properly in Maniphest.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T9132: Build an ApplicationEditor abstraction.

chad accepted this revision.Dec 4 2015, 6:45 PM

chad edited edge metadata.

chad added inline comments.

src/applications/maniphest/editor/ManiphestEditEngine.php
73	Can we split assign/claim now into two different actions?

This revision is now accepted and ready to land.Dec 4 2015, 6:45 PM

We can't trivially split them, since they'd be unlike other actions in two ways:

They'd be mutually exclusive, while all actions currently don't interact with other actions (although Differential actions probably will, when we get there).
The "Claim" action probably needs a new "don't render anything" control or something associated with it (not sure if you have ideas offhand for what the control should look like).

We can look at this once things are more stable. I also considered just dropping the "/ Claim" part since it's awkward and not really necessary, maybe, but I don't really know how much users are going to freak out yet and erred on the side of familiarity.

Closed by commit rPf9e84d1a88af: Make "Assign / Claim" stacked action work properly in Maniphest (authored by epriestley, committed by epriestley). · Explain WhyDec 5 2015, 12:31 AM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in T7848: Allow herald rules to change task status in HeraldManiphestTaskAdapter Actions.Dec 5 2015, 7:19 PM