Bug 37316 - Cannot add items to basket via file if barcodes not supplied
Summary: Cannot add items to basket via file if barcodes not supplied
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Lucas Gass
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 33171
Blocks:
  Show dependency treegraph
 
Reported: 2024-07-10 20:13 UTC by Nick Clemens (kidclamp)
Modified: 2024-09-05 14:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an error when using order files from a vendor to stage and add records with items to a basket. If barcodes are not specified for the items (either from the file or manually), this caused an error when saving. Order files without barcodes can now be used.
Version(s) released in:
24.11.00,24.05.02
Circulation function:


Attachments
Sample order file (13.70 KB, text/xml)
2024-07-10 20:21 UTC, Nick Clemens (kidclamp)
Details
Bug 37316: Add tests (1.40 KB, patch)
2024-07-11 17:24 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 37316: Treat empty string barcodes as undef (1.59 KB, patch)
2024-07-11 17:24 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 37316: Adjust tests (987 bytes, patch)
2024-07-11 17:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 37316: Adjust tests (1.01 KB, patch)
2024-07-11 17:54 UTC, Eric Garcia
Details | Diff | Splinter Review
Bug 37316: Add tests (1.46 KB, patch)
2024-07-12 14:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 37316: Treat empty string barcodes as undef (1.65 KB, patch)
2024-07-12 14:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 37316: Adjust tests (1.35 KB, patch)
2024-07-12 14:25 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 (kidclamp) 2024-07-10 20:13:28 UTC
Bug 33171 added the ability to supply barcodes in MarcItemFieldsToOrder

We didn't seem take into account that if no value is supplied, the default value from an html input is ""

When you try to add multiple items with an empty string as the barcode, Koha dies.


Duplicate ID at /usr/share/perl5/Exception/Class/Base.pm line 88
	Exception::Class::Base::throw('Koha::Exceptions::Object::DuplicateID', 'error', 'Duplicate ID', 'duplicate_id', 'itembarcodeidx') called at /kohadevbox/koha/Koha/Object.pm line 178
	Koha::Object::catch {...} ('DBIx::Class::Exception=HASH(0x558905ec0958)') called at /usr/share/perl5/Try/Tiny.pm line 123
	Try::Tiny::try('CODE(0x558905ebb9d0)', 'Try::Tiny::Catch=REF(0x558905f638f0)') called at /kohadevbox/koha/Koha/Object.pm line 209
Comment 1 Nick Clemens (kidclamp) 2024-07-10 20:21:38 UTC
Created attachment 168767 [details]
Sample order file

Below are the settigns that go with this file - for this bug remove the "barcode" from MarcItemFieldsToOrder


MarcItemFieldsToOrder

homebranch: 949$a
holdingbranch: 949$b
itype: 949$y
nonpublic_note: 949$x
public_note: 949$z
loc: 949$c
ccode: 949$8
notforloan: 949$7
uri: 949$u
copyno: 949$t
price: 949$g|949$j
replacementprice: 949$v
itemcallnumber: 949$o
quantity: 949$k
budget_code: 949$l
barcode: 949$p
coded_location_qualifier: 949$f
enumchron: 949$h


MarcFieldsToOrder

price: 949$g
quantity: 949$k
budget_code: 949$l
discount: 949$m
sort1: 949$n
sort2: 949$q
Comment 2 Lucas Gass 2024-07-10 21:17:35 UTC
I cannot recreate this in current main.
Comment 3 Lucas Gass 2024-07-11 17:24:13 UTC
Created attachment 168853 [details] [review]
Bug 37316: Add tests
Comment 4 Lucas Gass 2024-07-11 17:24:15 UTC
Created attachment 168854 [details] [review]
Bug 37316: Treat empty string barcodes as undef

To test:
1.Add the following to MarcItemFieldsToOrder:

homebranch: 949$a
holdingbranch: 949$b
itype: 949$y
nonpublic_note: 949$x
public_note: 949$z
loc: 949$c
ccode: 949$8
notforloan: 949$7
uri: 949$u
copyno: 949$t
price: 949$g|949$j
replacementprice: 949$v
itemcallnumber: 949$o
quantity: 949$k
budget_code: 949$l
coded_location_qualifier: 949$f
enumchron: 949$h

2. Add the following to MarcFieldsToOrder:

price: 949$g
quantity: 949$k
budget_code: 949$l
discount: 949$m
sort1: 949$n
sort2: 949$q

3. Acquisitions > Find a vendor
4. Create a new basket -> Add to basket -> From new file
5. Use the file uplodaded in this bug report.
6. Set format to MARCXML and stage for import.
7. Add staged files to basket
8. Select all, make sure you add an item type, and choose 'Do not look for matching records'
9. Save and kaboom.
10. APPLY PATCH and restart_all
11. Follow the steps again, this time no kaboom.
Comment 5 Lucas Gass 2024-07-11 17:38:28 UTC
Created attachment 168855 [details] [review]
Bug 37316: Adjust tests
Comment 6 Eric Garcia 2024-07-11 17:54:48 UTC
Created attachment 168858 [details] [review]
Bug 37316: Adjust tests

Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Comment 7 Martin Renvoize 2024-07-12 14:24:57 UTC
Created attachment 168917 [details] [review]
Bug 37316: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2024-07-12 14:24:59 UTC
Created attachment 168918 [details] [review]
Bug 37316: Treat empty string barcodes as undef

To test:
1.Add the following to MarcItemFieldsToOrder:

homebranch: 949$a
holdingbranch: 949$b
itype: 949$y
nonpublic_note: 949$x
public_note: 949$z
loc: 949$c
ccode: 949$8
notforloan: 949$7
uri: 949$u
copyno: 949$t
price: 949$g|949$j
replacementprice: 949$v
itemcallnumber: 949$o
quantity: 949$k
budget_code: 949$l
coded_location_qualifier: 949$f
enumchron: 949$h

2. Add the following to MarcFieldsToOrder:

price: 949$g
quantity: 949$k
budget_code: 949$l
discount: 949$m
sort1: 949$n
sort2: 949$q

3. Acquisitions > Find a vendor
4. Create a new basket -> Add to basket -> From new file
5. Use the file uplodaded in this bug report.
6. Set format to MARCXML and stage for import.
7. Add staged files to basket
8. Select all, make sure you add an item type, and choose 'Do not look for matching records'
9. Save and kaboom.
10. APPLY PATCH and restart_all
11. Follow the steps again, this time no kaboom.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2024-07-12 14:25:01 UTC
Created attachment 168919 [details] [review]
Bug 37316: Adjust tests

Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Katrin Fischer 2024-07-15 09:07:10 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 11 Lucas Gass 2024-07-17 21:32:22 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 12 Fridolin Somers 2024-07-19 07:29:33 UTC
Depends on Bug 33171 not in 23.11.x
Comment 13 David Nind 2024-07-26 10:09:04 UTC
I'm not sure I have got this release note right. Please update or clarify what this fixes if required.