-
Notifications
You must be signed in to change notification settings - Fork 4
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
Alert plan integration #599
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work Luke!! Just a few minor typescript details and we should be good to go, make sure to finish your todos / resolve hardcoded components
Could we use hash instead of query for having alerts tab open by default? Queries are often used for determining what content to be displayed(hence search "query"), whereas hashes are used for determining the landing position of the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some preliminary comments, please make sure to incorporate feedback from Courses leads.
method: "POST", | ||
credentials: "include", | ||
mode: "same-origin", | ||
headers: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this a constant? getCsrf()
frontend/plan/actions/index.js
Outdated
if (!res.ok) { | ||
throw new Error(JSON.stringify(res)); | ||
} else { | ||
// update on front end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnnecessary comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want more perfection.
Good work.
Alert + Plan api connection