Bug 30600 - Recalls sync problem between DBIx and kohastructure.sql
Summary: Recalls sync problem between DBIx and kohastructure.sql
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on: 19532
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-22 20:01 UTC by Fridolin Somers
Modified: 2022-05-17 12:38 UTC (History)
2 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:
22.05.00


Attachments
DBIx schema diff (2.12 KB, patch)
2022-04-22 20:01 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30600: Fix recall reasons for branch transfers (3.95 KB, patch)
2022-04-27 02:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30600: Fix recall reasons for branch transfers (4.20 KB, patch)
2022-04-29 02:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30600: Fix recall reasons for branch transfers (4.25 KB, patch)
2022-04-29 10:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30600: Fix recall reasons for branch transfers (4.28 KB, patch)
2022-05-04 20:09 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2022-04-22 20:01:16 UTC
Created attachment 133696 [details] [review]
DBIx schema diff

Bug 19532 Recalls added several db changes.

Comparing a structure from v21.11.00 + updatedatabase.pl and a structure generated from current master kohastructure.sql, after updating DBIx schema (via misc/devel/update_dbix_class_files.pl not dbic) we see a difference in Koha/Schema/Result/Branchtransfer.pm.
Comment 1 Fridolin Somers 2022-04-22 20:12:27 UTC
We see that kohastructure.sql has 'Recall' and 'CancelRecall' in branchtransfers.reason and branchtransfers.cancellation_reason :
https://git.koha-community.org/Koha-community/Koha/src/commit/74792b35a83a863fa5e68c37f0c0f552f0c21a7b/installer/data/mysql/kohastructure.sql#L1560

But DBRev only add in branchtransfers.reason :
https://git.koha-community.org/Koha-community/Koha/src/commit/74792b35a83a863fa5e68c37f0c0f552f0c21a7b/installer/data/mysql/db_revs/211200018.pl#L93

Which one is correct :/
Comment 2 Fridolin Somers 2022-04-22 20:22:40 UTC
Second :

DBRev removes from ENUM branchtransfers.reason the value 'TransferCancellation' added by Bug 12362
Comment 3 Aleisha Amohia 2022-04-27 01:18:03 UTC
Hi Fridolin, I will take a look at this asap.

Aleisha
Comment 4 Aleisha Amohia 2022-04-27 02:43:40 UTC
Created attachment 134008 [details] [review]
Bug 30600: Fix recall reasons for branch transfers

When a recall triggers a branch transfer, the reason for the transfer is
'Recall'.

When a transfer is cancelled that was triggered by a recall, it still
uses the default 'Manual' reason, so we don't need the 'CancelRecall'
cancellation reason that was added by Bug 19532.

This patch removes 'Recall' and 'CancelRecall' from
branchtransfers.cancellation_reason, and removes 'CancelRecall' from
branchtransfers.reason.

Sponsored-by: Catalyst IT
Comment 5 Fridolin Somers 2022-04-27 08:46:43 UTC
(In reply to Aleisha Amohia from comment #3)
> Hi Fridolin, I will take a look at this asap.
> 
> Aleisha

Thanks a lot.

The recalls feature is not yet in a release.
May we fix directly its DBRev 21.12.00.018 ?
Comment 6 Aleisha Amohia 2022-04-29 02:06:46 UTC
Created attachment 134344 [details] [review]
Bug 30600: Fix recall reasons for branch transfers

When a recall triggers a branch transfer, the reason for the transfer is
'Recall'.

When a transfer is cancelled that was triggered by a recall, depending
on where it is cancelled, will use the 'Manual' or 'CancelRecall'
cancellation reasons.

This patch ensures branchtransfers.cancellation_reason adds 'CancelRecall',
and branchtransfers.reason adds 'Recall' as options.

Sponsored-by: Catalyst IT
Comment 7 Katrin Fischer 2022-04-29 10:57:26 UTC
Created attachment 134356 [details] [review]
Bug 30600: Fix recall reasons for branch transfers

When a recall triggers a branch transfer, the reason for the transfer is
'Recall'.

When a transfer is cancelled that was triggered by a recall, depending
on where it is cancelled, will use the 'Manual' or 'CancelRecall'
cancellation reasons.

This patch ensures branchtransfers.cancellation_reason adds 'CancelRecall',
and branchtransfers.reason adds 'Recall' as options.

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Fridolin Somers 2022-05-04 20:09:49 UTC
Created attachment 134618 [details] [review]
Bug 30600: Fix recall reasons for branch transfers

When a recall triggers a branch transfer, the reason for the transfer is
'Recall'.

When a transfer is cancelled that was triggered by a recall, depending
on where it is cancelled, will use the 'Manual' or 'CancelRecall'
cancellation reasons.

This patch ensures branchtransfers.cancellation_reason adds 'CancelRecall',
and branchtransfers.reason adds 'Recall' as options.

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Fridolin Somers 2022-05-04 20:11:30 UTC
Fixed db_rev 211200018.pl was missing 'TransferCancellation'.

I really need this one for 22.05 release.
Consider I QA.
Comment 10 Fridolin Somers 2022-05-05 00:33:12 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄