Bug 17708

Summary: Renewal log seems empty
Product: Koha Reporter: vimal kumar <vimalibre>
Component: Transaction logsAssignee: Baptiste <baptiste.wojtkowski>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P1 - high CC: baptiste.wojtkowski, fridolin.somers, jean-manuel.broust, jonathan.druart, julien.comte, katrin.fischer, kohapatch, kyle, marjorie.barry-vila, mtj, sally.healey, veron, vimalibre
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 19076    
Attachments: Renewal log
Added a logaction function in Addrenewal in order to send renew logs
Added a logaction function in Addrenewal in order to send renew logs
BUG 8010: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Manual Merged
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
BUG 17708: Manual Merged
Bug 17708: Added a logaction function in Addrenewal in order to send renew logs
Bug 17708: Add the IGNORE clause
Bug 17708: Fix use statements
Bug 17708: Rename 'RENEW' with 'RENEWAL'

Description vimal kumar 2016-12-01 16:31:56 UTC
Created attachment 57861 [details]
Renewal log

Koha version 16.05.5

Tools > Log > Circulation > Renewal does not display result.

Please check the screenshots.
Comment 1 Baptiste 2017-02-08 09:06:21 UTC
Created attachment 60007 [details] [review]
Added a logaction function in Addrenewal in order to send renew logs

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 2 vimal kumar 2017-02-14 10:39:41 UTC
This patch available with latest versions?
Comment 3 Marc Véron 2017-02-17 09:56:04 UTC
Created attachment 60383 [details] [review]
Added a logaction function in Addrenewal in order to send renew logs

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.

https://bugs.koha-community.org/show_bug.cgi?id=17708

Followed test plan, works as expected. Circulation.t passes OK
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 4 Marc Véron 2017-02-17 09:57:39 UTC
Note: Tested on current master, switching version to master.
Comment 5 Jonathan Druart 2017-02-20 08:49:56 UTC
Baptiste,
An action log should be tied to a syspref. Take a look at the different logaction calls in C4::Circulation.
Moreover it seems that the call is wrong, third arg should be the borrowernumber, not itemnumber (which should be the 4th I guess).
Comment 6 Jonathan Druart 2017-02-20 08:51:30 UTC
(In reply to vimal kumar from comment #2)
> This patch available with latest versions?

Patches are always provided for master, unless they fix a bug in a specific version.
Comment 7 Baptiste 2017-02-20 09:42:27 UTC
Created attachment 60454 [details] [review]
BUG 8010: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 8 Baptiste 2017-02-20 09:47:32 UTC
Created attachment 60455 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 9 Marc Véron 2017-02-20 10:14:31 UTC
Hi Babtiste,

There is a tiny typo in the prefs:
"when item are renewed. " (items)

Do not forget to switch back to Needs Signoff.

Marc
Comment 10 Baptiste 2017-02-20 10:20:38 UTC
Created attachment 60458 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 11 Baptiste 2017-02-20 10:25:39 UTC
Created attachment 60459 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 12 Marc Véron 2017-02-20 10:43:43 UTC
There is somethng more in bug_17708_add-RenewalLog.sql and sysprefs.sql

log information about reports - I think you mean: log information about renewals
Comment 13 Baptiste 2017-02-20 10:53:40 UTC
Created attachment 60460 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 14 Marc Véron 2017-02-22 09:22:21 UTC
Created attachment 60541 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.

https://bugs.koha-community.org/show_bug.cgi?id=17708

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 15 Jonathan Druart 2017-02-23 16:08:03 UTC
The test does not pass for me, I guess it is because you forget to mock pref.
What you need to do is:

  mock_preference('RenewalLog', 0)
  AddRenewal
  test that there is no log added
  mock_preference('RenewalLog', 1)
  AddRenewal
  test that 1 log has been added

IMO the test should be
  is ($new_log_size, $old_log_size +1, '');
which make more sense than
  is ($new_log_size - 1, $old_log_size, '');
Comment 16 Baptiste 2017-02-24 08:43:43 UTC
Created attachment 60641 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog
24/20/17 : added a test for the syspref Renewal Log

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 17 jmbroust 2017-03-20 09:57:20 UTC
The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 17708:
<h1>Something went wrong !</h1>Applying: BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
Using index info to reconstruct a base tree...
M C4/Circulation.pm
M installer/data/mysql/sysprefs.sql
M t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Circulation.pm
Patch failed at 0001 BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
error: Failed to merge in the changes.
Bug 17708 - Renewal log seems empty

60641 - BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

Apply? [(y)es, (n)o, (i)nteractive] Patch left in tmpBUG-17708-Added-a-logaction-function-in-Addrenewal-dBVGdR.patch .
Comment 18 Baptiste 2017-03-21 08:53:12 UTC
Created attachment 61353 [details] [review]
BUG 17708: Manual Merged

Merged to be up to date with master
Comment 19 julien.comte 2017-03-21 10:39:30 UTC
Some problems occurred applying patches from bug 17708:
<h1>Something went wrong !</h1>Applying: BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
Using index info to reconstruct a base tree...
M       C4/Circulation.pm
M       installer/data/mysql/sysprefs.sql
M       t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Circulation.pm
Patch failed at 0001 BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
error: Failed to merge in the changes.
Bug 17708 - Renewal log seems empty

60641 - BUG 17708: Added a logaction function in Addrenewal in order to send renew logs
61353 - BUG 17708: Manual Merged

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/BUG-17708-Added-a-logaction-function-in-Addrenewal-4Iph8r.patch .
Comment 20 Baptiste 2017-03-21 11:28:58 UTC
Created attachment 61368 [details] [review]
BUG 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog
24/20/17 : added a test for the syspref Renewal Log

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Comment 21 Baptiste 2017-03-21 11:29:06 UTC
Created attachment 61369 [details] [review]
BUG 17708: Manual Merged

Merged to be up to date with master
Comment 22 julien.comte 2017-03-21 12:55:46 UTC
The renew appears when I set RenewalLog to 1.
Comment 23 Jonathan Druart 2017-03-21 19:37:25 UTC
Created attachment 61417 [details] [review]
Bug 17708: Added a logaction function in Addrenewal in order to send renew logs

20/02/17 : added the syspref RenewalLog
24/20/17 : added a test for the syspref Renewal Log

test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew

I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.

Signed-off-by: Julien Comte <julien.comte@u-psud.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2017-03-21 19:37:46 UTC
Created attachment 61418 [details] [review]
Bug 17708: Add the IGNORE clause

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2017-03-21 19:37:51 UTC
Created attachment 61419 [details] [review]
Bug 17708: Fix use statements

For an unknown reason, the use_ok('Circulation') does not work as
intended (see 3660c451a36d).
With the new use of C4::Log, the trick does no longer work.
It does not make sense to add the use_ok('C4::Log') in Circulation.t,
removing it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Jonathan Druart 2017-03-21 19:37:56 UTC
Created attachment 61420 [details] [review]
Bug 17708: Rename 'RENEW' with 'RENEWAL'

Sounds more appropriate and consistent with existing action logs.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Kyle M Hall 2017-03-31 13:18:55 UTC
Pushed to master for 17.05, thanks Baptiste, Jonathan!
Comment 28 Katrin Fischer 2017-04-02 17:08:12 UTC
This adds a new system preference and appears more like an enh to me.
Not pushing to 16.11.x.
Comment 29 Nick Clemens 2017-08-10 14:55:52 UTC
*** Bug 8752 has been marked as a duplicate of this bug. ***