noahmanger commented 7 years ago

This fixes a few interrelated snags on the candidate pages having to do with the logic around cycles / election years / full 4 and 6 year time periods.

The problem: So you're going to a presidential candidate who ran in 2016. The current behavior when going to a candidate page is to load it with cycle=2016 and election_full=true, and then the page will show aggregated data for the 2014 and 2016 cycles. This works fine.

But the problem is with future dates. Senate and presidential candidates often file to run for office for a year that's 4 or 6 years in the future. The current behavior in this cases is to load the candidate page with cycle=2020 and election_full=true. This causes three problems:

Behind the scenes, we use the 2020 value for all API queries for totals and itemized data; e.g. /schedule_a/?two_year_transaction_period=2020. While there might be data for the 2018 cycle, which we're in, there will never be data for the 2020 cycle until we're in the year 2019. This results in empty contribution and disbursement tables when looking at the default time period for a candidate currently running for president or senate (https://github.com/18F/openFEC-web-app/issues/1985).
When calling the /totals/ endpoint with full_election=true, it currently returns no results because the time period requested includes a time period that hasn't happened yet. That's why when you go to Schumer's profile there are no results, but if you toggle to 2017-2018, the data shows up.
As @jontours uncovered in https://github.com/18F/openFEC/issues/2364, passing a future cycle to the schedule_e aggregates also results in null data.

The solution: Rather than use the regular cycle value for all of the logic explained above, I added logic to set a max_cycle. If the cycle is before or equal to the current cycle we're in, it stays the same. If it's in the future (i.e. 2020 or later), max_cycle will be our current cycle. All of the API calls then use max_cycle to get data.

Similarly, if the request was for election_full=true but the cycle is in the future, it sets show_full_election to false so that we're not trying to aggregate data that doesn't yet exist.

However! In doing this, I uncovered what seems to be an API bug. #2000 fixed the /totals/ call by using the correct full_election param instead of election_full. However, there seems to be a bug in the case of a candidate who only has financial info for part of a full cycle (like this), the API returns nothing. This isn't visible on production because the API call is passing the wrong param, so it's just being ignored, but you can see on dev.

Resolves https://github.com/18F/openFEC-web-app/issues/1985 Resolves https://github.com/18F/openFEC/issues/2364

codecov-io commented 7 years ago

Codecov Report

Merging #2013 into develop will decrease coverage by 0.03%. The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #2013      +/-   ##
===========================================
- Coverage    55.35%   55.32%   -0.04%     
===========================================
  Files           48       48              
  Lines         3268     3270       +2     
  Branches       383      383              
===========================================
  Hits          1809     1809              
- Misses        1443     1445       +2     
  Partials        16       16

Impacted Files	Coverage Δ
openfecwebapp/views.py	`42.18% <0%> (-0.67%)`	:arrow_down:

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 7127a8c...eabf510. Read the comment docs.

jontours commented 7 years ago

Ah nice fix! Makes sense to me. I can look into the bug with the /totals endpoint.

fecgov / openFEC-web-app

Feature/fix candidate cycle logic #2013

Codecov Report