Koha-clms/t/db_dependent/Circulation
Jonathan Druart 979bc2e6fa Bug 18966: Do not deal with duplicate issue_id on checkin
Koha suffers of big bugs due to its history: When data are deleted, they are moved to another tables.
For instance issues and old_issues: when a checkin is done, it is moved to the old_issues table.
That leads to a main problem that is described on https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix

However we tried first to fix the problem (for issues/old_issues) at code level on bug 18242.
The goal was to prevent data lost.
Data lost may happens in this case:
Check an item out (issue_id = 1)
Check an item in (issue_id = 1)
Restart MySQL (reset auto increment for issue_id to 1)
Check an item out (issue_id = 1)
Check an item in => BOOM, the issue_id is a PK in old_issues and the move fails.
Before bug 18242 the data were lost, we inserted the value into old_issues, which fails silently (because of RaiseError set to 0 in Koha::Database), then delete the row from issues.
That has been fixed using a transaction.

This patch introduced a regression we tried to fix on bug 18651 comment 0, the patron was charged even if the checkin was rejected.
A good way to fix that would have been to LOCK the tables:
1- Start a transaction
2- LOCK the table to make sure nobody will read id and avoid race conditions
3- Move the content from one table to the other, dealing with ids
4- UNLOCK the table
5- Commit the transaction
But there were problems using LOCK and DBIx::Class (See commit 905572910b - Do no LOCK/UNLOCK the table).

Finally the solution implemented is not acceptable for several reasons:
- batch checkins may fail
- issue_id will always stay out of sync (between issues and old_issues)
See 18651 comment 66.

Since the next stable releases are very soon, and we absolutely need to fix this problem, I am suggesting to:
1- Execute the move in a transaction to avoid data lost and reject the checkin if we face IDs dup
=> It will only reject 1 checkin (max is 1 * MySQL restart), no need to deal with race conditions,
2- Display a warning on the checkin page and link to a solution/explanation
3- Communicate as much as we can on the proper fix: Update auto increment values when the DBMS is restarted - https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
4- Display a warning on the about page for corrupted data (see bug 18931)
5- Write and make available a maintenance script to fix corrupted data (TODO LATER)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-07-25 11:51:43 +12:00
..
IssuingRules Bug 17592: Remove itemtype-related maxsuspensiondays.t warnings 2016-12-12 14:54:09 +13:00
AnonymiseIssueHistory.t Bug 16155: Adjust a few other tests 2016-05-04 13:47:58 +00:00
Branch.t Bug 16404: Fix Circulation/Branch.t (tests 8-11) 2016-05-03 15:21:31 +00:00
CalcDateDue.t Bug 12787: (QA followup) small QA fixes 2016-04-22 00:54:35 +00:00
CalcFine.t Bug 16155: Adjust a few other tests 2016-05-04 13:47:58 +00:00
CheckIfIssuedToPatron.t Bug 17653: Remove itemtype-related t/db_dependent/Circulation* warnings 2016-12-12 14:54:13 +13:00
CheckValidBarcode.t Bug 15629: Koha::Libraries - Remove ModBranch 2016-02-24 03:55:06 +00:00
GetHardDueDate.t Bug 12787: Reorganise t/db_dependent files 2016-04-22 00:54:35 +00:00
GetIssues.t Bug 17636: Remove itemtype-related GetIssues.t warnings 2016-12-12 14:54:12 +13:00
GetTopIssues.t Bug 11998: Use t::lib::Mocks::mock_preference in tests 2016-03-15 07:08:28 +00:00
IsItemIssued.t Bug 17653: Remove itemtype-related t/db_dependent/Circulation* warnings 2016-12-12 14:54:13 +13:00
MarkIssueReturned.t Bug 17430: Make MarkIssueReturned.t create its own data 2016-10-22 14:57:56 +02:00
NoIssuesChargeGuarantees.t Bug 16405: Fix Circulation/NoIssuesChargeGuarantees.t test 2 2016-05-04 14:23:34 +00:00
OfflineOperation.t Bug 12787: Reorganise t/db_dependent files 2016-04-22 00:54:35 +00:00
Returns.t Bug 18966: Do not deal with duplicate issue_id on checkin 2017-07-25 11:51:43 +12:00
TooMany.t Bug 8361: Do not allow checkouts if no rules are defined 2017-02-22 21:57:36 +13:00
dateexpiry.t Bug 12787: Reorganise t/db_dependent files 2016-04-22 00:54:35 +00:00
issue.t Bug 17781 - Improper branchcode set during renewal 2017-01-25 12:48:38 +13:00
transfers.t Bug 17653: Remove itemtype-related t/db_dependent/Circulation* warnings 2016-12-12 14:54:13 +13:00