Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reminder command #806

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Akinator31
Copy link
Contributor

This pull request adds a new reminder command to the bot, allowing users to set reminders that will be sent as mentions at specified times. The reminders are stored in a database and scheduled using cron. The following changes were made:

  1. New Command: Added a !rem command that takes a time (HH:MM) and a message as arguments. The command schedules a reminder at the specified time and stores the reminder in the database.

  2. Cron Job Scheduling: Implemented cron jobs to handle the scheduled reminders. When a reminder is due, the bot sends a message mentioning the user who set the reminder. The cron module is included in the "package.json".

  3. Database Integration: Stored reminders in a database table named reminders. Each reminder includes the thread ID, user ID, reminder time, and message. If the table does not exist, it is automatically created when the bot starts.

  4. Allowed Mentions: Ensured that user mentions in reminder messages are active by using the allowedMentions parameter when sending messages.

  5. Deletion Command: Added a !delrap command to delete a scheduled reminder by its ID. This stops the associated cron job and removes the reminder from the database.

How to Use

  • Set a Reminder: Use the command !rem HH:MM message to set a reminder. The bot will confirm the scheduled time and message.

  • Delete a Reminder: Use the command !delrap ID to delete a reminder by its ID.

Example

  • To set a reminder for 14:30, use: !rem 14:30 Don't forget the meeting!
  • To delete a reminder with ID 1, use: !delrap 1

Code Changes

The following files were modified:

Testing

  • Verified that reminders are stored in the database and scheduled correctly.

  • Ensured that reminders are sent as mentions at the correct times.

  • Tested the deletion of reminders and confirmed that the associated cron jobs are stopped.

  • Tested that reminders still work after restarting the bot

Do not hesitate to notify me of any problems.

@Dragory
Copy link
Owner

Dragory commented Oct 14, 2024

I like the idea of reminders, thank you for making the PR! A couple things that come to mind:

  • Create a separate migration for the reminder tables, otherwise the table won't be created for existing setups. You can do this with npm run create-migration -- create_reminders_table
  • It might be worth moving the cronjob creation to a shared function that's used in both loadReminders() and when adding a new reminder. Currently the two pieces of code creating the cronjob are slightly different, with different messages as well.
  • On the topic of cronjobs, we might not need a cronjob for this at all, unless the plan is to have them repeat. As it currently is, it looks like the reminders will keep coming up again after the bot restarts, since they're not deleted from the database. Was that intentional?
  • For the database operations, please create a separate entity class (see e.g. Note and Snippet), and then repository functions that return these (see e.g. notes.js and snippets.js). In general, we should move any direct DB access (or Knex usage) to a repository function rather than the module itself.
  • For the commands, to keep things consistent, we should name them !reminder and !delete_reminder. These can then have shortcuts like !rem and !delrem.
  • There's a function call that currently uses fr-FR for the locale. Let's change this to en-US for now, until there's proper localization support!

@Akinator31
Copy link
Contributor Author

Thank you for your feedback. I will make the changes you just mentioned as soon as I have time ;)

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.

2 participants