Bug 24288 - Add wording to authorized value interface to indicate valid code length
Summary: Add wording to authorized value interface to indicate valid code length
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ava
QA Contact: Kyle M Hall
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2019-12-20 16:13 UTC by Andrew Fuerste-Henry
Modified: 2022-09-21 12:25 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24288: Add wording to authorized value interface to indicate valid code length (2.00 KB, patch)
2020-04-16 08:46 UTC, David Roberts
Details | Diff | Splinter Review
Bug 24288: Add wording to authorized value interface to indicate valid code length (2.03 KB, patch)
2020-04-16 12:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 24288: Add wording to authorized value interface to indicate valid code length (2.03 KB, patch)
2020-08-18 18:11 UTC, Marco Abi-Ramia
Details | Diff | Splinter Review
Bug 24288: Add wording to authorized value interface to indicate valid code length (1.90 KB, patch)
2021-01-21 03:24 UTC, Ava
Details | Diff | Splinter Review
Bug 24288: Add wording to authorized value interface to indicate valid code length (1.96 KB, patch)
2021-01-21 16:11 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 24288: Add wording to authorized value interface to indicate valid code length (2.03 KB, patch)
2021-01-21 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-12-20 16:13:52 UTC
Several standard authorized values feed into fields that require a single character code (LOST, NOTFORLOAN, DAMAGED, RESTRICTED, WITHDRAWN). It'd be good to have some wording in the authorized value interface that would indicate this.
Comment 1 David Roberts 2020-04-16 08:46:14 UTC
Created attachment 103060 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

This patch adds a warning that the authorized value field needs to be a
single numeric character, where appropriate

To test:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED, RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the Authorized value field stating "The authorized value field in the X category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized value
category.
Comment 2 Andrew Fuerste-Henry 2020-04-16 11:41:54 UTC
Sorry! I gave the wrong name for the Nor For Loan auth value in my bug and that mistake followed into the patch. It should be NOT_LOAN. Otherwise this does a good job of adding some guidance.

Part of me feels like we should go a step further and actually block folks from saving a value that's not valid, but that's a whole other thing.
Comment 3 Andrew Fuerste-Henry 2020-04-16 12:43:01 UTC
Created attachment 103083 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

Chnged NOTFORLOAN to NOT_LOAN

This patch adds a warning that the authorized value field needs to be a
single numeric character, where appropriate

To test:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED, RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the Authorized value field stating "The authorized value field in the X category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized value
category.
Comment 4 David Roberts 2020-04-16 15:22:23 UTC
Sorry, I should have noticed that myself!
Comment 5 Marco Abi-Ramia 2020-08-18 18:11:21 UTC
Created attachment 108536 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

Chnged NOTFORLOAN to NOT_LOAN

This patch adds a warning that the authorized value field needs to be a
single numeric character, where appropriate

To test:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED, RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the Authorized value field stating "The authorized value field in the X category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized value
category.
Comment 6 Kelly McElligott 2020-08-19 13:14:16 UTC
Hi,
I tested this and did not receive any warning / or see any text to indicate that these values needed to be a single digit numeric value.  I was able to create a a new value for both NOT_LOAN and LOST with letters and was successful.
Comment 7 Ava 2021-01-21 03:24:24 UTC
Created attachment 115498 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

TEST PLAN:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED,
RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the
Authorized value field stating "The authorized value field in the X
category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized
value category.
Comment 8 PTFS Europe Sandboxes 2021-01-21 16:11:00 UTC
Created attachment 115569 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

TEST PLAN:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED,
RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the
Authorized value field stating "The authorized value field in the X
category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized
value category.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Comment 9 Barbara Johnson 2021-01-21 16:29:14 UTC
The patch worked as described.  I could see the "must be a one-digit numeric value" warning.  I was able to add two and three digit values and alpha values as well.  So I agree that it would be great to add functionality to block adding anything that is not a one digit numeric value.
Comment 10 Andrew Fuerste-Henry 2021-01-21 16:41:46 UTC
Wording is a great start, though! Filed a new bug to actually prevent invalid values: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27512
Comment 11 Kyle M Hall 2021-01-21 17:34:18 UTC
Created attachment 115581 [details] [review]
Bug 24288: Add wording to authorized value interface to indicate valid code length

TEST PLAN:

1) Apply the patch
2) Add a new authorised value policy in the LOST, NOTFORLOAN, DAMAGED,
RESTRICTED, and WITHDRAWN categories
3) Check that there is a new warning displaying next to the
Authorized value field stating "The authorized value field in the X
category must be a one-digit numeric value"
4) Check that this same warning appears when modifying the same
authorised values
5) Check that the warning doesn't appear in any other authorized
value category.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Jonathan Druart 2021-01-22 13:43:32 UTC
1. Maybe also YES_NO?

2. NOT_LOAN can be negative. I don't think negative numbers are "one-digit" numbers (being pedantic I suppose but that may be confusing?)

3. Shouldn't we then add a check inputmode="numeric" and pattern when appropriate?
Comment 13 Andrew Fuerste-Henry 2022-09-15 13:41:03 UTC
(In reply to Jonathan Druart from comment #12)
> 1. Maybe also YES_NO?
Makes sense to me to include this.

> 
> 2. NOT_LOAN can be negative. I don't think negative numbers are "one-digit"
> numbers (being pedantic I suppose but that may be confusing?)
I see your point, technically. Would it be more clear an understandable to say something like "whole number between -9 and 9"?

> 
> 3. Shouldn't we then add a check inputmode="numeric" and pattern when
> appropriate?
Sounds good!
Comment 14 Kyle M Hall 2022-09-21 12:25:06 UTC
(In reply to Andrew Fuerste-Henry from comment #13)
> (In reply to Jonathan Druart from comment #12)
> > 1. Maybe also YES_NO?
> Makes sense to me to include this.
> 
> > 
> > 2. NOT_LOAN can be negative. I don't think negative numbers are "one-digit"
> > numbers (being pedantic I suppose but that may be confusing?)
> I see your point, technically. Would it be more clear an understandable to
> say something like "whole number between -9 and 9"?

All those tinyint(1) columns can be any value between -128 and 127. The 1 only specifies the default column display width and does not affect the size of the number that can be held within.

Given that, maybe we should write -128 to 127, since they are valid values.
Should we go a step further and explain the different behaviors between negative and positive values for a given category?

> > 
> > 3. Shouldn't we then add a check inputmode="numeric" and pattern when
> > appropriate?

Sounds good to me!



> Sounds good!