Bug 18572 - Improper branchcode set during OPAC renewal
Summary: Improper branchcode set during OPAC renewal
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17781
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-10 15:31 UTC by Nick Clemens (kidclamp)
Modified: 2019-06-27 09:24 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 18572 - Improper branchcode set during OPAC renewal (3.68 KB, patch)
2017-06-23 13:21 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 18572 - Improper branchcode set during OPAC renewal (3.73 KB, patch)
2017-06-24 20:14 UTC, David Kuhn
Details | Diff | Splinter Review
Bug 18572 - Improper branchcode set during OPAC renewal (3.73 KB, patch)
2017-06-24 20:30 UTC, David Kuhn
Details | Diff | Splinter Review
Bug 18572 - Improper branchcode set during OPAC renewal (3.84 KB, patch)
2017-07-14 10:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18572 - Improper branchcode set during OPAC renewal (3.80 KB, patch)
2017-07-19 18:33 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 18572 - QA Followup (3.29 KB, patch)
2017-07-19 18:33 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 18572: Remove AddRenewal parameter (1.17 KB, patch)
2017-07-24 17:08 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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