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

Fix candidate page 404 errors due to missing district #2500

Merged
merged 4 commits into from
Nov 13, 2018

Conversation

lbeaufort
Copy link
Member

@lbeaufort lbeaufort commented Nov 8, 2018

Summary (required)

Found while testing #2463

  • Resolves Candidate with missing district 404's #2499: Fix candidate page 404 errors due to missing district
  • Fix candidate page 404 errors due to missing district.
  • Only show compare button if link can be generated
  • Hide grey box if compare link can't be generated
  • (code cleanup) Remove variable self-assignment

Impacted areas of the application

List general components of the application that this PR will affect:

  • Candidate profile pages

Screenshots

Before

screen shot 2018-11-08 at 4 26 35 pm

screen shot 2018-11-08 at 4 31 59 pm

screen shot 2018-11-08 at 4 29 33 pm

After

screen shot 2018-11-08 at 4 18 31 pm

screen shot 2018-11-08 at 4 20 40 pm

How to test

@codecov-io
Copy link

Codecov Report

Merging #2500 into develop will decrease coverage by 0.1%.
The diff coverage is 44.44%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2500      +/-   ##
===========================================
- Coverage    75.03%   74.92%   -0.11%     
===========================================
  Files          113      113              
  Lines         6929     6932       +3     
  Branches       598      598              
===========================================
- Hits          5199     5194       -5     
- Misses        1730     1738       +8
Impacted Files Coverage Δ
fec/data/views.py 48.38% <ø> (-0.71%) ⬇️
fec/data/templatetags/filters.py 63.04% <44.44%> (-2.08%) ⬇️
fec/fec/static/js/modules/toc.js 87.17% <0%> (-10.26%) ⬇️

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 4b19c75...a2c951d. Read the comment docs.

Copy link
Contributor

@johnnyporkchops johnnyporkchops left a comment

Choose a reason for hiding this comment

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

Tested locally, works well. Code looks good.

@JonellaCulmer
Copy link
Contributor

@lbeaufort Just a couple questions, how many candidates would you say this affects? I'm assuming very few. And are they more likely to be new, first-time, filers?

I pulled it down locally and the changes look good/make sense since there's no district and we're unable to compare the candidate to other candidates. At this point, the only thing I would add, and this can go in a followup ticket would be to address the blank committees section.

@lbeaufort
Copy link
Member Author

@JonellaCulmer I'd say pretty few candidates have blank districts, they're usually new filers, and they're usually corrected fairly quickly. I agree about the blank committees section - do you mind putting in an issue?

@JonellaCulmer
Copy link
Contributor

@lbeaufort Great! Yup, I'll put in an issue. But this one looks good otherwise. Will review and merge.

Copy link
Contributor

@JonellaCulmer JonellaCulmer left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for catching!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants