Bug 6460

Summary: No log generated if Action set to All or Modify or Delete
Product: Koha Reporter: Janusz Kaczmarek <januszop>
Component: Transaction logsAssignee: Janusz Kaczmarek <januszop>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: normal    
Priority: PATCH-Sent (DO NOT USE) CC: chris, katrin.fischer, koha.sekjal, magnus, mjr, paul.poulain
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6911
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed patch
Signed-off patch
Bug 6460: alternative fix, updating template and script instead of C4::Log - a bit rushed
Bug 6460: alternative fix, updating template and script instead of C4::Log
Bug 6460: alternative fix, updating template and script instead of C4::Log
[Signed Off] Bug 6460: alternative fix, updating template and script instead of C4::Log

Description Janusz Kaczmarek 2011-06-04 23:14:08 UTC
In "Browse system logs" dialogue, if Action is set to All or Modify or Delete, no logs are generated, despite of other settings.  

This is due to modifications introduced to sub C4::GetLogs and to tools::viewlog by the patch to Bug 6014.

1) Actions are logged as MODIFY, DELETE etc., whereas template viewlog.t* uses mod, del and passes it to viewlog.pl.  That is why, consequently, wildcards (%) was used in GetLogs for action (prior to patch 6014).  This has to be restored.

2) Also, @action is, after changes made by 6014, a one element list, even if the first and only element is an empty string ("") for All (actions).  That is why, in GetLogs, one has to check if the first element of the list is not "" instead of checking the length of the list (which is always 1). 

A patch will follow.
Comment 1 Janusz Kaczmarek 2011-06-04 23:21:23 UTC Comment hidden (obsolete)
Comment 2 Janusz Kaczmarek 2011-06-04 23:26:24 UTC
BTW, the patch applies to 3.2.9 too.
Comment 3 Magnus Enger 2011-06-15 11:01:16 UTC
Before applying the patch, no results are shown when Action is set to All, Modify or Delete in Tools > Log Viewer. 

After applying the patch results are shown for all of the Actions, including the ones mentioned above. 

Signing off!
Comment 4 Magnus Enger 2011-06-15 11:01:59 UTC Comment hidden (obsolete)
Comment 5 Paul Poulain 2011-08-11 10:36:30 UTC
QA comment

I think this patch should be rejected: the diagnostic is correct (Actions are logged as MODIFY, DELETE etc., whereas template viewlog.t* uses mod, del and passes it to viewlog.pl) so, why not fix the template to pass MODIFY, DELETE,...
It would be a better solution than using LIKE.

Also note that your patch prevent to change the select to have a "MULTIPLE", that would be a handy feature.
Comment 6 MJ Ray (software.coop) 2011-09-22 20:22:46 UTC Comment hidden (obsolete)
Comment 7 MJ Ray (software.coop) 2011-09-23 11:22:14 UTC Comment hidden (obsolete)
Comment 8 MJ Ray (software.coop) 2011-09-23 16:01:29 UTC
(In reply to comment #7)
> Created attachment 5560 [details] [review]
> Bug 6460: alternative fix, updating template and script instead of C4::Log
> 
> Add some test cases and fix a bug in C4::Log found by in review.
> Thanks-to: Katrin Fischer.

And fixing that bug also seems to have fixed the fix.  Please hold off for now while I recode it.
Comment 9 MJ Ray (software.coop) 2011-09-23 16:29:39 UTC Comment hidden (obsolete)
Comment 10 Liz Rea 2011-09-23 17:25:34 UTC
Created attachment 5570 [details] [review]
[Signed Off] Bug 6460: alternative fix, updating template and script instead of C4::Log

Add some test cases and fix a bug in C4::Log found by in review.
Thanks-to: Katrin Fischer.

Signed-off-by: Liz Rea <lrea@nekls.org>
Passes test, interface works as expected.

Signed-off-by: Liz Rea <lrea@nekls.org>
Comment 11 Ian Walls 2011-09-23 17:33:14 UTC
Changes template instead of script.  Also adds a test case, which is double-plus good.  Marking as Passed QA
Comment 12 Chris Cormack 2011-09-23 22:03:22 UTC
Pushed, I agree, bonus points for the test case