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

Remove mur-specific query fields - use 'case' #3505

Merged
merged 2 commits into from
Feb 8, 2019

Conversation

lbeaufort
Copy link
Member

@lbeaufort lbeaufort commented Nov 23, 2018

Summary (required)

How to test the changes locally

Impacted areas of the application

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

  • MUR search

@codecov-io
Copy link

codecov-io commented Nov 23, 2018

Codecov Report

Merging #3505 into develop will increase coverage by 0.12%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3505      +/-   ##
===========================================
+ Coverage    88.14%   88.27%   +0.12%     
===========================================
  Files           76       76              
  Lines         6420     6396      -24     
===========================================
- Hits          5659     5646      -13     
+ Misses         761      750      -11
Impacted Files Coverage Δ
webservices/resources/legal.py 72.55% <0%> (+1.84%) ⬆️

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 4b328d2...db06ba1. Read the comment docs.

@lbeaufort lbeaufort changed the title [WIP] Remove mur-specific query fields - use 'case' Remove mur-specific query fields - use 'case' Dec 6, 2018
@lbeaufort lbeaufort requested review from fec-jli, qqss88 and pkfec December 6, 2018 19:45
Copy link
Contributor

@qqss88 qqss88 left a comment

Choose a reason for hiding this comment

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

everything look great to me.

@fec-jli
Copy link
Contributor

fec-jli commented Dec 11, 2018

I have two questions when I review on local.
1)I load 3 Murs into local ES. when I run http://localhost:5000/v1/legal/search/?case_no=7400
it works well. return one mur(7400)
if I run old field name like: http://localhost:5000/v1/legal/search/?mur_no=7400
I got all 3 murs return. Is this ok? we control field name from cms side?

2)if I run DSL to check mur data on local (new field name)
curl localhost:9200/docs/murs/case_7400 | python -m json.tool
no result.

I have to run this and get result (old field name):
curl localhost:9200/docs/murs/mur_7400 | python -m json.tool

not sure these two questions related with this ticket or not. Thank You.

@lbeaufort
Copy link
Member Author

lbeaufort commented Feb 8, 2019

@fec-jli thanks for taking a look!

if I run old field name like: http://localhost:5000/v1/legal/search/?mur_no=7400
I got all 3 murs return. Is this ok? we control field name from cms side?

Yes, because we're removing mur_no as a parameter, the filter won't work, and will return all results

2)if I run DSL to check mur data on local (new field name)
curl localhost:9200/docs/murs/case_7400 | python -m json.tool
no result.
I have to run this and get result (old field name):
curl localhost:9200/docs/murs/mur_7400 | python -m json.tool

This query uses the doc_id, which will start with the case type (in your example, mur_7400). doc_id needs to be unique, and some cases have the same number (something like AF 100, ADR 100).

'doc_id': '{0}_{1}'.format(case_type.lower(), row['case_no']),

@lbeaufort lbeaufort merged commit 1c1edea into develop Feb 8, 2019
@lbeaufort lbeaufort deleted the feature/use-generic-filters branch June 21, 2019 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants