Differential D19403

Add "Content type" and "Rule type" fields to Herald rules for Herald rules
ClosedPublic
Actions

Authored by epriestley on Apr 24 2018, 10:43 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13130: Plans: 2018 Week 17/18 Bonus Content

Commits

rPb4796d28374c: Add "Content type" and "Rule type" fields to Herald rules for Herald rules

Summary

Depends on D19400. Ref T13130. Currently, when you write Herald rules about other Herald rules, you can't pick a rule type or content type, so there's no way to get notified about edits to just global rules (which is the primary driving use case).

Add a "Content type" field to let the rule match rules that affect revisions, tasks, commits, etc.

Add a "Rule type" field to let the rule match global, personal, or object rules.

Test Plan

Wrote a global rule for other rules about global Herald rules:

Screen Shot 2018-04-24 at 3.37.33 PM.png (1×1 px, 160 KB)

Screen Shot 2018-04-24 at 3.37.32 PM.png (1×1 px, 163 KB)

Ran it against itself which matched:

Screen Shot 2018-04-24 at 3.37.50 PM.png (1×1 px, 148 KB)

Ran it against another rule (not a global rule about Herald rules), which did not match:

Screen Shot 2018-04-24 at 3.38.09 PM.png (1×1 px, 141 KB)

Also reviewed the fields in those transcripts in more detail to make sure they were extracting matching correctly.

Diff Detail

Repository

rP Phabricator

Branch

herald2

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 20220
Build 27440: Run Core Tests
Build 27439: arc lint + arc unit

Event Timeline

epriestley created this revision.Apr 24 2018, 10:43 PM

Harbormaster completed remote builds in B20220: Diff 46411.Apr 24 2018, 10:44 PM

epriestley requested review of this revision.Apr 24 2018, 10:44 PM

The only other field which seems possibly useful here is "Name", but I can't really come up with any reasons why you'd ever want to do something when, e.g., a rule's name matches some regexp.

amckinley accepted this revision.Apr 25 2018, 3:05 AM

amckinley added inline comments.

src/applications/herald/typeahead/HeraldRuleTypeDatasource.php
11	"Enter" a rule type? "Specify"?

This revision is now accepted and ready to land.Apr 25 2018, 3:05 AM

"Type a rule type..." reads a little awkwardly because of the repetition of the word "type", but the "Type a..." language is pretty consistent across datasources so I figured consistency was better than making it sound smoother:

$ git grep -i "'Type a"
src/applications/almanac/typeahead/AlmanacInterfaceDatasource.php:    return pht('Type an interface name...');
src/applications/almanac/typeahead/AlmanacServiceDatasource.php:    return pht('Type a service name...');
src/applications/almanac/typeahead/AlmanacServiceTypeDatasource.php:    return pht('Type a service type name...');
src/applications/badges/typeahead/PhabricatorBadgesDatasource.php:    return pht('Type a badge name...');
src/applications/calendar/typeahead/PhabricatorCalendarInviteeDatasource.php:    return pht('Type a user or project name, or function...');
src/applications/calendar/typeahead/PhabricatorCalendarInviteeUserDatasource.php:    return pht('Type a user name...');
src/applications/conpherence/typeahead/ConpherenceThreadDatasource.php:    return pht('Type a room title...');
src/applications/dashboard/typeahead/PhabricatorDashboardDatasource.php:    return pht('Type a dashboard name...');
src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php:    return pht('Type a panel name...');
...

(If you think none of these should be "Type a...", feel free to counterdiff? In the general case, I think "Type" is slightly more clear than "Enter" or "Specify" in conveying that you only need to start typing something, it just sounds a bit weird in this case.)

Closed by commit rPb4796d28374c: Add "Content type" and "Rule type" fields to Herald rules for Herald rules (authored by epriestley). · Explain WhyApr 25 2018, 1:54 PM

This revision was automatically updated to reflect the committed changes.