Page MenuHomePhorge

Add workboard column trigger to assign only unassigned tasks
Needs ReviewPublic

Authored by aklapper on Tue, May 13, 11:39.

Details

Summary

Allow defining a project workboard column trigger which sets a task assignee on move only when the task was previously unassigned.

This change requires / depends on D25991 (which allows defining Current User performing the column move instead of a specific user as assignee).

Closes T16064

Test Plan
  • Go to a project workboard like http://phorge.localhost/project/board/1/ and define a second column
  • On the non-default column, click the Cogs button in its column header and select "New Trigger..."
  • Set some random trigger name
  • In the Rules condition dropdown, select new "Assign unowned task to"
  • In the Rules condition value field, either set a user account (or write/select "viewer()" or "Current Viewer" if D25991 applied)
  • Click "Create Trigger"
  • Move tasks (both assigned and unassigned) into the column with the trigger, see preview in bottom right workboard corner, see that the card assignee avatar becomes the current user only when previously unassigned; open the task and see the timeline entry that the assignee was changed to the current user in case of previously unassigned tasks
  • Edit the trigger to include a second action (e.g. Change Priority); still works as expected
  • Move task back to previous column without trigger on workboard; no assignee changes
  • On the non-default column, click the Pencil button in its column header and select "Bulk Edit Tasks..."; in the Bulk Editor, under Bulk Edit Actions, see that no new option "Assign unowned task to" exists, as expected as this is only affects workboard triggers

Diff Detail

Repository
rP Phorge
Branch
T16064triggerUnassignedOnly (branched from master)
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningsrc/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php:3XHP87Class Not `abstract` Or `final`
Unit
Tests Passed
Build Status
Buildable 1976
Build 1976: arc lint + arc unit

Event Timeline

What's the expectation for classes which have a child class extending them? Marking them as abstract though they are really not abstract at all? protected class? Uhm.

I wonder if we should just extend PhabricatorProjectTriggerRule. So, leaving PhabricatorProjectTriggerManiphestOwnerRule as-is 🤔 any opinion about that?

That was my patch initially but it felt like too much duplicated code... I don't have a strong opinion though; I'm also fine with that other approach. :)