Page MenuHomePhabricator

Add trigger rule for adding projects to a task
ClosedPublic

Authored by amckinley on Apr 5 2019, 7:55 PM.
Tags
None
Referenced Files
F14395625: D20379.diff
Sun, Dec 22, 4:49 AM
Unknown Object (File)
Sat, Dec 21, 11:40 AM
Unknown Object (File)
Thu, Dec 19, 2:11 AM
Unknown Object (File)
Wed, Dec 18, 11:47 AM
Unknown Object (File)
Sat, Dec 14, 2:37 PM
Unknown Object (File)
Tue, Dec 10, 12:29 PM
Unknown Object (File)
Tue, Dec 10, 4:08 AM
Unknown Object (File)
Mon, Dec 9, 10:03 AM
Subscribers

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
Branch
add-proj (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22533
Build 30854: Run Core Tests
Build 30853: arc lint + arc unit

Event Timeline

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.Apr 6 2019, 9:30 PM

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

Most of this code looks perfect to me!

This revision was automatically updated to reflect the committed changes.