Bug 37551 - MarcFieldsToOrder price is overriding MarcItemFieldsToOrderPrice
Summary: MarcFieldsToOrder price is overriding MarcItemFieldsToOrderPrice
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 24.05
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 33171
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-01 19:52 UTC by Nick Clemens (kidclamp)
Modified: 2024-10-21 12:48 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.04
Circulation function:


Attachments
Marc file for testing (946 bytes, application/marc)
2024-08-26 19:44 UTC, Lucas Gass (lukeg)
Details
Bug 37551: Only delete price when there is already an itemprice (1.68 KB, patch)
2024-08-26 20:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37551: (24.05.x) Only delete price when there is already an itemprice (1.69 KB, patch)
2024-08-26 20:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37551: (24.05.x) Only delete price when there is already an itemprice (1.76 KB, patch)
2024-09-17 14:19 UTC, Martin Renvoize (ashimema)
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-08-01 19:52:43 UTC
In the code:

# Price is handled as "itemprice" in the UI form to distinguish from MarcFieldsToOrder
$iteminfos->{itemprice}  = delete $iteminfos->{price}  if $iteminfos->{price};

and later

$iteminfos->{price} ||= $price;    # Fallback to order price if no item price was defined

Well, this means we will always fallback, because we deleted it.

itemprice is the variable name when processing the form, but for populating we use 'price'

I touched all of these lines, so I am blamed, but seems a clear mistake
Comment 1 Lucas Gass (lukeg) 2024-08-26 17:04:13 UTC
This seems to resolved in main by Bug 35026.
Comment 2 Lucas Gass (lukeg) 2024-08-26 19:44:16 UTC
Created attachment 170735 [details]
Marc file for testing
Comment 3 Lucas Gass (lukeg) 2024-08-26 20:10:23 UTC
To recreate:
1. Have a mrc file with some different prices. In my case I am using the mrc file attached here with different price values in the 975$p and 975$a
2. Set the MarcFieldsToOrder to "price: 975$p" and  MarcItemFieldsToOrder to "price: 975$a"
3. In Acquisitions create a new basket > add to basket > from a new file 
4. The value from MarcFieldsToOrder (975$p) is always set as the price when it should be the value from MarcItemFieldsToOrder (975$a)
Comment 4 Lucas Gass (lukeg) 2024-08-26 20:15:21 UTC
Created attachment 170736 [details] [review]
Bug 37551: Only delete price when there is already an itemprice

To test:
1. Have a mrc file with some different prices. In my case I am using the mrc file attached here with different price values in the 975$p and 975$a
2. Set the MarcFieldsToOrder to "price: 975$p" and  MarcItemFieldsToOrder to "price: 975$a"
3. In Acquisitions create a new basket > add to basket > from a new file
4. The value from MarcFieldsToOrder (975$p) is always set as the price when it should be the value from MarcItemFieldsToOrder (975$a)
5. APPLY PATCH, restart_all
6. Try steps 1 - 3 again, this time item price should be correctly handled.
Comment 5 Lucas Gass (lukeg) 2024-08-26 20:15:57 UTC
Created attachment 170737 [details] [review]
Bug 37551: (24.05.x) Only delete price when there is already an itemprice

To test:
1. Have a mrc file with some different prices. In my case I am using the mrc file attached here with different price values in the 975$p and 975$a
2. Set the MarcFieldsToOrder to "price: 975$p" and  MarcItemFieldsToOrder to "price: 975$a"
3. In Acquisitions create a new basket > add to basket > from a new file
4. The value from MarcFieldsToOrder (975$p) is always set as the price when it should be the value from MarcItemFieldsToOrder (975$a)
5. APPLY PATCH, restart_all
6. Try steps 1 - 3 again, this time item price should be correctly handled.
Comment 6 Lucas Gass (lukeg) 2024-08-26 20:16:57 UTC
While Bug 35026 I don't think we'll want to backport all of that to 24.05? So I made a patch for 24.05.x
Comment 7 Donna 2024-09-06 19:58:55 UTC
Tested, works as expected.  Signing off.
Comment 8 Martin Renvoize (ashimema) 2024-09-17 14:19:09 UTC
Created attachment 171627 [details] [review]
Bug 37551: (24.05.x) Only delete price when there is already an itemprice

To test:
1. Have a mrc file with some different prices. In my case I am using the mrc file attached here with different price values in the 975$p and 975$a
2. Set the MarcFieldsToOrder to "price: 975$p" and  MarcItemFieldsToOrder to "price: 975$a"
3. In Acquisitions create a new basket > add to basket > from a new file
4. The value from MarcFieldsToOrder (975$p) is always set as the price when it should be the value from MarcItemFieldsToOrder (975$a)
5. APPLY PATCH, restart_all
6. Try steps 1 - 3 again, this time item price should be correctly handled.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Lucas Gass (lukeg) 2024-09-17 14:51:32 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 10 Fridolin Somers 2024-10-21 12:48:36 UTC
Depends on Bug 33171 not in 23.11.x