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 Coverage end date to Source reports #1701

Merged
merged 11 commits into from
Feb 22, 2018
Merged

Conversation

johnnyporkchops
Copy link
Contributor

@johnnyporkchops johnnyporkchops commented Jan 12, 2018

This PR addresses a few issues that are in grooming. It may be salvaged to implement an interim solution that is being discussed, if/when one is agreed upon. See issues referenced below for discussions:

  • Addresses 1735 (in grooming)
  • Addresses 1470 (Currently implemented solution)

Also these two OpenFEC PRs have been merged by @lbeaufort to accommodate possible solutions:
https://github.com/18F/openFEC/pull/2901- Add committee_pcc to /elections/ endpoint
https://github.com/18F/openFEC/pull/2898 -Add coverage end date to /elections/endpoint

Completion criteria, Issue 1470:

  • change source reports link text from "view", to "View all"
  • /elections/ EndPoint API needs to be modified to return coverage end date
  • add handlebars template to handle this particular mullt-line/conditional content within datatable column

@hcaofec has done some research into backend changes that need to take place and may be of help.

Screenshots

jan-12-2018 15-42-46

@codecov-io
Copy link

codecov-io commented Feb 2, 2018

Codecov Report

Merging #1701 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1701   +/-   ##
========================================
  Coverage    79.39%   79.39%           
========================================
  Files           45       45           
  Lines         3261     3261           
  Branches       488      488           
========================================
  Hits          2589     2589           
  Misses         672      672

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 68dfd27...357fa75. Read the comment docs.

@johnnyporkchops johnnyporkchops mentioned this pull request Feb 2, 2018
3 tasks
@johnnyporkchops
Copy link
Contributor Author

johnnyporkchops commented Feb 2, 2018

feb-02-2018 06-46-29

I took a first pass at changing the link in reports column to link to the location @paul mentioned here. https://github.com/18F/fec-cms/issues/1735

  • [x ] I did not figure out how to link to PCC if there is more than one committee. Resolved-Laura added endpoint param for this,
    - [ ] One candidate does not show a committee ID in the link that is built. (Example: SHOEMAKER, FRANKLIN ED) Resolved: this was a mistake in data that i believe has been corrected

Since this is a priority, anyone feel free to push it forward (or redo it if I missed the mark), I think have taken it a far as I can without some pairing help. , cc @lbeaufort, @LindsayYoung.

@johnnyporkchops johnnyporkchops changed the title [WIP] Add Coverage end date to Source reports column on election pages [WIP] Add Coverage end date to Source reports and link to committee filings tab on election pages Feb 2, 2018
@johnnyporkchops johnnyporkchops changed the title [WIP] Add Coverage end date to Source reports and link to committee filings tab on election pages [WIP] Add Coverage end date to Source reports and link to committee page >filings tab Feb 2, 2018
@johnnyporkchops johnnyporkchops changed the title [WIP] Add Coverage end date to Source reports and link to committee page >filings tab [WIP] Add Coverage end date to Source reports Feb 2, 2018
@johnnyporkchops johnnyporkchops changed the title [WIP] Add Coverage end date to Source reports Add Coverage end date to Source reports Feb 20, 2018
@johnnyporkchops
Copy link
Contributor Author

johnnyporkchops commented Feb 20, 2018

  • Added the source reports coverage end date and language. Conditionally rendering the coverage_end_date or "No processed data this period."-- if no coverage_end_date.
  • Created handlebars templates to render in column

final

@JonellaCulmer
Copy link
Contributor

@johnnyporkchops What's the line height in the source reports column?

Copy link
Member

@patphongs patphongs left a comment

Choose a reason for hiding this comment

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

@johnnyporkchops just a comment about the view all link

No processed data this period.
{{/if}}
<br>
{{#if url}}
Copy link
Member

Choose a reason for hiding this comment

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

If there is no processed data for this period, the link to the view all should not exist. I found that this becomes problematic if there is no committee connected to a candidate. When attempting to click on the view all, it will not filter anything because there is no committee connected to the candidate at all.

Can we move this {{#if url}} block into the `{{#if coverage_end_date }} block so that the link won't show up if there's no processed data?

This is currently a production bug too, so doing this would also fix that bug.

@johnnyporkchops
Copy link
Contributor Author

@JonellaCulmer , line height on the td is 1.8, inherited. A similar example of multiline text in columns is the Version column on the reports datatables.

Copy link
Member

@patphongs patphongs left a comment

Choose a reason for hiding this comment

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

Thanks for the change @johnnyporkchops, this looks good to me

@johnnyporkchops
Copy link
Contributor Author

Thanks @patphongs . I was thinking I do not need the if url statement because there will always be a URL, but we can change that when/if we refine this for 1735

@lbeaufort lbeaufort merged commit a7952f6 into develop Feb 22, 2018
@lbeaufort lbeaufort deleted the feature/coverage-end-date branch February 22, 2018 21:52
@johnnyporkchops
Copy link
Contributor Author

@lbeaufort -- Is this an issue where this one candidate who does not return a committee ID in the URL that is built for this page?
The example page: https://www.fec.gov/data/elections/senate/FL/2018/
SHOEMAKER, FRANKLIN ED
The URL created: https://www.fec.gov/data/reports/house-senate/?committee_id&cycle=2018&is_amended=false
I think you said you had some insight into this when we discovered it while pairing on this task.

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.

5 participants