Bug 34444

Summary: Statistic 1/2 not saving when updating fund after receipt
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: AcquisitionsAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to oldoldstable --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: major    
Priority: P5 - low CC: dch, fridolin.somers, lucas, michaela.sieber, nick, victor, wainuiwitikapark
Version: MainKeywords: rel_22_11_candidate, rel_23_05_candidate, rel_23_11_candidate
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.02,23.11.07,23.05.15
Circulation function:
Bug Depends on: 25763    
Bug Blocks:    
Attachments: Bug 34444: [22.11.x] Correct handling of sort1 and sort2 values
Bug 34444: Correct handling of sort1 and sort2 values
Bug 34444: Correct handling of sort1 and sort2 values
Bug 34444: Correct handling of sort1 and sort2 values

Description Katrin Fischer 2023-07-30 11:53:46 UTC
OK in master, not ok in 22.11, untested in 23.05.

When the fund is changed after receipt, the statistic values are also updated and show the correct funds, but selected values are not saved. If you change the fund again you can see that pull-downs jump back to the first entry instead of showing the previously selected values.

To test:

Setup:
* Make sure you have at least 2 funds with different stat settings, using AV and not
* Create a basket with an order line
* Close it and receive shipment
* Create an invoice and receive the order line
* Finish receiving

* Click "Modify fund"
* Switch fund, verify the stat fields are updated accordingly
* Change values for statistical values
* Update fund
* Edit fund again 
  = pull downs will jump back to first entry instead of showing the previously
    picked value

free text input seems to work ok at first glance.
Comment 1 Katrin Fischer 2023-10-29 23:32:57 UTC
22.11 is the LTS... it would be great to get some help here.
Comment 2 Nick Clemens (kidclamp) 2024-05-08 13:26:19 UTC
Created attachment 166417 [details] [review]
Bug 34444: [22.11.x] Correct handling of sort1 and sort2 values

Removed extraneous sort_1 data elements
Update selectors to use field names for statistics field
Updated code to set the value after finding the correct selector

To test:
* Make sure you have at least 2 funds with different stat settings, using AV and not
* Create a basket with an order line
* Close it and receive shipment
* Create an invoice and receive the order line
* Finish receiving

* Click "Modify fund"
* Switch fund, verify the stat fields are updated accordingly
* Change values for statistical values
* Update fund
* Edit fund again, pull downs are correct
* Change values in form and close, do not update
* Click 'Modify fund' - confrim form is filled with the saved values
Comment 3 Nick Clemens (kidclamp) 2024-05-08 13:54:00 UTC
Created attachment 166419 [details] [review]
Bug 34444: Correct handling of sort1 and sort2 values

Removed extraneous sort_1 data elements
Update selectors to use field names for statistics field
Updated code to set the value after finding the correct selector

To test:
* Make sure you have at least 2 funds with different stat settings, using AV and not
* Create a basket with an order line
* Close it and receive shipment
* Create an invoice and receive the order line
* Finish receiving

* Click "Modify fund"
* Switch fund, verify the stat fields are updated accordingly
* Change values for statistical values
* Update fund
* Edit fund again, pull downs are correct
* Change values in form and close, do not update
* Click 'Modify fund' - confrim form is filled with the saved values
Comment 4 Nick Clemens (kidclamp) 2024-05-08 13:55:01 UTC
I recreated on master - the same patch applied and works so reattached here - there is more cleanup that can be done, but this should fix the issue
Comment 5 Chris Cormack 2024-05-10 00:02:45 UTC
Created attachment 166481 [details] [review]
Bug 34444: Correct handling of sort1 and sort2 values

Removed extraneous sort_1 data elements
Update selectors to use field names for statistics field
Updated code to set the value after finding the correct selector

To test:
* Make sure you have at least 2 funds with different stat settings, using AV and not
* Create a basket with an order line
* Close it and receive shipment
* Create an invoice and receive the order line
* Finish receiving

* Click "Modify fund"
* Switch fund, verify the stat fields are updated accordingly
* Change values for statistical values
* Update fund
* Edit fund again, pull downs are correct
* Change values in form and close, do not update
* Click 'Modify fund' - confrim form is filled with the saved values

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 6 Katrin Fischer 2024-05-10 07:29:55 UTC
Hm, I see there was a patch for 22.11 and the current patch doesn't apply on 22.11. I am restoring it for some testing with our 22.11 version.
Comment 7 Katrin Fischer 2024-05-10 08:47:40 UTC
Hm, still doesn't apply on 22.11 - Nick, could you have a look?
Comment 8 Nick Clemens (kidclamp) 2024-05-10 15:21:30 UTC
(In reply to Katrin Fischer from comment #7)
> Hm, still doesn't apply on 22.11 - Nick, could you have a look?

Both apply clean to 22.11.x for me
Comment 9 Katrin Fischer 2024-05-10 15:23:16 UTC
(In reply to Nick Clemens (kidclamp) from comment #8)
> (In reply to Katrin Fischer from comment #7)
> > Hm, still doesn't apply on 22.11 - Nick, could you have a look?
> 
> Both apply clean to 22.11.x for me

I might need to take a closer look and maybe rebase our branch. Sorry for the noise and thanks!
Comment 10 Victor Grousset/tuxayo 2024-05-28 18:48:25 UTC
Just to be sure, is it expected that when
- switching from a fund with AV stats
- to one with free text stat
- then the free field contains the raw AV values

This fix is not just about when
- going back and forth between funds with AV stats and free values
- going back to the AV one
- then it will remember a changed AV not saved yet
Comment 11 Martin Renvoize (ashimema) 2024-06-05 15:13:09 UTC
Created attachment 167451 [details] [review]
Bug 34444: Correct handling of sort1 and sort2 values

Removed extraneous sort_1 data elements
Update selectors to use field names for statistics field
Updated code to set the value after finding the correct selector

To test:
* Make sure you have at least 2 funds with different stat settings, using AV and not
* Create a basket with an order line
* Close it and receive shipment
* Create an invoice and receive the order line
* Finish receiving

* Click "Modify fund"
* Switch fund, verify the stat fields are updated accordingly
* Change values for statistical values
* Update fund
* Edit fund again, pull downs are correct
* Change values in form and close, do not update
* Click 'Modify fund' - confrim form is filled with the saved values

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2024-06-06 14:16:58 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 13 Lucas Gass (lukeg) 2024-06-27 13:39:48 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 14 Fridolin Somers 2024-07-17 13:11:05 UTC
Pushed to 23.11.x for 23.11.07
Comment 15 Wainui Witika-Park 2024-07-19 03:12:52 UTC
The test plan is not working for me on 23.05.x - if required for 23.05.x please provide a specific patch :)
Comment 16 Katrin Fischer 2024-07-19 07:39:19 UTC
Maybe the 22.11 patch works for 23.05 as well? As far as we know it was broken all the way down, I can confirm the bug for 22.11.
Comment 17 Wainui Witika-Park 2024-07-21 03:49:06 UTC
(In reply to Katrin Fischer from comment #16)
> Maybe the 22.11 patch works for 23.05 as well? As far as we know it was
> broken all the way down, I can confirm the bug for 22.11.

Thanks Katrin, I will try that next month (string freeze already announced for this month) :)
Comment 18 Victor Grousset/tuxayo 2024-07-22 18:36:15 UTC
There are no string changes on this one so string freeze isn't an issue. (though it might still be too late to have the time to test and wait for CI so no worries)
Comment 19 Wainui Witika-Park 2024-09-16 01:59:57 UTC
Would someone please be able to provide a more detailed test plan for this for 23.05?
Comment 20 Katrin Fischer 2024-09-16 06:55:57 UTC
Hi Wainui, the test plan is:

Setup:
* Go to administration budgets and make sure you have an active budget added
* Create at least 2 funds, with different stat settings:
  Fund 1: Stat 1 = Any AV from the list, Stat 2 = none
  Fund 2: Stat 1 = none, Stat 2 = Any AV but the one from Fund 1
* Create a basket with an order line, it doesn't matter how it's created
* Close the basket and receive shipment for this vendor
* Add the invoice details and receive the order line
* Click Finish receiving at the bottom of the page

Finally on the right page: Invoice summary
* Click "Modify fund" link in the table of received order lines
* Switch fund, verify the stat fields are updated accordingly
* Change the values for the statistical values
* Save the changes with "Update fund"
* Edit fund again 
  = pull downs will jump back to first entry instead of showing the previously
    picked value. Changes haven't been saved.

I have added some steps and explanations, please let me know if you get stuck at a certain step.
Comment 21 Wainui Witika-Park 2024-09-17 01:35:54 UTC
(In reply to Katrin Fischer from comment #20)

> I have added some steps and explanations, please let me know if you get
> stuck at a certain step.

Thanks Katrin!!

Patch applied to 23.05.x for 23.05.15