Added New Welcome Message and !tos Command#146
Conversation
|
No errors, all tests pass. Consider changing the user messages to incorporate tagging and mentions of the username so that the user can be notified around the channel. |
|
Looks good to me everything worked all tests passed. |
reactivepixel
left a comment
reactivepixel
left a comment
There was a problem hiding this comment.
Very good work, only small items to be addressed. These small touches of tone are something that once you are thinking about you will start to naturally bake these into the features you are developing.
bot/client.js
Outdated
| // Send Direct Message with Terms of Service | ||
| member.send(`Welcome ${member} to the Full Sail Armada! Read our Terms of Service`); | ||
| // Send Welcome User Message in the General Channel | ||
| member.guild.channels.find('name', 'general').send(`Welcome ${member} to the Full Sail Armada!`); |
There was a problem hiding this comment.
When we bury strings inside of code that has been chained, even somewhat non-complex code, we make it hard to maintain the text messages we are trying to send. This is strongly related to magic numbers in that if we implement a variable, and control that string within, using the variable on the sending of the message method will come out cleaner. If you look at the email being sent in the verify controller you can see how quickly inline strings used in this manner begin to degrade.
const targetChannel = 'general';
const greeting = 'Welcome ${member.user.username} to the Family!';
// Send Welcome User Message in the General Channel
member.guild.channels.find('name', targetChannel).send(greeting);
bot/controllers/tos.js
Outdated
| '!tos', | ||
| '!tos', | ||
| 'Terms of Service', | ||
| 'Send Copy of the Terms of Service', |
There was a problem hiding this comment.
Where is this copy sent? Does it send it to the channel? We need to shape our messages to the user carefully and accurately so that we craft an expected user experience.
bot/controllers/tos.js
Outdated
| tosAction() { | ||
| // Send Direct Message with the Terms of Service | ||
| const { message } = this; | ||
| return `<@${message.author.id}> read our Terms of service`; |
There was a problem hiding this comment.
This is a different welcome message than originally sent.
bot/client.js
Outdated
| // Message for new users | ||
| client.on('guildMemberAdd', (member) => { | ||
| // Send Direct Message with Terms of Service | ||
| member.send(`Welcome ${member} to the Full Sail Armada! Read our Terms of Service`); |
There was a problem hiding this comment.
Duplicate code logic in your controller.
There was a problem hiding this comment.
If I understand correctly you are talking about the direct message here being the same as the direct message in my tos.js tosAction. I see that I could fire off the tosAction inside here but that would require me to import my specific controller, TosController. Is this your preferred way to solve the issue?
4 participants
No description provided.