This is kinda bad in terms of UI (It just makes a json of the thing and diffs that), but it's a start.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rP6dc30ecc8ef1: Drive Herald edits via transactions
edit rule, create rule, add/remove/edit conditions, actions
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- herald-tx
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 11335 Build 14104: Run Core Tests Build 14103: arc lint + arc unit
Event Timeline
Comment Actions
What I thought was a failing test is just strange validation error (It throws an exception if you submit a rule with no actions/conditions; Just reporting them as "errors" makes things break later; Maybe do this validation in the client side?).
Comment Actions
For consistency, I think TYPE_RENAME should just be TYPE_NAME. There's a special awkward case when creating a rule right now:
You could technically ignore that because it will be fixed by EditEngine, but maybe worth special casing it in the meantime since it could be a while before EditEngine arrives here.
In an ideal world it might be nice for this to be split into smaller pieces (repetition, match rule, conditions, actions?) but I think this is a big step forward and it's not particularly difficult to split apart later.
| src/applications/herald/storage/HeraldRuleTransaction.php | ||
|---|---|---|
| 80 | Maybe include a TYPE_EDIT here -- the default value that it's picking up is actually fairly reasonable, but can't be translated. | |
Comment Actions
- s/rename/name/
- change first "renamed" transaction to "created".
The first "Edited" tx still shows as "edited", but this is harder to test for.