Bug 22632 - Add logging of merged patrons
Summary: Add logging of merged patrons
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: axel
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2019-04-03 19:32 UTC by Andrew Fuerste-Henry
Modified: 2024-01-17 16:17 UTC (History)
15 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:


Attachments
without this patch , when patrons are merged , you can't see the merge in the log viewer. (2.05 KB, patch)
2019-04-11 15:32 UTC, axel
Details | Diff | Splinter Review
Bug 21003: Made Edit keywords more consistent in add_items-step2.tt (2.65 KB, patch)
2019-04-12 14:11 UTC, axel
Details | Diff | Splinter Review
Unit Testing of patch 22632 (3.69 KB, patch)
2019-04-12 14:15 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.22 KB, patch)
2019-04-12 14:16 UTC, axel
Details | Diff | Splinter Review
Unit Testing of patch 22632 (3.69 KB, patch)
2019-04-12 14:16 UTC, axel
Details | Diff | Splinter Review
adding of the rescearch of merge in the log viewer (1.78 KB, patch)
2019-04-12 15:10 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.22 KB, patch)
2019-04-15 07:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.70 KB, patch)
2019-04-15 07:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (1.74 KB, patch)
2019-04-15 07:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Change for the research of Modules : Patrons (896 bytes, patch)
2019-04-15 07:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Change for the research of Modules : Patrons (940 bytes, patch)
2019-04-15 08:28 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.22 KB, patch)
2019-04-16 08:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.70 KB, patch)
2019-04-16 08:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (1.74 KB, patch)
2019-04-16 08:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Change for the research of Modules : Patrons (1.01 KB, patch)
2019-04-16 08:51 UTC, axel
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.23 KB, patch)
2019-05-14 09:16 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.70 KB, patch)
2019-05-14 09:16 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (1.74 KB, patch)
2019-05-14 09:16 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.57 KB, patch)
2019-05-14 10:10 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.23 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (3.01 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.57 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Use PATRON_MERGE as log action (5.25 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm (1.40 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t (4.89 KB, patch)
2023-10-17 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.28 KB, patch)
2023-10-17 16:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (3.05 KB, patch)
2023-10-17 16:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.62 KB, patch)
2023-10-17 16:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: (follow-up) Use PATRON_MERGE as log action (5.30 KB, patch)
2023-10-17 16:30 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm (1.45 KB, patch)
2023-10-17 16:30 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t (4.94 KB, patch)
2023-10-17 16:30 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: (follow-up) Tidy and fix tests (8.58 KB, patch)
2023-10-17 18:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22632: (follow-up) Tidy and fix tests (8.96 KB, patch)
2023-10-17 19:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22632: (follow-up) Tidy and fix tests (9.00 KB, patch)
2023-10-17 19:43 UTC, David Nind
Details | Diff | Splinter Review
Bug 22632: Add logging of merged patrons (1.23 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: adding of the rescearch of merge in the log viewer (3.03 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: Unit Testing of patch 22632 (3.57 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Use PATRON_MERGE as log action (5.28 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm (1.40 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t (4.89 KB, patch)
2023-12-04 17:34 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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
Created attachment 157266 [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: David Nind <david@davidnind.com>
Comment 38 David Nind 2023-10-17 16:29:55 UTC
Created attachment 157267 [details] [review]
Bug 22632: adding of the rescearch of merge in the log viewer

Signed-off-by: David Nind <david@davidnind.com>
Comment 39 David Nind 2023-10-17 16:29:58 UTC
Created attachment 157268 [details] [review]
Bug 22632: Unit Testing of patch 22632

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 David Nind 2023-10-17 16:30:02 UTC
Created attachment 157269 [details] [review]
Bug 22632: (follow-up) Use PATRON_MERGE as log action

Signed-off-by: David Nind <david@davidnind.com>
Comment 41 David Nind 2023-10-17 16:30:06 UTC
Created attachment 157270 [details] [review]
Bug 22632: (follow-up) Perltidy fixes for Koha/Patron.pm

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 David Nind 2023-10-17 16:30:10 UTC
Created attachment 157271 [details] [review]
Bug 22632: (follow-up) Apply perltidy fixes and add missing exec flag to Log.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 43 David Nind 2023-10-17 16:35:25 UTC Comment hidden (obsolete)
Comment 44 Nick Clemens 2023-10-17 18:45:57 UTC
Created attachment 157278 [details] [review]
Bug 22632: (follow-up) Tidy and fix tests
Comment 45 Nick Clemens 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 2023-10-17 19:24:40 UTC Comment hidden (obsolete)
Comment 48 David Nind 2023-10-17 19:43:56 UTC
Created attachment 157286 [details] [review]
Bug 22632: (follow-up) Tidy and fix tests

Signed-off-by: David Nind <david@davidnind.com>
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 2023-12-04 20:01:11 UTC
Your tests include a deleted subroutine, C4::Logs::GetLogs. You must now use Koha::ActionLogs->search instead.