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

[BUG]: Change email fix #6761

Merged
merged 4 commits into from
Jul 9, 2020
Merged

[BUG]: Change email fix #6761

merged 4 commits into from
Jul 9, 2020

Conversation

androolloyd
Copy link
Contributor

@androolloyd androolloyd commented Jun 3, 2020

Description

A 500 error occurs during email change, updated to use the update_or_create functionality of the Django orm to simplify the process.

Refers/Fixes

#6561

@codecov
Copy link

codecov bot commented Jun 3, 2020

Codecov Report

Merging #6761 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6761      +/-   ##
==========================================
- Coverage   26.25%   26.23%   -0.02%     
==========================================
  Files         300      300              
  Lines       29329    29323       -6     
  Branches     4319     4317       -2     
==========================================
- Hits         7700     7693       -7     
+ Misses      21358    21355       -3     
- Partials      271      275       +4     
Impacted Files Coverage Δ
app/marketing/utils.py 26.11% <100.00%> (-2.93%) ⬇️
app/quests/views.py 16.22% <0.00%> (ø)
app/dashboard/views.py 10.56% <0.00%> (ø)
...rketing/management/commands/no_applicants_email.py 0.00% <0.00%> (ø)
...eting/management/commands/assemble_leaderboards.py 39.73% <0.00%> (ø)

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 77fd4ef...9a470c1. Read the comment docs.

@danlipert
Copy link
Contributor

@androolloyd you were unable to replicate the bug, correct? What convinces you that this is the correct fix?

@danlipert danlipert marked this pull request as draft June 8, 2020 08:31
@androolloyd
Copy link
Contributor Author

androolloyd commented Jun 8, 2020

the issue was from a 500 error during the change, it happens when updating to an email in the master branch, you already used in the email marketing table, and lead me to the lines i changed.

Once moving to an update or create, the error stops, and from what i can tell everything is operating as expected, i am able to change between any email associated to me.

@androolloyd androolloyd marked this pull request as ready for review June 8, 2020 12:05
@androolloyd
Copy link
Contributor Author

@androolloyd you were unable to replicate the bug, correct? What convinces you that this is the correct fix?

I was able to reproduce a 500 error and that error prevented me from updating my email address, there was a comment on a slack thread about a user getting a 500, once i saw that i investigated further as I had solved a 500 issue just moments before.

@danlipert danlipert merged commit 1aa584d into master Jul 9, 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.

4 participants