Page MenuHomePhabricator

Mostly make the editor UI for triggers work
ClosedPublic

Authored by epriestley on Mar 21 2019, 4:42 PM.

Details

Summary

Ref T5474. This provides a Herald-like UI for editing workboard trigger rules.

This probably has some missing pieces and doesn't actually save anything to the database yet, but the basics at least roughly work.

Test Plan

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

epriestley created this revision.Mar 21 2019, 4:42 PM
epriestley requested review of this revision.Mar 21 2019, 4:44 PM
amckinley accepted this revision.Mar 25 2019, 7:37 PM

I'm gonna guess a lot of this JS is more-or-less copy/pasted from the Herald editor? Is there enough code in common to factor out into something more reusable?

src/applications/project/controller/trigger/PhabricatorProjectTriggerEditController.php
68–69

These are unused, I'm assuming because we aren't yet writing anything to the DB.

This revision is now accepted and ready to land.Mar 25 2019, 7:37 PM

I think we have five of these editors now: Herald, Bulk Edit, Owners Package Paths, Custom Policy, and now Trigger Rule. Depending on how you want to count, the action stack on comment forms is also a similar editor.

I think they share a mostly-reasonable amount of code already. All but the action stack use "MultirowManager", which is super ancient but not too terrible. The newer ones (bulk edit, comment forms, trigger rules, maybe owners?) share PHUIX controls for rendering the actual dropdowns and text inputs and stuff, although the older ones (herald/policy?) could be updated. They do all have some amount of unique behavior. For example, Herald has two sections for conditions/actions, and conditions has three columns for [ field ] [ operation ][ value ]. Owners has an AJAX validation callback and an associated UI. So they're never going to share all of the same code, and the pieces that need to be specialized vary from instance to instance.

I think this stuff probably could be cleaned up a bit and more code could be shared, but it doesn't feel too bad to me right now.

This revision was landed with ongoing or failed builds.Mar 25 2019, 8:25 PM
This revision was automatically updated to reflect the committed changes.