Page MenuHomePhorge

Workboard Trigger Rule: allow to Add/Remove Subscribers
ClosedPublic

Authored by valerio.bozzolan on Mar 22 2023, 14:17.

Details

Summary

This is a natural expansion of the Workboard Trigger system
to support actions about Subscribers.

Context: even before this change - Workboard users
were already able to move Tasks to Columns with
nice automatisms. For instance, you can already:

  • Play a nice Sound
  • Change Task Priority
  • Change Task Status
  • Add or Remove a Task Owner
  • Add or Remove a Task Project Tag

With this change, you can also:

  • Add or Remove Task Subscribers

If you need inspiration, this feature is useful for adding
more eyes on a given work area; lighten the notifications
of certain people after certain workflows have already been
done, to increase happiness and mitigate Burnout.

If your goal is to oppress yourself or your coworkers - of
course you can also use this feature to increase the Burnout
of yourself or your coworkers, adding random people as
Subscriber.

Note: every trigger action can be confusing in general, but
it is the fault of Triggers in general and not related to
this specific feature - that is totally loveable,
just like you.

Closes T15162

Test Plan
  • Workboard > Column > Create Trigger
  • Add Action > Add Subscriber > (your best friend who hates NodeJS)
  • Add Action > Remove Subscriber > (your antagonist who hates PHP)
  • Move some Tasks there and here, and note that Add/Remove works

Diff Detail

Repository
rP Phorge
Branch
trigger-subscriber
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 145
Build 145: arc lint + arc unit

Unit TestsFailed

TimeTest
693 msPhabricatorCelerityTestCase::testCelerityMaps
Assertion failed, expected 'true' (at PhabricatorCelerityTestCase.php:32): When this test fails, it means the Celerity resource map is out of date. Run `bin/celerity map` to rebuild it. ACTUAL VALUE
948 msPhabricatorLibraryTestCase::testLibraryMap
Assertion failed, expected 'true' (at PhutilLibraryTestCase.php:51): The library map is out of date. Rebuild it with `arc liberate`. These entries differ: class.PhabricatorProjectTriggerAddSubscriberRule, class.PhabricatorProjectTriggerRemoveSubscriberRule, xmap.PhabricatorProjectTriggerAddSubscriberRule, xmap.PhabricatorProjectTriggerRemoveSubscriberRule.
607 msPhabricatorConduitTestCase::testConduitMethods
1 assertion passed.
227 msPhabricatorInfrastructureTestCase::testApplicationsInstalled
1 assertion passed.
230 msPhabricatorInfrastructureTestCase::testRejectMySQLNonUTF8Queries
1 assertion passed.
View Full Test Results (2 Failed · 28 Passed)

Event Timeline

I will be 100% honest with you: for unknown reasons this works, even if I totally have no idea how. I just copy-pasted stuff and put everything in a standard kitchen blender.

Small capitalization nitpick, seems all other options use sentence case instead of title case:

Screenshot from 2024-04-11 17-09-44.png (472×564 px, 37 KB)

Same for some other strings here, e.g. You must select at least one User or Project Tag.

Also, Subscriber Rule value should be a list should probably for consistency be Add subscriber rule value should be a list respectively Remove subscriber rule value should be a list.

UI wise this seems to work as expected, and I think there's no way to move stuff on a workboard via Conduit. Just needs some smaller string adjustments.

fix red color for removal (like in PhabricatorProjectTriggerRemoveProjectsRule)

I still think that the two Subscriber Rule value should be a list strings should for consistency be Add subscriber rule value should be a list respectively Remove subscriber rule value should be a list.

After that it looks like this is ready to get merged.

I still think that the two Subscriber Rule value should be a list strings should for consistency be Add subscriber rule value should be a list respectively Remove subscriber rule value should be a list.

Eeeh but in that way it's 84 and 87 chars long. Amend welcome btw :)

Giving a +1 as I had tested this in D25080#16527 but I would prefer to see the two strings adjusted in D25080#16549

This revision is now accepted and ready to land.May 10 2024, 09:10

adjust strings as suggested, also, renamed classes from "subscriber" to "subscribers"