This is a more challenging one, as issue_id refers to not one but two distinct tables .. issues and old_issues.. as such we either need to merge issues and old_issues or split this constraint into two distinct fields and ensure they're properly populated. I've opted for the distinct fields in this patch for now as it's the simpler approach and can be easily switched at a later date for the issue/old_issue merge.
Created attachment 85773 [details] [review] Bug 22421: Add tests for new constraints
Created attachment 85774 [details] [review] Bug 22421: Add missing constraints
Created attachment 85775 [details] [review] Bug 22421: Ensure old_issue_id is populated in accountlines
Sorry, can't apply: Applying: Bug 22421: Add tests for new constraints error: sha1 information is lacking or useless (t/db_dependent/Koha/Account/Lines.t). There are also some templates referencing issue_id, it should be fixed, and Koha::Accout::Line->checkout should be adjusted too...
Created attachment 90352 [details] [review] Bug 22421: Add tests for new constraints Test plan: * Read the updated tests and verify that they make sense and do not remove anything previously tested for. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90353 [details] [review] Bug 22421: Add missing constraints DB update to add a new old_issue_id field to accountlines and set foreign key constraints for both the new field and the existing issue_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90354 [details] [review] Bug 22421: Ensure old_issue_id is populated in accountlines Update C4::Circulation::MarkIssueReturned to handle moving issue_id to old_issue_id in accountlines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90359 [details] [review] Bug 22421: Add tests for new constraints Test plan: * Read the updated tests and verify that they make sense and do not remove anything previously tested for. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90360 [details] [review] Bug 22421: Add missing constraints DB update to add a new old_issue_id field to accountlines and set foreign key constraints for both the new field and the existing issue_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90361 [details] [review] Bug 22421: Ensure old_issue_id is populated in accountlines Update C4::Circulation::MarkIssueReturned to handle moving issue_id to old_issue_id in accountlines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90362 [details] [review] Bug 22421: (follow-up) Correct checkout relation The Koha::Account::Line->checkout relationship accessor needed an update to respect the new fields and foreign keys introduced here. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90365 [details] [review] Bug 22421: Add tests for new constraints Test plan: * Read the updated tests and verify that they make sense and do not remove anything previously tested for. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 90366 [details] [review] Bug 22421: Add missing constraints DB update to add a new old_issue_id field to accountlines and set foreign key constraints for both the new field and the existing issue_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 90367 [details] [review] Bug 22421: Ensure old_issue_id is populated in accountlines Update C4::Circulation::MarkIssueReturned to handle moving issue_id to old_issue_id in accountlines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 90368 [details] [review] Bug 22421: (follow-up) Correct checkout relation The Koha::Account::Line->checkout relationship accessor needed an update to respect the new fields and foreign keys introduced here. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Trying to test this, but I am having issues getting a lost replacement fee to charge. I have set a replacement default cost on item level and set WhenLostChargeReplacementFee to Charge. Items are returned on setting the lost status. Not sure if this is related yet, but maybe you can help verify.
I also tried charging overdue fines and returning the items. After returning them, the due date and return date information are no longer visible in accountlines (not implemented yet?). old_issue_id is set, so that works. I'll have to reset my db to check the lost behaviour, running a bit low on time right now, help appreciated.
I got myself into trouble here: 1) Check out item backdated, so it's overdue 2) Run fines.pl to accrue fines 3) Return it 4) Verify title etc. still show - Apply patchset 1) Run updatedatabase and dbic, restart_all 2) Check out same item backdated again 3) Return from patron account DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`accountlines`, CONSTRAINT `accountlines_ibfk_issues` FOREIGN KEY (`issue_id`) REFERENCES `issues` (`issue_id`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "INSERT INTO `accountlines` ( `accounttype`, `amount`, `amountoutstanding`, `borrowernumber`, `branchcode`, `date`, `description`, `interface`, `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `status`) VALUES ( ?, ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='OVERDUE', 1=0.9, 2=0.9, 3=51, 4=undef, 5='Conspirator : 07/01/2019 23:59', 6='intranet', 7=4, 8=22, 9=undef, 10='', 11=undef, 12='UNRETURNED'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88 Exception::Class::Base::throw('Koha::Exceptions::Object::FKConstraint', 'error', 'Broken FK constraint', 'broken_fk', 'issue_id') called at /home/vagrant/kohaclone/Koha/Object.pm line 163