AlexanderOtavka / ride-board

Ride sharing for Grinnell students
https://rideboard.app
MIT License
1 stars 3 forks source link

Notify per ride #69

Closed AlexanderOtavka closed 4 years ago

AlexanderOtavka commented 4 years ago

Resolves #22

Changes

Manual Testing

Create an initial test user with your real Grinnell email so you will actually receive email notifications.

For both passenger and driver apps:

  1. Play with user-level notifications
    1. Go to notification preferences from the rides page
    2. Turn on email notifications and text message notifications, you should receive an email and a text confirming
    3. Turn off text notifications to save money (email notifications are free)
  2. Play with ride-level notifications
    1. Create a ride
      • You should be subscribed for notifications automatically
    2. In an incognito window, create a separate user with a fake email
    3. Post a message as the other user
      • This should automatically subscribe that user
      • You should receive an email about the message posted
    4. Join the ride as the other user, then leave
      • You should receive an email about them joining
      • And another about them leaving
    5. Unsubscribe yourself from that ride's notifications
    6. Send another message from the fake user
      • You should not receive an email
    7. Join and leave the ride again as the other user
      • You should receive no emails about this either
    8. Create a ride as the other user
    9. Post a message on the other user's ride
      • This should subscribe you to the ride
    10. Respond to that message as the other user
      • You should receive an email
AlexanderOtavka commented 4 years ago

@yashdavisgupta I just updated the testing instructions. Could you go through them as a driver, I will go through them as a passenger.

AlexanderOtavka commented 4 years ago

Actually, hold off. It looks like email notifications are super broken.

yashdavisgupta commented 4 years ago

But it looks like SMS is working well!

yashdavisgupta commented 4 years ago

Texts saying "You are now receiving notifications..." should probably not go through every time you modify your notification preferences. In addition, if you are only turning on notifications for a single ride, it might be useful to list what that ride is in a human readable way rather than saying "You are..."

Thoughts?

AlexanderOtavka commented 4 years ago

@yashdavisgupta yeah, that is a bug. There is code that only sends the "You are now receiving notifications" message when the notification method is changed, but it wasn't working.

AlexanderOtavka commented 4 years ago

These changes should fix that issue.

AlexanderOtavka commented 4 years ago

Ready for testing!

yashdavisgupta commented 4 years ago

There is still what I consider to be an issue: When a user with text notifications enabled updates notification preferences to include email notification, a user gets a text message saying they are subscribed in addition to an email. This seems like the wrong behavior, only an email should be sent.

Edit: The vice versa is also true.

yashdavisgupta commented 4 years ago

Testing as a Driver user: Play with user-level notifications Things seem to work as expected, with the exception of the comment above Go to notification preferences from the rides page Works Turn on email notifications and text message notifications, you should receive an email and a text confirming Works Turn off text notifications to save money (email notifications are free) Works, with the exception of the above comment Play with ride-level notifications Works, with the exception of the above comment Create a ride You should be subscribed for notifications automatically True. In an incognito window, create a separate user with a fake email Post a message as the other user This should automatically subscribe that user True You should receive an email about the message posted True Join the ride as the other user, then leave You should receive an email about them joining True And another about them leaving True Unsubscribe yourself from that ride's notifications Send another message from the fake user You should not receive an email False, I still receive notifications. Join and leave the ride again as the other user You should receive no emails about this either False, I still receive notifications. Create a ride as the other user Post a message on the other user's ride This should subscribe you to the ride Works as expected Respond to that message as the other user You should receive an email Works as expected

Edit: Formatting

AlexanderOtavka commented 4 years ago

Join and leave the ride again as the other user You should receive no emails about this either False, I still receive notifications.

@yashdavisgupta I fixed the message notifications, but I wasn't able to reproduce this case. Could you double check?