Bug 33783 - Populate actual cost with estimated cost if actual cost not set when receiving (bug 8179 follow-up)
Summary: Populate actual cost with estimated cost if actual cost not set when receivin...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 8179
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-19 10:44 UTC by Nick Clemens
Modified: 2023-06-08 17:20 UTC (History)
4 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:
23.05.00


Attachments
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set (2.37 KB, patch)
2023-05-19 10:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set (1.98 KB, patch)
2023-05-26 12:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set (2.03 KB, patch)
2023-05-26 13:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33783: Make sure the value is populated on load time (2.00 KB, patch)
2023-05-29 19:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set (2.11 KB, patch)
2023-05-30 12:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33783: Make sure the value is populated on load time (2.07 KB, patch)
2023-05-30 12:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33783: Only use ecost if unitprice has not been set on ordering (2.03 KB, patch)
2023-05-30 17:24 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-05-19 10:44:22 UTC

    
Comment 1 Nick Clemens 2023-05-19 10:47:06 UTC
Created attachment 151469 [details] [review]
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set

This restores behavior prior to 8179 to use the estimated cost when receiving if the
actual cost is not set

To test:
1 - Add some orders to a basket, do not set actual cost field
2 - Close basket, receive orders
3 - Note actual cost field is blank
4 - Cancel receipt
5 - Apply patch
6 - Receive again
7 - Note actual cost is populated
8 - Complete receipt and confirm actual cost correctly saved
Comment 2 Sam Lau 2023-05-19 20:32:07 UTC
Created a new basket and added various items to basket, giving each one a vendor price, however leaving actual cost blank. After adding to basket, I closed basket and received the orders. Actual cost field was blank on the receipt. Canceled the receipt and applied patch. Received orders again. This time when I was receiving I noticed that the actual cost was showing up while receiving, however after finishing and viewing the receipt, the actual cost was still left blank.
Comment 3 Nick Clemens 2023-05-24 16:00:14 UTC
The problem is that the form expects you to enter a value - if you don't we don't fire an event to update the table rows, which are considered the source of truth.

We need some way to check the form values when  'Save changes' is clicked
Comment 4 Nick Clemens 2023-05-26 12:52:18 UTC
Created attachment 151758 [details] [review]
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set

This restores behavior prior to 8179 to use the estimated cost when receiving if the
actual cost is not set.

We set the unitprice in the table row so that it will be used when editing and will be saved even if not adjusted

To test:
1 - Add some orders to a basket, do not set actual cost field
2 - Close basket, receive orders
3 - Note actual cost field is blank
4 - Cancel receipt
5 - Apply patch
6 - Receive again
7 - Note actual cost is populated
8 - Complete receipt and confirm actual cost correctly saved
Comment 5 Tomás Cohen Arazi 2023-05-26 13:41:46 UTC
Created attachment 151761 [details] [review]
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set

This restores behavior prior to 8179 to use the estimated cost when receiving if the
actual cost is not set.

We set the unitprice in the table row so that it will be used when editing and will be saved even if not adjusted

To test:
1 - Add some orders to a basket, do not set actual cost field
2 - Close basket, receive orders
3 - Note actual cost field is blank
4 - Cancel receipt
5 - Apply patch
6 - Receive again
7 - Note actual cost is populated
8 - Complete receipt and confirm actual cost correctly saved

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Katrin Fischer 2023-05-29 13:59:54 UTC
Applied patch, restart_all, but the actual cost remains 0 on receive, for both multi and single receive :(

Any hints?
Comment 7 Katrin Fischer 2023-05-29 14:16:09 UTC
Just noting: I tested the process in 20.11 and the actual cost is pre-populated there. So I can confirm this bug is a regression.
Comment 8 Tomás Cohen Arazi 2023-05-29 19:18:27 UTC
Created attachment 151789 [details] [review]
Bug 33783: Make sure the value is populated on load time

The original patch makes the form set the right values in the order row
(before storing it) but misses to load the right value on page load,
which is still a regression. This patch solves that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Jonathan Druart 2023-05-30 12:36:25 UTC
Created attachment 151794 [details] [review]
Bug 33783: (bug 8179 follow-up) Populate actual cost with ecost during receiving if not set

This restores behavior prior to 8179 to use the estimated cost when receiving if the
actual cost is not set.

We set the unitprice in the table row so that it will be used when editing and will be saved even if not adjusted

To test:
1 - Add some orders to a basket, do not set actual cost field
2 - Close basket, receive orders
3 - Note actual cost field is blank
4 - Cancel receipt
5 - Apply patch
6 - Receive again
7 - Note actual cost is populated
8 - Complete receipt and confirm actual cost correctly saved

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2023-05-30 12:36:28 UTC
Created attachment 151795 [details] [review]
Bug 33783: Make sure the value is populated on load time

The original patch makes the form set the right values in the order row
(before storing it) but misses to load the right value on page load,
which is still a regression. This patch solves that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2023-05-30 12:49:23 UTC
wait
Comment 12 Jonathan Druart 2023-05-30 12:56:07 UTC
416             [% IF (invoiceincgst) %]
417                 [% SET unitprice = order.unitprice_tax_included > 0 ? order.unitprice_tax_included : order.ecost_tax_included %]
418                 <input type="text" size="20" name="unitprice" id="unitprice" value="[% unitprice | $Price on_editing => 1 %]" /> <span class="hint">(tax inclusive)</span>
419             [% ELSE %]
420                 [% SET unitprice = order.unitprice_tax_included > 0 ? order.unitprice_tax_excluded : order.ecost_tax_excluded %]
421                 <input type="text" size="20" name="unitprice" id="unitprice" value="[% unitprice | $Price on_editing => 1 %]" /> <span class="hint">(tax exclusive)</span>
422             [% END %]
Comment 13 Jonathan Druart 2023-05-30 12:57:37 UTC
You need to get ecost only if unitprice has not been passed when ordering.
Comment 14 Tomás Cohen Arazi 2023-05-30 17:24:16 UTC
Created attachment 151817 [details] [review]
Bug 33783: Only use ecost if unitprice has not been set on ordering

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Jonathan Druart 2023-05-31 08:20:24 UTC
Pushed to master for 23.05.00
Comment 16 Matt Blenkinsop 2023-06-08 17:20:57 UTC
Dependencies missing - not backporting to 22.11.x