---- Reported by nengard@gmail.com 2009-11-30 15:46:42 ---- Created an attachment members data in log viewer When you view the Members data in the log viewer you see a series of question marks in the UPDATE statement. All other logs work well, it's just the members one that has issues. The Log Viewer should show what changes were made to the patron record. ---- Additional Comments From nengard@gmail.com 2009-11-30 15:47:04 ---- Created an attachment data on mod log on patron record --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:15 UTC --- This bug was previously known as _bug_ 3820 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3820 Imported an attachment (id=1584) Imported an attachment (id=1585) Actual time not defined. Setting to 0.0 The original submitter of attachment 1584 [details] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. The original submitter of attachment 1585 [details] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The display is now : UPDATE (executed w/ arg: 1)
I am not sure I understand what 'UPDATE (executed w/ arg: 1)' means. Ideally the log would tell which fields were updated and the old and new values. Or at least the new values, so you coudl check previous log entries for the original values.
frankly katrin, I've hesitated to say this display was not that much useless. The log let you know who did something and when, but don't say what. That's another issue I think. Reopen the ticket. Nicole your opinion ?
I tend to reopen. Thinking a bit more about it: I am not sure saving patron information in logs was avoided for privacy reasons? You could always turn off the borrowers log if you don't want this information to be saved. Or we could make the borrowers log system preference have different settings for the kind of information that should be saved. No information, information about time and staff updating and a complete log telling you the changes. Other logs like systempreferences show changes in detail.
I'm with Katrin on this one - many libraries want a record of who changed what on a patron's record - and isn't that the point of a log? Nicole
Well, the cataloguing log shows exactly what the biblio was before it has been modified. I agree the MembersLog should be the same. Who changed that behaviour ? git blame C4/Members.pm, sub ModMember : ae48ebbc (Chris Nighswonger 2010-11-12 09:34:52 -0500 784) logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "UPDATE (executed w/ ar git log C4/Members.pm shows that it was only a reindenting, so no blame to chris_n, digging earlier, I see from chris_n too, patch 35df86da596cdcd5e6d9fb0bc7c14f303a4a7e57, 2008-03-18, : - &logaction(C4::Context->userenv->{'number'},"MEMBERS","MODIFY",$data{'borrowernumber'},"") + &logaction(C4::Context->userenv->{'number'},"MEMBERS","MODIFY",$data{'borrowernumber'},"$query (executed w/ arg: $data{'borrowernumber'})") seems there was no info at all before this patch. I'm a little bit confused. Nicole, any idea ?
issue still here in 16.05, updating assignee
*** Bug 8578 has been marked as a duplicate of this bug. ***
Still valid, see recent discussion on the mailinglist: https://lists.katipo.co.nz/pipermail/koha/2016-June/045634.html
There is still interest from our customer base on this one.
Created attachment 85100 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } }
Created attachment 85102 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } }
Changed fields will be listed in canonical order now. Any opinions on 'before/after' vs 'from/to'.. if we switch to 'from/to' we get them in that order for free with the canonicalisation switch.. with 'before/after' we will get 'after' coming before 'before' in the output wherever the raw json is shown as obviously 'a' comes before 'b' in the alphabet ;)
Created attachment 85103 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } }
Created attachment 85104 [details] [review] Bug 3820: (follow-up) Clean up warnings
Not against QA'ing this one, but someone might want to beat me to it.
*** Bug 15296 has been marked as a duplicate of this bug. ***
Created attachment 85161 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } } Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 85162 [details] [review] Bug 3820: (follow-up) Clean up warnings Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 85196 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } } Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Created attachment 85197 [details] [review] Bug 3820: (follow-up) Clean up warnings Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
This is great addition and I like it, but as it changes the behaviour, the test should be adjusted too: t/db_dependent/Koha/Patrons.t .. 29/33 # Failed test 'Got correct new cardnumber' # at t/db_dependent/Koha/Patrons.t line 1414. # got: undef # expected: 'TESTCARDNUMBER' # Failed test 'Got correct old cardnumber' # at t/db_dependent/Koha/Patrons.t line 1415. # got: undef # expected: 'jY44Hy2e7OojC2Zx7vMo43gzub' # Failed test 'With BorrowerLogs, Change in cardnumber should be logged, as well as general alert of patron mod.' # at t/db_dependent/Koha/Patrons.t line 1416. # got: '1' # expected: '2' # Looks like you failed 3 tests of 3. # Failed test 'Log cardnumber change' # at t/db_dependent/Koha/Patrons.t line 1417.
Created attachment 85243 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } } Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Created attachment 85244 [details] [review] Bug 3820: (follow-up) Clean up warnings Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Created attachment 85245 [details] [review] Bug 3820: DB Update to make old cardnumber logs match the new format
Created attachment 85246 [details] [review] Bug 3820: Update tests to reflect the change of behaviour
Back to signed off now I've added the required update to the tests (and also added a db update to make old cardnumber logs match the new format.. although we can't retrospectively populate the logs for other actions the update should make writing future reports backwards compatible without adding effort. Let me know if you'd like any further additions to the tests or anything.
Created attachment 85256 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } } Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85257 [details] [review] Bug 3820: (follow-up) Clean up warnings Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85258 [details] [review] Bug 3820: DB Update to make old cardnumber logs match the new format Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85259 [details] [review] Bug 3820: Update tests to reflect the change of behaviour Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85260 [details] [review] Bug 3820: (QA follow-up) Change REGEXP_REPLACE to REPLACE for wider compatibility Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Just a question, if TrackLastPatornActivity is enabled then we record a modification on each update of the lastseen field. Is this desired behavior? Other fields like login_attempts do seem to be avoided
Created attachment 85269 [details] [review] Bug 3820: Add changes to MEMBERS MODIFY logaction This patch attemps to add a more refined and detailed info structure detailing what fields were changed and what they were changed from and to. Test Plan: Enable BorrowersLog in the system preferences and undertake a series of borrower detail changes. Go to view the actionlogs and note that MEMBER MODIFY logs now detail what fields you have changed using the following form: { FIELD1: { before: BEFORE_VALUE, after: AFTER_VALUE } } Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85270 [details] [review] Bug 3820: (follow-up) Clean up warnings Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85271 [details] [review] Bug 3820: (follow-up) Update cardnumber logs match the new format Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85272 [details] [review] Bug 3820: (follow-up) Update tests to reflect the change of behaviour Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85273 [details] [review] Bug 3820: (QA follow-up) Change REGEXP_REPLACE to REPLACE for wider compatibility Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 85274 [details] [review] Bug 3820: (follow-up) Address lastseen log spamming Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Good catch Nick, I've added a followup to exclude the lastseen field from logging as it would inevitably spam the logs pretty badly. Note: We'll get double log lines for patron renewals too.. but this isn't a change of behaviour.. the first log line will be the now more details MODIFY, followed immediately by a 'RENEW' action log.. these both got logged before just there was less detail in the MODIFY than there is now.
Created attachment 85276 [details] [review] Bug 3820: (follow-up) Address lastseen log spamming Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 85277 [details] [review] Bug 3820: (follow-up) Address lastseen log spamming Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Awesome work all! Pushed to master for 19.05
Changing to 'enhancement' as it's such a longstanding bug that many people may be relying on the existing format for reports and I don't want to break said reports in a stable version.
Just a note on this one: the DB rev for cardnumber changes has a side effect of updating the timestamp in the logs - historical changes are going to be lost by this upgrade as they will now all appear to have been done at upgrade time. We will still have the historical cardnumber, I am not sure if this is worth filing as a bug