The library limitations are used to limit down the selection in pull downs in several spots. If you are editing data from library B while logged in at library A, doing so might result in data loss as the value stored in the modified object might be missing from the drop downs your user sees. Example 1: Item values Itemtype: BK Books - limited to Centerville AV LOC: FIC Fiction - limited to Centerville Your user: logged in for Centerville * Edit an item and make sure it uses the itemtype Books and location Fiction. * Switch library to any but Centerville. * Edit item: Both item type and location drow downs will be empty. * Saving now will empty out itemtype (if not set mandatory) and location resulting in data loss. Example 2: Patron data Patron category: Patron PT - limited to Centerville Your user: logged in for Centerville * Find a patron with the limited patron category * Edit patron: Verify the patron category shows in pull down * Switch library to any but Centerville * Edit patron: the category pull down will show the first entry of the pull down visible for this library. Saving now, you will change the patron category unknowingly resulting in data loss. Additional bug: If you search for "all patrons", you will have an error in the console no patrons will display: Uncaught TypeError: categories_map[data] is undefined. Filed as bug 31421. I think we need to make sure that the pull downs show what the library can use, but also what the current setting of the object is in order to prevent data loss. IIRC the funds pull down on the receive page uses a mechanism for this. Otherwise library limitations are very dangerous to use and can only be recommended if we can make sure that objects from other libraries cannot be edited at all where library limitations have been applied.
This is quite frankly, horrifying. I know a lot of large library systems and consortiums use library limitations. I agree with Katrin's solution - show the user what it currently is, and allow me to change if it needed, but don't let me change it to something that I am not permitted to use based on limitations.
Thx Donna, I had missed you already found this one! Not getting much attention yet :(
Created attachment 142259 [details] [review] Bug 31422: Add patron's current category to dropdown while editing
Created attachment 142285 [details] [review] Bug 31422: Add patron's current category to dropdown while editing Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using koha-testing-docker): 1. Replicated issue for example 2 in comment #1: . Patron used: Mary Burton (changed library to Centerville) 2. Applied the patch. 3. Retested: . When editing, now displays current patron category (previously it was blank) . Now has an ! icon next to it with this message when you hover over it: "The patron's current category (Patron) is limited to other branches" . If you save without changing the patron category, the patron category remains the same (so no data loss) . If you change the patron category and save, the patron category is changed - if you go to edit it again you can't choose the patron category with the limitation 4. Bug 31421 and bug 19361 deal with the other issues in the description.
If I am reading the comments, this patch only solves one issue but the description is broader than that? Am I correct? Or will the other issue(s) be addressed separately?
(In reply to Marcel de Rooy from comment #6) > If I am reading the comments, this patch only solves one issue but the > description is broader than that? Am I correct? Or will the other issue(s) > be addressed separately? Other issues handled on related bugs in 'See also' section
Created attachment 143050 [details] [review] Bug 31422: Add patron's current category to dropdown while editing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 143051 [details] [review] Bug 31422: (QA follow-up) Fix terminology and switch icon * fa-exclamation-circle looks more like 'info', but I feel this should be more of a warning, so switched to fa-exclamation-triangle * Changed branches to libraries in message Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Thanks Nick for tackling this, it was bothering us quite a bit. I wonder if the warning should stand out even more. I have updated the image to fa-exclamation-triangle to give it more of a warning character in the follow-up. Maybe it would be nice to also have a confirmation message if the category was changed? Commit message could have more words! ;)
Pushed to master for 22.11. Nice work everyone, thanks!
(In reply to Katrin Fischer from comment #10) > Commit message could have more words! ;) Yes, definitely! :D
*** Bug 32703 has been marked as a duplicate of this bug. ***
Can this be backported to 22.05?
(In reply to Christopher Brannon from comment #14) > Can this be backported to 22.05? This is a string patch, it can be backported during next month's cycle.
(In reply to Lucas Gass from comment #15) > (In reply to Christopher Brannon from comment #14) > > Can this be backported to 22.05? > > This is a string patch, it can be backported during next month's cycle. Nice. Thanks!
This, and Bug 32119, contain too many merge conflicts for 22.05.x. So no backport.
Created attachment 147698 [details] [review] Bug 31422: [22.05.x] Add patron's current category to dropdown while editing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 31422: (QA follow-up) Fix terminology and switch icon * fa-exclamation-circle looks more like 'info', but I feel this should be more of a warning, so switched to fa-exclamation-triangle * Changed branches to libraries in message Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Lucas Gass from comment #17) > This, and Bug 32119, contain too many merge conflicts for 22.05.x. So no > backport. Please test, I did not - you may not need 32119
*** Bug 33458 has been marked as a duplicate of this bug. ***
*** Bug 21255 has been marked as a duplicate of this bug. ***
(In reply to Lucas Gass from comment #17) > This, and Bug 32119, contain too many merge conflicts for 22.05.x. So no > backport. Oh no! We could really use this. That is too bad!
Can we close ?