From 3f751a49f9faa049fa0c7fbf6f6271e7542c54a0 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Mon, 25 Mar 2024 13:03:53 -0700 Subject: [PATCH 1/3] Make search str required and nonempty, non-whitespace-only in search submission Through the frontend. Backend already taken care of this by generating the corresponding errors --- datalad_registry/templates/overview.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datalad_registry/templates/overview.html b/datalad_registry/templates/overview.html index b28bdc42..d2287ce8 100644 --- a/datalad_registry/templates/overview.html +++ b/datalad_registry/templates/overview.html @@ -41,7 +41,7 @@
- Date: Mon, 25 Mar 2024 13:30:55 -0700 Subject: [PATCH 2/3] Remove conflation of `None` and `''` query in web UI Without this conflation, the behavior of handling empty string query args can be made more consistent across the app more easily. A proper message will be generated to the user if they bypass the frontend to submit an empty query string. --- datalad_registry/overview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datalad_registry/overview.py b/datalad_registry/overview.py index 1ecfd369..f8c5915f 100644 --- a/datalad_registry/overview.py +++ b/datalad_registry/overview.py @@ -39,7 +39,7 @@ def overview(): # No type hints due to mypy#7187. # as we would add search to individual files. query = request.args.get("query", None, type=str) search_error = None - if query: + if query is not None: lgr.debug("Search by '%s'", query) try: criteria = parse_query(query) From c70b3b9da0345818f96a0439b54347273ca9f5ce Mon Sep 17 00:00:00 2001 From: Isaac To Date: Mon, 25 Mar 2024 14:34:15 -0700 Subject: [PATCH 3/3] Add more tests for Web UI search Add tests for empty string query and query string that only contains whitespace chars --- datalad_registry/tests/test_overview.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/datalad_registry/tests/test_overview.py b/datalad_registry/tests/test_overview.py index 70d42294..3472ec65 100644 --- a/datalad_registry/tests/test_overview.py +++ b/datalad_registry/tests/test_overview.py @@ -182,12 +182,19 @@ def test_search_with_valid_query( @pytest.mark.usefixtures("populate_with_dataset_urls") @pytest.mark.parametrize( - "search_query", + "search_query, err_msg_prefix", [ - "unknown_field:example", + ("unknown_field:example", "Unknown field: 'unknown_field'. Known are:"), + ("", "Query string cannot be empty"), + (" \t \n", "Query string cannot contain only whitespace"), + (" ", "Query string cannot contain only whitespace"), + (" ", "Query string cannot contain only whitespace"), + (" \t \n \t ", "Query string cannot contain only whitespace"), ], ) - def test_search_with_invalid_query(self, search_query: Optional[str], flask_client): + def test_search_with_invalid_query( + self, search_query: str, err_msg_prefix: str, flask_client + ): """ Test searching with an invalid query """ @@ -197,9 +204,7 @@ def test_search_with_invalid_query(self, search_query: Optional[str], flask_clie soup = BeautifulSoup(resp.text, "html.parser") assert (error_span := soup.find("span", class_="error")) - assert error_span.text.startswith( - "ERROR: Unknown field: 'unknown_field'. Known are:" - ) + assert error_span.text.startswith(f"ERROR: {err_msg_prefix}") def test_pagination(self, populate_with_dataset_urls, flask_client): """