Unhide string comparison operators in QueryBuilder

[melton-jason]

Fixes #1035

Overview/Description

Adds a new Preference in the QueryBuilder -> Behavior section of User Preferences called Show comparison operators for text-based fields (default is unechecked/false)

When checked, the preference unhides the following operators for text fields in the Query Builder:

>, Greater than
<, Less than
>=, Greater than or equal to
<=, Less than or equal to

All of these were able to be accomplished previously using the BETWEEN operator, but doing so required knowledge of how MySQL used the BETWEEN operator 'behind the scenes' (which itself would require knowledge of ASCII characters and their values).

This preference can lead to potentially nonsensical results, especially when numeric strings are sorted lexicographically (see #3501 (review)).
This potential confusion is the primary motivation behind putting this functionality behind a preference and making it non-default.

Edge Case Behaviors/Notes

Viewing existing queries which have comparison-based filters on a text field when the preference is disabled

If you do not have the preference checked but encounter an existing comparison filter on a text field, then Specify respects the filter on the field: leaving it untouched. However, if you then switch the filter to a non-comparison operator, then the previous comparison filter will be removed from the list of available filters; only reloading the page, enabling the preference, or generally leaving the page and coming back (i..e, reloading the Query) will allow the user. to"go back" to the previous filter.

edge_case.mov

Let me know if this edge case deserves more attention! A more user-friendly implementation for example might keep the previous comparison-based filter in the list of possible filters to choose from: until the query is saved or the web-page is left.

Viewing queries which have comparison-based filters on a text field in older versions

Support for this feature is not retroactive: if such a filter exists on a query, the filter will be replaced with Any when viewing the query in older versions of Specify 7.
When viewing the Query in Specify 6, Specify 6 will replace the filter with Contains (see #3501 (review)). The Query, however, should be hidden in Specify 6 by default (see #3501 (comment))

@specify/ux-testing @specify/dev-testing
Feel free to give any feedback on the wording of the preference, the default value, etc.! 😄

We can also consider defining this User Preference as a Remote Preference instead, if the scope of the preference should be broader than user (which might become important when/if Shared Queries (#256) are implemented)

Checklist

• Self-review the PR after opening it to make sure the changes look good and
  self-explanatory (or properly documented)
• Add relevant issue to release milestone
• Add relevant documentation (Tester - Dev)
• Add automated tests

Testing instructions

• With the Show comparison operators for text-based fields preference set to false (the default behavior)
• Open a new or existing query with a table containing a text field which is not CatalogNumber (e.g., text1, text2, remarks)
  • CatalogNumber should already contain the comparison operators: regardless of preference value
  • If you do not have a text field in mind, https://files.specifysoftware.org/schema/version/2.10/
• Add the text field to the query if it is not already present
• Save the Query
• Click on/open the Filter Type dropdown (Any, Like, Equal, Between, etc.) and ensure that the Greater Than and Less Than filters are not shown
• Go into User Preferences and under QueryBuilder -> Behavior check/enable the Show comparison operators for text-based fields preference
  • Make sure the title and description of the preference makes sense!
• Without reloading the page, go back to the previous Query and click on/open the Filter Type dropdown
• Ensure the Greater Than and Less Than filters are present in the dropdown
• Select one of the Greater Than or Less Than filters on a text field and save the Query
  • Optionally, ensure that results are returned as expected* when using comparison-based filters on text fields
• Navigate back to User Preferences and disable the Show comparison operators for text-based fields preference
• Navigate back to the previously used Query and ensure that the comparison-based filter is present (and still respected by the query results) until changed

[grantfitzsimmons]

Interestingly, the test panel keeps saying this is the Git SHA anytime I deploy this PR:
8133b0f

[grantfitzsimmons]

Works as expected. Displays "Contains" in Specify 6, so this would further stray from what we consider "interoperability" with 6.

[grantfitzsimmons]

Another comment– string searching is still non-sensical to non-technical users. See this example.

[melton-jason]

Works as expected. Displays "Contains" in Specify 6, so this would further stray from what we consider "interoperability" with 6.

Interesting. I thought Specify 6 had the > and < symbols for strings, as shown by the screenshot in #1035 (comment). Or does 6 change what is available for Query operations based on the 'type' of Catalog Number?
>= and <= would not be supported in 6 as well.

---

Another comment– string searching is still non-sensical to non-technical users. See this example.

Yes, it is based on comparing the ASCII values of each character, which leads to results like this.
Perhaps a warning explaining this behavior would be okay?
I am not sure how useful this would be, but maybe a feature to treat a field in the Query Builder as a different data type could be considered. As in, for this query, treat this text field as numeric (or vice versa).

[maxpatiiuk]

Works as expected. Displays "Contains" in Specify 6, so this would further stray from what we consider "interoperability" with 6.

Interesting. Could it be that the same filter id means different things for different data types in sp6

[melton-jason]

Interesting. Could it be that the same filter id means different things for different data types in sp6

The comparator lists are stored in an Array in Specify 6. There are a lot of helper functions in QueryFieldPanel.java to get the correct comparison operators. (If you are interested, here are the relevant methods. Though I don't recommend going through them).

The comparison operators are determined on the Data Class (type) for the field (when the field is not a Picklist or Catalog Number. There is special logic for both of those cases).
There are hardcoded Operators for Strings, Booleans, and Timestamps. Every other data class gets the same operators.

Here is the Specify 6 logic for determining the comparison operators for the Catalog Number field...

https://github.com/specify/specify6/blob/945d28f00288d2d64979848fb2fb63fda34bdd1b/src/edu/ku/brc/specify/tasks/subpane/qb/QueryFieldPanel.java#L921-L929

        //CatalogNumber needs special treatment - works better as a number.
        //And other fields? Not sure how to tell. Maybe the formatter?????
        if (fieldInfo != null && fieldInfo.getName().equalsIgnoreCase("catalognumber") 
                && fieldInfo.getTableInfo().getClassObj().equals(CollectionObject.class))
        {
            if (fieldInfo.getFormatter() != null && fieldInfo.getFormatter().isNumeric())
            {
                return getComparatorListForClass(Number.class);
            }
            OperatorType[] stringCmps = getComparatorListForClass(String.class);
            OperatorType[] result = new OperatorType[stringCmps.length + 2];
            int c = 0;
            for (OperatorType ot : stringCmps)
            {
                result[c++] = ot;
            }
            result[c++] = SpQueryField.OperatorType.GREATERTHAN;
            result[c++] = SpQueryField.OperatorType.LESSTHAN;
            //result[c++] = SpQueryField.OperatorType.BETWEEN;
            return result;
        }

It quite literally is an amalgamation of hardcoded operator types based on the formatter for the field.

I am not sure on the logic, but more than likely, it is simply receiving a number which it interprets as the index for the Operator List. The 'index' in this case results in Contains being shown...

Because of this behavior, there will be a different operator in the query (for Catalog Number) depending on the formatter for the field.

All other string fields look like they'll display Contains

[maxpatiiuk]

🤦 🤦 🤦 🤦 🤦

[melton-jason]

We would not be able to make this compatible with Specify 6 without an update to 6.
Specify 6 only supports the following comparison operations for string fields :

Contains
Like
Equals
In
Between
Empty

We can close this pull request if the incompatibility is enough to warrant not implementing other operators in Specify 7.

[maxpatiiuk]

Sp7 Query builder is already not fully compatible (relative dates, OR filters, "current user" placeholder, ability to explicitly pick formatter aggregator)
Vinny added code that detected if the query has any sp7 exclusive features, and if so, hides the query in sp6 - if you think that's appropriate here, we could do the same -

specify7/specifyweb/frontend/js_src/lib/components/QueryBuilder/helpers.ts

Line 357 in e54d84a

export function isModern(query: SpecifyResource<SpQuery>): boolean {

[bronwyncombs]

Operators are understandable and working as expected!

[melton-jason]

Operators are understandable and working as expected!

Hi @bronwyncombs thank you for the review!
However, could you be a little more thorough in your testing, please? The catalog number field is a strange field in that it is very commonly treated as numeric (even though it is a string data type in the database).
This pull request is specifically about the comparison operators for string-type fields. You can see the data type of a field in the Schema Config or Datamodel Viewer. Fields like text and remarks are almost always string fields.

Can you test the following operators on a string field that is not Catalog Number and ensure they work? DO they make sense?

>, Greater than
<, Less than
>=, Greater than or equal to
<=, Less than or equal to

[bronwyncombs]

@melton-jason thanks for catching that and letting me know!

Good News: I did the test again on the string fields used for text in the forms and I believe it still checks out.

https://github.com/specify/specify7/assets/135047322/bde700c3-d66d-4c95-a714-a345b07a11d1

Does that look better?

[grantfitzsimmons]

Yes, it is based on comparing the ASCII values of each character, which leads to results like this.
Perhaps a warning explaining this behavior would be okay?
I am not sure how useful this would be, but maybe a feature to treat a field in the Query Builder as a different data type could be considered. As in, for this query, treat this text field as numeric (or vice versa).

@bronwyncombs mentioned this as being questionable as-is. Can we discuss this as a group before merging?

[bronwyncombs]

I don't think this should be implemented without including a preference to unhide or hide these operators for string fields. Unhiding them all by default may impact the non-technical users' ability to understand query building, causing unnecessary confusion and support requests.

[grantfitzsimmons]

Another request for this ability has come in from The College of Idaho– can this be added as a preference and included in v7.9.2?

[pashiav]

• Click on/open the Filter Type dropdown (Any, Like, Equal, Between, etc.) and
• Ensure the Greater Than and Less Than filters are present in the dropdown
• Navigate back to the previously used Query and ensure that the comparison-based filter is present (and still respected by the query results) until changed

---

Nice job!

---

• Make sure the title and description of the preference makes sense!

I think it's good! The description in preferences explains it well.

✅ Did not run into #3501 (review):

I did get this result as well when mixing numbers and letters since we're comparing ASCII.

[emenslin]

• Click on/open the Filter Type dropdown (Any, Like, Equal, Between, etc.) and ensure that the Greater Than and Less Than filters are not shown
• Ensure the Greater Than and Less Than filters are present in the dropdown
• Navigate back to the previously used Query and ensure that the comparison-based filter is present (and still respected by the query results) until changed

Looks good!

[Areyes42]

• Click on/open the Filter Type dropdown (Any, Like, Equal, Between, etc.) and ensure that the Greater Than and Less Than filters are not shown
• Ensure the Greater Than and Less Than filters are present in the dropdown
• Navigate back to the previously used Query and ensure that the comparison-based filter is present (and still respected by the query results) until changed

---

👍👍

[melton-jason]

Thanks for the reviews everyone! I will merge this PR with production now, given that there are three UX/UI testing approvals and one other Developer approval.
Given that there are still three old reviews requesting changes, I will dismiss these for now, but if there are any problems with the PR feel free to leave a comment here and open an Issue!

stale review