To recreate: 1 - Enable both issue and renewal logs 2 - Checkout an item to a patron 3 - View the logs - the issue is captured 4 - Checkout the item to the patron again and confirm renewal 5 - Both an issue and a renewal are logged
Created attachment 65829 [details] [review] Bug 19076 - unit tests
Created attachment 65830 [details] [review] Bug 19076 - Move issue logging in AddIssue into condtional clause Currently AddIssue tests if renewal, but logs an issue even if so. This patch moves the logging into the conditional so a log entry is only added if we aren't renewing (as renewals are logged separately) To test: 1 - prove t/db_dependent/Circulation.t - one test should fail 2 - Enable both issue and renewal logs 3 - Checkout an item to a patron 4 - View the logs - the issue is captured 5 - Checkout the item to the patron again and confirm renewal 6 - Both an issue and a renewal are logged 7 - Apply patch 8 Repeat 1-6, tests should pass and only renewal should be logged
Created attachment 65907 [details] [review] Bug 19076 - unit tests
Created attachment 65908 [details] [review] Bug 19076 - Move issue logging in AddIssue into condtional clause Currently AddIssue tests if renewal, but logs an issue even if so. This patch moves the logging into the conditional so a log entry is only added if we aren't renewing (as renewals are logged separately) To test: 1 - prove t/db_dependent/Circulation.t - one test should fail 2 - Enable both issue and renewal logs 3 - Checkout an item to a patron 4 - View the logs - the issue is captured 5 - Checkout the item to the patron again and confirm renewal 6 - Both an issue and a renewal are logged 7 - Apply patch 8 Repeat 1-6, tests should pass and only renewal should be logged
Created attachment 65916 [details] [review] Bug 19076 - followup - delete fines from additional renewal
Please rebase.
Applies cleanly for me on master, can you pull and retest?
Created attachment 65939 [details] [review] [SIGNED OFF ] Bug 19076 - followup - delete fines from additional renewal Works as described. Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Created attachment 66701 [details] [review] Bug 19076 - unit tests Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 66702 [details] [review] Bug 19076 - Move issue logging in AddIssue into condtional clause Currently AddIssue tests if renewal, but logs an issue even if so. This patch moves the logging into the conditional so a log entry is only added if we aren't renewing (as renewals are logged separately) To test: 1 - prove t/db_dependent/Circulation.t - one test should fail 2 - Enable both issue and renewal logs 3 - Checkout an item to a patron 4 - View the logs - the issue is captured 5 - Checkout the item to the patron again and confirm renewal 6 - Both an issue and a renewal are logged 7 - Apply patch 8 Repeat 1-6, tests should pass and only renewal should be logged Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 66703 [details] [review] Bug 19076 - followup - delete fines from additional renewal Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
(In reply to Christopher Brannon from comment #8) You only signed off on the last patch, please read the wiki page https://wiki.koha-community.org/wiki/Git_bz_configuration#Bugs_with_more_than_one_patch and/or follow the tutorial: https://wiki.koha-community.org/wiki/Koha_howto
(In reply to Nick Clemens from comment #5) > Created attachment 65916 [details] [review] [review] > Bug 19076 - followup - delete fines from additional renewal Why this patch here? Sounds like it should be moved somewhere else.
It should be squashed into unit tests patch, the extra renewal I add for tests added a fine, had to remove that to not break other tests
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.05.
Dependency on bug 17708 which is not in 16.11.x.