[Bug 18382] New: action_logs entry for module HOLDS, action SUSPEND is spammy
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Bug ID: 18382 Summary: action_logs entry for module HOLDS, action SUSPEND is spammy Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: barton@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Created attachment 61842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61842&action=edit Sample Action logs entry for suspend. When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text. See the attached 'action_logs.out' To replicate: To test: - Enable the HoldsLog syspref - Add a hold on a record/item - Click on the <Suspend> button Check the logs using module 'Holds' and Action 'Suspend' -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Barton Chittenden <barton@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14642 -- 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=18382 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #1 from Aleisha Amohia <aleisha@catalyst.net.nz> --- I am not seeing this error. When I click Suspend, this is the entry that gets inserted into the action log: $VAR1 = {'waitingdate' => undef,'itemnumber' => undef,'timestamp' => '2017-04-18 23:54:43','notificationdate' => undef,'priority' => '1','biblionumber' => '21','expirationdate' => undef,'cancellationdate' => undef,'reminderdate' => undef,'found' => undef,'reservedate' => '2017-04-18','suspend_until' => bless( {'locale' => bless( {'default_time_format_length' => 'medium','en_complete_name' => 'English United States','native_territory' => 'United States','default_date_format_length' => 'medium','native_language' => 'English','en_territory' => 'United States','native_complete_name' => 'English United States','en_language' => 'English','id' => 'en_US'}, 'DateTime::Locale::en_US' ),'tz' => bless( {'name' => 'UTC'}, 'DateTime::TimeZone::UTC' ),'utc_rd_days' => 736440,'utc_rd_secs' => 0,'local_rd_secs' => 0,'offset_modifier' => 0,'local_rd_days' => 736440,'rd_nanosecs' => 0,'local_c' => {'year' => 2017,'month' => 4,'day' => 21,'day_of_quarter' => 21,'quarter' => 2,'hour' => 0,'minute' => 0,'day_of_year' => 111,'day_of_week' => 5,'second' => 0},'formatter' => undef,'utc_year' => 2018}, 'DateTime' ),'lowestPriority' => '0','branchcode' => 'CPL','suspend' => 1,'reserve_id' => '232','reservenotes' => '','itemtype' => undef,'borrowernumber' => '51'}; It's a little long, sure, but it doesn't separate for each new line like in your attachment. -- 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=18382 --- Comment #2 from Barton Chittenden <barton@bywatersolutions.com> --- (In reply to Aleisha Amohia from comment #1)
I am not seeing this error. When I click Suspend, this is the entry that gets inserted into the action log:
$VAR1 = {'waitingdate' => undef,'itemnumber' => undef,'timestamp' => '2017-04-18 23:54:43','notificationdate' => undef,'priority' => '1','biblionumber' => '21','expirationdate' => undef,'cancellationdate' => undef,'reminderdate' => undef,'found' => undef,'reservedate' => '2017-04-18','suspend_until' => bless( {'locale' => bless( {'default_time_format_length' => 'medium','en_complete_name' => 'English United States','native_territory' => 'United States','default_date_format_length' => 'medium','native_language' => 'English','en_territory' => 'United States','native_complete_name' => 'English United States','en_language' => 'English','id' => 'en_US'}, 'DateTime::Locale::en_US' ),'tz' => bless( {'name' => 'UTC'}, 'DateTime::TimeZone::UTC' ),'utc_rd_days' => 736440,'utc_rd_secs' => 0,'local_rd_secs' => 0,'offset_modifier' => 0,'local_rd_days' => 736440,'rd_nanosecs' => 0,'local_c' => {'year' => 2017,'month' => 4,'day' => 21,'day_of_quarter' => 21,'quarter' => 2,'hour' => 0,'minute' => 0,'day_of_year' => 111,'day_of_week' => 5,'second' => 0},'formatter' => undef,'utc_year' => 2018}, 'DateTime' ),'lowestPriority' => '0','branchcode' => 'CPL','suspend' => 1,'reserve_id' => '232','reservenotes' => '','itemtype' => undef,'borrowernumber' => '51'};
It's a little long, sure, but it doesn't separate for each new line like in your attachment.
My formatting came from the fact that I queried action_logs via the command line, using mysql's '\G' query terminator, which puts in the line breaks. Apparently your 'locale' variable is fairly compact... mine isn't. In any case 'suspend_until' is still refers to a datetime object which is dumped in its entirity, that *may* get huge, and none of it is information that the user will need or want. -- 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=18382 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I agree it's a bit much and possibly hard to understand for people writing reports. Could we shorten the information to a normal timestamp like format? -- 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=18382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 63962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63962&action=edit Bug 18382 - action_logs entry for module HOLDS, action SUSPEND is spammy When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text. Test Plan: 1) Enable the HoldsLog syspref 2) Add a hold on a record/item 3) Suspend the hold with a date to resume 4) Note the massive amount of date in the suspend_until field 5) Apply this patch 6) Suspend another hold with a date to resume 7) Note the log has an acutal date in the suspend_until field Check the logs using module 'Holds' and Action 'Suspend' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- With this patch you can no longer suspend without a date set. Internal server error: Can't call method "ymd" on an undefined value at /home/vagrant/kohaclone/Koha/Hold.pm line 65. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #6 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 68281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68281&action=edit Bug 18382: [FOLLOW-UP] Ensure user can still enter an empty date Follow above test plan Also check that you can suspend with no date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63962|0 |1 is obsolete| | --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 68363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68363&action=edit Bug 18382 - action_logs entry for module HOLDS, action SUSPEND is spammy When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text. Test Plan: 1) Enable the HoldsLog syspref 2) Add a hold on a record/item 3) Suspend the hold with a date to resume 4) Note the massive amount of date in the suspend_until field 5) Apply this patch 6) Suspend another hold with a date to resume 7) Note the log has an acutal date in the suspend_until field Check the logs using module 'Holds' and Action 'Suspend' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68281|0 |1 is obsolete| | --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 68364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68364&action=edit Bug 18382: [FOLLOW-UP] Ensure user can still enter an empty date Follow above test plan Also check that you can suspend with no date Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=18382 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Your solution is not limited to action_logs. You truncate, so to speak, already at the holds level. Please add your fix at the hold unblessed level when sending it to the logs. Squash the patches too. Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Kyle M Hall <kyle@bywatersolutions.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=18382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68363|0 |1 is obsolete| | Attachment #68364|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72610&action=edit Bug 18382: action_logs entry for module HOLDS, action SUSPEND is spammy When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text. Test Plan: 1) Enable the HoldsLog syspref 2) Add a hold on a record/item 3) Suspend the hold with a date to resume 4) Note the massive amount of date in the suspend_until field 5) Apply this patch 6) Suspend another hold with a date to resume 7) Note the log has an acutal date in the suspend_until field Check the logs using module 'Holds' and Action 'Suspend' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #9)
Your solution is not limited to action_logs. You truncate, so to speak, already at the holds level. Please add your fix at the hold unblessed level when sending it to the logs. Squash the patches too. Thanks.
I'm not sure I understand your request. Can you give me a bit more detail? Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #11)
(In reply to Marcel de Rooy from comment #9)
Your solution is not limited to action_logs. You truncate, so to speak, already at the holds level. Please add your fix at the hold unblessed level when sending it to the logs. Squash the patches too. Thanks.
I'm not sure I understand your request. Can you give me a bit more detail? Thanks!
Yes, it was rather cryptic ;) I overlooked the truncate statement in the start of the sub. So just ignore it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72610|0 |1 is obsolete| | --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 73149 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73149&action=edit Bug 18382: action_logs entry for module HOLDS, action SUSPEND is spammy When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text. Test Plan: 1) Enable the HoldsLog syspref 2) Add a hold on a record/item 3) Suspend the hold with a date to resume 4) Note the massive amount of date in the suspend_until field 5) Apply this patch 6) Suspend another hold with a date to resume 7) Note the log has an acutal date in the suspend_until field Check the logs using module 'Holds' and Action 'Suspend' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 73150 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73150&action=edit Bug 18382: (QA follow-up) Replace ->ymd by ->datetime The first patch makes Hold.t fail on: not ok 8 - Hold is suspended with a date, truncation takes place automatically Failed test 'Hold is suspended with a date, truncation takes place automatically' at Hold.t line 94. got: '2018-03-22' expected: '2018-03-22T00:00:00' We could remove the midnight time there, but we also could replace ymd by datetime. The cpan doc tells us: Same as $dt->ymd('-') . 'T' . $dt->hms(':') Test plan: With this patch, run Hold.t again. It should pass now. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Passed QA |Failed QA --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The change reads wrong, please adjust. 94 if ( defined $dt ){ 95 $self->suspend_until( $dt->datetime ); 96 } else { 97 $self->suspend_until( $dt ); 98 } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I think we should clean the existing dirty entries: select from action_logs where suspend_until, dump the structure, modify the date and update it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #15)
The change reads wrong, please adjust.
94 if ( defined $dt ){ 95 $self->suspend_until( $dt->datetime ); 96 } else { 97 $self->suspend_until( $dt ); 98 }
Can you explain what you mean by it "reads wrong"? Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 73317 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73317&action=edit Bug 18382 [QA Followup]: Don't set suspend_until if dt if undefined -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kyle, I was not expecting your follow-up patch to work, I was expecting that ->suspend_hold would not update suspend_until What is supposed to do: $hold->suspend_hold( $dt ); $hold->suspend_hold; ? I would expect suspend_until to be set to undef -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73343&action=edit Bug 18382: Add a test This is what I am expecting but I may be wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 73420 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73420&action=edit Bug 18382 [QA Followup]: Make new unit test pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |nick@bywatersolutions.com Status|Pushed to Master |RESOLVED --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Enhancement, not backporting for 17.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18382 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28700 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org