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.
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.
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.
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.
Sorry, I should have noticed that myself!
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.
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.
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.
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>
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.
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
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>
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?
(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!
(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!
Is this still "in discussion"? It would be really useful when creating an authorized value category and when creating / editing authorized values, to have any kind of message indicating what format (length, no spaces, which characters are allowed) it needs to take.