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 ADR datatable page #2440

Merged
merged 7 commits into from
Oct 24, 2018
Merged

Add ADR datatable page #2440

merged 7 commits into from
Oct 24, 2018

Conversation

patphongs
Copy link
Member

@patphongs patphongs commented Oct 12, 2018

Summary

How to test

  1. vi ~/.bash_profile and insert this feature flag for ADRs export FEC_FEATURE_ADRS=true
  2. Open a new terminal window and run the branch
  3. Go to http://localhost:8000/data/legal/search/adrs/ to test the page. At this point, none of the canonical ADR links will work since that is phase 2 of the development for this feature.

Impacted areas of the application

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

  • Enforcement ADR datatable page

Screenshots

screen shot 2018-10-19 at 4 40 27 pm

- add ADR filter page template
- add ADR search results template
- add url route for adrs
- add ADR legal search results template to view
…respondent to coincide with the improvements made to the API
@codecov-io
Copy link

codecov-io commented Oct 12, 2018

Codecov Report

Merging #2440 into develop will decrease coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2440      +/-   ##
===========================================
- Coverage    74.25%   74.23%   -0.03%     
===========================================
  Files          111      111              
  Lines         6645     6659      +14     
  Branches       591      591              
===========================================
+ Hits          4934     4943       +9     
- Misses        1711     1716       +5
Impacted Files Coverage Δ
fec/fec/settings/base.py 88.75% <ø> (ø) ⬆️
fec/legal/views.py 56.16% <11.11%> (-6.34%) ⬇️
fec/data/api_caller.py 35.23% <66.66%> (+0.15%) ⬆️
fec/legal/urls.py 83.33% <66.66%> (-16.67%) ⬇️
fec/fec/static/js/modules/toc.js 100% <0%> (+12.82%) ⬆️

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 449436a...513f502. Read the comment docs.

- expanded width of case column
- added i-folder icon for ADR cases
- formatted keyword return to be inline
- updated scope of results so that the ADR totals can be accessed
@patphongs patphongs changed the title [WIP] Add ADR datatable page Add ADR datatable page Oct 18, 2018
@patphongs
Copy link
Member Author

@JonellaCulmer The additional datatable items noted here: #2436 (comment) have been added to this PR. Please do another design review.

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.

Looks, great! Thanks, @patphongs

@patphongs patphongs changed the title Add ADR datatable page [Don't merge yet] Add ADR datatable page Oct 23, 2018
@patphongs patphongs changed the title [Don't merge yet] Add ADR datatable page Add ADR datatable page Oct 23, 2018
@patphongs patphongs added this to the Sprint 7.4 milestone Oct 23, 2018
Copy link
Member

@lbeaufort lbeaufort left a comment

Choose a reason for hiding this comment

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

Looks great, @patphongs! I also think the results look great - nice design work @JonellaCulmer

Noting a small issue we discussed that shouldn't be a big issue - the ADR search says “keyword match” when we’re not searching on keywords:

screen shot 2018-10-24 at 10 23 58 am

@lbeaufort lbeaufort merged commit 52a4710 into develop Oct 24, 2018
@lbeaufort lbeaufort deleted the feature/2436-adr-datatable-page branch October 24, 2018 14:33
@lbeaufort
Copy link
Member

I also confirmed that the feature flag worked properly - I did need to unset it, as setting it to false didn't seem to work @patphongs

@patphongs
Copy link
Member Author

I also confirmed that the feature flag worked properly - I did need to unset it, as setting it to false didn't seem to work

Thanks @lbeaufort. The feature flags are strange in that the environment setting is checking for the existence of the feature flag, so technically you can set it to anything (not necessarily a boolean) and it would work. That logic should probably be fixed at some point, it can be confusing.

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