Bug 29010 - Weight input pattern wrong
Summary: Weight input pattern wrong
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Thomas Klausner
QA Contact:
URL:
Keywords:
Depends on: 18316
Blocks:
  Show dependency treegraph
 
Reported: 2021-09-14 09:10 UTC by Thomas Klausner
Modified: 2021-11-22 11:22 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 29010: fix pattern in search_field.weight (1.79 KB, patch)
2021-09-14 09:19 UTC, Thomas Klausner
Details | Diff | Splinter Review
Bug 29010: fix pattern in search_field.weight (2.64 KB, patch)
2021-10-14 19:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29010: fix pattern in search_field.weight (2.71 KB, patch)
2021-11-19 15:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29010: (QA follow-up) Change inputmode from numeric to decimal (1.51 KB, patch)
2021-11-19 15:09 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Klausner 2021-09-14 09:10:57 UTC
search_field.weight is of type NUMERIC(5,2) in the database, and values are rendered as floats in /admin/searchengine/elasticsearch/mappings.pl

But the field validation only accepts INTs:

<input type="text" inputmode="numeric" pattern="[0-9]*" name="search_field_weight" value="[% search_field.weight | html %]" />

To fix this, change the regexp in patter as follows:

  pattern="[0-9\.]*"

I can supply a proper patch in a few minutes...
Comment 1 Thomas Klausner 2021-09-14 09:19:31 UTC
Created attachment 124855 [details] [review]
Bug 29010: fix pattern in search_field.weight

search_field.weight is of type NUMERIC(5,2) in the database, and values
are rendered as floats in /admin/searchengine/elasticsearch/mappings.pl

But the field validation only accepts INTs. This patch fixes the pattern
to accept NUMERIC(ish) values

Sponsored-by: Steiermärkische Landesbibliothek
Comment 2 Thomas Klausner 2021-10-14 12:04:02 UTC
this is a rather simple bug with a rather simple fix. Still it would be nice to get it into 2021.11 :-)
Comment 3 David Nind 2021-10-14 13:28:04 UTC
Hi Thomas.

A test plan would help with testing to get it to the sign-off stage.

For example:

Test plan:
1. What is needed to setup testing e.g. Enable Elasticsearch and reindex
2. Steps to replicate current problem
3. Apply the patch
4. Steps to demonstrate that problem is fixed
5. Sign-off!

David
Comment 4 Thomas Klausner 2021-10-14 13:50:40 UTC
Test Plan:

* Enable Elasticsearch (but no need to actually index anyting)
* go to cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl
* Enter an integer (eg "8") into any "weight" column and click save
* Koha now displays the value as NUMERIC, eg. "8.00"
* Change nothing, and click save again
* Save does not work, you get a warning by the browser that the input does not match the requested format (because in the html field only ints are allowed, but the DB stored the value as numeric and returns it as such)
* Workaround: Change all the values back to ints (i.e. remove ".00"), but this is very cumbersome if you have several weights

* apply the patch

* Now try to save again (without changing eg "8.00" to "8". It works
* Add a new weight (eg "4"), save, it's turned into "4.00", but saving again still works
Comment 5 Owen Leonard 2021-10-14 19:22:23 UTC
Created attachment 126303 [details] [review]
Bug 29010: fix pattern in search_field.weight

search_field.weight is of type NUMERIC(5,2) in the database, and values
are rendered as floats in /admin/searchengine/elasticsearch/mappings.pl

But the field validation only accepts INTs. This patch fixes the pattern
to accept NUMERIC(ish) values

- Enable Elasticsearch (but no need to actually index anyting)
- go to cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl
- Enter an integer (eg "8") into any "weight" column and click save
- Koha now displays the value as NUMERIC, eg. "8.00"
- Change nothing, and click save again
- Save does not work, you get a warning by the browser that the input
  does not match the requested format (because in the html field only
  ints are allowed, but the DB stored the value as numeric and returns
  it as such)
- Workaround: Change all the values back to ints (i.e. remove ".00"),
  but this is very cumbersome if you have several weights

- Apply the patch
- Now try to save again (without changing eg "8.00" to "8". It works
- Add a new weight (eg "4"), save, it's turned into "4.00", but saving
  again still works

Sponsored-by: Steiermärkische Landesbibliothek

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Katrin Fischer 2021-11-01 21:34:17 UTC
Don't forget to assign a patch to yourself using the Assignee field once you start working on a patch :)
Comment 7 Katrin Fischer 2021-11-01 22:29:09 UTC
I was checking how we handle other decimal fields and for accounting we have:

<input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" name="amount" id="amount" required="required" min="0" value="" class="noEnterSubmit">

The patch has:

<input type="text" inputmode="numeric" pattern="[0-9\.]*" name="search_field_weight" value="[% search_field.weight | html %]" />

From documentation I think if we want to support decimals we should change the inputmode from numeric to decimal and maybe make the pattern a bit more specific.

For example see:
https://developer.mozilla.org/de/docs/Web/HTML/Global_attributes/inputmode

What do you think?
Comment 8 Katrin Fischer 2021-11-19 15:09:49 UTC
Created attachment 127881 [details] [review]
Bug 29010: fix pattern in search_field.weight

search_field.weight is of type NUMERIC(5,2) in the database, and values
are rendered as floats in /admin/searchengine/elasticsearch/mappings.pl

But the field validation only accepts INTs. This patch fixes the pattern
to accept NUMERIC(ish) values

- Enable Elasticsearch (but no need to actually index anyting)
- go to cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl
- Enter an integer (eg "8") into any "weight" column and click save
- Koha now displays the value as NUMERIC, eg. "8.00"
- Change nothing, and click save again
- Save does not work, you get a warning by the browser that the input
  does not match the requested format (because in the html field only
  ints are allowed, but the DB stored the value as numeric and returns
  it as such)
- Workaround: Change all the values back to ints (i.e. remove ".00"),
  but this is very cumbersome if you have several weights

- Apply the patch
- Now try to save again (without changing eg "8.00" to "8". It works
- Add a new weight (eg "4"), save, it's turned into "4.00", but saving
  again still works

Sponsored-by: Steiermärkische Landesbibliothek

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2021-11-19 15:09:53 UTC
Created attachment 127882 [details] [review]
Bug 29010: (QA follow-up) Change inputmode from numeric to decimal

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2021-11-22 11:14:54 UTC
Pushed to master for 21.11, thanks to everybody involved!