Bug 33849 - Duplicate patron warning resets patron's library if different than logged in user's library
Summary: Duplicate patron warning resets patron's library if different than logged in ...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-26 20:07 UTC by Lisette Scheer
Modified: 2024-05-29 11:59 UTC (History)
6 users (show)

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


Attachments
Bug 33849: Do not reset new patrons home library when error occurs (1.73 KB, patch)
2024-04-04 08:41 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 33849: Do not reset new patrons home library when error occurs (1.78 KB, patch)
2024-04-12 18:40 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33849: Do not reset new patrons home library when error occurs (1.84 KB, patch)
2024-05-10 13:03 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lisette Scheer 2023-05-26 20:07:16 UTC
When entering a new patron, if the patron's home library is not the same as the logged-in library of the user + the patron is flagged as a possible duplicate, the patron's home library reverts to the logged in user's library.
Comment 1 Emmi Takkinen 2023-09-15 09:42:27 UTC
For some reason $op isn't set as "duplicate" (as it should if I understand this correctly). So this condition isn't filled

if (defined ($data{'branchcode'}) and ( $op eq 'modify' || $op eq 'duplicate' || ( $op eq 'add' && $category->category_type eq 'C' ) )) {
    $userbranch = $data{'branchcode'};
}

and logged in users branchcode is used instead of selected branchcode.
Comment 2 Emmi Takkinen 2023-09-19 06:18:48 UTC
  (In reply to Emmi Takkinen from comment #1)
> For some reason $op isn't set as "duplicate" (as it should if I understand
> this correctly). So this condition isn't filled
> 
Actually, $op shouldn't be "duplicate". It's "duplicate" if patron is added with "Duplicate" button.

> if (defined ($data{'branchcode'}) and ( $op eq 'modify' || $op eq
> 'duplicate' || ( $op eq 'add' && $category->category_type eq 'C' ) )) {
>     $userbranch = $data{'branchcode'};
> }
> 
> and logged in users branchcode is used instead of selected branchcode.

Why to we to this ( $op eq 'add' && $category->category_type eq 'C' )? Shouldn't we use existing branchcode for all patron categorytypes, not only for "C"? And now when I think about this, shouldn't we use existing branchcode no matter what value $op has?
Comment 3 Emmi Takkinen 2024-04-03 10:20:49 UTC
Did some digging, code comes from bug 6116. Patch says:

>When adding a patron who is a child, the default library is not set when 
>defining a guarantor, this patch corrects the problem.
>More, for children with no guarantors, the default branch is the 
>editing branch.
Comment 4 Emmi Takkinen 2024-04-03 11:47:35 UTC
(In reply to Emmi Takkinen from comment #3)
> Did some digging, code comes from bug 6116. Patch says:
> 
> >When adding a patron who is a child, the default library is not set when 
> >defining a guarantor, this patch corrects the problem.
> >More, for children with no guarantors, the default branch is the 
> >editing branch.

Actually, it's not this, it's bug 1164. " Fix for bug 1164, child branch not defaulting to guarantor's on add child". It probably fixes the original issue it was supposed to fix, but this definitely causes issues with other patron categories.
Comment 5 Emmi Takkinen 2024-04-04 08:41:16 UTC
Created attachment 164404 [details] [review]
Bug 33849: Do not reset new patrons home library when error occurs

While adding new patron, if patron is flagged as duplicate
or another error occurs and their home library differs from
library user is logged in, patrons home library resets as
logged in users library. This happens with all patrons
expect those with category type C. This patch removes checking
if patrons category type is C from code so that all category
types use previously chosen home library even if error occurs.

To test:
1. Add new patron and set their library to a different
library than the one you're logged in.
2. Cause an error (wrong age, duplicate etc) while saving.
3. Attempt to save.
=> Note that patrons home library is set as one you're
logged in.
4. Apply this patch.
5. Repeat steps 1 to 3.
=> Note that patrons home library hasn't changed.

Sponsored-by: Koha-Suomi Oy
Comment 6 Emmi Takkinen 2024-04-04 08:44:44 UTC
Added patch which removes condition where patrons category has to be C to keep home library from changing. As far as I can tell, it doesn't anymore solve issue it did when it was added. When adding guarantor for guarantee, guarantees home library didn't change to same as guarantors. This doesn't happen even if we add guarantee for patron from patrons detail view. If that should happen, we probably need a new bug for that.
Comment 7 ByWater Sandboxes 2024-04-12 18:40:26 UTC
Created attachment 164863 [details] [review]
Bug 33849: Do not reset new patrons home library when error occurs

While adding new patron, if patron is flagged as duplicate
or another error occurs and their home library differs from
library user is logged in, patrons home library resets as
logged in users library. This happens with all patrons
expect those with category type C. This patch removes checking
if patrons category type is C from code so that all category
types use previously chosen home library even if error occurs.

To test:
1. Add new patron and set their library to a different
library than the one you're logged in.
2. Cause an error (wrong age, duplicate etc) while saving.
3. Attempt to save.
=> Note that patrons home library is set as one you're
logged in.
4. Apply this patch.
5. Repeat steps 1 to 3.
=> Note that patrons home library hasn't changed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Esther <esther@bywatersolutions.com>
Comment 8 Esther Melander 2024-04-12 18:41:39 UTC
This worked according to the test plan.
Comment 9 Nick Clemens (kidclamp) 2024-05-10 13:03:25 UTC
Created attachment 166554 [details] [review]
Bug 33849: Do not reset new patrons home library when error occurs

While adding new patron, if patron is flagged as duplicate
or another error occurs and their home library differs from
library user is logged in, patrons home library resets as
logged in users library. This happens with all patrons
expect those with category type C. This patch removes checking
if patrons category type is C from code so that all category
types use previously chosen home library even if error occurs.

To test:
1. Add new patron and set their library to a different
library than the one you're logged in.
2. Cause an error (wrong age, duplicate etc) while saving.
3. Attempt to save.
=> Note that patrons home library is set as one you're
logged in.
4. Apply this patch.
5. Repeat steps 1 to 3.
=> Note that patrons home library hasn't changed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Esther <esther@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Katrin Fischer 2024-05-13 08:42:19 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 11 Fridolin Somers 2024-05-29 11:59:18 UTC
Pushed to 23.11.x for 23.11.06