[Bug 20750] New: Allow timestamped auditing of ILL request events
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug ID: 20750 Summary: Allow timestamped auditing of ILL request events Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: andrew.isherwood@ptfs-europe.com Currently there is no audit trail provided for ILL requests, this is required. At the most basic form this would be a timestamped record of changes of a request's status. It would be useful to be able to also record other arbitrary events. This bug proposes the addition of a requests_actions log that allows either the core ILL code or backends to record arbitrary events relating to a request. This could then be displayed in the UI as an "audit trail" or reported upon. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |andrew.isherwood@ptfs-europ |ity.org |e.com Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #1 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Discussed with joubu on IRC, suggestion was to use actions_logs rather than bake our own recording mechanism -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #2 from Magnus Enger <magnus@libriotech.no> --- Sounds like a most excellent idea! Would this include some kind of diff between old and new metadata when a request is updated? That would be awesome! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #3 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- The initial implementation will only log status changes and this will indeed record "before" and "after" statuses. I'm implementing it in as extensible a way as possible to allow other events/actions to be logged with whatever data is required -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #4 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 75304 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75304&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |7317 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7317 [Bug 7317] Add an Interlibrary Loan Module to Circulation and OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #5 from Magnus Enger <magnus@libriotech.no> --- Looks like the file extension on the installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.sql file is wrong. It includes Perl code, so it should be .perl, not .sql: https://wiki.koha-community.org/wiki/Database_updates#How_to_write_an_atomic... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #6 from Magnus Enger <magnus@libriotech.no> --- The test plan does not mention the need to turn IllLog on. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #7 from Magnus Enger <magnus@libriotech.no> --- ILL is not in the list of available modules in the "Browse system logs" tool. It is of course possible to view the log related to a single ILL request, but it might also be interesting to use "Browse system logs" to see all ILL actions in a given period of time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #8 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Thanks very much for that feedback, very useful indeed. I'm about to push an updated patch that addresses all these points. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75304|0 |1 is obsolete| | --- Comment #9 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 75326 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75326&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #10 from Magnus Enger <magnus@libriotech.no> --- The latest patch touches Koha/Schema/Result/SocialData.pm. Is that supposed to happen? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #11 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 75329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75329&action=edit Squashed commit for ILL enhancements This squashed commit contains a number of ILL enhancements. It has been rebased on top of 17.11.x. Contains the following enhancements: - Bug 20041 - ILL module missing from more menu in staff when activated (Already present in 17.11.x) - Bug 20563 - ILL request list gives no indication of source and/or target - Bug 20651 - Improve display of "Toggle full supplier metadata" - Bug 20515 - "ILL Request" menu options displayed when user has no ILL permissions - Bug 20646 - "ILL Request" menu options not displayed when user has superlibrarian permission Bug 20548 - Remove copyright clearance workflow from staff created ILL requests - Bug 20284 - ILL: Adding a 'new request' from OPAC fails with template error if text exists in ILLModuleCopyrightClearance - Bug 20556 - Marking ILL request as complete results in "Internal server error" - Bug 20536 - ILL: authnotrequired not explicitly unset - Bug 20640 - Allow migrating a request between backends - Bug 20581 - Allow manual selection of custom ILL request statuses - Bug 20600 - Provide the ability for users to filter ILL requests - Bug 18837 - Add an unmediated Interlibrary Loans workflow - Bug 20639 - Allow setting a default/single backend for OPAC driven requests - Bug 18589 - Show ILLs as part of patron profile - Bug 20750 - Add audit trail of request actions. Currently change of request status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75329|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75326|0 |1 is obsolete| | --- Comment #12 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 75330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75330&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #13 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Well spotted Magnus, thanks! I've submitted a new patch which removes changes to Schema files. Also, ignore comment #11 and attachment #75329 - over zealous fingers typing faster than my brain... -- You are receiving this mail because: You are watching all bug changes.
From looking at the code, it seems that the test plan only covers part of the functionality, that of keeping a record of the actual status of a request. It looks like the new code can also track changes to the other parts of the
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #14 from Magnus Enger <magnus@libriotech.no> --- metadata. Is this correct, and would it be possible to enhance the Dummy or FreeForm backends in such a way that this other functionality could also be tested? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #15 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Magnus - Thanks for looking at this. Currently, all this code does is log status changes. I can see how you might have thought that other stuff was being logged as, in addition to logging during ->status calls, we also have potential logging during a ->store call. This was to catch the case where a status was being set directly in a ->store call, e.g. ->store({ status => 'REQ' }) Which obviously wouldn't have been caught by the logger in the status method. So, there is potential to log other "things" during the ->store call, but currently we don't. It is easily added by adding a new element to the Koha::ILLRequest::Logger $loggers class property: $self->{loggers} = { status => sub { $self->log_status_change(@_); } }; [...] sub log_maybe { my ($self, $req, $attrs) = @_; if (defined $req && defined $attrs) { foreach my $key (keys %{ $attrs }) { if (defined($self->{loggers}->{$key})) { $self->{loggers}->{$key}($req, $attrs->{$key}); } } } } [...] sub log_status_change { my ( $self, $req, $new_status ) = @_; $self->set_data({ actionname => 'STATUS_CHANGE', objectnumber => $req->id, infos => to_json({ log_origin => 'core', status_before => $req->{previous_status}, status_after => $new_status }) }); $self->log_something(); } As you can see, currently it says "if we get a call to $request->store and the thing we're storing contains a 'status' property, log the value of it" We also provide display templates for when it comes to displaying that stored "thing" back to the user. I *have* added a call to the logger in the BLDSS backend whenever an API request is sent to the BLDSS status check endpoint. It's a very simple addition and can be seen here: https://github.com/PTFS-Europe/BLDSS-backend/commit/40a4ccdce6e0429397025879... I hope this helps :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #16 from Magnus Enger <magnus@libriotech.no> --- (In reply to Andrew Isherwood from comment #15)
Hi Magnus - Thanks for looking at this. Currently, all this code does is log status changes.
Ah, gotcha! I'll sign off, then. If you do find the time to add logging of any changes to the metadata I'll be happy to test and sign off on it! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #17 from Magnus Enger <magnus@libriotech.no> --- Hm, the previous version of the patch was working as expected, but with the most recent version, when I go to "Manage request" i get an "Internal Server Error". The Plack error log says: Template process failed: undef error - The method logs is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 122 The code around line 122 looks like this: 120 my $data; 121 binmode( STDOUT, ":utf8" ); 122 $template->process( $self->filename, $vars, \$data ) 123 || die "Template process failed: ", $template->error(); 124 return $data; Not quite sure if there is a problem with the patch or if i screwed up my testing environment? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #18 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Magnus. I can't replicate this, seems to work fine for me. It sounds like your Illrequest object doesn't know about the "logs" method. Try setting up the testing env again maybe? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug 20750 depends on bug 7317, which changed state. Bug 7317 Summary: Add an Interlibrary Loan Module to Circulation and OPAC https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7317 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75330|0 |1 is obsolete| | --- Comment #19 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 77757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77757&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 C Hofstede <chloe.hofstede@educampus.ie> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chloe.hofstede@educampus.ie -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77757|0 |1 is obsolete| | --- Comment #20 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 77816 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77816&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77816|0 |1 is obsolete| | --- Comment #21 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 77817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77817&action=edit Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77817|0 |1 is obsolete| | --- Comment #22 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 77818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77818&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77818|0 |1 is obsolete| | --- Comment #23 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 77931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77931&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Just had to sort out some randomness with our sandboxes.. that final patch actually includes the signoff line this time.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18589 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589 [Bug 18589] Show ILLs as part of patron profile -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77931|0 |1 is obsolete| | --- Comment #25 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 79822 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79822&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #26 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 79823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79823&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79822|0 |1 is obsolete| | --- Comment #27 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 79824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79824&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #28 from Magnus Enger <magnus@libriotech.no> --- *** Bug 21461 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19588 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #29 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 80860 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80860&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20639 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20639 [Bug 20639] Allow setting a default/single backend for OPAC driven requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|18589 |21063 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18589 [Bug 18589] Show ILLs as part of patron profile https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21063 [Bug 21063] Allow columns in intranet ILL request datatable to be customisable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|7317 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7317 [Bug 7317] Add an Interlibrary Loan Module to Circulation and OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79823|0 |1 is obsolete| | --- Comment #30 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82082 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82082&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79824|0 |1 is obsolete| | --- Comment #31 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82083&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #80860|0 |1 is obsolete| | --- Comment #32 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82085&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #33 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Rebased on top of master then the current dependency tree -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82083|0 |1 is obsolete| | --- Comment #34 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82096 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82096&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82082|0 |1 is obsolete| | --- Comment #35 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82097 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82097&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82096|0 |1 is obsolete| | --- Comment #36 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82098 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82098&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82085|0 |1 is obsolete| | --- Comment #37 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82099&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82097|0 |1 is obsolete| | --- Comment #38 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82500 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82500&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82098|0 |1 is obsolete| | --- Comment #39 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82501&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82099|0 |1 is obsolete| | --- Comment #40 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 82502 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82502&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82501|0 |1 is obsolete| | --- Comment #41 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84193 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84193&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82502|0 |1 is obsolete| | --- Comment #42 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84194 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84194&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82500|0 |1 is obsolete| | --- Comment #43 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84195 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84195&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #44 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Rebased following changes to C4/Log.pm & t/Log.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #45 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84203 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84203&action=edit Bug 20750: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #46 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84266 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84266&action=edit Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #47 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84497 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84497&action=edit Bug 20750: (follow-up) Fix dependency merge error During the dependency merge the embed of status_alias in the API call had got lost, resulting in the user defined statuses not displaying. This patch fixes that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #48 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84498 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84498&action=edit Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84497|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84193|0 |1 is obsolete| | --- Comment #49 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84821&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84194|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84195|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84203|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84266|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84498|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84821|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #50 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84823&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #51 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84824&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #52 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84825&action=edit Bug 20750: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #53 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84826&action=edit Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #54 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84827&action=edit Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84193|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84194|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84195|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84203|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84266|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84497|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84498|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84823|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84824|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84825|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84826|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84827|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |katrin.fischer@bsz-bw.de --- Comment #55 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Patch currently doesn't apply, can you please rebase? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #56 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- This needs some serious rebasing due to the changes in Bug 20581. I'll get this sorted ASAP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84497|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84193|0 |1 is obsolete| | --- Comment #57 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84973 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84973&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84194|0 |1 is obsolete| | --- Comment #58 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84974 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84974&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84195|0 |1 is obsolete| | --- Comment #59 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84975 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84975&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84203|0 |1 is obsolete| | --- Comment #60 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84976 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84976&action=edit Bug 20750: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84266|0 |1 is obsolete| | --- Comment #61 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84977 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84977&action=edit Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84498|0 |1 is obsolete| | --- Comment #62 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84978&action=edit Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #63 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 84979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84979&action=edit Bug 20750: (follow-up) Fix merge problems Since we now merging on top of Bug 20581, we need some modifications to make sure status_alias changes are being logged and displayed correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #64 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Andrew, getting started with this one. 1) QA script: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt FAIL filters missing_filter at line 10 (from "[% display_before %]") FAIL forbidden patterns forbidden pattern: Do not use line breaks inside template tags (bug 18675) (line 5) forbidden pattern: Do not use line breaks inside template tags (bug 18675) (line 13) OK git manipulation OK js_in_body OK spelling OK tt_valid FAIL valid_template : filter not found 2) I am wondering a bit about the date operations in the Logger module, get_epoch especially. 3) This doesn't need to be part of the installer outside the sysprefs.sql file. Files ill_logging_pref.txt and ill_logging_pref.sql can be removed. sysprefs.sql will always be run during installation, installing all prefs. 4) I wonder if we should set the logger to off by default for updated installations and allow people to turn it on themselves. 5) For translations it's always nice to be specific. 'request' is a very general term that appears in multiple contexts. I am suggesting 'Display request log' > "ILL request log" (it being a button kind of transports the display part :) ) 6) In the log viewer, could we make the visibility of the ILL parts depend on the system preference? I think this would make it a little less confusing for people without the ILL module activated. 7) Just using this for a teaching moment :) <b>Status changed </b> will be turned into Status changed with a space at the end on translations, which will do nothing in this case, but can be a bit tedious. In other cases forgetting the space will be bad for display, so translators have to always look for those. 8) Tests are failing for me: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Illrequests.t t/db_dependent/Illrequests.t .. 1/11 # Failed test 'Status_alias getter works.' # at t/db_dependent/Illrequests.t line 66. # got: undef # expected: 'gCXbRHL2vgImhl1lv0AsoWfYj15fmIgBiFT9i_fmMnIzh1laXf' no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. # Looks like you failed 1 test of 25. t/db_dependent/Illrequests.t .. 3/11 # Failed test 'Basic object tests' # at t/db_dependent/Illrequests.t line 117. t/db_dependent/Illrequests.t .. 5/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. 7/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. 10/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. # Looks like you failed 1 test of 11. t/db_dependent/Illrequests.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/11 subtests Test Summary Report ------------------- t/db_dependent/Illrequests.t (Wstat: 256 Tests: 11 Failed: 1) Failed test: 3 Non-zero exit status: 1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #65 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85097 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85097&action=edit Bug 20750: (follow-up) Fix QA script errors As per item 1) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #66 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85098 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85098&action=edit Bug 20750: (follow-up) Remove ill_logging_pref.* As per item 3) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #67 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85099&action=edit Bug 20750: (follow-up) Modify display log wording As per item 5) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #68 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85101 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85101&action=edit Bug 20750: (follow-up) Remove wayward space As per item 7) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #69 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #64)
1) QA script:
Fixed
2) I am wondering a bit about the date operations in the Logger module, get_epoch especially.
What's concerning you about this? All it does is get the epoch value for dates returned so we can sort them for display. It doesn't modify them in any way
3) This doesn't need to be part of the installer outside the sysprefs.sql file. Files ill_logging_pref.txt and ill_logging_pref.sql can be removed.
Done
4) I wonder if we should set the logger to off by default for updated installations and allow people to turn it on themselves.
I don't know really. I guess we need to be consistent with what other logging modules do. Do they default to off or on?
5) For translations it's always nice to be specific.
Done
6) In the log viewer, could we make the visibility of the ILL parts depend on the system preference?
I agree. This is a great idea. However, looking at the log viewer code and template, there's currently no way of setting the visibility of a logging module based on a syspref. Presumably, the need has never arisen before. I'm not sure this bug is the place to add that, since it's a more generic mechanism in the log viewer, rather than just special casing ILL. As I say, it's a great idea, I'm just not sure it should be done here. Happy to do it if you disagree however.
7) Just using this for a teaching moment
Mmmm, good point. That will have been a typo, the space should never have been inside the <b> tag. Fixed
8) Tests are failing for me:
Fixed Thanks very much for the feedback, much appreciated! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #70 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85113&action=edit Bug 20750: (follow-up) Remove status_alias test As per item 8) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 This test is now redundant. We can't test the return of $req->status_alias against the value of $req->{status_alias} any more since we've overloaded the status_alias method and it won't always return the value of the object's property. We test the functionality of the status_alias method elsewhere so we're still covered. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |josef.moravec@gmail.com, | |tomascohen@gmail.com --- Comment #71 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I'd really like to get a second set of QA eyes on the code for this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #72 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #69)
(In reply to Katrin Fischer from comment #64)
1) QA script:
Fixed
2) I am wondering a bit about the date operations in the Logger module, get_epoch especially.
What's concerning you about this? All it does is get the epoch value for dates returned so we can sort them for display. It doesn't modify them in any way
I don't think the time formatting should be done here in the logger... it is helper method working with date and time... this way we could end up with one time formatter per class...
3) This doesn't need to be part of the installer outside the sysprefs.sql file. Files ill_logging_pref.txt and ill_logging_pref.sql can be removed.
Done
4) I wonder if we should set the logger to off by default for updated installations and allow people to turn it on themselves.
I don't know really. I guess we need to be consistent with what other logging modules do. Do they default to off or on?
Default logs: AuthoritiesLog IS ON BorrowersLog IS ON CataloguingLog IS ON FinesLog is ON IssueLog is ON LetterLog is ON ReturnLog is ON SubscriptionLog IS ON CronjobLog is OFF HoldsLog is OFF RenewalLog is OFF ReportsLog is OFF so 8 are ON and 4 are OFF, but I prefer to this log off by default too
5) For translations it's always nice to be specific.
Done
6) In the log viewer, could we make the visibility of the ILL parts depend on the system preference?
I agree. This is a great idea. However, looking at the log viewer code and template, there's currently no way of setting the visibility of a logging module based on a syspref. Presumably, the need has never arisen before. I'm not sure this bug is the place to add that, since it's a more generic mechanism in the log viewer, rather than just special casing ILL. As I say, it's a great idea, I'm just not sure it should be done here. Happy to do it if you disagree however.
7) Just using this for a teaching moment
Mmmm, good point. That will have been a typo, the space should never have been inside the <b> tag. Fixed
8) Tests are failing for me:
Fixed
Thanks very much for the feedback, much appreciated!
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #73 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Josef Moravec from comment #72)
(In reply to Andrew Isherwood from comment #69)
(In reply to Katrin Fischer from comment #64)
1) QA script:
Fixed
2) I am wondering a bit about the date operations in the Logger module, get_epoch especially.
What's concerning you about this? All it does is get the epoch value for dates returned so we can sort them for display. It doesn't modify them in any way
I don't think the time formatting should be done here in the logger... it is helper method working with date and time... this way we could end up with one time formatter per class...
3) This doesn't need to be part of the installer outside the sysprefs.sql file. Files ill_logging_pref.txt and ill_logging_pref.sql can be removed.
Done
4) I wonder if we should set the logger to off by default for updated installations and allow people to turn it on themselves.
I don't know really. I guess we need to be consistent with what other logging modules do. Do they default to off or on?
Default logs: AuthoritiesLog IS ON BorrowersLog IS ON CataloguingLog IS ON FinesLog is ON IssueLog is ON LetterLog is ON ReturnLog is ON SubscriptionLog IS ON
CronjobLog is OFF HoldsLog is OFF RenewalLog is OFF ReportsLog is OFF
so 8 are ON and 4 are OFF, but I prefer to this log off by default too
5) For translations it's always nice to be specific.
Done
6) In the log viewer, could we make the visibility of the ILL parts depend on the system preference?
I agree. This is a great idea. However, looking at the log viewer code and template, there's currently no way of setting the visibility of a logging module based on a syspref. Presumably, the need has never arisen before. I'm not sure this bug is the place to add that, since it's a more generic mechanism in the log viewer, rather than just special casing ILL. As I say, it's a great idea, I'm just not sure it should be done here. Happy to do it if you disagree however.
7) Just using this for a teaching moment
Mmmm, good point. That will have been a typo, the space should never have been inside the <b> tag. Fixed
8) Tests are failing for me:
Fixed
Thanks very much for the feedback, much appreciated!
Thx Joseph! I agree with default OFF, especially since the ILL module is optional. And I see a pattern there, all the default off were added after 3.2. So there is a pattern for adding new logs. Thx! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #74 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Josef Moravec from comment #72)
I don't think the time formatting should be done here in the logger... it is helper method working with date and time... this way we could end up with one time formatter per class...
Thanks for the feedback! OK. The obvious solution here in that case would be for me to add an additional parameter to C4::Log::GetLogs that allows you to specify if the logs should be returned sorted by date. Do you agree this would address your concerns? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #75 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85238 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85238&action=edit Bug 20750: (follow-up) Set logging default off As per comment #72 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c72 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #76 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #74)
(In reply to Josef Moravec from comment #72)
I don't think the time formatting should be done here in the logger... it is helper method working with date and time... this way we could end up with one time formatter per class...
Thanks for the feedback!
OK. The obvious solution here in that case would be for me to add an additional parameter to C4::Log::GetLogs that allows you to specify if the logs should be returned sorted by date. Do you agree this would address your concerns?
Maybe it would be better to create Koha::ActionLog[s] classes based on Koha::Object, it would be much better then adding new parameter to GetLogs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #77 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Josef Thanks for that. Sorry, I'm a bit lost. Are you suggesting I rewrite C4::Log as a Koha object? If so, that seems a bit out of scope just to get this bug through QA. Apologies if I'm misunderstanding! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22363 --- Comment #78 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #77)
Hi Josef
Thanks for that. Sorry, I'm a bit lost. Are you suggesting I rewrite C4::Log as a Koha object? If so, that seems a bit out of scope just to get this bug through QA. Apologies if I'm misunderstanding!
You do not need to rewrite whole C4::Log to Koha object, you just need to add Koha::ActionLog[s] objects and use them. Patch from Bug 22363 is enough for you ;) Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22363 [Bug 22363] Move C4::Logs::GetLogs to Koha namespace -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #79 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85266 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85266&action=edit Bug 20750: (follow-up) Use Koha::ActionLogs We now use Koha::ActionLogs in favour of C4::Log::GetLogs. This enables us to deprecate Koha::Illrequest::Logger::get_epoch This adds a dependency on Bug 22363. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #80 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Andrew, could you maybe sign-off on the dependency bug 22363? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #81 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Done :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #82 from Josef Moravec <josef.moravec@gmail.com> --- Sorry, i got fatal sha1 error, could you rebase please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug 20750 depends on bug 20639, which changed state. Bug 20639 Summary: Allow setting a default/single backend for OPAC driven requests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20639 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84973|0 |1 is obsolete| | --- Comment #83 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85697&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84974|0 |1 is obsolete| | --- Comment #84 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85698 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85698&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84975|0 |1 is obsolete| | --- Comment #85 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85699&action=edit Bug 20750: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84976|0 |1 is obsolete| | --- Comment #86 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85700&action=edit Bug 20750: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84977|0 |1 is obsolete| | --- Comment #87 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85701&action=edit Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84978|0 |1 is obsolete| | --- Comment #88 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85702&action=edit Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84979|0 |1 is obsolete| | --- Comment #89 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85703&action=edit Bug 20750: (follow-up) Fix merge problems Since we now merging on top of Bug 20581, we need some modifications to make sure status_alias changes are being logged and displayed correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85097|0 |1 is obsolete| | --- Comment #90 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85704&action=edit Bug 20750: (follow-up) Fix QA script errors As per item 1) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85098|0 |1 is obsolete| | --- Comment #91 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85705&action=edit Bug 20750: (follow-up) Remove ill_logging_pref.* As per item 3) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85099|0 |1 is obsolete| | --- Comment #92 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85706 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85706&action=edit Bug 20750: (follow-up) Modify display log wording As per item 5) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85101|0 |1 is obsolete| | --- Comment #93 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85707&action=edit Bug 20750: (follow-up) Remove wayward space As per item 7) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85113|0 |1 is obsolete| | --- Comment #94 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85708&action=edit Bug 20750: (follow-up) Remove status_alias test As per item 8) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 This test is now redundant. We can't test the return of $req->status_alias against the value of $req->{status_alias} any more since we've overloaded the status_alias method and it won't always return the value of the object's property. We test the functionality of the status_alias method elsewhere so we're still covered. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85238|0 |1 is obsolete| | --- Comment #95 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85709 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85709&action=edit Bug 20750: (follow-up) Set logging default off As per comment #72 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c72 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85266|0 |1 is obsolete| | --- Comment #96 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85710&action=edit Bug 20750: (follow-up) Use Koha::ActionLogs We now use Koha::ActionLogs in favour of C4::Log::GetLogs. This enables us to deprecate Koha::Illrequest::Logger::get_epoch This adds a dependency on Bug 22363. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off --- Comment #97 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Josef - I've just rebased and reattached. Make sure you apply the 22363 dependency first, hopefully this bug will then apply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #98 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #97)
Hi Josef - I've just rebased and reattached. Make sure you apply the 22363 dependency first, hopefully this bug will then apply.
Thanks Andrew, I just applied it cleanly, so I can now work on it ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #99 from Josef Moravec <josef.moravec@gmail.com> --- Comment on attachment 85697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85697 Bug 20750: Allow logging of arbitrary actions Review of attachment 85697: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20750&attachment=85697) ----------------------------------------------------------------- ::: Koha/Illrequest/Logger.pm @@ +110,5 @@
+ +sub log_status_change { + my ( $self, $req, $new_status ) = @_; + + $self->set_data({
I do not like to use 'data' variable to send data to logger. What happens when log_something is called accidentaly? ::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ +443,5 @@
+ <div class="modal-body"> + [% IF request.logs.size > 0 %] + [% FOREACH log IN request.logs %] + [% tpl = log.template %] + [% INCLUDE $tpl %]
I would call it with param log, to be sure what is rendered ::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ +54,5 @@
[% CASE 'CHANGE PASS' %]Change password [% CASE 'ADDCIRCMESSAGE' %]Add circulation message [% CASE 'DELCIRCMESSAGE' %]Delete circulation message +[% CASE 'STATUS_CHANGE' %]Change ILL request status +[% CASE 'BLDSS_STATUS_CHECK' %]Check ILL request status with BLDSS
Why do you add BLDSS_STATUS_CHECK action? Koha can't be awared of all existing backends... it should instead be get from installed backends, maybe easier to do it after switch backend to regular Koha plugins? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #100 from Josef Moravec <josef.moravec@gmail.com> --- Also, when you need more than one param in method call, you should use hashref: Koha::Illrequest::Logger - log_maybe - log_status_change - get_log_tenmplate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #101 from Josef Moravec <josef.moravec@gmail.com> --- Andrew, have you some plans with this logger to the future? It is interesting idea. But the implementation is just for one module and action. It would be nice to have it a bit more generalized. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #102 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Josef Moravec from comment #99)
Comment on attachment 85697 [details] [review] Bug 20750: Allow logging of arbitrary actions
::: Koha/Illrequest/Logger.pm @@ +110,5 @@
+ +sub log_status_change { + my ( $self, $req, $new_status ) = @_; + + $self->set_data({
I do not like to use 'data' variable to send data to logger. What happens when log_something is called accidentaly?
Interesting. This does seem overly convoluted, I'm sure there must have been a reason why I implemented it this way, rather than just send the data to be logged to log_something. I'll do some digging and see if I can remember. If I can't find any good reason, I'll just pass the data to log_something, rather than set it as an instance variable. Thanks! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #103 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85756&action=edit Bug 20750: (follow-up) Don't use object variable We were unecessarily setting a property on the Logger object containing the data to be logged, prior to calling log_something. This was overcomplicated things and could be problematic if an unexpected call to log_something was made. Now we just explicitly pass the data to log_something. As per comment #99 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c99 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #104 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Josef Moravec from comment #99)
Comment on attachment 85697 [details] [review] Bug 20750: Allow logging of arbitrary actions
Review of attachment 85697 [details] [review]: -----------------------------------------------------------------
::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ +443,5 @@
+ <div class="modal-body"> + [% IF request.logs.size > 0 %] + [% FOREACH log IN request.logs %] + [% tpl = log.template %] + [% INCLUDE $tpl %]
I would call it with param log, to be sure what is rendered
Sorry Josef, I'm not clear what you mean, could you please elaborate? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #105 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Josef Moravec from comment #99)
Comment on attachment 85697 [details] [review] Bug 20750: Allow logging of arbitrary actions
Review of attachment 85697 [details] [review]: -----------------------------------------------------------
::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ +54,5 @@
[% CASE 'CHANGE PASS' %]Change password [% CASE 'ADDCIRCMESSAGE' %]Add circulation message [% CASE 'DELCIRCMESSAGE' %]Delete circulation message +[% CASE 'STATUS_CHANGE' %]Change ILL request status +[% CASE 'BLDSS_STATUS_CHECK' %]Check ILL request status with BLDSS
Why do you add BLDSS_STATUS_CHECK action? Koha can't be awared of all existing backends... it should instead be get from installed backends, maybe easier to do it after switch backend to regular Koha plugins?
You're absolutely right, we shouldn't be hard coding to particular backends. Ideally we'd have some method, probably in Illrequests.pm, which something like viewlog.pl could call to poll all the available backends and see if they have anything it needs to include. That would require some thought to make sure we do it right. And I think you're right, it's probably something that would be made easier once backends are plugins. I'm happy to remove this specific binding to the BLDSS backend here, if you agree? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85756|0 |1 is obsolete| | --- Comment #106 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85757&action=edit Bug 20750: (follow-up) Don't use object variable We were unecessarily setting a property on the Logger object containing the data to be logged, prior to calling log_something. This was overcomplicated things and could be problematic if an unexpected call to log_something was made. Now we just explicitly pass the data to log_something. As per comment #99 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c99 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #107 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85765&action=edit Bug 20750: (follow-up) Pass hashrefs to functions As per comment #100 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c100 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #108 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- (In reply to Josef Moravec from comment #101)
Andrew, have you some plans with this logger to the future? It is interesting idea. But the implementation is just for one module and action. It would be nice to have it a bit more generalized.
You're right, it is pretty specific to ILL but then it doesn't really do anything that the log viewer does already, it just does it a bit prettier. The functionality it provides was to address a request from customers relating to ILL. It does currently only log a single action, but the instantiator can be expanded as more actions are required to be logged. I dunno, it could be made more generic, I'm just not sure it's needed, can you see a use case where this would be required elsewhere? If it were to be made more generic, that would probably be better as separate development? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #109 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #104)
(In reply to Josef Moravec from comment #99)
Comment on attachment 85697 [details] [review] [review] Bug 20750: Allow logging of arbitrary actions
Review of attachment 85697 [details] [review] [review]: -----------------------------------------------------------------
::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ +443,5 @@
+ <div class="modal-body"> + [% IF request.logs.size > 0 %] + [% FOREACH log IN request.logs %] + [% tpl = log.template %] + [% INCLUDE $tpl %]
I would call it with param log, to be sure what is rendered
Sorry Josef, I'm not clear what you mean, could you please elaborate?
I think somethink like this: [% INCLUDE $tpl log=log %] then in the template you always know the "log" variable is what you need, and when you need other name in parents, that's no problem then. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #110 from Josef Moravec <josef.moravec@gmail.com> ---
[% CASE 'CHANGE PASS' %]Change password [% CASE 'ADDCIRCMESSAGE' %]Add circulation message [% CASE 'DELCIRCMESSAGE' %]Delete circulation message +[% CASE 'STATUS_CHANGE' %]Change ILL request status +[% CASE 'BLDSS_STATUS_CHECK' %]Check ILL request status with BLDSS
Why do you add BLDSS_STATUS_CHECK action? Koha can't be awared of all existing backends... it should instead be get from installed backends, maybe easier to do it after switch backend to regular Koha plugins?
You're absolutely right, we shouldn't be hard coding to particular backends. Ideally we'd have some method, probably in Illrequests.pm, which something like viewlog.pl could call to poll all the available backends and see if they have anything it needs to include. That would require some thought to make sure we do it right. And I think you're right, it's probably something that would be made easier once backends are plugins.
I'm happy to remove this specific binding to the BLDSS backend here, if you agree?
Yes remove it, we could deal with this on another report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #111 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Andrew Isherwood from comment #108)
(In reply to Josef Moravec from comment #101)
Andrew, have you some plans with this logger to the future? It is interesting idea. But the implementation is just for one module and action. It would be nice to have it a bit more generalized.
You're right, it is pretty specific to ILL but then it doesn't really do anything that the log viewer does already, it just does it a bit prettier. The functionality it provides was to address a request from customers relating to ILL.
It does currently only log a single action, but the instantiator can be expanded as more actions are required to be logged.
I dunno, it could be made more generic, I'm just not sure it's needed, can you see a use case where this would be required elsewhere? If it were to be made more generic, that would probably be better as separate development?
It would be definitely a separate work. I would like to see this pushed as soon as possible ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #112 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85826&action=edit Bug 20750: (follow-up) Explicitly pass log var As per comment #109 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c109 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #113 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 85827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85827&action=edit Bug 20750: (follow-up) Remove BLDSS_STATUS_CHECK As per comment #110 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c110 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85697|0 |1 is obsolete| | --- Comment #114 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85829&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85698|0 |1 is obsolete| | --- Comment #115 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85830 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85830&action=edit Bug 20750: (follow-up) Fix sorting of log entries The log entries are received from C4::Log::GetLogs with their timestamps formatted as '2018-18-10 16:09:29', this doesn't sort well. In fact, it causes a warning in the Koha logs: Argument "2018-10-04 15:48:15" isn't numeric in numeric comparison (<=>) at /home/koha/kohaclone/Koha/Illrequest/Logger.pm line 231 This patch converts the timestamps to be sorted into epoch dates prior to them being compared Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85699|0 |1 is obsolete| | --- Comment #116 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85831 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85831&action=edit Bug 20750: Add unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85700|0 |1 is obsolete| | --- Comment #117 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85832 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85832&action=edit Bug 20750: (follow-up) Fix QA errors This patch fixes the "missing_filter" QA error Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85701|0 |1 is obsolete| | --- Comment #118 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85833&action=edit Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85702|0 |1 is obsolete| | --- Comment #119 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85834 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85834&action=edit Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85703|0 |1 is obsolete| | --- Comment #120 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85835&action=edit Bug 20750: (follow-up) Fix merge problems Since we now merging on top of Bug 20581, we need some modifications to make sure status_alias changes are being logged and displayed correctly Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85704|0 |1 is obsolete| | --- Comment #121 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85836 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85836&action=edit Bug 20750: (follow-up) Fix QA script errors As per item 1) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85705|0 |1 is obsolete| | --- Comment #122 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85837&action=edit Bug 20750: (follow-up) Remove ill_logging_pref.* As per item 3) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85706|0 |1 is obsolete| | --- Comment #123 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85838 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85838&action=edit Bug 20750: (follow-up) Modify display log wording As per item 5) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85707|0 |1 is obsolete| | --- Comment #124 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85839&action=edit Bug 20750: (follow-up) Remove wayward space As per item 7) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85708|0 |1 is obsolete| | --- Comment #125 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85840 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85840&action=edit Bug 20750: (follow-up) Remove status_alias test As per item 8) here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c64 This test is now redundant. We can't test the return of $req->status_alias against the value of $req->{status_alias} any more since we've overloaded the status_alias method and it won't always return the value of the object's property. We test the functionality of the status_alias method elsewhere so we're still covered. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85709|0 |1 is obsolete| | --- Comment #126 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85841&action=edit Bug 20750: (follow-up) Set logging default off As per comment #72 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c72 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85710|0 |1 is obsolete| | --- Comment #127 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85842&action=edit Bug 20750: (follow-up) Use Koha::ActionLogs We now use Koha::ActionLogs in favour of C4::Log::GetLogs. This enables us to deprecate Koha::Illrequest::Logger::get_epoch This adds a dependency on Bug 22363. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85757|0 |1 is obsolete| | --- Comment #128 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85843&action=edit Bug 20750: (follow-up) Don't use object variable We were unecessarily setting a property on the Logger object containing the data to be logged, prior to calling log_something. This was overcomplicated things and could be problematic if an unexpected call to log_something was made. Now we just explicitly pass the data to log_something. As per comment #99 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c99 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85765|0 |1 is obsolete| | --- Comment #129 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85844&action=edit Bug 20750: (follow-up) Pass hashrefs to functions As per comment #100 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c100 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85826|0 |1 is obsolete| | --- Comment #130 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85845&action=edit Bug 20750: (follow-up) Explicitly pass log var As per comment #109 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c109 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85827|0 |1 is obsolete| | --- Comment #131 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85846&action=edit Bug 20750: (follow-up) Remove BLDSS_STATUS_CHECK As per comment #110 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c110 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #132 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85847&action=edit Bug 20750: (QA follow-up) Add ILL status change option to actions in log viewer Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #133 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 85848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85848&action=edit Bug 20750: (QA follow-up) Add use KohaDates to status change template Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #134 from Josef Moravec <josef.moravec@gmail.com> --- Hope we cought everything ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug 20750 depends on bug 20639, which changed state. Bug 20639 Summary: Allow setting a default/single backend for OPAC driven requests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20639 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug 20750 depends on bug 20639, which changed state. Bug 20639 Summary: Allow setting a default/single backend for OPAC driven requests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20639 What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85829|0 |1 is obsolete| | Attachment #85830|0 |1 is obsolete| | Attachment #85831|0 |1 is obsolete| | Attachment #85832|0 |1 is obsolete| | Attachment #85833|0 |1 is obsolete| | Attachment #85834|0 |1 is obsolete| | Attachment #85835|0 |1 is obsolete| | Attachment #85836|0 |1 is obsolete| | Attachment #85837|0 |1 is obsolete| | Attachment #85838|0 |1 is obsolete| | Attachment #85839|0 |1 is obsolete| | Attachment #85840|0 |1 is obsolete| | Attachment #85841|0 |1 is obsolete| | Attachment #85842|0 |1 is obsolete| | Attachment #85843|0 |1 is obsolete| | Attachment #85844|0 |1 is obsolete| | Attachment #85845|0 |1 is obsolete| | Attachment #85846|0 |1 is obsolete| | Attachment #85847|0 |1 is obsolete| | Attachment #85848|0 |1 is obsolete| | --- Comment #135 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86434 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86434&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #136 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86435 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86435&action=edit Bug 20750: Add unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Bug 20750: (follow-up) Remove status_alias test This test is now redundant. We can't test the return of $req->status_alias against the value of $req->{status_alias} any more since we've overloaded the status_alias method and it won't always return the value of the object's property. We test the functionality of the status_alias method elsewhere so we're still covered. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #137 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86436 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86436&action=edit Bug 20750: (QA follow-up) Fix templates Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off --- Comment #138 from Josef Moravec <josef.moravec@gmail.com> --- Setting back to signed off, would be nice to have another QA team member look. Katrin? ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Bug 20750 depends on bug 22363, which changed state. Bug 22363 Summary: Move C4::Logs::GetLogs to Koha namespace https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22363 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86434|0 |1 is obsolete| | Attachment #86435|0 |1 is obsolete| | Attachment #86436|0 |1 is obsolete| | --- Comment #139 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 86534 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86534&action=edit Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker <Niamh.Walker-Headon@it-tallaght.ie> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #140 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 86535 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86535&action=edit Bug 20750: Add unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Bug 20750: (follow-up) Remove status_alias test This test is now redundant. We can't test the return of $req->status_alias against the value of $req->{status_alias} any more since we've overloaded the status_alias method and it won't always return the value of the object's property. We test the functionality of the status_alias method elsewhere so we're still covered. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #141 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 86536 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86536&action=edit Bug 20750: (QA follow-up) Fix templates Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #142 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED --- Comment #143 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Enhancement, will not be backported to 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 --- Comment #144 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The output of the tests contains warnings: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Illrequests.t t/db_dependent/Illrequests.t .. 1/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. 4/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. 7/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. 10/11 no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. no query in themelanguage at /home/vagrant/kohaclone/C4/Templates.pm line 266. t/db_dependent/Illrequests.t .. ok All tests successful. Files=1, Tests=11, 6 wallclock secs ( 0.04 usr 0.00 sys + 4.94 cusr 1.16 csys = 6.14 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23040 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23040 [Bug 23040] The output of Illrequests.t displays warnings -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org