Page MenuHomePhabricator

On Harbormaster build plans, show which Herald rules trigger builds
ClosedPublic

Authored by epriestley on Mar 7 2019, 2:22 PM.
Tags
None
Referenced Files
F14407542: D20259.diff
Tue, Dec 24, 3:09 AM
Unknown Object (File)
Wed, Dec 18, 8:34 PM
Unknown Object (File)
Tue, Dec 10, 1:19 AM
Unknown Object (File)
Mon, Dec 9, 1:06 AM
Unknown Object (File)
Wed, Dec 4, 6:01 PM
Unknown Object (File)
Wed, Dec 4, 4:20 PM
Unknown Object (File)
Wed, Dec 4, 2:03 AM
Unknown Object (File)
Sat, Nov 30, 6:06 PM
Subscribers
Restricted Owners Package

Details

Summary

Ref T13258. Provide an easy way to find rules which trigger a particular build plan from the build plan page.

The implementation here ends up a little messy: we can't just search for actionType = 'build' AND targetPHID = '<build plan PHID>' since the field is a blob of JSON.

Instead, make rules indexable and write a "build plan is affected by rule actions" edge when indexing rules, then search on that edge.

For now, only "Run Build Plan: ..." rules actually write this edge, since I think (?) that it doesn't really have meaningful values for other edge types today. Maybe "Call Webhooks", and you could get a link from a hook to rules that trigger it? Reasonable to do in the future.

Things end up a little bit rough overall, but I think this panel is pretty useful to add to the Build Plan page.

This index needs to be rebuilt with bin/search index --type HeraldRule. I'll call this out in the changelog but I'm not planning to explicitly migrate or add an activity, since this is only really important for larger installs and they probably (?) read the changelog. As rules are edited over time, this will converge to the right behavior even if you don't rebuild the index.

Test Plan

Screen Shot 2019-03-07 at 6.21.04 AM.png (214×1 px, 29 KB)

Diff Detail

Repository
rP Phabricator
Branch
unlock4
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22210
Build 30365: Run Core Tests
Build 30364: arc lint + arc unit

Event Timeline

Owners added a subscriber: Restricted Owners Package.Mar 7 2019, 2:22 PM
  • Minor fixes for null/empty behaviors.
amckinley added inline comments.
src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
96

Should this be wrapped in an array?

And also, why does the HeraldActionRecord typehint get highlighted differently here than the typehint in HeraldAction.php?

src/applications/herald/edge/HeraldRuleActionAffectsObjectEdgeType.php
7

Nice.

Also, just out of curiosity, is there a script or something for coming up with the next unallocated EDGECONST?

This revision is now accepted and ready to land.Mar 7 2019, 9:47 PM
This revision was automatically updated to reflect the committed changes.
src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
96

For this rule type, the target is already a list (since you can have a rule run more than one build plan).

Colorization difference is an xhpast binary difference on some secure hosts, I think.

src/applications/herald/edge/HeraldRuleActionAffectsObjectEdgeType.php
7

Not really -- ConfigModulesEdge Types, then add 1.