[Bug 36716] New: Need a better way of looping through smart-rules ( circ table ) columns
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Bug ID: 36716 Summary: Need a better way of looping through smart-rules ( circ table ) columns Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: lucas@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com When a user clicks "Edit" ( .editrule ) we use JavaScript to loop through each of the columns in the table. This is done based on the column index like so: if ( i == 3 ) or } else if ( i == 5 || i == 6 || i == 26 || i == 27 || i == 28 || current_input_id === "holds_pickup_period" ) { This leads to problems like Bug 36708. We could probably give the columns names/classes so we don't have to rely on the index, which may vary between Koha versions. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |emily.lamancusa@montgomeryc |ity.org |ountymd.gov Severity|enhancement |normal CC| |emily.lamancusa@montgomeryc | |ountymd.gov --- Comment #1 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- +1 for this! Commit c654932f0c376b2b619c9bd4550aab0adf9acdab from bug 8367 did something like that for the holds pickup period - checking the input id rather than the index. Taking a stab at this using that method... I'm also changing this to a bug rather than an enhancement, because similar bugs keep cropping up (I'm seeing at least one, with no automatic renewal after (hard limit) again, in main currently). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 --- Comment #2 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 170214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170214&action=edit Bug 36716: Do special processing based on input id, not column index When a user clicks "Edit" ( .editrule ) we use JavaScript to loop through each of the columns in the table to copy the appropriate values into the input fields. Fields that need special processing are identified by the column index, which can lead to problems when the index varies between Koha versions or columns are shown/hidden based on syspref settings. In the current main, there is at least one such bug causing the value for "no automatic renewal before" not to propagate, but to get silently saved in the "no automatic renewal before (hard limit)" field instead. Identifying fields for special processing based on input id rather than index should fix the above issue and avoid similar regressions. To test: 1. Create a circulation rule that has: a) a value (such as 30) in the column "No automatic renewal after" b) no value in the column "No automatic renewal after (hard limit)" 2. Click the button to edit the circulation rule from step 1 --> The text field for "No automatic renewal after" is blank 3. Save the rule without making any changes --> "No automatic renewal after" is now blank for this rule, but "No automatic renewal after (hard limit)" has a date in it 4. Apply patch 5. Repeat steps 1-3 --> "No automatic renewal after" and "No automatic renewal after (hard limit)" now preserve their values correctly 6. Create a circulation rule that has a non-default value in every field 7. Edit the circulation rule from step 6 --> Confirm that all values are copied to the edit fields correctly 8. Save the rule without making any changes --> Confirm that the rule saved correctly 9. Create a circulation rule, leaving the following columns blank: "Current checkouts allowed" "Current on-site checkouts allowed" "Holds allowed (total)" "Holds allowed (daily)" "Holds per record (count)" --> The above columns should display as "Unlimited" 10. Edit the rule from step 9 --> The input fields for the above columns should be blank 11. Save the rule without making any changes --> The above fields should still display as "Unlimited" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170214|0 |1 is obsolete| | --- Comment #3 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 170233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170233&action=edit Bug 36716: Do special processing based on input id, not column index When a user clicks "Edit" ( .editrule ) we use JavaScript to loop through each of the columns in the table to copy the appropriate values into the input fields. Fields that need special processing are identified by the column index, which can lead to problems when the index varies between Koha versions or columns are shown/hidden based on syspref settings. In the current main, there is at least one such bug causing the value for "no automatic renewal before" not to propagate, but to get silently saved in the "no automatic renewal before (hard limit)" field instead. Identifying fields for special processing based on input id rather than index should fix the above issue and avoid similar regressions. To test: 1. Create a circulation rule that has: a) a value (such as 30) in the column "No automatic renewal after" b) no value in the column "No automatic renewal after (hard limit)" 2. Click the button to edit the circulation rule from step 1 --> The text field for "No automatic renewal after" is blank 3. Save the rule without making any changes --> "No automatic renewal after" is now blank for this rule, but "No automatic renewal after (hard limit)" has a date in it 4. Apply patch 5. Repeat steps 1-3 --> "No automatic renewal after" and "No automatic renewal after (hard limit)" now preserve their values correctly 6. Create a circulation rule that has a non-default value in every field 7. Edit the circulation rule from step 6 --> Confirm that all values are copied to the edit fields correctly 8. Save the rule without making any changes --> Confirm that the rule saved correctly 9. Create a circulation rule, leaving the following columns blank: "Current checkouts allowed" "Current on-site checkouts allowed" "Holds allowed (total)" "Holds allowed (daily)" "Holds per record (count)" --> The above columns should display as "Unlimited" 10. Edit the rule from step 9 --> The input fields for the above columns should be blank 11. Save the rule without making any changes --> The above fields should still display as "Unlimited" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #4 from Lucas Gass <lucas@bywatersolutions.com> --- Thanks, Emily! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170233|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 170533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170533&action=edit Bug 36716: Do special processing based on input id, not column index When a user clicks "Edit" ( .editrule ) we use JavaScript to loop through each of the columns in the table to copy the appropriate values into the input fields. Fields that need special processing are identified by the column index, which can lead to problems when the index varies between Koha versions or columns are shown/hidden based on syspref settings. In the current main, there is at least one such bug causing the value for "no automatic renewal before" not to propagate, but to get silently saved in the "no automatic renewal before (hard limit)" field instead. Identifying fields for special processing based on input id rather than index should fix the above issue and avoid similar regressions. To test: 1. Create a circulation rule that has: a) a value (such as 30) in the column "No automatic renewal after" b) no value in the column "No automatic renewal after (hard limit)" 2. Click the button to edit the circulation rule from step 1 --> The text field for "No automatic renewal after" is blank 3. Save the rule without making any changes --> "No automatic renewal after" is now blank for this rule, but "No automatic renewal after (hard limit)" has a date in it 4. Apply patch 5. Repeat steps 1-3 --> "No automatic renewal after" and "No automatic renewal after (hard limit)" now preserve their values correctly 6. Create a circulation rule that has a non-default value in every field 7. Edit the circulation rule from step 6 --> Confirm that all values are copied to the edit fields correctly 8. Save the rule without making any changes --> Confirm that the rule saved correctly 9. Create a circulation rule, leaving the following columns blank: "Current checkouts allowed" "Current on-site checkouts allowed" "Holds allowed (total)" "Holds allowed (daily)" "Holds per record (count)" --> The above columns should display as "Unlimited" 10. Edit the rule from step 9 --> The input fields for the above columns should be blank 11. Save the rule without making any changes --> The above fields should still display as "Unlimited" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Massive Dev quality of life improvement here.. thanks Lucas.. no idea why we haven't done this before now! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Martin Renvoize from comment #6)
Massive Dev quality of life improvement here.. thanks Lucas.. no idea why we haven't done this before now!
I agree! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- One question, why not update these? (visible above one of the deleted lines in the patch): if ( i == 0 || i == 1 ) { -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #8)
One question, why not update these? (visible above one of the deleted lines in the patch):
if ( i == 0 || i == 1 ) {
Never got a reply. Dropping. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36716 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to main |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org