Page MenuHomePhabricator

Add trigger rule for adding projects to a task
ClosedPublic

Authored by amckinley on Fri, Apr 5, 7:55 PM.

Details

Summary

Ref T13269. This is mostly copying code from the similar Herald implementation. Note that the drop effect preview always renders because we don't have the infrastructure to compare lists of edge targets.

Test Plan

Created some triggers, dragged some tasks around, checked that tasks that already had project membership didn't write additional edges.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

amckinley created this revision.Fri, Apr 5, 7:55 PM
amckinley requested review of this revision.Fri, Apr 5, 7:56 PM
epriestley accepted this revision.Sat, Apr 6, 9:30 PM

Note that the drop effect preview always renders because we don't have the infrastructure to compare lists of edge targets.

Oh, perfect -- yeah, this seems like a totally reasonable starting point to me per discussion elsewhere.

This (and the corresponding "remove" rule) will get us into a couple weird situations:

  • If you add a milestone or subproject of the current project, the card will teleport out of the current project and into the milestone/subproject.
  • If you remove the current project, the card will teleport off the board.

In both cases, I think "teleport" currently means "card stays visible, but vanishes on reload".

In both cases, I think this will probably be reasonably obvious what you're doing most of the time, and it's sort of legitimate if you want to make like an "Take it Out Behind the Shed" column that plays a sound and removes the current project or something. We could maybe warn/refine these cases later ("This trigger sends cards to the shadow realm, which is a legal operation, but just a heads up in case you didn't mean to do that.")

src/applications/project/trigger/PhabricatorProjectTriggerAddProjectsRule.php
30–44

I think you should be able to drop this logic and just apply the transaction unconditionally -- the Editor later on will do the right thing in terms of reducing or mooting the transaction. We might need a setContinueOnNoEffect(true) somewhere, but the TriggerRule shouldn't need to care about the current state of the object.

This revision is now accepted and ready to land.Sat, Apr 6, 9:30 PM
amckinley updated this revision to Diff 48656.Wed, Apr 10, 7:02 PM

Change rule to unconditionally add projects in transaction and let the editor figure it out.

Most of this code looks perfect to me!

amckinley updated this revision to Diff 48658.Wed, Apr 10, 7:06 PM

ok this time i got it

This revision was automatically updated to reflect the committed changes.