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

Add email unsubscribe UI and logic #5249

Merged
merged 3 commits into from
Nov 7, 2019
Merged

Add email unsubscribe UI and logic #5249

merged 3 commits into from
Nov 7, 2019

Conversation

iamonuwa
Copy link
Contributor

Description
  1. Unsubscribe user from a particular email type instead of all.
  2. Updated the emails to pass the email_type as part of the requests on the unsubscribe email
  3. Redirect to unsubscribe email page if successful
  4. Ensure that only valid email_types are processed. Others redirect to settings page
Refers/Fixes

#4465

Testing

Click on the unsubscribe link at the footer of an email from Gitcoin.

@owocki
Copy link
Contributor

owocki commented Sep 30, 2019

@danlipert i can review this if u guys want

@octavioamu
Copy link
Contributor

octavioamu commented Sep 30, 2019

@iamonuwa why you didn't used #4653 (comment) ? 187kb vs 51kb

@iamonuwa
Copy link
Contributor Author

iamonuwa commented Oct 1, 2019

@iamonuwa why you didn't used #4653 (comment) ? 187kb vs 51kb

Exported same file from figma as jpg. But will update it with your

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #5249 into master will increase coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5249      +/-   ##
==========================================
+ Coverage   29.81%   29.87%   +0.05%     
==========================================
  Files         236      231       -5     
  Lines       19884    19133     -751     
  Branches     2838     2734     -104     
==========================================
- Hits         5929     5716     -213     
+ Misses      13711    13178     -533     
+ Partials      244      239       -5
Impacted Files Coverage Δ
app/retail/emails.py 23.32% <0%> (+0.02%) ⬆️
app/marketing/views.py 11.38% <0%> (-0.51%) ⬇️
app/quests/models.py 44.44% <0%> (-3.31%) ⬇️
app/quests/views.py 22.77% <0%> (-2.01%) ⬇️
app/bounty_requests/forms.py 52% <0%> (-1.85%) ⬇️
app/grants/views.py 14.38% <0%> (-1.8%) ⬇️
app/economy/utils.py 83.87% <0%> (-0.98%) ⬇️
app/grants/models.py 60.88% <0%> (-0.7%) ⬇️
app/dashboard/admin.py 63.09% <0%> (-0.63%) ⬇️
app/marketing/models.py 61.72% <0%> (-0.61%) ⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6721d88...9d4d963. Read the comment docs.

@iamonuwa
Copy link
Contributor Author

@owocki any update on this one? We can chat on slack 🙈

app/marketing/views.py Outdated Show resolved Hide resolved
@owocki
Copy link
Contributor

owocki commented Oct 14, 2019

jusft left a handful of comments

@owocki
Copy link
Contributor

owocki commented Oct 23, 2019

@iamonuwa did u see my comments

@owocki
Copy link
Contributor

owocki commented Oct 31, 2019

@iamonuwa bump ..

@owocki
Copy link
Contributor

owocki commented Nov 7, 2019

ok since @iamonuwa went MIA; i've taken the liberty of addressing my own comnents so this can finally go live. 9d4d963

@owocki owocki merged commit 7b07ec1 into gitcoinco:master Nov 7, 2019
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.

3 participants