...by the surprising step of changing how this data is stored from id to phid. Also a small fix to not allow "disabled" rules to be used as herald rule conditions, i.e. can't make a rule that depends on a disabled rule.
Details
Details
- Reviewers
epriestley - Commits
- Restricted Diffusion Commit
rP183086800773: Herald - make herald condition of herald rule display better
viewed existing herald rule that had a rule condition and noted nice new display using handle. made a new rule that had a rule condition and verified it worked correctly.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- heraldrule
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
| src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php | ||
|---|---|---|
| 47 | i missed this cleanup in a prior diff | |
Comment Actions
This is really nice! One inline.
| src/applications/herald/controller/HeraldRuleController.php | ||
|---|---|---|
| 636–637 | Consider removing this, or maybe showing "(Disabled)" in the UI instead. The problem we run into is that if H2 depends on H1, but H1 is disabled, the mere act of editing H2 will modify it, since H1 won't be available in the dropdown. This is surprising and unexpected. A cleaner fix is probably to render names as "Rule Name" for enabled rules and "Rule Name (Disabled)" for disabled rules. | |
Comment Actions
- change the mfilter to a foreach() that adds pht('(Disabled)') to rule names in the select