Bug 18572

Summary: Improper branchcode set during OPAC renewal
Product: Koha Reporter: Nick Clemens <nick>
Component: OPACAssignee: Nick Clemens <nick>
Status: Pushed to Stable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: fridolin.somers, jdemuth, jonathan.druart, katrin.fischer, m.de.rooy, mtj, nick, techservspec
Version: master   
Hardware: All   
OS: All   
Whiteboard:
Change sponsored?: --- Patch complexity: Small patch
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Bug Depends on: 17781    
Bug Blocks:    
Attachments: Bug 18572 - Improper branchcode set during OPAC renewal
Bug 18572 - Improper branchcode set during OPAC renewal
Bug 18572 - Improper branchcode set during OPAC renewal
Bug 18572 - Improper branchcode set during OPAC renewal
Bug 18572 - Improper branchcode set during OPAC renewal
Bug 18572 - QA Followup
Bug 18572: Remove AddRenewal parameter

Description Nick Clemens 2017-05-10 15:31:36 UTC
Bug 17781 fixed an inconsistency in renewals, but it broke functionality in the OPAC

To recreate:
1 - Set 'OpacRenewalBranch' to 'OPACrenew'
2 - Renew an item via the OPAC
3 - Check the statistics table
4 - Branch will be the user's branch, not OPACrenew
Comment 1 Nick Clemens 2017-06-23 13:21:46 UTC
Created attachment 64596 [details] [review]
Bug 18572 - Improper branchcode set during OPAC renewal

To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t
Comment 2 David Kuhn 2017-06-24 20:14:54 UTC
Created attachment 64609 [details] [review]
Bug 18572 - Improper branchcode set during OPAC renewal

To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t

Signed-off-by: David Kuhn <techservspec@gmail.com>
Comment 3 David Kuhn 2017-06-24 20:30:59 UTC
Created attachment 64610 [details] [review]
Bug 18572 - Improper branchcode set during OPAC renewal

To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t

Step 3 in the above test should refer to entries in the statistics table as that was where the problem was originally noticed.

Signed-off-by: David Kuhn <techservspec@gmail.com>
Comment 4 Marcel de Rooy 2017-07-14 10:06:03 UTC
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_master`.`clubs`, CONSTRAINT `clubs_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) [for Statement "DELETE FROM branches"] at t/db_dependent/Circulation/issue.t line 65.
Comment 5 Marcel de Rooy 2017-07-14 10:21:32 UTC
Created attachment 65048 [details] [review]
Bug 18572 - Improper branchcode set during OPAC renewal

To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t

Signed-off-by: David Kuhn <techservspec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Jonathan Druart 2017-07-17 16:13:05 UTC
Why do not you use C4::Context->interface instead of adding a new parameter to this subroutine?
Comment 7 Jonathan Druart 2017-07-17 16:13:29 UTC
Please also double check the indentation in tests.
Comment 8 Nick Clemens 2017-07-19 18:33:13 UTC
Created attachment 65123 [details] [review]
Bug 18572 - Improper branchcode set during OPAC renewal

To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t

Signed-off-by: David Kuhn <techservspec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Nick Clemens 2017-07-19 18:33:17 UTC
Created attachment 65124 [details] [review]
Bug 18572 - QA Followup
Comment 10 Jonathan Druart 2017-07-24 17:08:31 UTC
Created attachment 65201 [details] [review]
Bug 18572: Remove AddRenewal parameter
Comment 11 Jonathan Druart 2017-07-24 19:03:36 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 12 Fridolin SOMERS 2017-07-27 10:31:29 UTC
Pushed to 17.05.x, will be in 17.05.02.
Comment 13 Katrin Fischer 2017-07-27 23:30:00 UTC
These patches have been pushed to 16.11.x and will be in 16.11.10.
Comment 14 Mason James 2017-07-31 12:16:02 UTC
Pushed to 16.05.x, for 16.05.15 release