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.

Sign up for GitHub

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

Jump to bottom

21 push notification #145

Merged
merged 23 commits into from Nov 29, 2021
Merged

21 push notification #145

merged 23 commits into from Nov 29, 2021

Conversation

DDVD233
Copy link
Member

@DDVD233 DDVD233 commented Nov 17, 2021

It can now add a device to a user's device list and notify the user whenever a reminder is required.

Several todos:

  • We need a scheduler to send reminders periodically, as sending reminders are not invoked by accessing an API endpoint.
  • Frontend needs to request notification permission. We need to have a somewhat friendly interface to ask users for permission.
    The todos can be another ticket, separated from this PR to make the PR not too large.

@DDVD233 DDVD233 added the review ready This issue is ready for review! label Nov 17, 2021
@Foxworth22 Foxworth22 added this to In progress in Cheese & Quackers Board via automation Nov 17, 2021
@DDVD233 DDVD233 linked an issue Nov 17, 2021 that may be closed by this pull request
[21pts] Reminders/Push Notifications #21
Closed
5 tasks
RafaelPiloto10
RafaelPiloto10 requested changes Nov 19, 2021
View reviewed changes
app_backend/app/routers/notification.py Outdated Show resolved Hide resolved
app_backend/requirements.txt Show resolved Hide resolved
app_backend/app/routers/notification.py Outdated Show resolved Hide resolved
Cheese & Quackers Board automation moved this from In progress to Review in progress Nov 19, 2021
@Foxworth22
Copy link
Member

@oreorz please review this soon

brendacano
brendacano requested changes Nov 22, 2021
View reviewed changes
Copy link
Contributor

@brendacano brendacano left a comment

Choose a reason for hiding this comment

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

This looks really good. Please update this branch with the new changes merged into dev because it seems like you will have to make some changes based on that merged ticket.

@DDVD233
Copy link
Member Author

DDVD233 commented Nov 26, 2021

Fixed tests; integrated user authentication for both notification and toggle favorite

@brendacano
Copy link
Contributor

is this ready for review?

brendacano
brendacano approved these changes Nov 26, 2021
View reviewed changes
Copy link
Contributor

@brendacano brendacano left a comment

Choose a reason for hiding this comment

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

It looks really good even after having to make changes with the new authentication stuff.

RafaelPiloto10
RafaelPiloto10 approved these changes Nov 28, 2021
View reviewed changes
Copy link
Member

@RafaelPiloto10 RafaelPiloto10 left a comment

Choose a reason for hiding this comment

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

Everything looks good, just make sure to merge after the parent branch has been merged
#151

Cheese & Quackers Board automation moved this from Review in progress to Reviewer approved Nov 28, 2021
@RafaelPiloto10
Copy link
Member

@DDavid233 it has been merged. Feel free to merge when you get the chance!

@DDVD233
Merge branch 'dev' into 21_Push_notification
af521a8 
# Conflicts:
#	app_backend/app/auth_handler.py
#	app_backend/app/routers/friends.py
#	app_backend/app/routers/user.py
#	app_backend/database.py
#	app_backend/tests/friends_test.py
@DDVD233
Fix tests
a125e5b
@DDVD233
Fix tests
c06c884
@RafaelPiloto10
Copy link
Member

@DDavid233 let's merge this ASAP as it has some features we need

@DDVD233 DDVD233 merged commit 84c643f into dev Nov 29, 2021
Cheese & Quackers Board automation moved this from Reviewer approved to Ready for Demo Nov 29, 2021
@DDVD233
Copy link
Member Author

DDVD233 commented Nov 29, 2021

@RafaelPiloto10 Merged

@Foxworth22 Foxworth22 removed the review ready This issue is ready for review! label Nov 29, 2021
@DDVD233 DDVD233 deleted the 21_Push_notification branch December 4, 2021 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelPiloto10 RafaelPiloto10 RafaelPiloto10 approved these changes

@brendacano brendacano brendacano approved these changes

@MimiOlayeye MimiOlayeye Awaiting requested review from MimiOlayeye MimiOlayeye is a code owner

@oreorz oreorz Awaiting requested review from oreorz oreorz is a code owner

Assignees

@DDVD233 DDVD233

Labels
None yet
Projects
Cheese & Quackers Board
  
Ready for Demo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[21pts] Reminders/Push Notifications
4 participants
@DDVD233 @Foxworth22 @brendacano @RafaelPiloto10