Column triggers: Allow assigning user who performs the move
AcceptedPublic
Actions

Authored by aklapper on Tue, May 6, 17:42.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16058: Workboard column triggers: Allow assigning to user performing the move

Summary

Allow defining a project workboard column trigger which sets a "dynamic" assignee, similar to the "Current User" functionality already existing in other "Assigned to" fields, for example Maniphest's Advanced Search form.

The logic is similar to PhabricatorPeopleNoOwnerDatasource; it defines const FUNCTION_TOKEN = 'viewer()' to avoid Value for "Assign task to" rule is invalid: User PHID ("viewer()") is not a valid user.

Note: In the meantime my user clarified that they do not only want every task to always be assigned in a column but are interested in only assigning tasks to the current user when the task is currently unassigned. That would be a separate followup patch obviously.

Closes T16058

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 "Assign task to"
In the Rules condition value field, write/select "viewer()" or "Current Viewer"
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; open the task and see the timeline entry that the assignee was changed to the current user
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, select "Assign to" in the dropdown, type "viewer()" in the username field, no such option exists as expected

Diff Detail

Repository

rP Phorge

Branch

T16058assignColumnMoveCurrent (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1946
Build 1946: arc lint + arc unit

Event Timeline

aklapper created this revision.Tue, May 6, 17:42

Owners added a reviewer: O1: Blessed Committers.Tue, May 6, 17:42

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptTue, May 6, 17:42

aklapper requested review of this revision.Tue, May 6, 17:42

Harbormaster completed remote builds in B1938: Diff 2989.Tue, May 6, 17:42

valerio.bozzolan added inline comments.Wed, May 7, 07:25

src/applications/people/typeahead/PhabricatorViewerDatasource.php
13	We can add "stuff()" in apixes, like here https://we.phorge.it/source/phorge/browse/master/src/applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php$13
13	Bonus point: it's probably even better to introduce `pht('Type "%s"...', 'viewer()');` , so we do not introduce yet another hardcoded translation to be partially translated, and we give more creative space to uniform the others following this.

aklapper marked 2 inline comments as done.Wed, May 7, 08:23

aklapper added inline comments.

src/applications/people/typeahead/PhabricatorViewerDatasource.php

Yes but that would be a separate task/patch to fix the current inconsistency:

[acko@foo phorge (master $|u=)]$ grep -r " getPlaceholderText" -A1 . | grep "return pht" | grep "()"
./src/infrastructure/customfield/datasource/PhabricatorCustomFieldApplicationSearchNoneFunctionDatasource.php-    return pht('Type "none()"...');
./src/infrastructure/customfield/datasource/PhabricatorCustomFieldApplicationSearchAnyFunctionDatasource.php-    return pht('Type "any()"...');
./src/applications/maniphest/typeahead/ManiphestTaskClosedStatusDatasource.php-    return pht('Type closed()...');
./src/applications/maniphest/typeahead/ManiphestTaskOpenStatusDatasource.php-    return pht('Type open()...');
./src/applications/people/typeahead/PhabricatorPeopleAnyOwnerDatasource.php-    return pht('Type "anyone()"...');
./src/applications/people/typeahead/PhabricatorViewerDatasource.php-    return pht('Type viewer()...');
./src/applications/project/typeahead/PhabricatorProjectLogicalOnlyDatasource.php-    return pht('Type only()...');
./src/applications/project/typeahead/PhabricatorProjectLogicalViewerDatasource.php-    return pht('Type viewerprojects()...');
./src/applications/differential/typeahead/DifferentialRevisionClosedStatusDatasource.php-    return pht('Type closed()...');
./src/applications/differential/typeahead/DifferentialResponsibleViewerFunctionDatasource.php-    return pht('Type viewer()...');
./src/applications/differential/typeahead/DifferentialRevisionOpenStatusDatasource.php-    return pht('Type open()...');
./src/applications/calendar/typeahead/PhabricatorCalendarInviteeViewerFunctionDatasource.php-    return pht('Type viewer()...');

This also

src/applications/maniphest/typeahead/ManiphestAssigneeDatasource.php
11

valerio.bozzolan accepted this revision.Wed, May 7, 08:57

valerio.bozzolan added inline comments.

src/applications/project/trigger/PhabricatorProjectTriggerManiphestOwnerRule.php
132	The translation problem in my opinion is not here (that is good in pht), but in old line 132. Maybe better to adopt pht().
132	Feel free to ignore this tip. It's just to say that the new line 133 makes sense under pht(), and the problem is not there in my opinion but here. Maybe the TODO could be added here, or we can improve this right now, but leaving pht() in new line 133 maybe.