Skip to content

Conversation

@yuna878
Copy link
Contributor

@yuna878 yuna878 commented May 2, 2021

Overview

New notifications flow implemented in v2 api

Changes Made

  • Initially had a weird notifications flow due to miscommunication between backend & frontend
  • Refer to this documentation about the new flow
  • Also made it compatible for web sign-in logic
  • notifications mode = MOBILE || EMAIL instead of IOS || ANDROID || WEB || NONE

I hope it works :)

TODO

Have to migrate notification column on prod db from IOS || ANDROID to MOBILE
Update env var

@yuna878 yuna878 force-pushed the yuna/notif-flow branch from d2d88fc to 2b78a66 Compare May 2, 2021 14:31
@yuna878 yuna878 requested review from alanna-zhou and tedim52 May 2, 2021 14:34
Copy link
Contributor

@alanna-zhou alanna-zhou left a comment

Choose a reason for hiding this comment

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

nice! should probs update the PR title so when u merge in the commit name makes sense :p

return session.serialize_session_v2()

except ValueError:
raise Exception("Invalid token")
Copy link
Contributor

Choose a reason for hiding this comment

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

hm wouldnt people debugging want to know the reason behind the invalid token


# DEPRECATED!!!!
# NONE is a constant imported from constants.py with string value "None"
# Following line of code converts the NONE="None" string value into Python's null value None
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting i did not know this haha

Copy link

@tedim52 tedim52 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants