-
Notifications
You must be signed in to change notification settings - Fork 18
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This is a proposal for a change that allows you to leave the `action` column blank to signify that you want the rule to match messages and stop processing further rules, but not actually move the thread from its current state (similar to the current `mark_important` column behavior). I'm having trouble correctly routing code review emails. I don't want CI messages for other people's changes to move threads to my inbox, but if I specify `archive` on those, and a CI message is processed in the same run as a relevant message, it overrides the relevant message. I could move the CI filter rule below, but that precludes a "catch-all" review rule to route uncategorized reviews to my inbox. If this seems OK, let me know and I can add tests and make it a nicer thing.
- Loading branch information
1 parent
37b828f
commit d5c6531
Showing
5 changed files
with
50 additions
and
25 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,8 +37,9 @@ export class Processor { | |
break; | ||
} | ||
if (rule.condition.match(message_data)) { | ||
console.log(`rule ${rule} matches message ${message_data}, apply action ${rule.thread_action}`); | ||
console.log(`Rule ${rule} matches message ${message_data}, apply action ${rule.thread_action}`); | ||
thread_data.thread_action.mergeFrom(rule.thread_action); | ||
console.log(`Thread action after merging: ${thread_data.thread_action}`) | ||
let endThread = false; | ||
switch (rule.thread_action.action_after_match) { | ||
case ActionAfterMatchType.DONE: | ||
|
@@ -168,22 +169,8 @@ export class Processor { | |
expect(thread_data.thread_action.action_after_match).toBe(ActionAfterMatchType.DEFAULT); | ||
expect(thread_data.thread_action.important).toBe(BooleanActionType.DEFAULT); | ||
expect(thread_data.thread_action.label_names).toEqual(new Set(['abc', 'xyz'])); | ||
expect(thread_data.thread_action.move_to).toBe(InboxActionType.DEFAULT); | ||
expect(thread_data.thread_action.move_to).toBe(InboxActionType.NOTHING); | ||
expect(thread_data.thread_action.read).toBe(BooleanActionType.DEFAULT); | ||
}) | ||
it('Throws error when message matches action but has no actions', () => { | ||
expect(() => { | ||
test_proc([ | ||
{ | ||
conditions: '(sender [email protected])', | ||
stage: '5', | ||
}, | ||
], [ | ||
{ | ||
getFrom: () => '[email protected]', | ||
} | ||
]) | ||
}).toThrow(); | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,43 @@ | ||
import ThreadAction from './ThreadAction'; | ||
import ThreadAction, {InboxActionType} from './ThreadAction'; | ||
import Utils from './utils'; | ||
|
||
describe('ThreadAction Tests', () => { | ||
ThreadAction.testThreadActions(it, expect); | ||
}) | ||
it('Adds parent labels', () => { | ||
const labels = ['list/abc', 'bot/team1/test', 'bot/team1/alert', 'def']; | ||
const action = new ThreadAction(); | ||
const expected = new Set(['list', 'list/abc', 'bot', 'bot/team1', 'bot/team1/test', 'bot/team1/alert', 'def']) | ||
|
||
action.addLabels(labels); | ||
|
||
Utils.assert(action.label_names.size === expected.size, | ||
`Expected ${Array.from(expected).join(', ')}, | ||
but got ${Array.from(action.label_names).join(', ')}`); | ||
|
||
for (const label of expected) { | ||
Utils.assert(action.label_names.has(label), `Expected label ${label}, but not present in action.`); | ||
} | ||
}); | ||
|
||
it('Does not add parent labels for empty list', () => { | ||
const labels: string[] = []; | ||
const action = new ThreadAction(); | ||
|
||
action.addLabels(labels); | ||
|
||
Utils.assert(action.label_names.size === 0, | ||
`Expected empty set, but got ${Array.from(action.label_names).join(', ')}`); | ||
}); | ||
|
||
it('Correctly merges NOTHING actions', () => { | ||
const thread_data_action = new ThreadAction(); | ||
const rule_action = new ThreadAction(); | ||
rule_action.move_to = InboxActionType.NOTHING; | ||
|
||
Utils.assert(thread_data_action.move_to == InboxActionType.DEFAULT, | ||
`move_to should be DEFAULT, but is ${thread_data_action.move_to}`); | ||
thread_data_action.mergeFrom(rule_action); | ||
|
||
Utils.assert(thread_data_action.move_to == InboxActionType.NOTHING, | ||
`move_to should be NOTHING, but is ${thread_data_action.move_to}`); | ||
Utils.assert(rule_action.toString() == '>NOTHING +L', | ||
`rule_action should be '>NOTHING +L', but is ${rule_action}`); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters