May 2018 - Koha-bugs - lists.koha-community.org

[Bug 11943] Prevent double accepts in Koha::Virtualshelfshare
by bugzilla-daemon＠bugs.koha-community.org 07 May '18

07 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11943 Nick Clemens <nick(a)bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75050|0 |1 is obsolete| | --- Comment #13 from Nick Clemens <nick(a)bywatersolutions.com> --- Created attachment 75105 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75105&action=ed… Bug 11943: Add unit test for double accepts Test plan: Run t/db_dependent/Virtualshelves.t Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Nick Clemens <nick(a)bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 11943] Prevent double accepts in Koha::Virtualshelfshare
by bugzilla-daemon＠bugs.koha-community.org 07 May '18

07 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11943 Nick Clemens <nick(a)bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75049|0 |1 is obsolete| | --- Comment #12 from Nick Clemens <nick(a)bywatersolutions.com> --- Created attachment 75104 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75104&action=ed… Bug 11943: Prevent double accepts in Koha::Virtualshelfshare This 'bug' existed long enough now to finally remove it ;) We do so by deleting the invitation if the borrower already has a share on this list. Actually not that hard. We still need: a unit test and a db revision. Test plan: [1] Share a list. Let user B accept. [2] Without this patch: Share again and let B accept again. [3] Verify that you have two shares for this list in virtualshelfshares. [4] With this patch: Share another list, let B accept. [5] Share this other list again, let B accept again. [6] Verify that virtualshelfshares does not contain double entries now. (Note: This pertains to the second list only.) Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Signed-off-by: Nick Clemens <nick(a)bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 11943] Prevent double accepts in Koha::Virtualshelfshare
by bugzilla-daemon＠bugs.koha-community.org 07 May '18

07 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11943 Nick Clemens <nick(a)bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 11943] Prevent double accepts in Koha::Virtualshelfshare
by bugzilla-daemon＠bugs.koha-community.org 07 May '18

07 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11943 Nick Clemens <nick(a)bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #11 from Nick Clemens <nick(a)bywatersolutions.com> --- Applying: Bug 11943: Prevent double accepts in Koha::Virtualshelfshare fatal: sha1 information is lacking or useless (Koha/Virtualshelfshare.pm). -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 --- Comment #9 from Barton Chittenden <barton(a)bywatersolutions.com> --- (In reply to Saiful Amin from comment #7) > I've done some more digging into this and found that it was indeed a data > issue, the item itself was missing. Single record caused the failure. > > I feel that the error message does not do justice to the error. The script > should not crash on faulty data. The final fix that I would suggest is this > (as suggested in Comment #3): > > 109 my $branchcode = > 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} > 111 : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} > 112 : $overdue->{branchcode}; > + 113 if (!defined $branchcode ) { > + 114 carp "ERROR in getting branchcode. Skipping record.\n"; > + 115 next; > + 116 } > 117 > 118 # In final case, CircControl must be PickupLibrary. (branchcode comes > from issues table here). > 119 if ( !exists $is_holiday{$branchcode} ) { > 120 $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); As luck would have it, I was writing my patch at the same time that you were writing this; mine has the same logic but gives a bit more detail about which item failed. Here's my test plan: When the CircControl system preference is set to 'the library the item is from' and items.homebranch is not set, fines.pl will stop processing witha fatal error that looks something like this: Use of uninitialized value $branchcode in exists at /home/koha/kohaclone/misc/cronjobs/fines.pl line 110 No branchcode argument passed to Koha::Calendar->new at /home/koha/kohaclone/misc/cronjobs/fines.pl line 169. This patch will check the value of $branchcode before it is used. If $branchcode is undefined, a warning will be issued and, processing will be skipped for that item, and continue for the next item. To test: 1) Set CircControl to 'the library the item is from' 2) Check out items and back-date them to be overdue 3) Set homebranch of one of the items to NULL 4) Run misc/cronjobs/fines.pl, note that processing stops at that item. 5) Apply patch 6) Repeat step 4. not that processing completes. 7) Check logs, you should see a warning message for the item missing its homebranch. 8) Sign off. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 --- Comment #8 from Barton Chittenden <barton(a)bywatersolutions.com> --- Created attachment 75103 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75103&action=ed… Add warning if homebranch is undefined. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 Saiful Amin <saiful(a)semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff --- Comment #7 from Saiful Amin <saiful(a)semanticconsulting.com> --- I've done some more digging into this and found that it was indeed a data issue, the item itself was missing. Single record caused the failure. I feel that the error message does not do justice to the error. The script should not crash on faulty data. The final fix that I would suggest is this (as suggested in Comment #3): 109 my $branchcode = 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} 111 : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} 112 : $overdue->{branchcode}; + 113 if (!defined $branchcode ) { + 114 carp "ERROR in getting branchcode. Skipping record.\n"; + 115 next; + 116 } 117 118 # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here). 119 if ( !exists $is_holiday{$branchcode} ) { 120 $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 Saiful Amin <saiful(a)semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|FIXED |--- Status|RESOLVED |REOPENED --- Comment #6 from Saiful Amin <saiful(a)semanticconsulting.com> --- After posting that I thought of changing the status from RESOLVED to something else, but didn't see any option to modify. Coming back to testing, it was done on a fresh installation of 17.11.05.000 installed from Debian repo on Ubuntu 17.04, using MariaDB. - We migrated data from another ILS for all bibs, items, patrons and circulation transactions. - There is only one branch for all items and borrowers. - After migration we checked there are no NULL values in items.homebranch, issues.branchcode, borrowers.branchcode columns. - In preferences, - Checkout policy 'CircControl' is set to 'ItemHomeLibrary' ("the library the item is from.") - 'item-level_itypes' is set to 'specific item' (1) - 'finesMode' set to 'Calculate and charge' We run this command (copied from cron file): koha-foreach --chdir --enabled /usr/share/koha/bin/cronjobs/fines.pl The fines don't get generated and we see this error: Use of uninitialized value $branchcode in exists at /usr/share/koha/bin/cronjobs/fines.pl line 115. No branchcode argument passed to Koha::Calendar->new at /usr/share/koha/bin/cronjobs/fines.pl line 179. With the following change: - ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} + ( $control eq 'ItemHomeLibrary' ) ? $overdue->{branchcode} the error goes away and fines are getting calculated. My comment about 'issues' not containing 'homebranch' was not relevant since I realize now that the Getoverdues() function left joins the 'item' table (which contains the 'homebranch'). Should I add a pull request for this so that someone can run other tests? -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 --- Comment #5 from Barton Chittenden <barton(a)bywatersolutions.com> --- (In reply to Saiful Amin from comment #4) > This is not a data issue but a bug in the code. Here are the lines: > > 109 my $branchcode = > 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} > 111 : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} > 112 : $overdue->{branchcode}; > 113 > > The variable $overdue->{homebranch} is bad because the 'issues' table has no > column called 'homebranch'. It should be 'branchcode' instead. I'm able to > fix the error by changing the line 110 to this: > > 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{branchcode} Saiful, This bug was marked 'RESOLVED, FIXED'. Have you tested this in the most recent version of Koha? If it's still an issue, please let us know; this does cause support problems. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

1 0

[Bug 5789] Fines don't work when items have null homebranch
by bugzilla-daemon＠bugs.koha-community.org 06 May '18

06 May '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5789 Saiful Amin <saiful(a)semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED CC| |saiful(a)semanticconsulting.c | |om --- Comment #4 from Saiful Amin <saiful(a)semanticconsulting.com> --- This is not a data issue but a bug in the code. Here are the lines: 109 my $branchcode = 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} 111 : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} 112 : $overdue->{branchcode}; 113 The variable $overdue->{homebranch} is bad because the 'issues' table has no column called 'homebranch'. It should be 'branchcode' instead. I'm able to fix the error by changing the line 110 to this: 110 ( $control eq 'ItemHomeLibrary' ) ? $overdue->{branchcode} -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.

1 0