|
Description
Donna
2023-12-21 18:09:31 UTC
+1, this would be extremely useful. As a system librarian trying to work out if I've made a change to an auth value, or someone else did it without my input, I'd love this feature to exist! Created attachment 168747 [details] [review] Bug 35630: Add new system preference 'AuthorizedValuesLog' Created attachment 168748 [details] [review] Bug 35630: Add ability to log creation, modification, and deletion of authorized values These patches enable librarians to log any changes to authorized values. The feature is dependent on the new 'AuthorizedValuesLog' system preference. To test: 1) Apply patch, updatedatabase, restart_all 2) In Administration->System preferences, search for 'AuthorizedValuesLog' and change to 'Log'. Save your changes. 3) Now go to Administration->Authorized values. Keep this tab open 4) In a new tab, go to Tools -> Log viewer. Ensure that "Authorized values" is checked. 5) Now in AV, click on a category and create a new authorized value. 6) Refresh your log viewer, and the creation should be logged appropiately. 7) Modify an authroized value, this should also be logged correctly. 8) Now delete and authorized value, ensure the logging is correct here as well. 9) Now turn off the system preferece. 10) In the log viewer, 'Authorized values' should have a black triangle next to it, indicating the preference is turned off. 11) Change an AV, this should not be logged. It works as described it test plan, but: 1. QA - Not blocker: Sysprefs IntranetAddMastheadLibraryPulldown is bad placed (see bug 10610) 2. What should be recorded into 'info' column in action_logs table? Value before modification or value after mod.? For bibliographic record it is value before modification. (In reply to Roman Dolny from comment #5) > It works as described it test plan, but: > > 1. QA - Not blocker: Sysprefs IntranetAddMastheadLibraryPulldown is bad > placed (see bug 10610) This is unrelated to this bug. > 2. What should be recorded into 'info' column in action_logs table? Value > before modification or value after mod.? I would think the value after modification. If you wanted to see the value before modifcation, then you would just go back into the logs. > For bibliographic record it is value before modification. I see what you are saying about editing a record, but there's examples where it goes the other way as well. For example, when editing a specific item, the logs show the value after. I think this is better behavior (In reply to Sam Lau from comment #6) > > ... > > > 2. What should be recorded into 'info' column in action_logs table? Value > > before modification or value after mod.? > > I would think the value after modification. If you wanted to see the value > before modifcation, then you would just go back into the logs. > This works for new values and subsequent modifications of values before logging begins. But not for the first modification of values currently existing in the database. (But I'm not sure if this is really important.) Maybe we should record the old and new value in the "info" column? > Maybe we should record the old and new value in the "info" column?
I think that is confusing. Why not just log the new value as MODIFY?
We pushed bug 25159 to the end of the cycle and I feel like we should make use of it when adding new logs now. Adding Kyle and also a topic to next meeting agenda: https://wiki.koha-community.org/w/index.php?title=Development_IRC_meeting_17_July_2024&action=submit Created attachment 169027 [details] [review] Bug 35630: Handle library limits inside the store function This patch makes use of the diff action_logs as mentioned in comment 9. Furthermore, to fix the issue of calling store twice when creating a new AV, the "replace_library_limits" call has been moved inside to the store function. Use test plan from previous patch to test. Created attachment 169044 [details] [review] Bug 35630: Add new system preference 'AuthorizedValuesLog' Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 169045 [details] [review] Bug 35630: Add ability to log creation, modification, and deletion of authorized values These patches enable librarians to log any changes to authorized values. The feature is dependent on the new 'AuthorizedValuesLog' system preference. To test: 1) Apply patch, updatedatabase, restart_all 2) In Administration->System preferences, search for 'AuthorizedValuesLog' and change to 'Log'. Save your changes. 3) Now go to Administration->Authorized values. Keep this tab open 4) In a new tab, go to Tools -> Log viewer. Ensure that "Authorized values" is checked. 5) Now in AV, click on a category and create a new authorized value. 6) Refresh your log viewer, and the creation should be logged appropiately. 7) Modify an authroized value, this should also be logged correctly. 8) Now delete and authorized value, ensure the logging is correct here as well. 9) Now turn off the system preferece. 10) In the log viewer, 'Authorized values' should have a black triangle next to it, indicating the preference is turned off. 11) Change an AV, this should not be logged. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 169046 [details] [review] Bug 35630: Handle library limits inside the store function This patch makes use of the diff action_logs as mentioned in comment 9. Furthermore, to fix the issue of calling store twice when creating a new AV, the "replace_library_limits" call has been moved inside to the store function. Use test plan from previous patch to test. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Failing tests with this patch applied. Probably need to add tests here to cover the change? Maybe instead of converting the object to in Log.pm ( $infos = Data::Dumper::Dumper( $infos ) if ref $infos; ) which makes me a bit nervous it could be done in AuthorisedValue.pm and only when dealing with AUTHORIZEDVALUE changes? Created attachment 172629 [details] [review] Bug 35630: Move Data::Dump to AuthorisedValue.pm I think it's prudent to now get a 2nd sign-off, returning this one to NSO. |