| Summary: | Required format is not enforced for Patron Categories | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marc Véron <veron> |
| Component: | Patrons | Assignee: | Marc Véron <veron> |
| Status: | CLOSED FIXED | QA Contact: | Paul Poulain <paul.poulain> |
| Severity: | normal | ||
| Priority: | P3 | CC: | chris, dcook, gmcharlt, jonathan.druart, marc, paul.poulain, veron |
| Version: | 3.8 | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 7628 - Required format is not enforced for Patron Categories
Bug 7628 - Required format is not enforced for Patron Categories Bug 7628 - Required format is not enforced for Patron Categories [SIGNED-OFF] Bug 7628 - Required format is not enforced for Patron Categories Bug 7628: Escape characters for categorycode [alternative patch] [SIGNED-OFF] Bug 7628: Escape '+' characters for categorycode |
||
Created attachment 10918 [details] [review] Bug 7628 - Required format is not enforced for Patron Categories Enforces category code with allowed characters only. Test plan: - Before applying patch, create a category code like Test+ - Try to delete category with this code -> does not work - Apply patch - Try to create category with code containg other characters than letters, numbers, - and _ - Result: You are not allowed to do so, appropriate message pops up Created attachment 10921 [details] [review] Bug 7628 - Required format is not enforced for Patron Categories Enforces category code with allowed characters only. Test plan: - Before applying patch, create a category code like Test+ - Try to delete category with this code -> does not work - Apply patch - Try to create category with code containg other characters than letters, numbers, - and _ - Result: You are not allowed to do so, appropriate message pops up Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 10922 [details] [review] Bug 7628 - Required format is not enforced for Patron Categories Enforces category code with allowed characters only. Test plan: - Before applying patch, create a category code like Test+ - Try to delete category with this code -> does not work - Apply patch - Try to create category with code containg other characters than letters, numbers, - and _ - Result: You are not allowed to do so, appropriate message pops up Signed-off-by: David Cook <dcook@prosentient.com.au> Small patch that does what it says. I'm not sure why someone would put a "+" into a category code, but data validation is always a good thing, and without it you stumble into the problems that Marc has outlined. Created attachment 10923 [details] [review] [SIGNED-OFF] Bug 7628 - Required format is not enforced for Patron Categories Enforces category code with allowed characters only. Test plan: - Before applying patch, create a category code like Test+ - Try to delete category with this code -> does not work - Apply patch - Try to create category with code containg other characters than letters, numbers, - and _ - Result: You are not allowed to do so, appropriate message pops up Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 11034 [details] [review] Bug 7628: Escape characters for categorycode [alternative patch] This patch escape the following characters for categorycode: &, @, /, ;, :, =, +, ? and $ QA Comments: The patch works but it does not allow to delete categorycode with a '+' character. That could be considered as a bug for instance in this situation. I think, but I am not sure, the patch I propose is more adapted for this issue. Marc and David, what do you think about this patch ? For discussion... Jonathan, When our librarians tested Koha for the first time they wanted to have some categories similar to "KIDS 4 +" Then they wanted to delete the code and did not succeed. My patch would have prevented them to enter "illegal" characters (and to have a user experience like 'It does not work'). And if we do not allow to enter illegal characters there is no need to delete entries with illegal characters. I think the main question is: What characters should be / are allowed with category codes? I had a look at the manual for 3.8: http://manual.koha-community.org/3.8/en/patscirc.html#patcats -Snip--------------- The 'Category Code' is an identifier for your new code. Important The category code is limited to 10 characters (numbers and letters) -End snip--------------- Your patch assumes that &, @, /, ;, :, =, +, ? and $ are legal characters. Are they (without having side effects at other places in Koha)? Should all characters be allowed? What was (historically) the reason for the restriction in the manual? Why are the codes upper cased by the existing JavaScript? If there are no side effects, your patch is the better solution, because it gives the useres more flexiblity to choose a 'speaking' code. (Manual should then be updated, though). If we have to expect side effects, I would prefer my restricion on data entry. Marc The solution may be a combination of these 2 patchs. It would allow restriction (from documention) to always be guaranted and existing wrong-formatted category code to be deleted. Agree. Created attachment 11072 [details] [review] [SIGNED-OFF] Bug 7628: Escape '+' characters for categorycode uri escape the following characters: &, @, /, ;, :, =, +, ? and $ Signed-off-by: Marc Veron <veron@veron.ch> Test and sign off second patch (independently from first patch). Added categories like '+zwoelf' or '? Test' before applying the patch. Deletion was not possible. After applying patch deletion is possible, patch works as expected. QA comment: small patch, test-qa.pl OK, passes QA follow-up comment: would be good to add the same control for itemtypes and branches, because I'm not sure at all having them with a special character will work... Pushed to 3.8.x, will be in 3.8.4 |
Problem: -------- If you use characters like '+' in category code, like '+TEST', you can not edit or delete the category. Required format is not enforced. Solution: --------- In categorie.tt, function Check(ff), trim user input and test it for unwanted characters. Instead of: ----------- if (ff.categorycode.value.length==0) { ok=1; _alertString += _("- categorycode missing") + "\n"; } Put: ---- ff.categorycode.value = ff.categorycode.value.trim(); if (ff.categorycode.value.length==0) { ok=1; _alertString += _("- categorycode missing") + "\n"; } else{ var patt=/^[a-zA-Z0-9\-_]+$/g; if ( !patt.test(ff.categorycode.value) ) { ok=1; _alertString += _("- category code can only contain the following characters: letters, numbers, - and _") + "\n"; } }