Add `q=` free text param to facilities API endpoint #675

kellyi · 2019-07-15T18:14:15Z

Overview

Add q= free text param to facilities API endpoint
Update client to use q= param
Enable searches using q or name to search both name and OAR ID
fields
Update client search input labels to indicate searches happen over
names and OAR IDs
If only one facility returns from a given search, immediately push the details page for that facility using its OAR ID.

Connects #121

Demo

Notes

This currently uses id__icontains=q for the OAR ID which means that it'll return both 1 result which exactly matches the input OAR ID if it's a full OAR ID or any results that match a partial OAR ID. This is the logic we used for the name search, too.

I'm happy to make it an exact match but it seemed useful to have both available.

Testing Instructions

get branch then ./scripts/resetdb
visit localhost:6543 and search for some facilities to get an OAR ID
enter an OAR ID in the text search input and verify that it returns the single result, that the details page is immediately pushed, and that you can click back to see the facilities search tab with one result
search by partial OAR ID (US2019) and verify that that returns a list of results

Checklist

fixup! commits have been squashed
CI passes after rebase
CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

kellyi · 2019-07-15T19:17:51Z

src/app/src/components/FilterSidebar.jsx

@@ -131,7 +132,7 @@ class FilterSidebar extends Component {
                case filterSidebarTabsEnum.guide:
                    return <FilterSidebarGuideTab />;
                case filterSidebarTabsEnum.search:
-                    return <FilterSidebarSearchTab />;
+                    return <Route component={FilterSidebarSearchTab} />;


This was done to have history.push available in the mapDispatchToProps function.

Consider an inline comment explaining our reasoning.

jwalgran

The code looks good. I don't have a dev environment up and running yet, so I have not exercised it.

jwalgran · 2019-07-15T19:50:43Z

src/django/api/views.py

                coreapi.Field(
                    name='name',
                    location='query',
                    type='string',
                    required=False,
-                    description='Facility Name',
+                    description='Facility Name or OAR ID',


Consider adding DEPRECATED. Use "q" instead. to the description.

Sounds good. I added this in 56208cf

jwalgran · 2019-07-15T19:52:41Z

src/django/api/views.py

-                                        None)
+        free_text_query = request.query_params.get(FacilitiesQueryParams.Q,
+                                                   None)
+        name = request.query_params.get(FacilitiesQueryParams.NAME, None)


Consider adding an inline comment explaining that name is deprecated in favor of q

👍 Added in 56208cf

jwalgran · 2019-07-15T19:53:01Z

src/django/api/views.py

        if name is not None:
-            queryset = queryset.filter(name__icontains=name)
+            queryset = queryset.filter(Q(name__icontains=name) |


Consider adding an inline comment explaining that name is deprecated in favor of q

👍 see 56208cf

jwalgran · 2019-07-15T20:03:53Z

src/app/src/actions/facilities.js

+                ).length === 1;
+
+                if (responseHasOnlyOneFacility) {
+                    flow(


Consider this more procedural version that makes sure we don't push an invalid route and alerts us to a problem.

const id = get(data, `features[0].id`, '') if (id) { pushNewRoute(makeFacilitiesDetailLink(id)) } else { // Raise an exception that we will see in Rollbar }

Sounds good -- this is in 7327ad1

Since the code is in a promise the existing catch will handle the thrown error.

jwalgran · 2019-07-15T20:22:41Z

src/app/src/components/FilterSidebar.jsx

@@ -131,7 +132,7 @@ class FilterSidebar extends Component {
                case filterSidebarTabsEnum.guide:
                    return <FilterSidebarGuideTab />;
                case filterSidebarTabsEnum.search:
-                    return <FilterSidebarSearchTab />;
+                    return <Route component={FilterSidebarSearchTab} />;


Consider an inline comment explaining our reasoning.

kellyi · 2019-07-15T21:47:41Z

Consider an inline comment explaining our reasoning.

I added the comment in a3c186c

- Add `q=` free text param to facilities API endpoint - Update client to use `q=` param - Enable searches using `q` or `name` to search both name and OAR ID fields - Update client search input labels to indicate searches happen over names and OAR IDs

If only one facility returns in the search response, push the details page for that facility. (The search tab will remain populated on clicking back.)

kellyi · 2019-07-16T23:56:34Z

I rebased this on develop and squashed the fixups.

jwalgran

Works well.

kellyi · 2019-07-17T17:42:34Z

Thanks!

kellyi requested a review from jwalgran July 15, 2019 19:14

kellyi assigned jwalgran Jul 15, 2019

kellyi marked this pull request as ready for review July 15, 2019 19:15

kellyi commented Jul 15, 2019

View reviewed changes

jwalgran reviewed Jul 15, 2019

View reviewed changes

kellyi changed the title ~~WIP Add q= free text param to facilities API endpoint~~ Add q= free text param to facilities API endpoint Jul 16, 2019

Kelly Innes added 3 commits July 16, 2019 19:55

Push details page if only one facility returns

8a800ff

If only one facility returns in the search response, push the details page for that facility. (The search tab will remain populated on clicking back.)

Update CHANGELOG

d201bdd

kellyi force-pushed the ki/search-by-oar-id branch from 11338cc to d201bdd Compare July 16, 2019 23:55

jwalgran approved these changes Jul 17, 2019

View reviewed changes

jwalgran assigned kellyi and unassigned jwalgran Jul 17, 2019

kellyi merged commit 41820e7 into develop Jul 17, 2019

kellyi deleted the ki/search-by-oar-id branch July 17, 2019 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `q=` free text param to facilities API endpoint #675

Add `q=` free text param to facilities API endpoint #675

kellyi commented Jul 15, 2019 •

edited

kellyi Jul 15, 2019

jwalgran Jul 15, 2019

jwalgran left a comment

jwalgran Jul 15, 2019

kellyi Jul 15, 2019

jwalgran Jul 15, 2019

kellyi Jul 15, 2019

jwalgran Jul 15, 2019

kellyi Jul 15, 2019

jwalgran Jul 15, 2019

kellyi Jul 15, 2019

jwalgran Jul 15, 2019

kellyi commented Jul 15, 2019

kellyi commented Jul 16, 2019

jwalgran left a comment

kellyi commented Jul 17, 2019

Add q= free text param to facilities API endpoint #675

Add q= free text param to facilities API endpoint #675

Conversation

kellyi commented Jul 15, 2019 • edited

Overview

Demo

Notes

Testing Instructions

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwalgran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellyi commented Jul 15, 2019

kellyi commented Jul 16, 2019

jwalgran left a comment

Choose a reason for hiding this comment

kellyi commented Jul 17, 2019

Add `q=` free text param to facilities API endpoint #675

Add `q=` free text param to facilities API endpoint #675

kellyi commented Jul 15, 2019 •

edited