Fix a couple of long-standing bugs in job board validation logic (#429)

* Fix job post parsing when tags + description are on the same line

* Fix a source of random unhelpful messages in private threads

* Fix busted validation rules

* Apparently fix edited post detection

* Remove posts that are edited out of compliance with formatting rules

* Simplify thread/channel detection logic

This caused a heccin big bug on first merge unforunately. It was too complex, too difficult to understand, too easy to introduce weirdness. I tested this pretty comprehensively, testing message send in channel/thread/private thread, as well as editing messages in all those contexts.

* Add emoji tests, tune the sensitivity down

Author: vcarl

src/features/jobs-moderation.ts

@@ -106,75 +106,73 @@ const jobModeration = async (bot: Client) => {
   await deleteAgedPosts();
 
   bot.on("messageCreate", async (message) => {
-    const { channel } = message;
-    if (
-      message.author.bot ||
-      message.channelId !== CHANNELS.jobBoard ||
-      (channel.isThread() && channel.parentId !== CHANNELS.jobBoard) ||
-      // Don't treat newly fetched old messages as new posts
-      differenceInHours(new Date(), message.createdAt) >= 1
-    ) {
+    // Bail if it's a bot or staff message
+    if (message.author.bot || isStaff(message.member)) {
       return;
     }
+    const { channel } = message;
     // If this is an existing enforcement thread, process the through a "REPL"
     // that lets people test messages against the rules
     if (
       channel.type === ChannelType.PrivateThread &&
-      channel.ownerId === bot.user?.id &&
-      channel.parentId === CHANNELS.jobBoard
+      channel.parentId === CHANNELS.jobBoard &&
+      channel.ownerId === bot.user?.id
     ) {
-      validationRepl(message);
+      await validationRepl(message);
       return;
     }
-    // If this is a staff member, bail early
-    if (channel.type !== ChannelType.GuildText || isStaff(message.member)) {
+    // Bail if this isn't #job-board
+    if (
+      channel.type !== ChannelType.GuildText ||
+      message.channelId !== CHANNELS.jobBoard
+    ) {
       return;
     }
+
     const posts = parseContent(message.content);
     const errors = validate(posts, message);
     console.log(
       `[DEBUG] validating new job post from @${
         message.author.username
-      }, errors: [${JSON.stringify(errors)}]`,
+      }, errors: ${JSON.stringify(errors)}`,
     );
     if (errors) {
       await handleErrors(channel, message, errors);
     }
   });
 
-  bot.on("messageUpdate", async (_, newMessage) => {
-    const { channel } = newMessage;
-    if (newMessage.author?.bot) {
-      return;
-    }
-    if (channel.type === ChannelType.PrivateThread) {
-      validationRepl(await newMessage.fetch());
-      return;
-    }
+  bot.on("messageUpdate", async (_, message) => {
+    const { channel } = message;
     if (
-      newMessage.channelId !== CHANNELS.jobBoard ||
+      message.author?.bot ||
+      message.channelId !== CHANNELS.jobBoard ||
       channel.type !== ChannelType.GuildText ||
-      isStaff(newMessage.member)
+      isStaff(message.member)
     ) {
       return;
     }
-    const message = await newMessage.fetch();
-    const posts = parseContent(message.content);
-    // Don't validate hiring posts
-    if (posts.every((p) => p.tags.includes(PostType.hiring))) {
-      return;
+    if (message.partial) {
+      message = await message.fetch();
     }
+    const posts = parseContent(message.content);
     // You can't post too frequently when editing a message, so filter those out
     const errors = validate(posts, message).filter(
       (e) => e.type !== POST_FAILURE_REASONS.tooFrequent,
     );
 
     if (errors) {
+      const isRecentEdit =
+        differenceInMinutes(new Date(), message.createdAt) < REPOST_THRESHOLD;
+      errors.unshift({
+        type: POST_FAILURE_REASONS.circumventedRules,
+        recentEdit: isRecentEdit,
+      });
+      if (isRecentEdit) {
+        removeSpecificJob(message);
+      }
+      await handleErrors(channel, message, errors);
       if (posts.some((p) => p.tags.includes(PostType.forHire))) {
         reportUser({ reason: ReportReasons.jobCircumvent, message });
-        // await newMessage.delete();
-      } else {
-        await handleErrors(channel, message, errors);
       }
     }
   });

src/features/jobs-moderation/job-mod-helpers.ts

@@ -29,16 +29,12 @@ import {
   PostFailures,
   PostType,
   PostFailureLinkRequired,
+  CircumventedRules,
 } from "../../types/jobs-moderation";
 
-export class RuleViolation extends Error {
-  reasons: POST_FAILURE_REASONS[];
-  constructor(reasons: POST_FAILURE_REASONS[]) {
-    super("Job Mod Rule violation");
-    this.reasons = reasons;
-  }
-}
-
+export const failedCircumventedRules = (
+  e: PostFailures,
+): e is CircumventedRules => e.type === POST_FAILURE_REASONS.circumventedRules;
 export const failedMissingType = (
   e: PostFailures,
 ): e is PostFailureMissingType => e.type === POST_FAILURE_REASONS.missingType;
@@ -268,7 +264,7 @@ export const removeSpecificJob = (message: Message) => {
   const index = jobBoardMessageCache.hiring.findIndex(
     (m) => m.message.id === message.id,
   );
-  if (index) {
+  if (index !== -1) {
     jobBoardMessageCache.hiring.splice(index);
   } else
     jobBoardMessageCache.forHire.splice(

src/features/jobs-moderation/parse-content.test.ts

@@ -108,6 +108,28 @@ many long lines of text`,
     expect(parsed[0]).toMatchObject({ tags: ["hiring"], description: "" });
   });
 
+  it("correctly pulls description off tags line", () => {
+    let parsed = parseContent(`[hiring]Lorem ipsum dolor sit amet`);
+    expect(parsed[0]).toMatchObject({
+      tags: ["hiring"],
+      description: "Lorem ipsum dolor sit amet",
+    });
+
+    parsed = parseContent(`[hiring][remote][visa]Lorem ipsum dolor sit amet`);
+    expect(parsed[0]).toMatchObject({
+      tags: ["hiring", "remote", "visa"],
+      description: "Lorem ipsum dolor sit amet",
+    });
+
+    parsed = parseContent(
+      `[hiring] [remote]  [visa]   Lorem ipsum dolor sit amet`,
+    );
+    expect(parsed[0]).toMatchObject({
+      tags: ["hiring", "remote", "visa"],
+      description: "Lorem ipsum dolor sit amet",
+    });
+  });
+
   // Disable this, not relevant right now. Also broken as of May '23
   it.skip("parses contact", () => {
     const makePost = (contact: string) => `|

src/features/jobs-moderation/parse-content.ts

@@ -25,12 +25,31 @@ export const parseTags = (tags: string) => {
     .filter((tag) => tag !== "");
 };
 
-export const parseContent = (inputString: string): Post[] => {
+const splitTagsFromDescription = (
+  inputString: string,
+): { heading: string; body: string[] } => {
   const [tagsLine, ...lines] = inputString.trim().split("\n");
+
+  if (tagsLine.includes("[")) {
+    const cleanedTags = tagsLine.replace(/\]\w+\[/, "][");
+    const match = cleanedTags.match(/(.*)\](.*)/);
+    const trailingText = match?.[2] || "";
+    lines.unshift(trailingText.trim());
+    return { heading: match?.[1] || "", body: lines };
+  }
+  return { heading: tagsLine, body: lines };
+};
+
+export const parseContent = (inputString: string): Post[] => {
+  const { heading, body } = splitTagsFromDescription(inputString);
+  // TODO: Replace above .split() with some more logic around detecting tags
+  // If |, treat the complete line as tags
+  // if [], check for trailing text with no wrapper and add it to the description
+
   return [
     {
-      tags: parseTags(tagsLine),
-      description: lines.reduce((description, line) => {
+      tags: parseTags(heading),
+      description: body.reduce((description, line) => {
         if (line === "") {
           return description;
         }

src/features/jobs-moderation/validate.test.ts

@@ -1,11 +1,45 @@
 import { describe, expect, it } from "vitest";
 import { PostType, POST_FAILURE_REASONS } from "../../types/jobs-moderation";
-import { links } from "./validate";
+import { links, formatting } from "./validate";
 
 const makePost = (type: PostType, description: string) => [
   { tags: [type], description },
 ];
 
+describe("emoji", () => {
+  it("isn't too crazy about emoji", () => {
+    const noFailure = [
+      "for some role and stuff\nDM me to apply ✨",
+      "for some role and stuff\nDM me to apply ✨",
+      "👉 stuff: some more details afterwards and whatever shenanigans\n👉 stuff: some more details afterwards and whatever shenanigans\n👉 stuff: some more details afterwards and whatever shenanigans\n👉 stuff: some more details afterwards and whatever shenanigans\n👉 stuff: some more details afterwards and whatever shenanigans\n",
+    ];
+    for (const content of noFailure) {
+      expect(
+        formatting(
+          makePost(PostType.forHire, content),
+          // @ts-expect-error testing override
+          { content: `[forhire]\n${content}` },
+        ),
+      ).not.toContainEqual({ type: POST_FAILURE_REASONS.tooManyEmojis });
+    }
+  });
+  it("stops obnoxious emoji usage", () => {
+    const noFailure = [
+      "for ✨ some role and stuff\nDM ✨ me ✨ to ✨ apply ✨",
+      "for some role and stuff\nDM me to apply ✨✨✨✨✨✨",
+      "👉 stuff: some more ✨✨ details afterwards and whatever shenanigans\n👉 stuff: some more ✨✨ details afterwards and whatever shenanigans\n👉 stuff: some more ✨✨ details afterwards and whatever shenanigans\n👉 stuff: some more ✨✨ details afterwards and whatever shenanigans\n👉 stuff: some more ✨✨ details afterwards and whatever shenanigans\n",
+    ];
+    for (const content of noFailure) {
+      expect(
+        formatting(
+          makePost(PostType.forHire, content),
+          // @ts-expect-error testing override
+          { content: `[forhire]\n${content}` },
+        ),
+      ).toContainEqual({ type: POST_FAILURE_REASONS.tooManyEmojis });
+    }
+  });
+});
 describe("links", () => {
   it("requires links", () => {
     expect(

src/features/jobs-moderation/validate.ts

@@ -53,7 +53,7 @@ export const formatting: JobPostValidator = (posts, message) => {
   posts.forEach((post) => {
     // If > 1 in 150 chars is an emoji
     const emojiCount = extractEmoji(post.description).length;
-    if (emojiCount / post.description.length > 1 / 150) {
+    if (emojiCount / post.description.length > 1 / 30) {
       errors.push({ type: POST_FAILURE_REASONS.tooManyEmojis });
     }
     const lineCount = countLines(post.description.trim());

src/features/jobs-moderation/validation-messages.ts

@@ -1,11 +1,13 @@
 import {
+  CircumventedRules,
   POST_FAILURE_REASONS,
   PostFailures,
   PostFailureTooFrequent,
   PostFailureTooLong,
   PostFailureTooManyLines,
 } from "../../types/jobs-moderation";
 import {
+  failedCircumventedRules,
   failedMissingType,
   failedReplyOrMention,
   failedTooManyLines,
@@ -18,6 +20,8 @@ import {
 } from "./job-mod-helpers";
 
 const ValidationMessages = {
+  [POST_FAILURE_REASONS.circumventedRules]: (e: CircumventedRules) =>
+    `Your message was removed after you edited it so that it no longer complies with our formatting rules. ${e.recentEdit ? "Please re-post." : ""}`,
   [POST_FAILURE_REASONS.missingType]:
     "Your post does not include our required `[HIRING]` or `[FOR HIRE]` tag. Make sure the first line of your post includes `[HIRING]` if you’re looking to pay someone for their work, and `[FOR HIRE]` if you’re offering your services.",
   [POST_FAILURE_REASONS.inconsistentType]:
@@ -33,10 +37,13 @@ const ValidationMessages = {
   [POST_FAILURE_REASONS.tooFrequent]: (e: PostFailureTooFrequent) =>
     `You’re posting too frequently. You last posted ${e.lastSent} days ago, please wait at least 7 days.`,
   [POST_FAILURE_REASONS.replyOrMention]:
-    "Messages in this channel may not be replies or include @-mentions of users, to ensure the channel isn’t being used to discuss postings.",
+    "Messages in this channel may not be replies or include @-mentions of users due to a history of posters incorrectly attempting to 'apply' by replying within a thread or reply.",
 };
 
 export const getValidationMessage = (reason: PostFailures): string => {
+  if (failedCircumventedRules(reason)) {
+    return ValidationMessages[reason.type](reason);
+  }
   if (failedMissingType(reason)) {
     return ValidationMessages[reason.type];
   }

src/index.ts

@@ -48,12 +48,7 @@ export const bot = new discord.Client({
     IntentsBitField.Flags.DirectMessageReactions,
     IntentsBitField.Flags.MessageContent,
   ],
-  partials: [
-    Partials.Channel,
-    Partials.Message,
-    Partials.Reaction,
-    Partials.GuildMember,
-  ],
+  partials: [Partials.Channel, Partials.Reaction, Partials.GuildMember],
 });
 
 registerCommand(resetJobCacheCommand);

src/types/jobs-moderation.ts

@@ -28,11 +28,16 @@ export const enum POST_FAILURE_REASONS {
   tooManyGaps = "tooManyGaps",
   tooFrequent = "tooFrequent",
   replyOrMention = "replyOrMention",
+  circumventedRules = "circumventedRules",
   // invalidContact = 'invalidContact',
   // unknownLocation = 'unknownLocation',
   // invalidPostType = 'invalidPostType',
 }
 
+export interface CircumventedRules {
+  type: POST_FAILURE_REASONS.circumventedRules;
+  recentEdit: boolean;
+}
 export interface PostFailureMissingType {
   type: POST_FAILURE_REASONS.missingType;
 }
@@ -64,6 +69,7 @@ export interface PostFailureReplyOrMention {
   type: POST_FAILURE_REASONS.replyOrMention;
 }
 export type PostFailures =
+  | CircumventedRules
   | PostFailureMissingType
   | PostFailureInconsistentType
   | PostFailureTooFrequent