-
Notifications
You must be signed in to change notification settings - Fork 4
NRL-1205 dont ignore category if type is given for search #908
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
NRL-1205 dont ignore category if type is given for search #908
Conversation
|
💥 Something went wrong while deploying the pull request environment. |
|
🚀 PR environment successfully deployed. |
|
|
||
| event = create_test_api_gateway_event( | ||
| headers=create_headers(), | ||
| query_string_parameters={ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this right, the test is
- Creating a pointer of type: MHCP, cat: Care Plan
- Creating a pointer of type: NEWS2, cat: Obs
- Searching for pointers of type: MHCP, cat: Care Plan
It then expects the one result (pointer 1) since pointer 2 would be filtered out by both criteria.
I think to prove both filters are being applied there needs to be a test scenario where, e.g., you search for type: MHCP, cat: Obs
Then the expected result is an empty searchset bundle because 1 is filtered out on the basis of failure to match category, and 2 is filtered out on the basis of failure to match type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll add that I'll also add searching with multiple categories and the one type to show that the optimisation for the single type is still used and that it wont get other categories even if they're provided because it isnt for that type
|
Can we add feature tests? |
There are already some feature tests I wrote back when I did the category filter work but I can do an additional scenario that is basically the unit test you mentioned above but an integration test |
|
🚀 PR environment successfully deployed. |
|
🚀 PR environment successfully deployed. |
|
|
🚀 PR environment successfully deployed. |
mattdean3-nhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
|
💥 Something went wrong while destroying the pull request environment. |



No description provided.