镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Add loop messages feature - #18

Closed
rafaelalmeidatk wants to merge 6 commits into
reactiflux:mainfrom
rafaelalmeidatk:feature/msg-loop
Closed

rafaelalmeidatk wants to merge 6 commits into
reactiflux:mainfrom
rafaelalmeidatk:feature/msg-loop

Conversation

@rafaelalmeidatk

Copy link
Copy Markdown
Member

With this feature, the bot will keep a message as the last one in a channel, with an interval between each post, to keep important information more visible to users.

I swear I couldn't find a better name for the module 😅 Suggestions are welcome

Closes #13

@@ -0,0 +1,12 @@
module.exports = [

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using a JS file instead of JSON because of the comments that are useful with channelId and the template strings for the multi-line messages

@bpas247 bpas247 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome so far 😃 just left a few comments to discuss the implementation details a bit further, otherwise :shipit:

Comment thread features/loopMessages/index.js Outdated
sendLoopMessages();
}, INTERVAL_TIME);

sendLoopMessages();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to initially run it on startup?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, if the bot restarts for some reason then it already executes the function to update the channels

Comment thread features/loopMessages/index.js Outdated

async function sendLoopMessage(client, loopMessage) {
const channel = client.channels.get(loopMessage.channelId);
const channelMessages = await channel.fetchMessages({ limit: 50 });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing if the loop message is past 50 messages, it probably doesn't matter if we delete it or not 😛 I'm not exactly sure how the reactiflux website is parsing the job posts, not sure if it would pick up the bots messages for the jobs channel specifically.

@rafaelalmeidatk rafaelalmeidatk Oct 17, 2019 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing if the loop message is past 50 messages, it probably doesn't matter if we delete it or not

Yeah exactly.

I'm not exactly sure how the reactiflux website is parsing the job posts, not sure if it would pick up the bots messages for the jobs channel specifically.

I was gonna say that the bot only parses the messages that contain the tags ([FOR HIRE], [HIRING], etc) but I just realized that they are in the message content lol
Great catch, I will change the jobs script to ignore the messages from the bot

Comment thread features/loopMessages/index.js Outdated
const loopMessages = require("./messages.js");

// time between each check to send a new loop message
const INTERVAL_TIME = 5 * 60 * 1000; // 5 minutes (ms)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 5 minutes an appropriate amount of time? Should it be longer? I'm not sure if we want this to be customization depending on the loop message.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, to be honest. The bot won't keep sending new messages if the last one is from it so a low interval time won't create spam, but the messages on #jobs aren't that active to hide the bot message if the interval time is too big. 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want this to be customization depending on the loop message.

The initial implementation had an interval field on each message, but I decided to remove it to avoid making it more complex than it is initially needed. We can add when we feel necessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I increased the interval to 40 minutes

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok now each message can have a different interval

@vcarl

vcarl commented Oct 17, 2019

Copy link
Copy Markdown
Member

I haven't reviewed this much yet, but my initial thoughts for the feature was just to have it be straight up a schedule of how often a message gets posted, and posting it. i was thinking node-cron for scheduling to keep it simple, not sure we need to check recent messages etc

@rafaelalmeidatk

Copy link
Copy Markdown
Member Author

@vcarl I see, so it would be definitely more simple hm 🤔
I like this implementation because the post is almost always visible to someone that is going to post something in the channel (most of them won't scroll up that much before posting something) and it doesn't spam the channel with the same message.

Comment thread features/jobs.js
const jobs = {
tags: ["forhire", "for hire", "hiring", "remote", "local"],
handleMessage: ({ msg, user }) => {
if (msg.author.bot) return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this strictly necessary?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes? What benefit would we get if we had bots messaging/triggering each other?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary because the bot may post a message in the #jobs channel that does not contain these tags so it would receive a warning from itself

Base automatically changed from master to main January 22, 2021 23:29
@rosofo

rosofo commented Jan 31, 2021

Copy link
Copy Markdown
Contributor

May I ask why this wasn't merged? seems a waste

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable periodic messages

5 participants