From 7a672f01cb8b82a780d0dd273f77ff60543a92f3 Mon Sep 17 00:00:00 2001 From: Greg Walker Date: Tue, 27 Feb 2024 12:21:50 -0600 Subject: [PATCH 1/2] introduce strict mode --- src/scripts/inclusion-bot.js | 8 ++++++++ sync-inclusion-bot-words.js | 37 ++++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/scripts/inclusion-bot.js b/src/scripts/inclusion-bot.js index 75961d9a..4cab76e6 100644 --- a/src/scripts/inclusion-bot.js +++ b/src/scripts/inclusion-bot.js @@ -2,6 +2,7 @@ const fs = require("fs"); const path = require("path"); const yaml = require("js-yaml"); const { + optOut, slack: { addEmojiReaction, postEphemeralResponse }, stats: { incrementStats }, helpMessage, @@ -41,6 +42,12 @@ module.exports = async (app) => { "Charlie passively listens for language with racist, ableist, sexist, or other exclusionary histories. When it hears such words or phrases, it quietly lets the speaker know and offers some suggestions. What a great bot, helping nudge us all to thoughtful, inclusive language!", ); + const optout = optOut( + "inclusion_bot_strict", + "Inclusion Bot | Strict mode", + "Inclusion Bot operates in strict mode by default, highlighting all language that we have identified as racist, ableist, sexist, or otherwise exclusionary. We believe this is the ideal way to use Inclusion Bot. However, we recognize that some people may not want to be reminded about some of the language identified, so we offer a way to have Inclusion Bot only highlight language that we, as an organization, believe strongly should not be used. We encourage people to keep the stricter mode enabled as a learning aid, but if it makes you feel excluded yourself, please opt out of it.", + ); + // Use the module exported version here, so that it can be stubbed for testing const { link, message, triggers } = module.exports.getTriggers(); @@ -120,6 +127,7 @@ module.exports = async (app) => { }, ], }, + ...optout.button, ], }, ], diff --git a/sync-inclusion-bot-words.js b/sync-inclusion-bot-words.js index 95db42c2..ea510917 100644 --- a/sync-inclusion-bot-words.js +++ b/sync-inclusion-bot-words.js @@ -44,8 +44,8 @@ const markdownRowToTrigger = ([, matches, alternatives, why]) => ({ .replace(/this term/i, '":TERM:"'), }); -const getTriggerIgnoreMap = (currentConfig) => { - const triggerWithIgnore = (newTrigger) => { +const getTriggerExtraMetadataMap = (currentConfig) => { + const triggerWithExtraMetadata = (newTrigger) => { // Find a trigger in the existing config that has at least one of the same // matches as the new trigger. There may not be one, but if... const existing = currentConfig.triggers.find( @@ -53,23 +53,32 @@ const getTriggerIgnoreMap = (currentConfig) => { currentMatches.some((v) => newTrigger.matches.includes(v)), ); + // Create a new mapped trigger. We don't use an object spread notation here + // so we control the order the properties get rendered into YAML, to keep + // things consistent and nice. + const mapped = { + matches: newTrigger.matches, + alternatives: newTrigger.alternatives, + ignore: existing.ignore, + }; + // If there is an existing config that matches AND it has an ignore property - // return a new trigger that includes the ignore property. (We do it this - // way instead of using an object spread so we get the properties in the - // order we want for readability when it gets written to yaml.) + // add the existing ignore property on to the new trigger. if (existing?.ignore) { - return { - matches: newTrigger.matches, - alternatives: newTrigger.alternatives, - ignore: existing.ignore, - why: newTrigger.why, - }; + mapped.ignore = existing.ignore; + } + // If there is an existing config that matches AND it has an ignore property + // add the existing ignore property on to the new trigger. + if (existing?.strict) { + mapped.strict = existing.strict; } + mapped.why = newTrigger.why; + // Otherwise just return what we got. - return newTrigger; + return mapped; }; - return triggerWithIgnore; + return triggerWithExtraMetadata; }; const main = async () => { @@ -82,7 +91,7 @@ const main = async () => { // Also find the frontmatter comments so we can preserve it. const frontmatter = getYamlFrontmatter(currentYamlStr); - const triggerWithIgnore = getTriggerIgnoreMap(currentConfig); + const triggerWithIgnore = getTriggerExtraMetadataMap(currentConfig); // Read and parse the markdown. const mdf = await fs.readFile("InclusionBot.md", { encoding: "utf-8" }); From 58f0106efc6fcada7099d7adebc45a4ac17cdf63 Mon Sep 17 00:00:00 2001 From: Greg Walker Date: Tue, 27 Feb 2024 14:51:18 -0600 Subject: [PATCH 2/2] support 'optional' triggers, and allow opting out of them --- src/scripts/inclusion-bot.js | 19 ++++--- src/scripts/inclusion-bot.test.js | 82 ++++++++++++++++++++++++++++++- sync-inclusion-bot-words.js | 4 +- 3 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/scripts/inclusion-bot.js b/src/scripts/inclusion-bot.js index 4cab76e6..629d6c78 100644 --- a/src/scripts/inclusion-bot.js +++ b/src/scripts/inclusion-bot.js @@ -43,9 +43,9 @@ module.exports = async (app) => { ); const optout = optOut( - "inclusion_bot_strict", - "Inclusion Bot | Strict mode", - "Inclusion Bot operates in strict mode by default, highlighting all language that we have identified as racist, ableist, sexist, or otherwise exclusionary. We believe this is the ideal way to use Inclusion Bot. However, we recognize that some people may not want to be reminded about some of the language identified, so we offer a way to have Inclusion Bot only highlight language that we, as an organization, believe strongly should not be used. We encourage people to keep the stricter mode enabled as a learning aid, but if it makes you feel excluded yourself, please opt out of it.", + "inclusion_bot_extended", + "Inclusion Bot | Extended mode", + "Opt out of extended inclusive language notifications. These notifications are meant to help educate and remind us to be mindful of our colleagues.", ); // Use the module exported version here, so that it can be stubbed for testing @@ -61,10 +61,12 @@ module.exports = async (app) => { const combinedRegex = new RegExp(`\\b${combinedString}\\b`, "i"); app.message(combinedRegex, (msg) => { + const extended = !optout.isOptedOut(msg.event.user); + // Find the specific match that triggered this bot. At this point, go ahead // and remove things that should be ignored. const specificMatch = triggers - .map(({ alternatives, ignore, matches }) => { + .map(({ alternatives, ignore, optional, matches }) => { const { text } = msg.message; // Wrap the match in word boundaries, so we don't match something that's @@ -78,10 +80,12 @@ module.exports = async (app) => { // makes the regexes a lot simpler. return { alternatives, + optional, text: text.replace(ignoreRegex, "").match(matchRegex), }; }) .filter(({ text }) => !!text) + .filter(({ optional }) => extended || !optional) .map(({ text: [match], ...rest }) => ({ text: match, ...rest })); // If there aren't any specific matches, it means the bot was triggered only @@ -107,7 +111,11 @@ module.exports = async (app) => { color: "#2eb886", fallback: message, blocks: [ - { type: "section", text: { type: "mrkdwn", text: message } }, + { + type: "section", + text: { type: "mrkdwn", text: message }, + ...(extended ? optout.button : {}), + }, { type: "section", text: { type: "mrkdwn", text: pretexts.join("\n") }, @@ -127,7 +135,6 @@ module.exports = async (app) => { }, ], }, - ...optout.button, ], }, ], diff --git a/src/scripts/inclusion-bot.test.js b/src/scripts/inclusion-bot.test.js index 17656102..d41ccfa2 100644 --- a/src/scripts/inclusion-bot.test.js +++ b/src/scripts/inclusion-bot.test.js @@ -2,6 +2,7 @@ const fs = require("fs"); const { getApp, utils: { + optOut, slack: { addEmojiReaction, postEphemeralResponse }, }, } = require("../utils/test"); @@ -56,6 +57,9 @@ describe("Inclusion bot", () => { const app = getApp(); const msg = { + event: { + user: "abc123", + }, message: { id: "message id", room: "channel id", @@ -64,9 +68,18 @@ describe("Inclusion bot", () => { }, }; + const isOptedOut = jest.fn(); + beforeEach(() => { jest.resetAllMocks(); + isOptedOut.mockReturnValue(false); + + optOut.mockReturnValue({ + button: { button: "goes here" }, + isOptedOut, + }); + fs.readFileSync.mockReturnValue(` link: https://link.url message: "This is the message" @@ -85,6 +98,11 @@ triggers: - match 2b alternatives: - b1 + - matches: + - match 3 + alternatives: + - b3 + optional: true `); }); @@ -92,7 +110,7 @@ triggers: bot(app); expect(app.message).toHaveBeenCalledWith( - /\b(match 1)(?=[^"“”']*(["“”'][^"“”']*["“”'][^"“”']*)*$)|(match 2a|match 2b)(?=[^"“”']*(["“”'][^"“”']*["“”'][^"“”']*)*$)\b/i, + /\b(match 1)(?=[^"“”']*(["“”'][^"“”']*["“”'][^"“”']*)*$)|(match 2a|match 2b)(?=[^"“”']*(["“”'][^"“”']*["“”'][^"“”']*)*$)|(match 3)(?=[^"“”']*(["“”'][^"“”']*["“”'][^"“”']*)*$)\b/i, expect.any(Function), ); }); @@ -110,6 +128,7 @@ triggers: type: "mrkdwn", text: "This is the message", }, + button: "goes here", }, { accessory: { @@ -150,6 +169,8 @@ triggers: beforeEach(() => { bot(app); handler = app.getHandler(); + + expectedMessage.attachments[0].blocks[1].accessory.value = "match 1"; }); it("handles a single triggering phrase", () => { @@ -236,5 +257,64 @@ triggers: // Reset the parts of the expected message that we changed above. expectedMessage.attachments[0].blocks[1].accessory.value = "match 1"; }); + + it("does not add the opt-out button if the user is already opted-out", () => { + isOptedOut.mockReturnValue(true); + + const noButton = JSON.parse(JSON.stringify(expectedMessage)); + delete noButton.attachments[0].blocks[0].button; + + msg.message.text = "hello this is the match 1 trigger"; + handler(msg); + + expect(addEmojiReaction).toHaveBeenCalledWith(msg, "wave"); + + noButton.attachments[0].blocks[1].text.text = expect.stringMatching( + /• Instead of saying "match 1," how about \*(a1|a2|a3)\*\?/, + ); + expect(postEphemeralResponse).toHaveBeenCalledWith(msg, noButton); + }); + + it("does trigger on optional words if the user is not opted out", () => { + msg.message.text = "hello this is the match 3 trigger"; + handler(msg); + + expect(addEmojiReaction).toHaveBeenCalledWith(msg, "wave"); + + expectedMessage.attachments[0].blocks[1].accessory.value = "match 3"; + expectedMessage.attachments[0].blocks[1].text.text = + expect.stringMatching( + /• Instead of saying "match 3," how about \*b3\*\?/, + ); + expect(postEphemeralResponse).toHaveBeenCalledWith(msg, expectedMessage); + }); + + it("does not trigger if the user is opted-out and the trigger phrase is optional", () => { + isOptedOut.mockReturnValue(true); + + msg.message.text = "hello this is the match 3 trigger"; + handler(msg); + + expect(addEmojiReaction).not.toHaveBeenCalled(); + + expect(postEphemeralResponse).not.toHaveBeenCalled(); + }); + + it("does trigger on non-optional words but leaves out optional words if the user is opted-out", () => { + isOptedOut.mockReturnValue(true); + + msg.message.text = "hello this is the match 1 and the match 3 trigger"; + handler(msg); + + expect(addEmojiReaction).toHaveBeenCalledWith(msg, "wave"); + + const noButton = { ...expectedMessage }; + delete noButton.attachments[0].blocks[0].button; + + noButton.attachments[0].blocks[1].text.text = expect.stringMatching( + /• Instead of saying "match 1," how about \*(a1|a2|a3)\*\?/, + ); + expect(postEphemeralResponse).toHaveBeenCalledWith(msg, noButton); + }); }); }); diff --git a/sync-inclusion-bot-words.js b/sync-inclusion-bot-words.js index ea510917..7e9c8b21 100644 --- a/sync-inclusion-bot-words.js +++ b/sync-inclusion-bot-words.js @@ -69,8 +69,8 @@ const getTriggerExtraMetadataMap = (currentConfig) => { } // If there is an existing config that matches AND it has an ignore property // add the existing ignore property on to the new trigger. - if (existing?.strict) { - mapped.strict = existing.strict; + if (existing?.optional) { + mapped.optional = existing.optional; } mapped.why = newTrigger.why;