Bug 30600

Summary:	Recalls sync problem between DBIx and kohastructure.sql
Product:	Koha	Reporter:	Fridolin Somers <fridolin.somers>
Component:	Database	Assignee:	Aleisha Amohia <aleisha>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	critical
Priority:	P5 - low	CC:	aleisha, kyle
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	22.05.00
Circulation function:
Bug Depends on:	19532
Bug Blocks:
Attachments:	DBIx schema diff Bug 30600: Fix recall reasons for branch transfers Bug 30600: Fix recall reasons for branch transfers Bug 30600: Fix recall reasons for branch transfers Bug 30600: Fix recall reasons for branch transfers

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 ﷐[U+1F984]﷑