Page MenuHomePhorge

Avoid "Action with no effect" for auto-claim statuses after manually removing assignee
ClosedPublic

Authored by valerio.bozzolan on Mar 4 2024, 14:22.

Details

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)

Diff Detail

Repository
rP Phorge
Branch
arcpatch-D25550
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1108
Build 1108: arc lint + arc unit

Event Timeline

keep the original optimization about $is_claim, defined only in case

I'm a bit happy that:

  1. we gained a bit more readability than legacy code in my opinion, now without negations, without "is not something?" logics etc. also without mixing phases (example: the $actor_phid check should really had to be in original 358)
  2. the variable is not called anymore $is_claim so it cannot be confused anymore with "is actor claiming" (since it's not - it's just the status that suggests to do so)
  3. All logic is in one place. if (possible) { auto-claim } else { don't }

I forgot to invite the original author. This modification would not exist without aklapper. Eternal glory to aklapper.

Thanks, this is way cleaner (and slightly more performant) than my D25476. I've tested this locally, also in combination with other actions (e.g. opening another action field without changing it) and behavior is as expected.

This revision is now accepted and ready to land.Mar 13 2024, 14:24