Bug 6460 - No log generated if Action set to All or Modify or Delete
Summary: No log generated if Action set to All or Modify or Delete
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Transaction logs (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Janusz Kaczmarek
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-04 23:14 UTC by Janusz Kaczmarek
Modified: 2012-10-25 23:09 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (878 bytes, patch)
2011-06-04 23:21 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Signed-off patch (943 bytes, patch)
2011-06-15 11:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 6460: alternative fix, updating template and script instead of C4::Log - a bit rushed (5.64 KB, patch)
2011-09-22 20:22 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
Bug 6460: alternative fix, updating template and script instead of C4::Log (7.33 KB, patch)
2011-09-23 11:22 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
Bug 6460: alternative fix, updating template and script instead of C4::Log (7.84 KB, patch)
2011-09-23 16:29 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
[Signed Off] Bug 6460: alternative fix, updating template and script instead of C4::Log (8.01 KB, patch)
2011-09-23 17:25 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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