Back in the 3.0 days, in https://git.koha-community.org/Koha-community/Koha/commit/af466ca41ab02150f4f6c60bf6262b079b2e6e85 someone looked at the dead code in https://git.koha-community.org/Koha-community/Koha/blame/commit/c6013f3fd09ac5bd02f31e86bcc932c265c58502/C4/Biblio.pm#L3288 my $series = 0; if ( $biblio->{'seriestitle'} ) { $series = 1 } which did nothing because there wasn't any biblio.series for $series to be setting (maybe there had been at one time, maybe it was always dead code, dunno), and decided to "fix" it by changing it to # set the series flag my $serial = 0; if ( $biblio->{'seriestitle'} ) { $serial = 1 }; which turned it into actively wrong code, overriding someone actively setting serial with whether or not it had a series title. A few years later, someone at least let setting something mapped to biblio.serial cause biblio.serial to be set, while leaving the bogus setting of it based on seriestitle still there if serial wasn't already set. The added comment "# set the series flag" makes it clear that it wasn't done out of any belief that seriestitle implies serial, only thinking that serial was how you spell "has a series." As bug 34128 will tell you, it isn't. And since CheckPrevCheckout uses biblio.serial to only warn people if they have previously checked out a particular copy of People Weekly rather than warning them that they checked out last week's issue so they don't want this week's issue, had it noticed that as a result of seriestitle -> serial it doesn't warn people who've checked out copy 1 of the 62nd Stone Barrington novel that they probably don't want to check out copy 2, CheckPrevCheckout would also tell you that it isn't. For comic relief, AddBiblio only sets biblio.serial the first time that you save a biblio which has a seriestitle. If you edit the biblio again, ModBiblio unsets it, so it's both wrong and incapable of making its wrongness stick.
Created attachment 152921 [details] [review] Bug 34182 - AddBiblio shouldn't set biblio.serial based on biblio.seriestitle A misunderstanding of the intention of some dead code that probably wanted to set biblio.series (which doesn't exist) left us setting biblio.serial if biblio.seriestitle was set. The only thing series and serial have in common is the first four letters. We shouldn't set serial on something with a series (unless someone also sets serial on it, of course). Test plan: 1. Administration - MARC bibliographic framework - Actions button next to Default framework - MARC structure 2. In the Search for tag input type 942 and click search 3. Actions button next to 942 - Edit subfields 4. Tab s - check the checkbox for Editor, uncheck the checkbox for Collapsed - Save changes 5. Cataloging - New record 6. Click in the input for 000 and hold down Tab until you get past 008 to fill in mandatory default values, then type any character in 040 subfield c 7. Tab 2 - In 245 subfield a type Series not serial 8. Tab 4 - In 490 subfield a type any character 9. Tab 9 - Set the value of subfield c to Books 10. Click save and leave the tab open to keep the biblionumber 11. Cataloging - New record - repeat step 6 12. Tab 2 - In 245 subfield a type Serial not series 13. Tab 9 - Set the value of subfield c to Books - Type 1 in subfield s 14. Click save, the biblionumber should be one higher than the first one 15. Reports - Create from SQL 16. Type something in Report name, paste in the SQL SELECT biblio.serial, biblio.seriestitle, biblio.title FROM biblio WHERE biblionumber IN ("","") and put your first biblionumber in the first "", your second in the second. 17. Save report - Run report 18. Series not serial should have a blank in the serial column and the character you typed in the seriestitle column; Serial not series should have a 1 in the serial column and a blank in the seriestitle column.
Created attachment 152923 [details] [review] Bug 34182 - AddBiblio shouldn't set biblio.serial based on biblio.seriestitle A misunderstanding of the intention of some dead code that probably wanted to set biblio.series (which doesn't exist) left us setting biblio.serial if biblio.seriestitle was set. The only thing series and serial have in common is the first four letters. We shouldn't set serial on something with a series (unless someone also sets serial on it, of course). Test plan: 1. Administration - MARC bibliographic framework - Actions button next to Default framework - MARC structure 2. In the Search for tag input type 942 and click search 3. Actions button next to 942 - Edit subfields 4. Tab s - check the checkbox for Editor, uncheck the checkbox for Collapsed - Save changes 5. Cataloging - New record 6. Click in the input for 000 and hold down Tab until you get past 008 to fill in mandatory default values, then type any character in 040 subfield c 7. Tab 2 - In 245 subfield a type Series not serial 8. Tab 4 - In 490 subfield a type any character 9. Tab 9 - Set the value of subfield c to Books 10. Click save and leave the tab open to keep the biblionumber 11. Cataloging - New record - repeat step 6 12. Tab 2 - In 245 subfield a type Serial not series 13. Tab 9 - Set the value of subfield c to Books - Type 1 in subfield s 14. Click save, the biblionumber should be one higher than the first one 15. Reports - Create from SQL 16. Type something in Report name, paste in the SQL SELECT biblio.serial, biblio.seriestitle, biblio.title FROM biblio WHERE biblionumber IN ("","") and put your first biblionumber in the first "", your second in the second. 17. Save report - Run report 18. Series not serial should have a blank in the serial column and the character you typed in the seriestitle column; Serial not series should have a 1 in the serial column and a blank in the seriestitle column. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks so much for the digging here Phil, great bit of coding forensics there. This makes eminent sense now giveny our explainations and the challenges we've been having over on bug 34128. Signing off.
Thanks from me too, Phil! I had a hunch something was wrong, but you did the hard part of proving and fixing!
Created attachment 153225 [details] [review] Bug 34182: Don't set biblio.serial based on biblio.seriestitle in AddBiblio A misunderstanding of the intention of some dead code that probably wanted to set biblio.series (which doesn't exist) left us setting biblio.serial if biblio.seriestitle was set. The only thing series and serial have in common is the first four letters. We shouldn't set serial on something with a series (unless someone also sets serial on it, of course). Test plan: 1. Administration - MARC bibliographic framework - Actions button next to Default framework - MARC structure 2. In the Search for tag input type 942 and click search 3. Actions button next to 942 - Edit subfields 4. Tab s - check the checkbox for Editor, uncheck the checkbox for Collapsed - Save changes 5. Cataloging - New record 6. Click in the input for 000 and hold down Tab until you get past 008 to fill in mandatory default values, then type any character in 040 subfield c 7. Tab 2 - In 245 subfield a type Series not serial 8. Tab 4 - In 490 subfield a type any character 9. Tab 9 - Set the value of subfield c to Books 10. Click save and leave the tab open to keep the biblionumber 11. Cataloging - New record - repeat step 6 12. Tab 2 - In 245 subfield a type Serial not series 13. Tab 9 - Set the value of subfield c to Books - Type 1 in subfield s 14. Click save, the biblionumber should be one higher than the first one 15. Reports - Create from SQL 16. Type something in Report name, paste in the SQL SELECT biblio.serial, biblio.seriestitle, biblio.title FROM biblio WHERE biblionumber IN ("","") and put your first biblionumber in the first "", your second in the second. 17. Save report - Run report 18. Series not serial should have a blank in the serial column and the character you typed in the seriestitle column; Serial not series should have a 1 in the serial column and a blank in the seriestitle column. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
It would be nice if we could fix data with wrongly set info now somehow, but we might as well do that on a different bug and get this in ASAP. Maybe one of the existing scripts (touch.../rebuild...) could do the trick even?
Pushed to master for 23.11. Nice work everyone, thanks!
Thanks for all the hard work! Pushed to 23.05.x for the next release
Nice work everyone! Pushed to oldstable for 22.11.x
Backported to 22.05.x for upcoming 22.05.16