Bug 26516 - Importing records with unexpected format of copyrightdate fails
Summary: Importing records with unexpected format of copyrightdate fails
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 11046
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-23 14:40 UTC by Nick Clemens
Modified: 2020-12-10 13:14 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07, 19.11.13


Attachments
Bug 26516: Check if int before save (1.21 KB, patch)
2020-09-23 14:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear (2.78 KB, patch)
2020-09-23 14:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear (2.83 KB, patch)
2020-11-03 10:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear (2.89 KB, patch)
2020-11-16 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-09-23 14:40:18 UTC
Bug 11046 attempted to improve the parsing of publicationyear/copyrightdate from marc records.

Unfortunately copyrightdate is strictly an integer field, so now adds/imports fail if invalid data is passed

To recreate:
1 - Add a new record to Koha making sure data is valid except 260$c:
    198-
2 - Save the record
3 - It fails

DBD::mysql::st execute failed: Data truncated for column 'copyrightdate' at row 1 [for Statement "INSERT INTO biblio
        SET frameworkcode = ?,
            author = ?,
            title = ?,
            subtitle = ?,
            medium = ?,
            part_number = ?,
            part_name = ?,
            unititle =?,
            notes = ?,
            serial = ?,
            seriestitle = ?,
            copyrightdate = ?,
            datecreated=NOW(),
            abstract = ?
        " with ParamValues: 0='', 1=undef, 2="This record", 3=undef, 4=undef, 5=undef, 6=undef, 7=undef, 8=undef, 9=0, 10=undef, 11="198-", 12=undef] at /kohadevbox/koha/C4/Biblio.pm line 2745.
ERROR in _koha_add_biblio INSERT INTO biblio
Comment 1 Nick Clemens 2020-09-23 14:42:17 UTC
Created attachment 110602 [details] [review]
Bug 26516: Check if int before save

This is how we handle it when modifying a biblio

To test:
To recreate:
1 - Add a new record to Koha making sure data is valid except 260$c:
    198-
2 - Save the record
3 - It fails
4 - Apply patch
5 - restart all the things
6 - Repeat
7 - Success!
Comment 2 Nick Clemens 2020-09-23 14:45:55 UTC
Created attachment 110605 [details] [review]
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear

The previous patch rejects incorrect values when saving to the db specifically for copyrightdate
An error is thrown for the int conversion when it fails

Rather than catching things when saving we can fix the value when generated, simply returning null when parsing
the record

To test:
1 - Add a new record to Koha making sure data is valid except 260$c:
    198-
2 - Save the record
3 - It fails
4 - Apply this patch
5 - Restart all the thigns
6 - Repeat
7 - Success!
Comment 3 Lisette Scheer 2020-09-24 16:29:11 UTC
Before applying the patch, when adding a record with 198- in the 260$c: field, we got the following error on the sandbox:

Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88
in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm line 88

We were not able to import a record with 198- in the 260$c: field before applying the patch. 

After the patch was applied we still got the same error when trying to add a new record with 198- in the 260$c: field. We were able to export the record, edit it to have 198- and import the record successfully. 


Owen wasn't able to recreate the error below on his test server. 
 
Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88
in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm line 88

Lisette
Comment 4 David Nind 2020-11-03 10:27:26 UTC
Created attachment 112905 [details] [review]
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear

The previous patch rejects incorrect values when saving to the db specifically for copyrightdate
An error is thrown for the int conversion when it fails

Rather than catching things when saving we can fix the value when generated, simply returning null when parsing
the record

To test:
1 - Add a new record to Koha making sure data is valid except 260$c:
    198-
2 - Save the record
3 - It fails
4 - Apply this patch
5 - Restart all the thigns
6 - Repeat
7 - Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2020-11-03 10:35:40 UTC
(In reply to Lisette Scheer from comment #3)
> Before applying the patch, when adding a record with 198- in the 260$c:
> field, we got the following error on the sandbox:
> 
> Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88
> in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm
> line 88

I got this error as well locally using koha-testing-docker. For me the error mentioned in the description appeared in /var/log/koha/kohadev/plack-intranet-error.log (not sure whether this will be the same when using kohadevbox).

I imported a record using Z39.50 from the Library of Congress (title = IBM Style Guide), and changed the 260$c to 198- 

After applying the patch I went flush_memcached and restart_all, redid the import and everything worked as per the test plan - successfully able to add a record with 198- for $260$c.
Comment 6 Martin Renvoize 2020-11-16 09:57:52 UTC
Created attachment 113655 [details] [review]
Bug 26516: [alternate] Don't accept incorrect values for copyrightdate/publicationyear

The previous patch rejects incorrect values when saving to the db specifically for copyrightdate
An error is thrown for the int conversion when it fails

Rather than catching things when saving we can fix the value when generated, simply returning null when parsing
the record

To test:
1 - Add a new record to Koha making sure data is valid except 260$c:
    198-
2 - Save the record
3 - It fails
4 - Apply this patch
5 - Restart all the thigns
6 - Repeat
7 - Success!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2020-11-16 10:01:53 UTC
A simple elegant fix.. I did wonder if we aught to use the same form as we do for the ISBD rules and simply replace the '-' with '0'.. but I'm now really sure that would benefit us in any meaningful way here.


Passing QA
Comment 8 Jonathan Druart 2020-11-17 11:26:07 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Andrew Fuerste-Henry 2020-12-09 20:13:10 UTC
Pushed to 20.05.x for 20.05.07
Comment 10 Victor Grousset/tuxayo 2020-12-10 13:14:07 UTC
Backported to 19.11.x branch for 19.11.13