Bug 31422 - Library limitations might cause data loss when editing patrons
Summary: Library limitations might cause data loss when editing patrons
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords: rel_22_05_candidate
: 21255 32703 33458 (view as bug list)
Depends on:
Blocks: 35868 36376 32119
  Show dependency treegraph
 
Reported: 2022-08-19 15:11 UTC by Katrin Fischer
Modified: 2024-03-20 17:28 UTC (History)
12 users (show)

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


Attachments
Bug 31422: Add patron's current category to dropdown while editing (2.19 KB, patch)
2022-10-20 14:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31422: Add patron's current category to dropdown while editing (2.23 KB, patch)
2022-10-20 20:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 31422: Add patron's current category to dropdown while editing (2.29 KB, patch)
2022-11-02 22:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31422: (QA follow-up) Fix terminology and switch icon (1.80 KB, patch)
2022-11-02 22:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31422: [22.05.x] Add patron's current category to dropdown while editing (3.00 KB, patch)
2023-03-03 13:54 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-08-19 15:11:27 UTC
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.
Comment 1 Donna 2022-10-18 13:42:43 UTC
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.
Comment 2 Katrin Fischer 2022-10-18 14:05:12 UTC
Thx Donna, I had missed you already found this one! Not getting much attention yet :(
Comment 3 Nick Clemens 2022-10-20 14:54:22 UTC
Created attachment 142259 [details] [review]
Bug 31422: Add patron's current category to dropdown while editing
Comment 4 David Nind 2022-10-20 20:12:13 UTC
Created attachment 142285 [details] [review]
Bug 31422: Add patron's current category to dropdown while editing

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2022-10-20 20:28:33 UTC
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.
Comment 6 Marcel de Rooy 2022-10-21 07:13:57 UTC
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?
Comment 7 Nick Clemens 2022-10-21 11:33:29 UTC
(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
Comment 8 Katrin Fischer 2022-11-02 22:56:33 UTC
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>
Comment 9 Katrin Fischer 2022-11-02 22:56:38 UTC
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>
Comment 10 Katrin Fischer 2022-11-02 22:56:57 UTC
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! ;)
Comment 11 Tomás Cohen Arazi 2022-11-03 12:55:55 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 12 Jonathan Druart 2022-11-07 12:35:55 UTC
(In reply to Katrin Fischer from comment #10)
> Commit message could have more words! ;)

Yes, definitely! :D
Comment 13 Katrin Fischer 2023-01-23 21:23:44 UTC
*** Bug 32703 has been marked as a duplicate of this bug. ***
Comment 14 Christopher Brannon 2023-01-23 22:15:54 UTC
Can this be backported to 22.05?
Comment 15 Lucas Gass 2023-01-24 20:39:30 UTC
(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.
Comment 16 Christopher Brannon 2023-01-24 23:22:22 UTC
(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!
Comment 17 Lucas Gass 2023-03-01 13:42:08 UTC
This, and Bug 32119, contain too many merge conflicts for 22.05.x. So no backport.
Comment 18 Nick Clemens 2023-03-03 13:54:00 UTC
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>
Comment 19 Nick Clemens 2023-03-03 13:55:55 UTC
(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
Comment 20 Katrin Fischer 2023-04-08 21:44:14 UTC
*** Bug 33458 has been marked as a duplicate of this bug. ***
Comment 21 Katrin Fischer 2023-04-08 21:46:21 UTC
*** Bug 21255 has been marked as a duplicate of this bug. ***
Comment 22 Christopher Brannon 2023-04-08 23:38:19 UTC
(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!
Comment 23 Fridolin Somers 2023-11-02 20:52:30 UTC
Can we close ?