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

Email settings section in Account page to manage notifications #15933

Merged
merged 17 commits into from
Nov 19, 2020

Conversation

VictorVelarde
Copy link
Contributor

@VictorVelarde VictorVelarde commented Nov 16, 2020

To manage if the user wants email enabled for certain notifications (currently just do_subscriptions) from the /account page.

It adds the frontend to this previous API PR: #15930

Initial style for the row in a new Email Settings category, using harcoded data
WIP, without having any real interaction with data or api
Connect current <Save changes> button to launch both updateUser + updateNotifications settings. No hitting the real API yet.
To manage getting & putting configuration related to new email notification
@VictorVelarde VictorVelarde force-pushed the email-settings branch 4 times, most recently from d5ef1af to 1244d41 Compare November 17, 2020 23:25
@VictorVelarde VictorVelarde changed the title Email settings to manage notifications from Account page Email settings section in Account page to manage notifications Nov 19, 2020
@VictorVelarde VictorVelarde marked this pull request as ready for review November 19, 2020 11:47
@VictorVelarde
Copy link
Contributor Author

Deploying to staging...

@VictorVelarde
Copy link
Contributor Author

VictorVelarde commented Nov 19, 2020

Once we have it, this could be a simple acceptance: https://email-settings.carto-staging.com/

  • Login with email-settings > go to settings/account

    • It should display the new Email settings option with DO notification 'On' (default value, currently true)
  • Modify DO setting to Off / On and save

    • It should display the confirmation dialog, and then save (both user settings / email notifications)

@thedae
Copy link
Contributor

thedae commented Nov 19, 2020

Confirmed, it's working perfectly, thanks!

@VictorVelarde VictorVelarde requested a review from thedae November 19, 2020 15:00
Copy link
Contributor

@thedae thedae left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@VictorVelarde VictorVelarde merged commit 06fe379 into master Nov 19, 2020
@VictorVelarde VictorVelarde deleted the email-settings branch November 19, 2020 15:17
@thedae thedae mentioned this pull request Nov 20, 2020
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.

2 participants