mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 06:42:42 +01:00
Avoid "Action with no effect" for auto-claim statuses after manually removing assignee
Summary: Sometime you just want to flag something as Resolved and keep that task claimed by nobody. But, there are some task statuses that can auto-claim, and "Resolved" is one of these. So, if you "Resolved", Phorge tries to set yourself as claimer. Keeping that "claimed by nobody" is a bit tricky and also generates a confusing warning. In fact, after you "Resolved", you can override the defaults with: - Add Action > Assign / Claim > (nobody) The problem is, on saving, the above action causes this warning: > **Action With No Effect** > One of your actions has no effect: > The task already has the selected owner. > Apply remaining actions? > [ Cancel ] [ Apply Remaining Actions ] That warning "The task already has the selected owner" really means "The task is already claimed by nobody" and, indeed, that is exactly what the user wants. This patch intercepts the above action, and prevents the related confusing "non-effect" warning. Thanks to hard troubleshooting from user https://we.phorge.it/p/aklapper/ See also https://we.phorge.it/D25476 Closes T15164 Test Plan: Task 1 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, and **set <nothing>** 4. Save and, instead of any confusing warning, only the Status changes. Task 2 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, keep it as-is 4. Save and, it works as expected (just like before) Task 3 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, set to somebody else 4. Save and, it works as expected (just like before) Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15164 Differential Revision: https://we.phorge.it/D25550
This commit is contained in:
parent
faf43d7edf
commit
8fe3d68577
1 changed files with 18 additions and 11 deletions
|
@ -327,11 +327,11 @@ final class ManiphestTransactionEditor
|
|||
|
||||
$is_unassigned = ($object->getOwnerPHID() === null);
|
||||
|
||||
$any_assign = false;
|
||||
$any_xassign = null;
|
||||
foreach ($xactions as $xaction) {
|
||||
if ($xaction->getTransactionType() ==
|
||||
ManiphestTaskOwnerTransaction::TRANSACTIONTYPE) {
|
||||
$any_assign = true;
|
||||
$any_xassign = $xaction;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
@ -355,15 +355,22 @@ final class ManiphestTransactionEditor
|
|||
|
||||
// If the task is not assigned, not being assigned, currently open, and
|
||||
// being closed, try to assign the actor as the owner.
|
||||
if ($is_unassigned && !$any_assign && $is_open && $is_closing) {
|
||||
$is_claim = ManiphestTaskStatus::isClaimStatus($new_status);
|
||||
|
||||
// Don't assign the actor if they aren't a real user.
|
||||
// Don't claim the task if the status is configured to not claim.
|
||||
if ($actor_phid && $is_claim) {
|
||||
$results[] = id(new ManiphestTransaction())
|
||||
->setTransactionType(ManiphestTaskOwnerTransaction::TRANSACTIONTYPE)
|
||||
->setNewValue($actor_phid);
|
||||
// Don't assign the actor if they aren't a real user.
|
||||
if ($is_unassigned && $is_open && $is_closing && $actor_phid) {
|
||||
$is_autoclaim = ManiphestTaskStatus::isClaimStatus($new_status);
|
||||
if ($is_autoclaim) {
|
||||
if ($any_xassign === null) {
|
||||
$results[] = id(new ManiphestTransaction())
|
||||
->setTransactionType(ManiphestTaskOwnerTransaction::TRANSACTIONTYPE)
|
||||
->setNewValue($actor_phid);
|
||||
} else if ($any_xassign->getNewValue() === null) {
|
||||
// We have an explicit "Assign / Claim" = nothing in the frontend.
|
||||
// The user is trying to "undo" the above automatic auto-claim.
|
||||
// When saving, this would cause the "no effect" warning.
|
||||
// So we suppress that confusing warning.
|
||||
// https://we.phorge.it/T15164
|
||||
$any_xassign->setIgnoreOnNoEffect(true);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue