Refactor the target user check to reuse snark

[aedwardg]

Purpose

Fede wrote some amazing snarky responses from the bot for when you forget to include the target user. Unfortunately, we are only using them in a couple of commands, when we could use them everywhere.

We should refactor, so that we can re-use this same code in every command that takes a target user.

Proposed Solution

Make a helper module that checks for the target user and returns snark if no user is provided (or the user left).

Acceptance Criteria

Please list the requirements the implementation of this feature should meet.

• [ ] Logic for checking target user is in only one place in the code
• [ ] Code checks if user is provided/exists and responds with snark if not.
• [ ] Code passes all linting/formatting

Additional context

Example of what this is referring to: https://github.com/CodecademyCommunity/codecademy-discord-bot/blob/26ddcb48a0dab76794e847c9599b06962dd09e38/commands/moderation/addnote.js#L81-L101

[lyallstewart]

Hi @aedwardg , Some of the snark messages (like "Please provide the user you are making a note about") are specific to a single command - do you think these should be made more generic so they could be applied to all commands, or should there be a seperate set of snark messages for each command in the helper function?

[aedwardg]

@lyallstewart great question.

To make it as reusable as possible, the helper module should basically just export a single function that contains one set of snarky responses. This function should take a string argument of the action type (note, verbal, warn, etc.) and interpolate that into the message, if the selected message references an action. Then when the command is checking for the target user, if it can't find it this function gets called with the appropriate action name.

For example:

function hasTargetUser(msg, targetUser) {
  if (targetUser) {
    return true;
  } else {
    msg.reply(failSnark("note"));
    return false;
  }
}

Obviously, if you can come up with a better name that failSnark then go for it! 😂

[lyallstewart]

That makes sense thanks!

(I think failSnark is the perfect name lol 🤣 )

[aedwardg]

Awesome!

One last note: please scope this to just writing out the helper module -- don't change any of the commands just yet. We can do that in 1-3 followup PRs. That way it's easier to review and approve and easier for contributors to tackle. (And also easier to roll-back a piece if something slips through the cracks and breaks the bot in prod).

Obviously, you'll want to test it out with one of the commands to make sure your code works, but I prefer that we try to keep most of our issues and PRs bite-sized when possible.

[aedwardg]

@lyallstewart actually, I just re-read my initial issue and realized I wanted to abstract away the entire target user check so that could be imported into all the commands.

So in this case, the helper module should export that hasTargetUser function, (which in turn can call the failSnark function) and it should have three parameters: msg, targetUser and action with action being passed to failSnark.

Let me know if that doesn't make sense.

[lyallstewart]

Yup that makes sense, thanks! I've just got my local environment working so I'll give it a go 😄