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

Reminder notifications #121

Merged
merged 15 commits into from
Aug 15, 2023
Merged

Reminder notifications #121

merged 15 commits into from
Aug 15, 2023

Conversation

nzeager
Copy link
Member

@nzeager nzeager commented Aug 1, 2023

Pull Request Template

1. Targeted Issue

Ticket #93 15/60 min notifications BE: Adding backend functionality to send users 15 and 60 minute notifications before a session begins.

2. Overview of Solution

We used Redis, Celery, and Celery Beat to check every 5 minutes if we are within 10-15 or 55-60 minutes of a session. If so we call the appropriate function we set up in models.py to email mentors and mentees to remind them how soon their session is starting. celery.py uses Celery Beat to say we want to run this check every 5 minutes. tasks.py uses Celery and checks if the session is in the appropriate time range.

3. Tools Used

We used Redis, Celery, and Celery Beat.

4. Testing Strategy

Run the local server, Redis, Celery, and Celery Beat (see README for instructions on how to do that) and set up sessions on my local server. Then check to see if the emails are sent at the appropriate times.

5. Future Implications

We'll want to add testing to the code we added in model.py down the line (added TODO comments).

6. Screenshots

n/a

7. Code Reviewers

@GitLukeW @esparr @jeromecox This was a team effort, thanks for everyone's contributions!

@nzeager nzeager requested review from GitLukeW and jeromecox August 1, 2023 16:20
@nzeager nzeager marked this pull request as ready for review August 11, 2023 20:39
Copy link
Contributor

@GitLukeW GitLukeW left a comment

Choose a reason for hiding this comment

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

Looks like it has a conflict in the pipfile. We will need to get that fixed before merging.

@nzeager nzeager removed the request for review from jeromecox August 12, 2023 14:59
@nzeager
Copy link
Member Author

nzeager commented Aug 12, 2023

Need to merge some updates from the main branch. Please hold off on reviewing until noted.

UPDATE: Now ready for review.

import ssl
from celery import Celery

os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'config.settings')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's look into how to automate the production versus development environments for celery. Here is the Render documentation for Celery: https://render.com/docs/deploy-celery

Copy link
Collaborator

@esparr esparr left a comment

Choose a reason for hiding this comment

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

Looks great! Excited about getting this implemented. I added a few comments and approved.

Copy link
Contributor

@GitLukeW GitLukeW left a comment

Choose a reason for hiding this comment

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

Thank you!

@GitLukeW GitLukeW merged commit 7a22dce into main Aug 15, 2023
@GitLukeW GitLukeW deleted the reminder-notifications branch September 9, 2023 16:01
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.

3 participants