From b060508cf63e840e780fbdbd00c134712fc0dd29 Mon Sep 17 00:00:00 2001 From: Aaron Stacy Date: Mon, 15 Jun 2020 22:47:45 -0500 Subject: [PATCH] Add `NOTHING` action 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. --- Processor.ts | 3 ++- Rule.ts | 2 +- ThreadAction.test.ts | 46 ++++++++++++++++++++++++++++++++++++++++---- ThreadAction.ts | 2 +- ThreadData.ts | 6 +++--- 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/Processor.ts b/Processor.ts index ae7836b..d4b580a 100644 --- a/Processor.ts +++ b/Processor.ts @@ -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: diff --git a/Rule.ts b/Rule.ts index a8b9555..63b79a5 100644 --- a/Rule.ts +++ b/Rule.ts @@ -70,7 +70,7 @@ export class Rule { private static parseInboxActionType(str: string): InboxActionType { if (str.length === 0) { - return InboxActionType.DEFAULT; + return InboxActionType.NOTHING; } const result = InboxActionType[str.toUpperCase() as keyof typeof InboxActionType]; Utils.assert(result !== undefined, `Can't parse inbox action value ${str}.`); diff --git a/ThreadAction.test.ts b/ThreadAction.test.ts index 4375d0b..ae284a2 100644 --- a/ThreadAction.test.ts +++ b/ThreadAction.test.ts @@ -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}`); +}); diff --git a/ThreadAction.ts b/ThreadAction.ts index 3e1f0cc..a9449ec 100644 --- a/ThreadAction.ts +++ b/ThreadAction.ts @@ -16,7 +16,7 @@ export enum BooleanActionType {DEFAULT, ENABLE, DISABLE} -export enum InboxActionType {DEFAULT, INBOX, ARCHIVE, TRASH} +export enum InboxActionType {DEFAULT, INBOX, ARCHIVE, TRASH, NOTHING} export enum ActionAfterMatchType {DEFAULT, DONE, FINISH_STAGE, NEXT_STAGE} diff --git a/ThreadData.ts b/ThreadData.ts index dc5297e..0d87a92 100644 --- a/ThreadData.ts +++ b/ThreadData.ts @@ -122,19 +122,19 @@ export class ThreadData { } validateActions() { - if (!this.thread_action.hasAnyAction()) { + if (!this.thread_action.hasAnyAction() && this.thread_action.move_to != InboxActionType.NOTHING) { const messages = this.raw.getMessages(); const last_message = messages[messages.length - 1]; const from = last_message.getFrom(); const to = last_message.getTo(); - throw `Thread "${this.raw.getFirstMessageSubject()}" from ${from} to ${to} has no action, does it match any rule?`; + throw `Thread "${this.raw.getFirstMessageSubject()}" from ${from} to ${to} has default action (${this.thread_action}), does it match any rule?`; } } static applyAllActions(session_data: SessionData, all_thread_data: ThreadData[]) { const label_action_map: { [key: string]: GoogleAppsScript.Gmail.GmailThread[] } = {}; const moving_action_map = new Map([ - [InboxActionType.DEFAULT, []], [InboxActionType.INBOX, []], [InboxActionType.ARCHIVE, []], [InboxActionType.TRASH, []] + [InboxActionType.DEFAULT, []], [InboxActionType.INBOX, []], [InboxActionType.ARCHIVE, []], [InboxActionType.TRASH, []], [InboxActionType.NOTHING, []] ]); const important_action_map = new Map([ [BooleanActionType.DEFAULT, []], [BooleanActionType.ENABLE, []], [BooleanActionType.DISABLE, []]