search

Police-Data-Accessibility-Project / data-sources-app

An API and UI for using and maintaining the Data Sources database
MIT License
4 stars 5 forks source link

Search fails in v2 app #401

Closed EvilDrPurple closed 3 months ago

EvilDrPurple commented 3 months ago

Context

The v2 app search functionality is not working currently in both the live app and on local setups Searching for anything without clicking any typeahead suggestions will result in this being displayed: image When clicking a typeahead suggestion, pressing the search button does nothing and will not show the error message until the page is refreshed The nasty part about this error is, it slipped past our tests and an error message is not outputting to the console on local setups. I did have an error message print once, which was this: 

127.0.0.1 - - [19/Aug/2024 13:33:57] "GET /search/typeahead-suggestions?query=Pittsb HTTP/1.1" 500 -
[2024-08-19 13:33:57,414] ERROR in app: Exception on /search/typeahead-suggestions [GET]
Traceback (most recent call last):
  File "/home/kylie/Programming/data-sources-app-v2/resources/PsycopgResource.py", line 43, in wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask_limiter/extension.py", line 1303, in __inner
    return cast(R, flask.current_app.ensure_sync(obj)(*a, **k))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/resources/TypeaheadSuggestions.py", line 81, in get
    response = get_typeahead_suggestions_wrapper(db_client, query)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/middleware/typeahead_suggestion_logic.py", line 14, in get_typeahead_suggestions_wrapper
    suggestions = db_client.get_typeahead_suggestions(query)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/database_client/database_client.py", line 674, in get_typeahead_suggestions
    display_name=row[1],
    ^^^^^^^^^^^
AttributeError: 'DatabaseClient' object has no attribute 'cursor'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask/app.py", line 1484, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask/app.py", line 1469, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask_restx/api.py", line 402, in wrapper
    resp = resource(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask/views.py", line 109, in view
    return current_app.ensure_sync(self.dispatch_request)(**kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask_restx/resource.py", line 41, in dispatch_request
    resp = meth(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/kylie/Programming/data-sources-app-v2/resources/PsycopgResource.py", line 47, in wrapper
    abort(
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask_restx/errors.py", line 28, in abort
    flask.abort(code)
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/flask/helpers.py", line 277, in abort
    current_app.aborter(code, *args, **kwargs)
  File "/home/kylie/Programming/data-sources-app-v2/venv/lib/python3.11/site-packages/werkzeug/exceptions.py", line 861, in __call__
    raise self.mapping[code](*args, **kwargs)
werkzeug.exceptions.InternalServerError: 500 Internal Server Error: The server encountered an internal error and was unable to complete your request. Either the server is overloaded or there is an error in the application.
127.0.0.1 - - [19/Aug/2024 13:33:57] "GET /search/typeahead-suggestions?query=Pittsbu HTTP/1.1" 500 -
'DatabaseClient' object has no attribute 'cursor'

Though this may be a red herring because the line it's pointing to and the error message don't exactly match up. This error predates the recent changes to the connection logic and may have been around for weeks without detection. My hypothesis is this could be a frontend issue since all backend tests pass.

Requirements

Tests

maxachis commented 3 months ago

@EvilDrPurple @joshuagraber @josh-chamberlain This seems to be related to a discussion me and Joshua and Josh were having about how we expect users to use the search functionality.

  1. One idea was that the only valid searches would be from Typeahead suggestions. If that's the case, maybe we want to disable the search button until the user selects a typeahead suggestion.
  2. Another idea was that we should include a secondary search functionality if they type and search for something that isn't accounted for by the Typeahead Suggestion. One point I made was that, realistically, any location they search for should already be pulled by the Typeahead suggestion, making it redundant. Still, if it's normal user behavior for someone to do that, then maybe we should.

Regardless, I think Kylie's right that, with the web design currently as is, some users probably would try to make a search without using the typeahead suggestion. So we'd need to figure out how to approach that.

Additionally, it's worth pointing out that if a user types "Pittsburgh, Allegheny, PA" (as is currently suggested above the search bar), no results would be returned. Partly, that's on me, as the search bar's contents is being fed into my typeahead endpoint, which only searches on a single entity at a time (whether that be a State, City, or Locality). Maybe we want to consider adjusting that.

It's also worth bearing in mind that the frontend is in a very alpha stage right now, so a lot of this is subject to substantial change.

EvilDrPurple commented 3 months ago

Regardless, I think Kylie's right that, with the web design currently as is, some users probably would try to make a search without using the typeahead suggestion. So we'd need to figure out how to approach that.

@maxachis I just wanted to make it clear that the search currently doesn't work at all -- regardless of whether a typeahead suggestion is selected or not. If a typeahead suggestion is selected it doesn't show an error but the search button does not react to being pressed and the results page is not loaded at all

maxachis commented 3 months ago

Regardless, I think Kylie's right that, with the web design currently as is, some users probably would try to make a search without using the typeahead suggestion. So we'd need to figure out how to approach that.

@maxachis I just wanted to make it clear that the search currently doesn't work at all -- regardless of whether a typeahead suggestion is selected or not. If a typeahead suggestion is selected it doesn't show an error but the search button does not react to being pressed and the results page is not loaded at all

@EvilDrPurple That part is intentional! If you check the console log in the browser, you can see the endpoint path being generated. @joshuagraber is working on developing that component.

EvilDrPurple commented 3 months ago

@maxachis ah okay thanks for clearing that up!

joshuagraber commented 3 months ago

All of this is still very much in development. It is definitely premature to test against these front-end views.

FWIW, we also have an issue to set up client-side end-to-end (integration) tests once this work is completed, which will catch errors like these.

Regardless, I think Kylie's right that, with the web design currently as is, some users probably would try to make a search without using the typeahead suggestion. So we'd need to figure out how to approach that.

Yeah, we can just disable the button until a user selects something. That's on my list to add during the search results work.

Additionally, it's worth pointing out that if a user types "Pittsburgh, Allegheny, PA" (as is currently suggested above the search bar), no results would be returned. Partly, that's on me, as the search bar's contents is being fed into my typeahead endpoint, which only searches on a single entity at a time (whether that be a State, City, or Locality). Maybe we want to consider adjusting that.

Agreed, we should probably figure out how to allow such a search to proceed. A non-trivial task (where does one split the string, etc.), but worthwhile to look into.

josh-chamberlain commented 3 months ago

@EvilDrPurple Thank you for reporting this in such detail! I'm going to close this as a non-bug as others have mentioned; the main branch up at https://data-sources-v2.pdap.io/ breaking would be unexpected. https://data-sources-v2.pdap.dev/ is not expected to be totally functional except when we are doing testing for a dev → main merge.

@maxachis @joshuagraber it's a good point that the help text is not helpful! The functionality changed out from under it. I made an issue here about searching more complex types of names with comma separation: https://github.com/Police-Data-Accessibility-Project/data-sources-app/issues/389

Thank you all for being thoughtful testers, and caring about the humans who will have to use this stuff!