Bug 10237 - Enforce authorized value character limitations
Summary: Enforce authorized value character limitations
Status: RESOLVED DUPLICATE of bug 7013
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact:
URL: /cgi-bin/koha/admin/authorised_values.pl
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-13 14:49 UTC by Owen Leonard
Modified: 2013-08-05 09:09 UTC (History)
3 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 10237 - Enforce authorized value character limitations (6.30 KB, patch)
2013-05-13 16:36 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10237 - Enforce authorized value character limitations (6.31 KB, patch)
2013-06-14 12:40 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 Owen Leonard 2013-05-13 14:49:06 UTC
According to the manual, "Authorized value is limited to 80 characters and cannot have spaces or special characters other than underscores and hyphens in it." We know that breaking this rule causes problems with searching and bugs like Bug 10229. We should enforce this rule on the client and server side to prevent the user from entering invalid data.
Comment 1 Owen Leonard 2013-05-13 16:36:08 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2013-05-14 05:51:06 UTC
Should we also discourage the use dashes? Dashes in collection and location can cause problems with your search not being as exact as you expected it to be (being interpretated as multiple words by zebra).
Comment 3 Owen Leonard 2013-05-14 09:46:46 UTC
(In reply to comment #2)
> Should we also discourage the use dashes?

It's easy to modify the regex if that's the right thing to do. I was simply following what the manual recommended.
Comment 4 Katrin Fischer 2013-05-15 08:49:01 UTC
It took me a bit to find it, but Joy wrote something on the topic a while ago:
http://bywatersolutions.com/2012/07/05/naming-conventions-in-koha/
Comment 5 Katrin Fischer 2013-05-15 09:02:05 UTC
Hm, thinking more about this - there are values where this won't work so nicely - one of them is MANUAL_INV. The code is the value that will be shown for your custom fee type and therefore enforcing upper case, no spaces and no umlauts would not be good.
Comment 6 Kyle M Hall 2013-06-14 12:40:16 UTC
Created attachment 19002 [details] [review]
Bug 10237 - Enforce authorized value character limitations

An authorized value should not have spaces or special characters other
than underscores and hyphens in it, but this limitation is not enforced.
This patch adds client-side validation blocking entries which contain
anything other than numbers, letters, underscores, or dashes. Changes
include:

- The addition of a custom alphanumeric validation scheme which can be
  used anywhere in Koha.
- The addition of a new translatable warning message which appears when
  the alphanumeric validation scheme is violated.
- The addition of rules to the authorized_values.tt template to enable
  the new alphanumeric validation scheme.

To test, apply the patch and clear your browser cache.

With an existing category of authorized values (like ccode):

- Click to add new authorized value
- Try to enter invalid data in the "authorized value" form field. You
  should see a warning.
- Try to submit a form with the invalid data. You should be prevented
  from doing to.
- Correct the invalid data to conform to the validation rules. Warnings
  should disappear.
- Submit the form. It should submit correctly.

With a new category:

- Click to create a new category.
- Try to enter invalid data in the "category" and "authorized value"
  form fields. You should see warnings.
- Try to submit a form with the invalid data. You should be prevented
  from doing to.
- Correct the invalid data to conform to the validation rules. Warnings
  should disappear.
- Submit the form. It should submit correctly.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Katrin Fischer 2013-06-14 12:48:35 UTC
Kyle, did you test what I noted in comment 5?
Comment 8 Jonathan Druart 2013-06-14 15:14:11 UTC
(In reply to comment #5)
> Hm, thinking more about this - there are values where this won't work so
> nicely - one of them is MANUAL_INV. The code is the value that will be shown
> for your custom fee type and therefore enforcing upper case, no spaces and
> no umlauts would not be good.

I confirm that many values contain spaces (I found '|', '(', ')', '-', '/' too).
Marked as Failed QA.
Comment 9 Owen Leonard 2013-06-25 13:06:13 UTC
Okay, let's approach it this way: Which default authorized value categories can contain non-alphanumeric characters? I'll build an explicit check applying the validation rules for only categories which require it.

Asort1
Asort2
BOR_NOTES
Bsort1
Bsort2
CCODE
DAMAGED
DEPARTMENT
INST
LOC
LOST
MANUAL_INV
NOT_LOAN
PA_CLASS
REPORT_GROUP
REPORT_SUBGROUP
RESTRICTED
SUGGEST
SUGGEST_STATUS
SUPPRESS
School
TERM
WITHDRAWN
YES_NO
Comment 10 Katrin Fischer 2013-06-25 15:14:37 UTC
Hm, I don't have school in my installations. I think MANUAL_INV and BOR_NOTES would be candidates probably.
Comment 11 Jonathan Druart 2013-06-26 07:42:04 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to comment #5)
> > Hm, thinking more about this - there are values where this won't work so
> > nicely - one of them is MANUAL_INV. The code is the value that will be shown
> > for your custom fee type and therefore enforcing upper case, no spaces and
> > no umlauts would not be good.
> 
> I confirm that many values contain spaces (I found '|', '(', ')', '-', '/'
> too).
> Marked as Failed QA.

Hum... forget that, I don't know how I found that...

On my side, I found SUGGEST and LOC:

installer/data/mysql/fr-FR/marcflavour/unimarc_lecture_pub/Obligatoire/framework_DEFAULT.sql:INSERT INTO `authorised_values` (`id`, `category`, `authorised_value`, `lib`) VALUES (144, 'SUGGEST', 'Pas assez de budget', 'Pas assez de budget');

installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql:INSERT INTO `authorised_values` ( `category`, `authorised_value`, `lib`) VALUES ( 'LOC', 'Salle de lecture', 'Salle de lecture');

I think it is not correct and the authorised_value field should be a code. But they exist as it in sample data.
Comment 12 Katrin Fischer 2013-07-01 07:39:53 UTC
I think having spaces in the LOC code can lead to unexpected search results. I know there is a bug reporting problems for very similar codes, like EBOOK and EBOOKX. I tend to have the restrictions being put on LOC, because in general it seems more safe to use good codes there.

For SUGG it probably doesn't give you trouble, but I think the codes are not displayed anywhere, so maybe the codes could be cleaned up?
Comment 13 Katrin Fischer 2013-08-05 09:09:48 UTC

*** This bug has been marked as a duplicate of bug 7013 ***