When one tries to edit circulation rule "When to charge" columns value is always set as "End of interval". This happens because column doesn't have data-code attribute which defines existing rules value. To reproduce: 1. Navigate to Circulation and fine rules page. 2. Find or create a rule with a "When to charge" value set as "Start of interval". 3. Edit rule. => Note that "When to charge" value is now "End of interval", while as the other values should be copied as they were.
Created attachment 154688 [details] [review] Bug 34585: Add data-code attribute to chargeperiod_charge_at selector When one tries to edit circulation rule "When to charge" columns value is always set as "End of interval". This patch adds data-code attribute to chargeperiod_charge_at selector to define existing rules value. To reproduce: 1. Navigate to Circulation and fine rules page. 2. Find or create a rule with a "When to charge" value set as "Start of interval". 3. Edit rule. => Note that "When to charge" value is now "End of interval", while as the other values should be copied as they were. 4. Apply this patch. 5. Edit rule again. => "When to charge" should now be copied correctly. Sponsored-by: Koha-Suomi Oy
This isn't working for me. When I edit a rule with the setting "Start of interval," the "edit" button correctly updates the dropdown to that value, but when I click the "Clear" button and then edit a rule with the setting "End of interval," the value doesn't get set correctly.
After applying the patch, value on "When to charge" column is copied as it is in the rule before editing. In my tests the patch works. I can't repeat the situation that Owen is describing. But, there are couple things I noticed. 1) On our current production Koha (23.05.01.003, custom build: KK-Koha 23.05.848) value on "When to charge" column is copied as it is when editing the rule. So, our library doesn't seem to have this problems Emmi is reporting in the first place. Maybe we already have some fix for this... 2) I think the one problem is with the Clear button and how it behaves. This happens in our production Koha and in sandbox tests before and after applying the patch. 1. Edit rule A, values are copied, but then decide to cancel, click Clear 2. Edit rule B, values might be copied, but then decide to cancel, click Clear 3. Edit rule C, not all values are not copied All in all, it seems that even after the first time, when one cancels by clicking Clear, it is most reliable to first refresh the page and and then start editing the same rule or the next rule. I feel silly writing this, but I thing Emmi's patch is partially fixing the error at least on test sandbox. But if one cancels the editing process, the Koha/page can't keep up and editing mode starts behaving weirdly, and can't copy data on rule row as it maybe should.
I was able to reproduce Owens problem on our 22.xx version. This also happens on current master, so it might be that National library indeed has some sort of fix for this. I also noticed that Owens problem happens also with rules "OPAC item level holds", "On shelf holds allowed", "Automatic renewal", "Hard due date", "Unit", "Days mode" and "Patron category". Basically with every dropdown selector except "Item type". This could also have something to do with problem Johanna described. But this should probably have its own bug.
Pressing "Cancel" button should set first select option as selected and it does. However previous selection is never cleared. So every time rule row is selected to be edited it's select options receive "selected" attribute which persists. When new value is selected, it's select options still have previous rules options as selected.
Created attachment 160370 [details] [review] Bug 34585: Fix behaviour of select dropdowns when editing circulation rules When chancing rule to edit via "Edit" button in circulation rules table, dropdown values aren't always changed correctly. Pressing "Edit" first time works, but after that previous rules values persist. Same happens when one tries to clear edit row, values aren't set to default values. This happens with every input with a dropdown in it. This mostly happanes because we set attribute "selected" to selected options, but never clear them which leads to values persisting. In columns "Patron category" and "Itemtype" value "All", or "*" as it is in code, doesn't match to any existing values from database and can't be changed via pressing "Edit" button. To reproduce: 1. Select a rule to edit and then select another rule to edit. 2. Repeat this couple of times and observe what values dropdowns fields display in edit row. => Selected values in dropdowns aren't always changed based on rule that you are currently editing. Instead after changing rule couple of times value doesn't change at all. 3. Press "Clear" button to return edit rows values back to default values. => Values in dropdowns aren't set back as default, instead they have same values as rule you edited before pressing "Clear". After reproducing problem, apply this patch and proceed to test "Clear" and "Edit" buttons. To test "Edit" button: 1. Again, select a rule to edit and then select a another rule to edit. 2. Observe what values dropdowns fields display in edit row. => Confirm that values are changed based on rule that you are currently editing and this behaviour doesn't change after selecting new rule to edit couple of times. To test "Clear" button: 1. Press "Clear" button. => Values in dropdowns should now be set as default values. Sponsored-by: Koha-Suomi Oy
Created attachment 160488 [details] [review] Bug 34585: Fix behaviour of select dropdowns when editing circulation rules When chancing rule to edit via "Edit" button in circulation rules table, dropdown values aren't always changed correctly. Pressing "Edit" first time works, but after that previous rules values persist. Same happens when one tries to clear edit row, values aren't set to default values. This happens with every input with a dropdown in it. This mostly happanes because we set attribute "selected" to selected options, but never clear them which leads to values persisting. In columns "Patron category" and "Itemtype" value "All", or "*" as it is in code, doesn't match to any existing values from database and can't be changed via pressing "Edit" button. To reproduce: 1. Select a rule to edit and then select another rule to edit. 2. Repeat this couple of times and observe what values dropdowns fields display in edit row. => Selected values in dropdowns aren't always changed based on rule that you are currently editing. Instead after changing rule couple of times value doesn't change at all. 3. Press "Clear" button to return edit rows values back to default values. => Values in dropdowns aren't set back as default, instead they have same values as rule you edited before pressing "Clear". After reproducing problem, apply this patch and proceed to test "Clear" and "Edit" buttons. To test "Edit" button: 1. Again, select a rule to edit and then select a another rule to edit. 2. Observe what values dropdowns fields display in edit row. => Confirm that values are changed based on rule that you are currently editing and this behaviour doesn't change after selecting new rule to edit couple of times. To test "Clear" button: 1. Press "Clear" button. => Values in dropdowns should now be set as default values. Sponsored-by: Koha-Suomi Oy Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
What's the status of 1st attachment? "Bug 34585: Add data-code attribute to chargeperiod_charge_at selector" Is it valid? Was is applied when signing off? It doesn't change the same area of the code so it's not sure it's an old version. And it has a different test plan.
CCing Lucas to have confirmation about what they signed off.
(In reply to Victor Grousset/tuxayo from comment #8) > What's the status of 1st attachment? > "Bug 34585: Add data-code attribute to chargeperiod_charge_at selector" > > Is it valid? > Was is applied when signing off? > It doesn't change the same area of the code so it's not sure it's an old > version. And it has a different test plan. Yes it's still valid. These are two separate patches fixing two different problems. First just adds data-code attribute to chargeperiod_charge_at selector so that its value is copied correctly. Second one fixes input fields with dropdowns which currently aren't always cleared as they should be.
Looking this again and I'm starting to think we need separate bug for the second patch. It's a totally different problem and it seems that patch doesn't fully fix it.
Tested again in current master, editing rules works(!), clearing them does not. Neither of these work in version 24.05.01. Filed bug 37254 and setting this as "Needs Signoff" to get more feedback.
Created attachment 170216 [details] [review] Bug 34585: Add data-code attribute to chargeperiod_charge_at selector When one tries to edit circulation rule "When to charge" columns value is always set as "End of interval". This patch adds data-code attribute to chargeperiod_charge_at selector to define existing rules value. To reproduce: 1. Navigate to Circulation and fine rules page. 2. Find or create a rule with a "When to charge" value set as "Start of interval". 3. Edit rule. => Note that "When to charge" value is now "End of interval", while as the other values should be copied as they were. 4. Apply this patch. 5. Edit rule again. => "When to charge" should now be copied correctly. Sponsored-by: Koha-Suomi Oy Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Created attachment 170234 [details] [review] Bug 34585: Add data-code attribute to chargeperiod_charge_at selector When one tries to edit circulation rule "When to charge" columns value is always set as "End of interval". This patch adds data-code attribute to chargeperiod_charge_at selector to define existing rules value. To reproduce: 1. Navigate to Circulation and fine rules page. 2. Find or create a rule with a "When to charge" value set as "Start of interval". 3. Edit rule. => Note that "When to charge" value is now "End of interval", while as the other values should be copied as they were. 4. Apply this patch. 5. Edit rule again. => "When to charge" should now be copied correctly. Sponsored-by: Koha-Suomi Oy Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Pushed for 24.11! Well done everyone, thank you!
Can this be ported to 24.05 as well?
Backported to 24.05.x for upcoming 24.05.05
Pushed to 23.11.x for 23.11.11