https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29220
Bug ID: 29220
Summary: Minor fixes and improved code readability in
circulation.pl
Change sponsored?: ---
Product: Koha
Version: master
Hardware: All
OS: All
Status: NEW
Severity: minor
Priority: P5 - low
Component: Circulation
Assignee: [email protected]
Reporter: [email protected]
QA Contact: [email protected]
CC: [email protected], [email protected]
While debugging another issue I found myself severely confused by some parts of
the code in circulation.pl. By reviewing some earlier commits I worked out what
is most likely the intent of the code and refactored it to clearer communicate
that intent.
The culprit of my confusion was this:
https://github.com/Koha-Community/Koha/commit/1a8ca21875f86f25c72baf38e29d27af05c988b0#diff-57dd130c647858ddbf96b97d8dc5bd4cd5344ce8b0420ae8e7f1c3c17050c810
While the previous version was straight forward, this change is confusing since
trying to do at least two things at once.
What we want to do as far as I understand is to check for some serious errors
(in this case UNKNOWN_BARCODE, but theoretically could be others as well) and
allow forced onsite checkouts for all errors except for those ones.
Since the display of all non blocking errors are hidden in the staff client for
forced onsite checkouts, for consistency, these errors should also be hidden if
some blocking error occurs. The alternative behavior would be very confusing.
Now, only by chance, UNKNOWN_BARCODE is the first error in CanBookBeIssued to
be checked for and retursn immediately after, so no other errors are ever
registered in that case. But this could change in the future, and would cause a
behavior where all errors are shown if a blocking error occur, but no errors
are shown if not (for forced on site checkouts).
I refactored this part to fix this issue and more clearly communicate the
intended behavior.
I also discovered some minor bugs introduced in various commits:
https://github.com/Koha-Community/Koha/commit/2e72eb888016f60ce15958ecb37e0ae64f0c8454#diff-57dd130c647858ddbf96b97d8dc5bd4cd5344ce8b0420ae8e7f1c3c17050c810
"delete $question->{'DEBT'} if ($debt_confirmed);" is accidentally moved inside
an if condition, when it should really always be performed as far as I can
tell.
https://github.com/Koha-Community/Koha/commit/7a7a0a2474c958e85b96db55bba04e8d90fbed31#diff-57dd130c647858ddbf96b97d8dc5bd4cd5344ce8b0420ae8e7f1c3c17050c810
Here an explicit check for DEBT_GUARANTORS is added, as a blocking error (an
error that will not be ignored for on site checkouts with OnSiteCheckoutsForce
enabled. That seems inconsistent with the settings description "Enable/Disable
the on-site for all cases (Even if a user is debarred, etc." and the fact that
all other errors returned by CanBookBeIssued UNKNOWN_BARCODE are ignored. This
includes
DEBT_GUARANTEES, DEBT, RESTRICTED etc.
--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
