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.
Created attachment 4359 [details] [review] Proposed patch
BTW, the patch applies to 3.2.9 too.
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!
Created attachment 4478 [details] [review] Signed-off patch
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.
Created attachment 5546 [details] [review] Bug 6460: alternative fix, updating template and script instead of C4::Log - a bit rushed
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.
(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.
Created attachment 5569 [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.
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>
Changes template instead of script. Also adds a test case, which is double-plus good. Marking as Passed QA
Pushed, I agree, bonus points for the test case