Skip to content

Conversation

@candleindark
Copy link
Collaborator

The main purpose of this PR is to remove the conflation of the case in which the search query string is not provided and the case in which the search query string is an empty string. Removing this conflation will help to develop will permit better consistency and flexibility and make the intent more clear across the app.

Per ChatGPT:

The pattern if query: in the context of checking if a request parameter (query in this case) is provided and non-empty is common in web development. This approach effectively treats a missing parameter (None) and an empty string ("") the same way, which simplifies handling request parameters that are expected to be non-empty strings when provided.

This practice can be considered good or bad depending on the specific requirements of your application:

Pros

  1. Simplicity: It simplifies the code by reducing the need for explicit checks for both None and empty strings.
  2. Usability: From a user's perspective, an empty string often implies "no input" or "default behavior," so this approach aligns with typical user expectations.
  3. Error Prevention: Automatically filtering out empty strings can prevent errors or unnecessary processing later in your code, where a non-empty string might be assumed.

Cons

  1. Potential Inconsistency: If different parts of your program or different endpoints treat empty strings differently (e.g., sometimes as valid input), it might lead to inconsistencies in how input is handled.
  2. Reduced Flexibility: Automatically treating empty strings as None can reduce flexibility. There might be cases where you want to distinguish between a parameter being absent and it being intentionally set to an empty value.
  3. Implicit Behavior: This pattern can sometimes obscure the intent of the code, making it less clear to others (or your future self) that empty strings are being treated as no input.

Additionally, this PR limits user input for the search string to be non-empty strings that contain more than just whitespace chars to ensure user expectations regarding in that aspect are met.

Note: This PR is related to #344 and #345 and will be consistent with #343 in handling string query arguments.

…submission

Through the frontend. Backend already taken care of this
by generating the corresponding errors
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.
Add tests for empty string query and query string that only
contains whitespace chars
@codecov
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.72%. Comparing base (8c7b160) to head (c70b3b9).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #346   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files          52       52           
  Lines        2519     2519           
=======================================
  Hits         2487     2487           
  Misses         32       32           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@candleindark candleindark changed the title Handle web UI empty query Handle empty search query string in web UI Mar 25, 2024
@candleindark candleindark requested a review from yarikoptic March 25, 2024 23:30
@yarikoptic yarikoptic merged commit 4da200b into datalad:master Mar 29, 2024
@candleindark candleindark deleted the handle-web-ui-empty-query branch March 29, 2024 17:47
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.

2 participants