Bug 22632

Summary: Add logging of merged patrons
Product: Koha Reporter: Andrew Fuerste-Henry <andrew>
Component: PatronsAssignee: axel <axel.amghar>
Status: Pushed to main --- QA Contact: Nick Clemens (kidclamp) <nick>
Severity: enhancement    
Priority: P5 - low CC: axel.amghar, axel.amghar, david, gmcharlt, kebliss, koha, kyle, lucas, marie-luce.laflamme, marjorie.barry-vila, mathsabypro, nick, rcoert, severine.queune, sharon.dugdale
Version: MainKeywords: Academy, additional_work_needed
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
This enhancement adds details of patron merges to the log viewer (when BorrowersLog is enabled).
Version(s) released in:
25.11.00
Circulation function:
Bug Depends on:    
Bug Blocks: 40641    
Attachments: without this patch , when patrons are merged , you can't see the merge in the log viewer.
Bug 21003: Made Edit keywords more consistent in add_items-step2.tt
Unit Testing of patch 22632
Bug 22632: Add logging of merged patrons
Unit Testing of patch 22632
adding of the rescearch of merge in the log viewer
Bug 22632: Add logging of merged patrons
Bug 22632: Unit Testing of patch 22632
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Change for the research of Modules : Patrons
Bug 22632: Change for the research of Modules : Patrons
Bug 22632: Add logging of merged patrons
Bug 22632: Unit Testing of patch 22632
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Change for the research of Modules : Patrons
Bug 22632: Add logging of merged patrons
Bug 22632: Unit Testing of patch 22632
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t
Bug 22632: (follow-up) Tidy and fix tests
Bug 22632: (follow-up) Tidy and fix tests
Bug 22632: (follow-up) Tidy and fix tests
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) QA tool corrections for Log.t
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) QA tool corrections for Log.t
Bug 22632: Add logging of merged patrons
Bug 22632: adding of the rescearch of merge in the log viewer
Bug 22632: Unit Testing of patch 22632
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Bug 22632: (follow-up) QA tool corrections for Log.t
Bug 22632: (QA follow-up) Add no warnings test

Description Andrew Fuerste-Henry 2019-04-03 19:32:01 UTC
When patrons are merged, we don't retain any record of that merging. One patron is retained and the other goes into deletedborrowers and that's it. It's be great to be able to somehow identify if a current patron was previously merged and which deleted patron they were merged with.
Comment 1 axel 2019-04-11 15:32:57 UTC Comment hidden (obsolete)
Comment 2 axel 2019-04-12 14:11:13 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2019-04-12 14:13:36 UTC
I think something went wrong here with the patch attached. Obsoleting the last one.
Comment 4 axel 2019-04-12 14:15:21 UTC Comment hidden (obsolete)
Comment 5 axel 2019-04-12 14:16:36 UTC Comment hidden (obsolete)
Comment 6 axel 2019-04-12 14:16:39 UTC Comment hidden (obsolete)
Comment 7 axel 2019-04-12 15:10:17 UTC Comment hidden (obsolete)
Comment 8 axel 2019-04-15 07:51:48 UTC Comment hidden (obsolete)
Comment 9 axel 2019-04-15 07:51:52 UTC Comment hidden (obsolete)
Comment 10 axel 2019-04-15 07:51:55 UTC Comment hidden (obsolete)
Comment 11 axel 2019-04-15 07:51:58 UTC Comment hidden (obsolete)
Comment 12 axel 2019-04-15 08:28:35 UTC Comment hidden (obsolete)
Comment 13 axel 2019-04-16 08:51:21 UTC Comment hidden (obsolete)
Comment 14 axel 2019-04-16 08:51:25 UTC Comment hidden (obsolete)
Comment 15 axel 2019-04-16 08:51:28 UTC Comment hidden (obsolete)
Comment 16 axel 2019-04-16 08:51:32 UTC Comment hidden (obsolete)
Comment 17 Andrew Fuerste-Henry 2019-05-07 16:13:52 UTC
Would it be possible for this to get entered in the log with the retained patron as the object, rather than the deleted patron? That way it would show up in the modification log for the extant patron, which seems a bit more helpful than having it tie to the deleted patron.

Other than that small nitpick, this is great!
Comment 18 axel Amghar 2019-05-13 09:16:59 UTC
(In reply to Andrew from comment #17)
> Would it be possible for this to get entered in the log with the retained
> patron as the object, rather than the deleted patron? That way it would show
> up in the modification log for the extant patron, which seems a bit more
> helpful than having it tie to the deleted patron.
> 
> Other than that small nitpick, this is great!

hi,

in info we have :
patron 1 has been been merged into patron 2, so we see both patrons.

Did you mean to reverse the sentence in:
patron 2 has been merged with patron 1  ?
Comment 19 Katrin Fischer 2019-05-13 11:01:28 UTC
(In reply to axel Amghar from comment #18)
> (In reply to Andrew from comment #17)
> > Would it be possible for this to get entered in the log with the retained
> > patron as the object, rather than the deleted patron? That way it would show
> > up in the modification log for the extant patron, which seems a bit more
> > helpful than having it tie to the deleted patron.
> > 
> > Other than that small nitpick, this is great!
> 
> hi,
> 
> in info we have :
> patron 1 has been been merged into patron 2, so we see both patrons.
> 
> Did you mean to reverse the sentence in:
> patron 2 has been merged with patron 1  ?

Hi Axel, there is an object column in the action_log table - I think he refers to that. Not to the info part.
Comment 20 Andrew Fuerste-Henry 2019-05-13 12:10:33 UTC
Hi folks,

Right, as Katrin says, I'm looking at the Object column. I realize both patrons are in the Info column, so this works ok. But as it is, if I merge patrons 1 & 2, retaining patron 1, and then click Modification Log from patron 1's record the merge doesn't appear in the log search Koha generates.
Comment 21 Katrin Fischer 2019-05-13 12:55:42 UTC
What Andrew suggests makes sense to me!
Comment 22 axel Amghar 2019-05-14 09:16:15 UTC Comment hidden (obsolete)
Comment 23 axel Amghar 2019-05-14 09:16:18 UTC Comment hidden (obsolete)
Comment 24 axel Amghar 2019-05-14 09:16:22 UTC Comment hidden (obsolete)
Comment 25 axel Amghar 2019-05-14 09:21:05 UTC
(In reply to Katrin Fischer from comment #21)
> What Andrew suggests makes sense to me!

Hi, thank you for for the explanations :)
I think i get it, tell me if it's better.
Comment 26 axel Amghar 2019-05-14 09:42:26 UTC
i have issues with the unit test so i put the status in assigned
Comment 27 axel Amghar 2019-05-14 10:10:36 UTC Comment hidden (obsolete)
Comment 28 Marie-Luce Laflamme 2019-05-31 19:56:07 UTC
Patch doesn't apply.

The 'Merge' is not recorded in the log viewer, only the deleted account is shown.
Comment 29 Sharon Dugdale 2022-11-02 13:11:18 UTC
Hi
This has come up as an issue with Cumbria Libraries and the ability to see the previous information on a merged patron record in the modification logs would be really useful. I am told that the information is retained in the deleted borrowers table however that would require a report to access the information and our frontline staff would find it very useful to be able to see these deleted personal details from the merged account. We get alot of duplicate memberships from patrons re-joining online, sometimes with more uptodate information than their existing record has and it would be useful for staff to have this information if a patron queries the merge.
Comment 30 Owen Leonard 2023-10-17 16:21:06 UTC Comment hidden (obsolete)
Comment 31 Owen Leonard 2023-10-17 16:21:09 UTC Comment hidden (obsolete)
Comment 32 Owen Leonard 2023-10-17 16:21:12 UTC Comment hidden (obsolete)
Comment 33 Owen Leonard 2023-10-17 16:21:16 UTC Comment hidden (obsolete)
Comment 34 Owen Leonard 2023-10-17 16:21:19 UTC Comment hidden (obsolete)
Comment 35 Owen Leonard 2023-10-17 16:21:22 UTC Comment hidden (obsolete)
Comment 36 Owen Leonard 2023-10-17 16:23:47 UTC
I've rebased this and added some cleanup follow-ups, but I think this still needs work to address all the QA tool issues.

Hopefully someone else can take this to the finish line?
Comment 37 David Nind 2023-10-17 16:29:51 UTC Comment hidden (obsolete)
Comment 38 David Nind 2023-10-17 16:29:55 UTC Comment hidden (obsolete)
Comment 39 David Nind 2023-10-17 16:29:58 UTC Comment hidden (obsolete)
Comment 40 David Nind 2023-10-17 16:30:02 UTC Comment hidden (obsolete)
Comment 41 David Nind 2023-10-17 16:30:06 UTC Comment hidden (obsolete)
Comment 42 David Nind 2023-10-17 16:30:10 UTC Comment hidden (obsolete)
Comment 43 David Nind 2023-10-17 16:35:25 UTC Comment hidden (obsolete)
Comment 44 Nick Clemens (kidclamp) 2023-10-17 18:45:57 UTC Comment hidden (obsolete)
Comment 45 Nick Clemens (kidclamp) 2023-10-17 18:46:29 UTC
(In reply to David Nind from comment #43)
> I signed off before seeing comment #36
> 
> What should the status be changed to? Failed QA?
> 
Tests should pass now, and tidied - can you confirm tests and sign off David?
Comment 46 David Nind 2023-10-17 19:10:08 UTC Comment hidden (obsolete)
Comment 47 Nick Clemens (kidclamp) 2023-10-17 19:24:40 UTC Comment hidden (obsolete)
Comment 48 David Nind 2023-10-17 19:43:56 UTC Comment hidden (obsolete)
Comment 49 David Nind 2023-10-17 19:44:48 UTC
Thanks Nick! Tests now pass, signed off.
Comment 50 Katrin Fischer 2023-11-04 23:58:18 UTC
Can you please rebase?


Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 22632: Add logging of merged patrons
Applying: Bug 22632: adding of the rescearch of merge in the log viewer
Applying: Bug 22632: Unit Testing of patch 22632
Applying: Bug 22632: (follow-up) Use PATRON_MERGE as log action
Applying: Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm
Applying: Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t
Applying: Bug 22632: (follow-up) Tidy and fix tests

error: sha1 information is lacking or useless (Koha/Patron.pm).

error: could not build fake ancestor
Patch failed at 0001 Bug 22632: (follow-up) Tidy and fix tests
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-22632-follow-up-Tidy-and-fix-tests-o_fvd6hr.patch
Comment 51 Owen Leonard 2023-12-04 17:34:09 UTC
Created attachment 159553 [details] [review]
Bug 22632: Add logging of merged patrons

without this patch , when patrons are merged , you can't see the merge in the log viewer.

    To test:
    -Apply the patch
    -Make sure you have "BorrowersLog" on in you System Preference
    -Search patrons
    -Check two ore more patrons
    -Merge selected patrons
    -Merge patrons
    -Go to log viewer
    - Make sure there is the line with the action : "Merge"
Comment 52 Owen Leonard 2023-12-04 17:34:12 UTC
Created attachment 159554 [details] [review]
Bug 22632: adding of the rescearch of merge in the log viewer
Comment 53 Owen Leonard 2023-12-04 17:34:16 UTC
Created attachment 159555 [details] [review]
Bug 22632: Unit Testing of patch 22632
Comment 54 Owen Leonard 2023-12-04 17:34:19 UTC
Created attachment 159556 [details] [review]
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Comment 55 Owen Leonard 2023-12-04 17:34:22 UTC
Created attachment 159557 [details] [review]
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm
Comment 56 Owen Leonard 2023-12-04 17:34:25 UTC
Created attachment 159558 [details] [review]
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t
Comment 57 Lucas Gass (lukeg) 2023-12-04 20:01:11 UTC
Your tests include a deleted subroutine, C4::Logs::GetLogs. You must now use Koha::ActionLogs->search instead.
Comment 58 Katrin Fischer 2025-05-07 09:18:45 UTC
+1 We'd love to have this as we are trying to restore lost data from bug 39841.
Comment 59 Sharon Dugdale 2025-05-08 11:43:16 UTC
We would love to have this too please
Comment 60 Owen Leonard 2025-07-22 17:50:37 UTC
Created attachment 184493 [details] [review]
Bug 22632: Add logging of merged patrons

without this patch , when patrons are merged , you can't see the merge in the log viewer.

    To test:
    -Apply the patch
    -Make sure you have "BorrowersLog" on in you System Preference
    -Search patrons
    -Check two ore more patrons
    -Merge selected patrons
    -Merge patrons
    -Go to log viewer
    - Make sure there is the line with the action : "Merge"
Comment 61 Owen Leonard 2025-07-22 17:50:41 UTC
Created attachment 184494 [details] [review]
Bug 22632: adding of the rescearch of merge in the log viewer
Comment 62 Owen Leonard 2025-07-22 17:50:45 UTC
Created attachment 184495 [details] [review]
Bug 22632: Unit Testing of patch 22632
Comment 63 Owen Leonard 2025-07-22 17:50:49 UTC
Created attachment 184496 [details] [review]
Bug 22632: (follow-up) Use PATRON_MERGE as log action
Comment 64 Owen Leonard 2025-07-22 17:50:53 UTC
Created attachment 184497 [details] [review]
Bug 22632: (follow-up) QA tool corrections for Log.t
Comment 65 Andrew Fuerste-Henry 2025-07-22 19:24:49 UTC
Created attachment 184503 [details] [review]
Bug 22632: Add logging of merged patrons

without this patch , when patrons are merged , you can't see the merge in the log viewer.

    To test:
    -Apply the patch
    -Make sure you have "BorrowersLog" on in you System Preference
    -Search patrons
    -Check two ore more patrons
    -Merge selected patrons
    -Merge patrons
    -Go to log viewer
    - Make sure there is the line with the action : "Merge"

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 66 Andrew Fuerste-Henry 2025-07-22 19:24:53 UTC
Created attachment 184504 [details] [review]
Bug 22632: adding of the rescearch of merge in the log viewer

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 67 Andrew Fuerste-Henry 2025-07-22 19:24:57 UTC
Created attachment 184505 [details] [review]
Bug 22632: Unit Testing of patch 22632

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 68 Andrew Fuerste-Henry 2025-07-22 19:25:01 UTC
Created attachment 184506 [details] [review]
Bug 22632: (follow-up) Use PATRON_MERGE as log action

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 69 Andrew Fuerste-Henry 2025-07-22 19:25:05 UTC
Created attachment 184507 [details] [review]
Bug 22632: (follow-up) QA tool corrections for Log.t

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 70 Andrew Fuerste-Henry 2025-07-22 19:27:46 UTC
Thanks, Owen!
Comment 71 Nick Clemens (kidclamp) 2025-08-01 16:19:47 UTC
Created attachment 185058 [details] [review]
Bug 22632: Add logging of merged patrons

without this patch , when patrons are merged , you can't see the merge in the log viewer.

    To test:
    -Apply the patch
    -Make sure you have "BorrowersLog" on in you System Preference
    -Search patrons
    -Check two ore more patrons
    -Merge selected patrons
    -Merge patrons
    -Go to log viewer
    - Make sure there is the line with the action : "Merge"

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 72 Nick Clemens (kidclamp) 2025-08-01 16:19:51 UTC
Created attachment 185059 [details] [review]
Bug 22632: adding of the rescearch of merge in the log viewer

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 73 Nick Clemens (kidclamp) 2025-08-01 16:19:55 UTC
Created attachment 185060 [details] [review]
Bug 22632: Unit Testing of patch 22632

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 74 Nick Clemens (kidclamp) 2025-08-01 16:19:59 UTC
Created attachment 185061 [details] [review]
Bug 22632: (follow-up) Use PATRON_MERGE as log action

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 75 Nick Clemens (kidclamp) 2025-08-01 16:20:03 UTC
Created attachment 185062 [details] [review]
Bug 22632: (follow-up) QA tool corrections for Log.t

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 76 Nick Clemens (kidclamp) 2025-08-01 16:20:07 UTC
Created attachment 185063 [details] [review]
Bug 22632: (QA follow-up) Add no warnings test

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 77 Lucas Gass (lukeg) 2025-08-01 19:24:17 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 78 Lucas Gass (lukeg) 2025-08-01 19:31:29 UTC
Please add release notes!
Comment 79 Lucas Gass (lukeg) 2025-08-13 19:22:38 UTC
Tests are failing here, please follow-up