Bug 22069 - Log viewer not displaying item renewals
Summary: Log viewer not displaying item renewals
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 22363
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-04 13:18 UTC by barry.cannon
Modified: 2019-05-11 21:04 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
This patch fixes the search for 'renewal', so both item renewals and patron renewals are found.
Version(s) released in:
19.05.00, 18.11.05


Attachments
Bug 22069: Make log viewer find item renewals (2.15 KB, patch)
2019-04-07 23:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22069: Make log viewer find item renewals (2.19 KB, patch)
2019-04-09 17:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22069: (QA Follow-up) Fix other cases (1.05 KB, patch)
2019-04-09 17:00 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22069: Make log viewer find item renewals (2.23 KB, patch)
2019-04-09 17:02 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22069: Make log viewer find item renewals (2.21 KB, patch)
2019-04-09 17:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22069: Make log viewer find item renewals (2.26 KB, patch)
2019-04-09 17:08 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22069: Make log viewer find item renewals (2.31 KB, patch)
2019-04-14 22:11 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description barry.cannon 2019-01-04 13:18:07 UTC
If Tools > Log Viewer is selected and run using the module "Circulation" with the action of "Renew" no results are shown. Selecting the action of "All" shows all results including renewals. The reason appears to be because when selecting the action of "Renew" the form POST uses the action parameter value of "RENEW". This, in turn, gets passed to GetLogs where is used in the SQL query. However, the actual value that is logged in the action_logs table is "RENEWAL" and not "RENEW". Changing the SQL query to "RENEWAL" obtains the correct results.

RENEW is correct for the MEMBERS module but RENEWAL should be used for Circ.
Comment 1 Katrin Fischer 2019-04-07 22:56:33 UTC
The problem is that we have membership renewals that log with RENEW and item renewals that log as RENEWAL, trying to work out a fix.
Comment 2 Katrin Fischer 2019-04-07 23:51:47 UTC
Created attachment 87486 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Circulation and Renew are selected in the form.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected
Comment 3 axel 2019-04-09 15:11:02 UTC
Hello,
I tested the patch, and he doesn't work.
When I tested the different searches,
- Modules All - Actions All  works
- Modules All - Actions Renew return No log found 

- Modules All - Actions Renew  return No log found 
because you forgot parentheses around the grep:

if ( (grep { $_ eq 'CIRCULATION'} @modules) && (grep { $_ eq 'RENEW'} @actions) ) {
   push @actions, 'RENEWAL';	 
}


But i tkink a better solution will be to change the value in the form:

In the actions label
replace <option value="RENEW">Renew</option>
by <option value="RENEWAL">Renew</option>

and the patch will work.
Comment 4 axel 2019-04-09 15:21:50 UTC
(In reply to axel from comment #3)
> Hello,
> I tested the patch, and he doesn't work.
> When I tested the different searches,
> - Modules All - Actions All  works
> - Modules All - Actions Renew return No log found 
> 
> - Modules Circulation - Actions Renew  return No log found 
> because you forgot parentheses around the grep:
> 
> if ( (grep { $_ eq 'CIRCULATION'} @modules) && (grep { $_ eq 'RENEW'}
> @actions) ) {
>    push @actions, 'RENEWAL';	 
> }
> 
> 
> But i tkink a better solution will be to change the value in the form:
> 
> In the actions label
> replace <option value="RENEW">Renew</option>
> by <option value="RENEWAL">Renew</option>
> 
> and the patch will work.


When you add the parentheses, this will only resolve the research "- Modules Circulation - Actions Renew"
I encourage you to apply the last solution because all the case will works
Comment 5 Katrin Fischer 2019-04-09 16:50:02 UTC
Axel, it sadly won't work because then you won't be able to find Membership renewals - Module Patrons > Renw.
Comment 6 Katrin Fischer 2019-04-09 16:50:22 UTC
We have both in the database, RENEW and RENEWAL for different modules.
Comment 7 Katrin Fischer 2019-04-09 17:00:07 UTC
Created attachment 87645 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Circulation and Renew are selected in the form.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected
Comment 8 Liz Rea 2019-04-09 17:00:28 UTC
Created attachment 87646 [details] [review]
Bug 22069: (QA Follow-up) Fix other cases

Same test plan as original patch. :)
Comment 9 Liz Rea 2019-04-09 17:02:36 UTC
Created attachment 87647 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Circulation and Renew are selected in the form.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 10 Katrin Fischer 2019-04-09 17:02:59 UTC
Created attachment 87648 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Renew was selected in the form, so both terms are
included in the query.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected
Comment 11 Katrin Fischer 2019-04-09 17:03:39 UTC
I've changed the patch slightly, thx for testing! I've also added a code comment to make things clearer.
Comment 12 Liz Rea 2019-04-09 17:08:49 UTC
Created attachment 87649 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Renew was selected in the form, so both terms are
included in the query.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 13 Chris Cormack 2019-04-14 22:11:38 UTC
Created attachment 87958 [details] [review]
Bug 22069: Make log viewer find item renewals

Without this patch renewals for checkouts wouldn't
show in the log viewer. The log viewre was using
RENEW, which we use for patrons, but for issue
RENEWAL is used.

The patch adds RENEWAL to the search params, when
Renew was selected in the form, so both terms are
included in the query.

To test:
- Make sure you have RenewalLog turned on
- Make a patron renewal
- Check something out and renew it
- Go to Tools > Log viewer and test different
  searches
  - Modules All - Actions All
  - Modules All - Actions Renew
  - Modules Circulation - Actions Renew
  ...
- Make sure results show as expected

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 14 Nick Clemens 2019-04-16 13:25:15 UTC
Awesome work all!

Pushed to master for 19.05
Comment 15 Martin Renvoize 2019-04-25 13:48:02 UTC
Re-implemented for 18.11.x series as we don't have the refactor that took place in bug 22363.

Pushed to 18.11.x for 18.11.05
Comment 16 Lucas Gass 2019-04-26 16:01:48 UTC
not backporting to 18.05.x