Add ADRs and AFs to API #3365

lbeaufort · 2018-09-06T21:02:46Z

Summary (required)

Resolves #3359

retrieve ADRs and AFs from Postgres
load any attached documents into S3
index the ADRs and AFs in Elasticsearch
make ADRs and AFs searchable
Update view with _vw suffix
Figure out fields specific to ADR/AF: https://docs.google.com/spreadsheets/d/1i8GeWHEeTzt3jdN3RZg3wKA_dSq2J-6CR5JGtq6G2nQ/edit#gid=0
Update load/search with those new fields

Remaining dev work

Figure out if existing query parameters work properly
Reuse query parameters: Add ADRs and AFs to API #3365 (review). Don't get rid of existing mur_ query fields or we'll break the front end, but build generic ones as for MUR/ADR/AF. Put in an issue to refactor front end to use generic filters for MURs.
check codecov and add tests if needed

How to test the changes locally

Test migration with invoke create_sample_db
In webservices/legal/current_cases.py, change fecmur.cases_with_parsed_case_serial_numbers_vw to fecmur.cases_with_parsed_case_serial_numbers_lb_tmp
Test MUR reload celery task
Re-create the indexes and load ADR/AF/MUR:

./manage.py initialize_current_legal_docs
./manage.py load_adrs -s 820
./manage.py load_adrs
./manage.py load_admin_fines -s 820
./manage.py load_admin_fines
./manage.py load_current_murs
./manage.py load_current_murs -s 7000
./manage.py load_current_legal_docs

Search ADR/AF/MUR
http://localhost:5000/v1/legal/search/?type=admin_fines
ADR/AF/MUR "canonical" page

Impacted areas of the application

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

Legal API

codecov-io · 2018-09-06T22:46:15Z

Codecov Report

Merging #3365 into develop will decrease coverage by 0.28%.
The diff coverage is 70.39%.

@@             Coverage Diff             @@
##           develop    #3365      +/-   ##
===========================================
- Coverage    88.75%   88.47%   -0.29%     
===========================================
  Files           75       75              
  Lines         6199     6314     +115     
===========================================
+ Hits          5502     5586      +84     
- Misses         697      728      +31

Impacted Files	Coverage Δ
webservices/tasks/__init__.py	`68% <ø> (ø)`	⬆️
webservices/args.py	`97.52% <ø> (ø)`	⬆️
webservices/tasks/legal_docs.py	`0% <0%> (ø)`	⬆️
webservices/legal_docs/index_management.py	`36.61% <100%> (+4.8%)`	⬆️
webservices/legal_docs/__init__.py	`59.25% <33.33%> (-4.75%)`	⬇️
webservices/resources/legal.py	`70.21% <63.33%> (-2.98%)`	⬇️
webservices/legal_docs/current_cases.py	`87.39% <81.18%> (ø)`

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 842d5bc...52fcbeb. Read the comment docs.

vrajmohan · 2018-09-07T16:38:07Z

webservices/resources/legal.py

@@ -145,7 +154,12 @@ def mur_query_builder(q, type_, from_hit, hits_returned, **kwargs):
        .index('docs_search') \
        .sort("sort1", "sort2")

-    return apply_mur_specific_query_params(query, **kwargs)
+    if type_ == 'murs':


Instead of having distinctly named query parameters for every document type, we may want to consider combining them and having common parameters like document_no, respondents, dispositions, min_date, etc.

vrajmohan

Nice work! Made some minor comments.

vrajmohan · 2018-09-12T14:41:56Z

data/migrations/V0105__add_adr_af.sql

 SET search_path = fecmur, pg_catalog;

-CREATE OR REPLACE VIEW cases_with_parsed_case_serial_numbers AS
+CREATE OR REPLACE VIEW cases_with_parsed_case_serial_numbers_vw AS


Are we dropping the old view cases_with_parsed_case_serial_numbers somewhere?

vrajmohan · 2018-09-12T14:52:04Z

locustfile.py

@@ -200,6 +200,20 @@ def get_mur(self, term=None):
        }
        self.client.get('legal/docs/murs/7074', name='legal_get_mur', params=params)

+    @locust.task


This appears to be pre-existing - the term parameter is unused.

vrajmohan · 2018-09-12T14:56:25Z

locustfile.py

+        params = {
+            'api_key': API_KEY,
+        }
+        self.client.get('legal/docs/adrs/668', name='legal_get_adr', params=params)


Again pre-existing - we may want to select a random document from a list of known document numbers. Also, applies to AOs, AFs and MURs.

I agree, I'll put in a new commit.

qqss88

2 WHERE keywords in the query
RECENTLY_MODIFIED_MURS = """
SELECT case_no, pg_date
FROM fecmur.cases_with_parsed_case_serial_numbers
WHERE pg_date >= NOW() - '8 hour'::INTERVAL
WHERE case_type = 'MUR'
ORDER BY case_serial
"""

qqss88

doc strings need to be updated too - like Mur --> case -

to avoid confusion.

in the mapping file, 'url' mapping could be discussed a little more.

qqss88

in current_murs.py:
def load_current_murs(mur_no=None):
load_cases(mur_no, 'MUR') load_cases(mur_no, 'MUR')

to be unified with adrs and AF, better to change mur_no --> case_no

qqss88

I see different string formatting patterns in the code:

       if case_type == 'MUR':
        return mur	                case['mur_type'] = 'current'
            case['url'] = '/legal/matter-under-review/%s/' % row['case_no']
        else:
            case['url'] = '/legal/{0}/{1}'.format(case_type.lower(),

not a big issue, but better to be unified.

qqss88

later on, we can move hard-coded 'MUR', 'AF', 'ADR' to a case config file or something like that.

lbeaufort · 2018-09-12T16:12:26Z

Thanks for your feedback, @qqss88! Re: #3365 (review), I unify them in this commit: cc6e4d8 sorry if that doesn't address your comment.

qqss88 · 2018-09-12T16:22:09Z

Great job!!! Ignore my comments for those pre-existing stuff. We can discuss how to improve the code the base in a general discussion. Thx.

vrajmohan · 2018-09-12T18:52:09Z

data/migrations/V0105__add_adr_af.sql

+GRANT SELECT ON TABLE cases_with_parsed_case_serial_numbers_vw TO fec_read;
+GRANT SELECT ON TABLE cases_with_parsed_case_serial_numbers_vw TO openfec_read;
+
+DROP VIEW cases_with_parsed_case_serial_numbers


Missing semicolon?

Thank you! I'm surprised the migrations run - probably because it's at the end.

lbeaufort · 2018-09-12T19:03:45Z

@vrajmohan @qqss88 I've made your suggested changes and split the celery task for automatic reloading of ADR/AF to a new PR: #3374

lbeaufort mentioned this pull request Sep 6, 2018

[WIP] Feature/add adr af to legal #3364

Closed

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from 9964dc7 to aed34d5 Compare September 6, 2018 22:37

lbeaufort changed the title ~~Feature/add adr af to legal 2~~ [WIP] Add ADRs and AFs to API Sep 6, 2018

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch 3 times, most recently from 85c04e3 to 4809fe2 Compare September 7, 2018 00:33

vrajmohan reviewed Sep 7, 2018

View reviewed changes

lbeaufort mentioned this pull request Sep 7, 2018

Add ADR and AF to API #3359

Closed

10 tasks

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from 9de7bda to 0906c2c Compare September 11, 2018 14:43

lbeaufort mentioned this pull request Sep 11, 2018

Refactor MUR search to use generic search parameters fecgov/fec-cms#2351

Closed

2 tasks

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch 2 times, most recently from 97f0617 to ac0d5d7 Compare September 11, 2018 21:26

lbeaufort changed the base branch from develop to release/public-20180911 September 11, 2018 21:27

lbeaufort changed the base branch from release/public-20180911 to develop September 11, 2018 21:27

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from ac0d5d7 to 3c4c7d8 Compare September 11, 2018 21:41

lbeaufort changed the title ~~[WIP] Add ADRs and AFs to API~~ Add ADRs and AFs to API Sep 11, 2018

lbeaufort requested review from vrajmohan and pkfec September 11, 2018 22:48

lbeaufort added the Please review label Sep 11, 2018

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from c079002 to 66944f1 Compare September 11, 2018 23:53

lbeaufort changed the base branch from develop to release/public-20180911 September 11, 2018 23:54

lbeaufort changed the base branch from release/public-20180911 to develop September 11, 2018 23:54

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from 66944f1 to 4ac25f7 Compare September 12, 2018 00:02

lbeaufort requested a review from qqss88 September 12, 2018 01:39

vrajmohan approved these changes Sep 12, 2018

View reviewed changes

qqss88 suggested changes Sep 12, 2018

View reviewed changes

qqss88 reviewed Sep 12, 2018

View reviewed changes

qqss88 approved these changes Sep 12, 2018

View reviewed changes

lbeaufort changed the title ~~Add ADRs and AFs to API~~ [WIP] Add ADRs and AFs to API Sep 12, 2018

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from eb1855f to bd6cb12 Compare September 12, 2018 18:16

lbeaufort added 6 commits September 12, 2018 14:33

Modify cases_with_parsed_case_serial_numbers view to include AF, ADR

e43087e

Change load_current_murs to use generic load_current_cases

aa0582c

Add load_adrs and load_admin_fines

855b0ac

Rename current_murs module to current_cases

49c1c60

Add ADRs and Admin Fines to legal search

d688462

Standardize flag for loading specific cases (-s)

05b9754

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch 2 times, most recently from c2f6def to a50986a Compare September 12, 2018 18:51

lbeaufort changed the title ~~[WIP] Add ADRs and AFs to API~~ Add ADRs and AFs to API Sep 12, 2018

vrajmohan suggested changes Sep 12, 2018

View reviewed changes

lbeaufort added 6 commits September 12, 2018 14:59

Add _vw suffix to view

c95e4b3

Add Admin Fine-specific fields

0f62006

Add generic case filters and specific admin fine filters

67c5986

Make case_type required parameter for load_cases

0502f87

Add tests for ADRs and Admin Fines

836b5a4

Add locust tests for ADRs and Admin Fines

52fcbeb

lbeaufort force-pushed the feature/add-adr-af-to-legal-2 branch from a50986a to 52fcbeb Compare September 12, 2018 18:59

lbeaufort mentioned this pull request Sep 12, 2018

Update legal locust tests to test different AO/MUR/ADR/AFs #3375

Open

vrajmohan approved these changes Sep 12, 2018

View reviewed changes

vrajmohan merged commit d06d283 into develop Sep 12, 2018

vrajmohan deleted the feature/add-adr-af-to-legal-2 branch September 12, 2018 19:15

This was referenced Nov 23, 2018

Update MUR filters to use generic case_ filters fecgov/fec-cms#2442

Merged

Remove the mur_ filters from the API #3504

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ADRs and AFs to API #3365

Add ADRs and AFs to API #3365

lbeaufort commented Sep 6, 2018 •

edited

Loading

codecov-io commented Sep 6, 2018 •

edited

Loading

vrajmohan Sep 7, 2018

vrajmohan left a comment

vrajmohan Sep 12, 2018

vrajmohan Sep 12, 2018

vrajmohan Sep 12, 2018

lbeaufort Sep 12, 2018

qqss88 left a comment

qqss88 left a comment

qqss88 left a comment

qqss88 left a comment

qqss88 left a comment

lbeaufort commented Sep 12, 2018

qqss88 commented Sep 12, 2018

vrajmohan Sep 12, 2018

lbeaufort Sep 12, 2018

lbeaufort commented Sep 12, 2018

Add ADRs and AFs to API #3365

Add ADRs and AFs to API #3365

Conversation

lbeaufort commented Sep 6, 2018 • edited Loading

Summary (required)

How to test the changes locally

Impacted areas of the application

codecov-io commented Sep 6, 2018 • edited Loading

Codecov Report

vrajmohan Sep 7, 2018

Choose a reason for hiding this comment

vrajmohan left a comment

Choose a reason for hiding this comment

vrajmohan Sep 12, 2018

Choose a reason for hiding this comment

vrajmohan Sep 12, 2018

Choose a reason for hiding this comment

vrajmohan Sep 12, 2018

Choose a reason for hiding this comment

lbeaufort Sep 12, 2018

Choose a reason for hiding this comment

qqss88 left a comment

Choose a reason for hiding this comment

qqss88 left a comment

Choose a reason for hiding this comment

qqss88 left a comment

Choose a reason for hiding this comment

qqss88 left a comment

Choose a reason for hiding this comment

qqss88 left a comment

Choose a reason for hiding this comment

lbeaufort commented Sep 12, 2018

qqss88 commented Sep 12, 2018

vrajmohan Sep 12, 2018

Choose a reason for hiding this comment

lbeaufort Sep 12, 2018

Choose a reason for hiding this comment

lbeaufort commented Sep 12, 2018

lbeaufort commented Sep 6, 2018 •

edited

Loading

codecov-io commented Sep 6, 2018 •

edited

Loading