Bug 20623 - PDF export of a basket group fails when an item has an itemtype that is not in the itemtype table
Summary: PDF export of a basket group fails when an item has an itemtype that is not i...
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Victor Grousset/tuxayo
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-19 17:36 UTC by Victor Grousset/tuxayo
Modified: 2018-05-11 17:13 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20623 : Fix basket group PDF when itemtype not itemtype table (3.41 KB, patch)
2018-04-20 09:27 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20623 : Fix basket group PDF when itemtype not itemtype table (2.88 KB, patch)
2018-04-20 09:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20623 : Fix basket group PDF when itemtype not itemtype table (3.30 KB, patch)
2018-04-27 15:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20623: Fix basket group PDF when itemtype not itemtype table (3.35 KB, patch)
2018-05-10 15:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20623: Fix basket group PDF when itemtype not itemtype table (3.45 KB, patch)
2018-05-11 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20623: (QA follow-up) Remove one find call (1.49 KB, patch)
2018-05-11 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20623: (QA follow-up) avoid fetch() calls in some cases (1.16 KB, patch)
2018-05-11 15:47 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2018-04-19 17:36:43 UTC
Does it matter?
Yes, using authorized values like ccode to populate the itemtypes of the biblioitems (instead of the itemtype table) can lead to such data.
Or importing records with invalid itemtype codes. Koha doesn't do enough checks to at least warn about these issues.

== Reproduce ==
1. have/create a budget
2. have/create a fund
3. have/create a vendor with minimal info
4. create a basket with minimal info
5. add a item to the basket
6. go to the basket. URL should be
   /cgi-bin/koha/acqui/basket.pl?basketno=XXXXX
7. close this basket
8. create a basket group with the basket
   - your vendor page => Basket groups
   - create basket group
   - (tick the "close this basket group" check box)
9. go to the basket group
      your vendor page => Basket groups => Closed
10. export as PDF, it should work, keep the page opened
11. find the itemtype code of item in the basket
12. delete it with the following SQL[1] (directly or use sql reports)
      replace with the relevant type code
      DELETE from itemtypes where itemtype = "BOOK";
13. reexport the basket as PDF
14. It should fail (internal server error)

[1] Or you can find a more realistic way to have a biblioitem whose itemtype is not in the itemtype table
Comment 1 Victor Grousset/tuxayo 2018-04-20 09:27:34 UTC Comment hidden (obsolete)
Comment 2 Victor Grousset/tuxayo 2018-04-20 09:40:03 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2018-04-20 14:56:36 UTC
This is due to migration issue, see this thread: http://koha.1045719.n5.nabble.com/Expected-behaviour-if-itemtype-does-not-exist-td5939989.html

It would be better to add a warning on the about page about data inconsistency.
Comment 4 Victor Grousset/tuxayo 2018-04-20 16:13:59 UTC
TODO: find a realistic test plan.

After discussing with Jonathan on IRC, I need to provide a test plan that shows that it's not only a migration issue.
Comment 5 Victor Grousset/tuxayo 2018-04-27 15:50:22 UTC
Created attachment 74913 [details] [review]
Bug 20623 : Fix basket group PDF when itemtype not itemtype table

When an item has an itemtype not in the itemtype table. Trying to fetch
it's description lead to an error.
Using authorized values like ccode to populate the itemtypes of the
biblioitems (instead of the itemtype table) can lead to such data.
Or importing records with invalid itemtype codes. Koha doesn't do enough
checks at import to at least warn about these issues.

== Test plan ==
1. first we need an item with an itype not in the item_types table
   1. download a record as MARCXML
   2. find it's item type in administration; and the related code
   3. open the MARCXML file and search for occurences of the code
        and replace them by some non-existing value like "FOOBAR"
   4. also change the barcode so it won't be ignored because
        it's a duplicate
   5. also change the title to easily find it later in the search
   6. tools → "Stage MARC records for import"
   7. upload your file
   8. "stage for import"
   9. click "Manage staged records"
        You should end on the page related to your staged record
   10. "Import this batch into the catalog"

2. now we need it in a basket group
   1. have/create a active budget
   2. have/create a fund
   3. have/create a vendor with minimal info
   4. create a basket with minimal info
   5. add our imported item to the basket
        for example search it by name
   6. go to the basket. URL should be
      /cgi-bin/koha/acqui/basket.pl?basketno=XXXXX
   7. close this basket
        and tick "Attach this basket to a new basket group with the same name"
   8. you will end up in the basket groups lists
   9. go to the "closed" tab
   11. go to the basket group
          your vendor page => Basket groups => Closed

3. export as PDF, it should fail (internal server error)
     this is the bug (no kidding ^_^)
4. apply this patch
5. reexport the basket as PDF
6. it should work
7. create an item type (in administration)
     that has the same code as the one that you put in the MARCXML
8. reexport the basket as PDF
9. check that in the PDF that the description is here:
     table at the bottom of the document → "Document" column
Comment 6 Victor Grousset/tuxayo 2018-04-27 16:01:27 UTC
oleonard:
> Catalogers here do some cataloging outside of Koha and then import records.
> If they have made a typo when entering the item type or collection code
> an error could get in that way.

> [...]

> The catalogers stage and import files of MARC records
Comment 7 Owen Leonard 2018-05-10 15:15:29 UTC
Created attachment 75225 [details] [review]
Bug 20623: Fix basket group PDF when itemtype not itemtype table

When an item has an itemtype not in the itemtype table. Trying to fetch
it's description lead to an error.
Using authorized values like ccode to populate the itemtypes of the
biblioitems (instead of the itemtype table) can lead to such data.
Or importing records with invalid itemtype codes. Koha doesn't do enough
checks at import to at least warn about these issues.

== Test plan ==
1. first we need an item with an itype not in the item_types table
   1. download a record as MARCXML
   2. find it's item type in administration; and the related code
   3. open the MARCXML file and search for occurences of the code
        and replace them by some non-existing value like "FOOBAR"
   4. also change the barcode so it won't be ignored because
        it's a duplicate
   5. also change the title to easily find it later in the search
   6. tools → "Stage MARC records for import"
   7. upload your file
   8. "stage for import"
   9. click "Manage staged records"
        You should end on the page related to your staged record
   10. "Import this batch into the catalog"

2. now we need it in a basket group
   1. have/create a active budget
   2. have/create a fund
   3. have/create a vendor with minimal info
   4. create a basket with minimal info
   5. add our imported item to the basket
        for example search it by name
   6. go to the basket. URL should be
      /cgi-bin/koha/acqui/basket.pl?basketno=XXXXX
   7. close this basket
        and tick "Attach this basket to a new basket group with the same name"
   8. you will end up in the basket groups lists
   9. go to the "closed" tab
   11. go to the basket group
          your vendor page => Basket groups => Closed

3. export as PDF, it should fail (internal server error)
     this is the bug (no kidding ^_^)
4. apply this patch
5. reexport the basket as PDF
6. it should work
7. create an item type (in administration)
     that has the same code as the one that you put in the MARCXML
8. reexport the basket as PDF
9. check that in the PDF that the description is here:
     table at the bottom of the document → "Document" column

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Marcel de Rooy 2018-05-11 09:39:29 UTC
Created attachment 75263 [details] [review]
Bug 20623: Fix basket group PDF when itemtype not itemtype table

When an item has an itemtype not in the itemtype table. Trying to fetch
it's description lead to an error.
Using authorized values like ccode to populate the itemtypes of the
biblioitems (instead of the itemtype table) can lead to such data.
Or importing records with invalid itemtype codes. Koha doesn't do enough
checks at import to at least warn about these issues.

== Test plan ==
1. first we need an item with an itype not in the item_types table
   1. download a record as MARCXML
   2. find it's item type in administration; and the related code
   3. open the MARCXML file and search for occurences of the code
        and replace them by some non-existing value like "FOOBAR"
   4. also change the barcode so it won't be ignored because
        it's a duplicate
   5. also change the title to easily find it later in the search
   6. tools → "Stage MARC records for import"
   7. upload your file
   8. "stage for import"
   9. click "Manage staged records"
        You should end on the page related to your staged record
   10. "Import this batch into the catalog"

2. now we need it in a basket group
   1. have/create a active budget
   2. have/create a fund
   3. have/create a vendor with minimal info
   4. create a basket with minimal info
   5. add our imported item to the basket
        for example search it by name
   6. go to the basket. URL should be
      /cgi-bin/koha/acqui/basket.pl?basketno=XXXXX
   7. close this basket
        and tick "Attach this basket to a new basket group with the same name"
   8. you will end up in the basket groups lists
   9. go to the "closed" tab
   11. go to the basket group
          your vendor page => Basket groups => Closed

3. export as PDF, it should fail (internal server error)
     this is the bug (no kidding ^_^)
4. apply this patch
5. reexport the basket as PDF
6. it should work
7. create an item type (in administration)
     that has the same code as the one that you put in the MARCXML
8. reexport the basket as PDF
9. check that in the PDF that the description is here:
     table at the bottom of the document → "Document" column

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2018-05-11 09:39:34 UTC
Created attachment 75264 [details] [review]
Bug 20623: (QA follow-up) Remove one find call

Imo these somewhat weird lines ask for improvement, but I don't want to change
the exact conditions here. Just removing the need to call find twice.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2018-05-11 13:27:10 UTC
We could avoid the fetch in some cases.
Maybe more:
  my $itemtype = ( $ord->{itemtype} and $biblioitem->itemtype )
    ? Koha::ItemTypes->find( $biblioitem->itemtype )
    : undef;

  $ord->{itemtype} = $itemtype ? $itemtype->description : undef;
Comment 11 Jonathan Druart 2018-05-11 13:27:28 UTC
Please fix ASAP, I'd like to push it very soon.
Comment 12 Jonathan Druart 2018-05-11 14:14:48 UTC
I pushed these 2 patches by mistake.
Comment 13 Victor Grousset/tuxayo 2018-05-11 15:47:50 UTC
Created attachment 75273 [details] [review]
Bug 20623: (QA follow-up) avoid fetch() calls in some cases
Comment 14 Jonathan Druart 2018-05-11 17:12:43 UTC
Pushed to master for 18.05, thanks to everybody involved!