Description
Julian Maurice
2017-06-07 15:29:58 UTC
Created attachment 64083 [details] [review] Bug 18745 - Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t Created attachment 65791 [details] [review] Bug 18745 - kohastructure.sql: Move aqbooksellers above serial Otherwise we can't define a foreign key in serial that references aqbooksellers Hi Julian When performing step 2 (creating a new subscription for supplier S1) I found that a software error was thrown (see below). You will need to add the aqbooksellerid to the Koha::Schema::Result::Serial Software error: DBIx::Class::Row::store_column(): No such column 'aqbooksellerid' on Koha::Schema::Result::Serial at /home/vagrant/kohaclone/Koha/Object.pm line 72 Hi Alex, Something is missing in the test plan, you need to run the database update and update the DBIx::Class schema files first. perl installer/data/mysql/updatedatabase.pl perl misc/devel/update_dbix_class_files.pl --db_name DBNAME --db_user DBUSER --db_passwd DBPASS Hi Julian, I've run the database update, dbic, and restart_all in this sequence. The database updates look fine, the aqbooksellerid is populated in serial. But when I try to receive an issue, I have an error: Internal server error koha-plack-error: Can't call method "title" on an undefined value at /home/vagrant/kohaclone/serials/serials-edit.pl line 424. Hi, when creating a serial without a vendor, there is an internal server error. logs: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`serial`, CONSTRAINT `serial_aqbooksellerid_fk` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`)) [for Statement "INSERT INTO `serial` ( `aqbooksellerid`, `biblionumber`, `planneddate`, `publisheddate`, `serialseq`, `serialseq_x`, `serialseq_y`, `serialseq_z`, `status`, `subscriptionid`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='0', 1="12345", 2='2019-01-01', 3='2019-01-01', 4='Vol 145, No 1,2019', 5='145', 6='1', 7='2019', 8=1, 9='123'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834. DBIx::Class::Storage::DBI::_dbh_execute(): Cannot add or update a child row: a foreign key constraint fails (`koha`.`serial`, CONSTRAINT `serial_aqbooksellerid_fk` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`)) at /home/koha/src/Koha/Object.pm line 121 It seems that the issue is that 0 is trying to get in "aqbooksellerid". And there is no aqbooksellers.aqbooksellerid egal to 0 Created attachment 85240 [details] [review] Bug 18745: Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t Created attachment 85241 [details] [review] Bug 18745: kohastructure.sql: Move aqbooksellers above serial Otherwise we can't define a foreign key in serial that references aqbooksellers Created attachment 85242 [details] [review] Bug 18745: Add foreign key constraint on subscription.aqbooksellerid So now aqbookseller is mandatory in the form right? (In reply to Victor Grousset/tuxayo from comment #10) > So now aqbookseller is mandatory in the form right? I'd like to avoid this if possible. Right now there is only a warning and I think assigning a 'fake' vendor in some cases is not as nice as the current behaviour. Are there cases where a serial wouldn't always have a vendor? From a librarian POV. (In reply to Katrin Fischer from comment #11) > (In reply to Victor Grousset/tuxayo from comment #10) > > So now aqbookseller is mandatory in the form right? > > I'd like to avoid this if possible. Right now there is only a warning and I > think assigning a 'fake' vendor in some cases is not as nice as the current > behaviour. Bookseller is still optional, but now subscription.aqbooksellerid would be NULL instead of 0 (In reply to Victor Grousset/tuxayo from comment #12) > Are there cases where a serial wouldn't always have a vendor? From a > librarian POV. Some don't use the claim feature or acq... and then the vendor might be left empty. Also maybe if you are tracking your own publications or loose leaf things? I am not sure, but I have been asked if the field can be left empty. Created attachment 87102 [details] [review] Bug 18745: Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t Created attachment 87103 [details] [review] Bug 18745: kohastructure.sql: Move aqbooksellers above serial Otherwise we can't define a foreign key in serial that references aqbooksellers Created attachment 87104 [details] [review] Bug 18745: Add foreign key constraint on subscription.aqbooksellerid Applying: Bug 18745: kohastructure.sql: Move aqbooksellers above serial Applying: Bug 18745: Add foreign key constraint on subscription.aqbooksellerid fatal: sha1 information is lacking or useless (C4/Serials.pm). error: could not build fake ancestor Patch failed at 0001 Bug 18745: Add foreign key constraint on subscription.aqbooksellerid Created attachment 124951 [details] [review] Bug 18745: Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t All patches rebased on master and squashed. Ready to be tested error: sha1 information is lacking or useless (installer/data/mysql/kohastructure.sql). You shouldn't need to reorder kohastructure.sql (In reply to Jonathan Druart from comment #21) > error: sha1 information is lacking or useless > (installer/data/mysql/kohastructure.sql). > > You shouldn't need to reorder kohastructure.sql Indeed patch "kohastructure.sql: Move aqbooksellers above serial" is obsolete, aqbooksellers is above serial because tables are alphabetical now Created attachment 138423 [details] [review] Bug 18745: Serial claims: save supplier info on serial-level This allows to change the subscription's supplier while keeping the info of supplier for previous serials, which allows to claim late/missing serials to the right supplier Test plan: 1. Create 2 suppliers, S1 and S2 2. Create a new subscription with S1 3. Generate some late/missing serials 4. Change supplier to S2 5. Generate some more late/missing serials 6. Go to /cgi-bin/koha/serials/claims.pl and see you have late/missing issues for both suppliers 7. Check that you have the expected issues for each supplier 8. prove t/db_dependent/Serials/Claims.t Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 138424 [details] [review] [DO NOT PUSH] Bug 18745: DBIC schema changes Happy birthday, here is a rebase [U+1F381] ^o^ I compared the old and new patch files to double check that I didn't change anything unknowingly. There was some mess in the ticket. Some old patches weren't obsoleted and this was the actual reason the patch didn't apply at first. Fixed missing comma in kohastructure.sql Removed DBIC schema changes, regenerated then and put then in their own commit. (tests still pass) It seems there is an issue (but I'm not sure I'm using serials right)
> 3. Generate some late/missing serials
So I click "Receive" and replace "expected" by "late" or "missing" and then "save"
In my case I did one late and one missing.
Then changed the sub vendor to S2.
And did one late and one missing.
Now on claims for S1 I have one late, one missing, **one late** I had already changed to S2!
For S2 I have one missing, one expected. **But not one late**
Tried again and same result.
So the 1st after the transition (editing the sub to change vendor) is on the wrong vendor.
====
The tests pass.
the first "# 3 is late" looks like it should say "# 2 is late", should it?
Why the does the test expects
2 late issues for bookseller1
and
1 late issue for bookseller2
Shouldn't it be 1 and 2?
Or maybe I don't get how late serial work and the test is correct and the behavior of my manual testing is correct?
+ ADD CONSTRAINT `serial_aqbooksellerid_fk` FOREIGN KEY (`aqbooksellerid`) REFERENCES `aqbooksellers` (`id`) Needs an explicit on delete/update cascade no ? atomicupdate bug_18745.perl needs to be updated with current format. |