Bug 25893 - Log viewer no longer searches using wildcards
Summary: Log viewer no longer searches using wildcards
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 22363
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-29 18:15 UTC by Nick Clemens
Modified: 2021-06-14 21:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03, 19.11.09


Attachments
Bug 25893: Use wildcards when searching logs (2.75 KB, patch)
2020-06-29 18:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25893: Use wildcards when searching logs (2.79 KB, patch)
2020-06-30 14:52 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25893: Don't wildcard object (757 bytes, patch)
2020-07-06 18:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25893: Don't wildcard object (814 bytes, patch)
2020-07-15 06:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25893: Use wildcards when searching logs (2.74 KB, patch)
2020-07-20 10:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25893: Use wildcards when searching logs (2.70 KB, patch)
2020-08-04 19:21 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25893: (follow-up) Undo unrelated changes (855 bytes, patch)
2020-08-04 19:21 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25893: Use wildcards when searching logs (2.32 KB, patch)
2020-08-06 08:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-06-29 18:15:20 UTC
Bug 22363 updated the log viewer from using C4::Logs::GetLogs to Koha::ActionLogs->search, however, GetLogs appended wildcards to the search and viewlogs.pl does not
Comment 1 Nick Clemens 2020-06-29 18:20:12 UTC
Created attachment 106415 [details] [review]
Bug 25893: Use wildcards when searching logs

The restores the wildcards that were used in the past

I also clean up a few warns and unused param

To test:
 0 - Ensure cataloguing logs and issues logs are enabled
 1 - Edit an item
 2 - Circulate that same item
 3 - Browse to tools-> log viewer
 4 - Put the itemnumber in the 'info' box
 5 - You see only the circulation
 6 - Put the itemnumber in the object box
 7 - You see only the cataloging modification
 8 - Apply patch
 9 - Put itemnumber in info box
10 - you see both actions
11 - Put the itemnumber in the object box
12 - You see only the catalogiong modification
Comment 2 Andrew Fuerste-Henry 2020-06-30 14:52:01 UTC
Created attachment 106433 [details] [review]
Bug 25893: Use wildcards when searching logs

The restores the wildcards that were used in the past

I also clean up a few warns and unused param

To test:
 0 - Ensure cataloguing logs and issues logs are enabled
 1 - Edit an item
 2 - Circulate that same item
 3 - Browse to tools-> log viewer
 4 - Put the itemnumber in the 'info' box
 5 - You see only the circulation
 6 - Put the itemnumber in the object box
 7 - You see only the cataloging modification
 8 - Apply patch
 9 - Put itemnumber in info box
10 - you see both actions
11 - Put the itemnumber in the object box
12 - You see only the catalogiong modification

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 3 Matthias Meusburger 2020-07-03 14:45:35 UTC
The patch works as intended.

However, using a wildcard on object (which is an identifier) does not seem to be a good idea, since you will retrieve many unwanted results.

Moreover, the wildcard in C4::Logs::GetLogs was only on info, and not on object:

if ($object) {
$query .= " AND object = ? ";
push( @parameters, $object );
}

Hence, it doesn't restore the old behavior, but changes it.

What do you think?
Comment 4 Nick Clemens 2020-07-06 18:04:48 UTC
Created attachment 106602 [details] [review]
Bug 25893: Don't wildcard object
Comment 5 Nick Clemens 2020-07-06 18:05:19 UTC
(In reply to Matthias Meusburger from comment #3)
> The patch works as intended.
> 
> However, using a wildcard on object (which is an identifier) does not seem
> to be a good idea, since you will retrieve many unwanted results.
> 
> Moreover, the wildcard in C4::Logs::GetLogs was only on info, and not on
> object:
> 
> if ($object) {
> $query .= " AND object = ? ";
> push( @parameters, $object );
> }
> 
> Hence, it doesn't restore the old behavior, but changes it.
> 
> What do you think?

Yup, I just got mixed up, removed wildcards from object
Comment 6 Katrin Fischer 2020-07-15 06:05:39 UTC
I think a better example for why this is useful is searching for system preference changes. Like put any pref in info to see when it was toggled and to which settings. It doesn't work without this patch.
Comment 7 Katrin Fischer 2020-07-15 06:06:14 UTC
Created attachment 106891 [details] [review]
Bug 25893: Don't wildcard object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Jonathan Druart 2020-07-20 10:58:35 UTC
Created attachment 107084 [details] [review]
Bug 25893: Use wildcards when searching logs

The restores the wildcards that were used in the past

I also clean up a few warns and unused param

To test:
 0 - Ensure cataloguing logs and issues logs are enabled
 1 - Edit an item
 2 - Circulate that same item
 3 - Browse to tools-> log viewer
 4 - Put the itemnumber in the 'info' box
 5 - You see only the circulation
 6 - Put the itemnumber in the object box
 7 - You see only the cataloging modification
 8 - Apply patch
 9 - Put itemnumber in info box
10 - you see both actions
11 - Put the itemnumber in the object box
12 - You see only the catalogiong modification

Signed-off-by: Jason Robb <jrobb@sekls.org>

Bug 25893: Don't wildcard object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Jonathan Druart 2020-07-20 10:59:24 UTC
Patches squashed
Comment 10 Jonathan Druart 2020-07-20 11:03:52 UTC
The removal of enabled_staff_search_views is not correct.

koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
316                         [% INCLUDE 'biblio-view-menu.inc' %]

Usually it's not a good idea to embed unrelated changes ;)
Comment 11 Nick Clemens 2020-08-04 19:21:24 UTC
Created attachment 107802 [details] [review]
Bug 25893: Use wildcards when searching logs

The restores the wildcards that were used in the past

To test:
 0 - Ensure cataloguing logs and issues logs are enabled
 1 - Edit an item
 2 - Circulate that same item
 3 - Browse to tools-> log viewer
 4 - Put the itemnumber in the 'info' box
 5 - You see only the circulation
 6 - Put the itemnumber in the object box
 7 - You see only the cataloging modification
 8 - Apply patch
 9 - Put itemnumber in info box
10 - you see both actions
11 - Put the itemnumber in the object box
12 - You see only the catalogiong modification

Signed-off-by: Jason Robb <jrobb@sekls.org>

Bug 25893: Don't wildcard object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Nick Clemens 2020-08-04 19:21:27 UTC
Created attachment 107803 [details] [review]
Bug 25893: (follow-up) Undo unrelated changes
Comment 13 Jonathan Druart 2020-08-06 08:59:51 UTC
Created attachment 107869 [details] [review]
Bug 25893: Use wildcards when searching logs

The restores the wildcards that were used in the past

To test:
 0 - Ensure cataloguing logs and issues logs are enabled
 1 - Edit an item
 2 - Circulate that same item
 3 - Browse to tools-> log viewer
 4 - Put the itemnumber in the 'info' box
 5 - You see only the circulation
 6 - Put the itemnumber in the object box
 7 - You see only the cataloging modification
 8 - Apply patch
 9 - Put itemnumber in info box
10 - you see both actions
11 - Put the itemnumber in the object box
12 - You see only the catalogiong modification

Signed-off-by: Jason Robb <jrobb@sekls.org>

Bug 25893: Don't wildcard object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 25893: (follow-up) Undo unrelated changes
Comment 14 Jonathan Druart 2020-08-06 09:38:10 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 15 Lucas Gass 2020-08-13 18:45:00 UTC
backported to 20.05.x for 20.05.03
Comment 16 Aleisha Amohia 2020-08-16 22:04:55 UTC
backported to 19.11.x for 19.11.09
Comment 17 Victor Grousset/tuxayo 2020-09-20 13:31:52 UTC
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.