Bug - points command: Symbols aren't escaped

[T0nci]

Complete the following REQUIRED checkboxes:

• [x] I have thoroughly read and understand The Odin Project Contributing Guide

• [x] The title of this issue follows the Bug - location of bug: brief description of bug format, e.g. Bug - points command: Extra whitespace causes command to not be called

The following checkbox is OPTIONAL:

• [x] I would like to be assigned this issue to work on it

---

1. Description of the Bug:

When people have special/punctuation characters in their names it causes unexpected side effects because of Discord's message formatting. For example I added "`test`" into my name and that caused the points command to show my name as "name test" instead of "name `test`".

That can happen with other symbols as well(the 2 examples are shown in the screenshots below). The second example shows that adding || into ones name triggers the spoiler format(if there is another set of || after) which hides lines of the message in between the vertical lines.

2. How To Reproduce:

1. Use the points command
2. Find a name where there is Discord formatting or rename your server name to include Discord formatting

3. Expected Behavior:

1. Use the points command
2. You see your name with the characters not formatted, instead they are escaped

4. Desktop/Device:

• Device: Laptop and mobile so far(probably a problem on all devices)
• OS:
• Browser:
• Version:

5. Additional Information:

[Asartea]

This should be a relatively easy fix since discord.js has its own built-in escaper, escapeMarkdown, which is included in the discord.js package, so it should be as simply as calling that on any string that might contain markdown where it isn't wanted

[MaoShizhong]

Yep, escapeMarkdown from the discord.js package is all we'll need for this.

services/points/points.service.js has stuff that deals with .displayName e.g. interaction.guild.members.cache.get(user.id).displayName that should be escapeMarkdown(interaction.guild.members.cache.get(user.id).displayName), or member.displayName which should be escapeMarkdown(member.displayName).

Various methods in services/rotations/rotation.service.js also deal with extracting display names that currently don't have markdown escaped.

Both of these have their corresponding .test.js files that would need any mock data amended to include an example of a display name that should be escaped.

@T0nci is this still something you wish to work on?

[T0nci]

@MaoShizhong Yes, I'd love to work on this! Although since this is my first contribution involving code that isn't HTML or markdown lol, if I can't finish the task I'll ask for help or ask for it to be reassigned to someone that can complete it.

[MaoShizhong]

@MaoShizhong Yes, I'd love to work on this! Although since this is my first contribution involving code that isn't HTML or markdown lol, if I can't finish the task I'll ask for help or ask for it to be reassigned to someone that can complete it.

Assigned. Feel free to ask about any problems or uncertainties. Local bot setup instructions are in this repo's wiki.

I think for the points stuff you won't be able to directly test some points things in your test discord server since there's an intermediary bot that I don't think we have a dev version for (not too familiar with that part myself but it was mentioned at some point in our staff channels), but the Jest tests for those should still run fine, hence we need them amended to test the escaped behaviour.

[MaoShizhong]

@T0nci just a courtesy check in on this in case you need any help or clarification?

[T0nci]

@MaoShizhong Will get on it in the next few days! Had a break from coding for a while and just got into the flow again, so I'll look into it. Apologies for the long period it's taking me to complete this contribution.

[MaoShizhong]

No worries, real life comes first and breaks are important :+1: Let me know here if you need any assistance at any points.

[T0nci]

@MaoShizhong So while fixing the leaderboard subcommand, I noticed that there is an extra backslash when there is a `, although I'm guessing that's because jest is using template literals and needs to escape all `s. Is it fine if I leave this test(and possibly all other tests that will include `s) like this? All other markdown is escaped correctly.

The line in question is where user103 is ^

[MaoShizhong]

As long as the visual output from the command is correct and all tests pass with the appropriate snapshots, that's fine. If the snapshot requires additional backslashes, then it needs them.

\\\` would be needed in this case because the discord embed needs to enter \`, so we need to escape the backtick to prevent clashing with the template literal, and also escape the first blackslash so it doesnt end up escaping the backtick's backslash instead.

[T0nci]

@MaoShizhong I have a problem with completing the second part of the task. I'm confused how the code works, and in turn don't know what and how to test. I may have pinpointed the part where I'm supposed to include the escapeMarkdown function(line 35 and 37), but in any case I might not be able to continue with the contribution.

I have successfully completed integrating the escapeMarkdown function in the points commands. The commits are located on the points_escape_markdown branch for review.

[MaoShizhong]

@T0nci Yep, you've isolated the correct function. You can actually test the rotation commands locally in your test server since you'll be an admin in that. /triage add and add yourself (change your server name to include some sort of markdown like spoilers), and you can use /triage rotate to view the output. escapeMarkdown() on both username returns will do the trick. Tests will then need amending so that at least one markdown username is included and tested to be escaped properly on output.

We could probably even change that map callback's returns to

    this.displayNames = members.map(async (memberId) => {
      const member = await server.members.fetch(memberId);
      return escapeMarkdown(member.nickname || member.user.username);
    });

[T0nci]

@MaoShizhong That's great, I'll try to implement this too once I get better from my illness.

The part that was bugging me and still is, is that I don't really know how the command works so I can change the tests correctly. I've tried using it in my discord server with an odin-bot setup(through the steps in the docs) but crashes after the points leader and user commands and the rotation commands(not too sure about all rotation commands that's why I'll try to use them again tommorow) so I couldn't use the commands to try to see how it works.

[MaoShizhong]

The points commands won't work because it relies on a separate bot we don't have a dev version for (which we have an issue open about it - waiting on a reply from others who know more).

The /triage commands are what you want for the rotation stuff and I can confirm on my private test server, /triage add and /triage rotate work perfectly fine (they're not tied to a database or anything unlike the points stuff, that's why - they're just part of an internal staff queue). The rotation tests get their dummy data not from botCommands/mockData.js like the points stuff, but I think somewhere in utils/. Should be traceable from the imports in the rotations test file.

No rush - recover well! Health comes first.