From 18528ce184446b6b21594c319024b97aa5a446c0 Mon Sep 17 00:00:00 2001 From: Emily Lamancusa Date: Fri, 9 Aug 2024 13:04:17 -0400 Subject: [PATCH] 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" --- .../prog/en/modules/admin/smart-rules.tt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 31907687dc..2299f5e57d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -1505,11 +1505,12 @@ itm_text = $(this).text(); itm_text = itm_text.replace(/^\s*|\s*$/g,''); var current_column = $("#edit_row td:eq("+i+")"); - if ( i == 3 ) { + var current_input_id = $(current_column).children('input').first().attr('id'); + if ( current_input_id === "note" ) { // specific processing for the Note column var note = $(this).find("a[id='viewnote']").data("content"); $(current_column).find("input[type='text']").val(note); - } else if ( i == 9 ) { + } else if ( current_input_id === "hardduedatecompare" ) { // specific processing for Hard due date $(current_column).find("select").val(itm_code); var hardduedate = $(this).data('duedate'); @@ -1518,7 +1519,7 @@ hardduedate = fp.parseDate( hardduedate, flatpickr_dateformat_string ); if( hardduedate) fp.setDate( hardduedate, 1 ); } - } else if ( i == 25 ) { + } else if ( current_input_id === "no_auto_renewal_after_hard_limit" ) { // specific processing for No automatic renewal after (hard limit) var renewdate = itm_text; if (renewdate) { @@ -1526,7 +1527,7 @@ renewdate = fp.parseDate( renewdate, flatpickr_dateformat_string ); if( renewdate) fp.setDate( renewdate, 1 ); } - } else if ( i == 16 ) { + } else if ( current_input_id === "cap_fine_to_replacement_price" ) { // specific processing for cap_fine_to_replacement_price var cap_fine_to_replacement_price = $(this).find("input[type='checkbox']"); $('#cap_fine_to_replacement_price').prop('checked', cap_fine_to_replacement_price.is(':checked') ); @@ -1552,7 +1553,6 @@ // After setting the correct option, update the select to reflect the change $(current_column).find('select').trigger('change'); - var current_input_id = $(current_column).children('input').first().attr('id'); if ( i == 0 || i == 1 ) { // Disable the 2 first columns, we cannot update them. var val = $(current_column).find("select option:selected").val(); @@ -1563,7 +1563,7 @@ // Remove potential previous input added $(current_column).find("input").remove(); $(current_column).append(""); - } else if ( i == 5 || i == 6 || i == 26 || i == 27 || i == 28 || current_input_id === "holds_pickup_period" ) { + } else if ( current_input_id === "maxissueqty" || current_input_id === "maxonsiteissueqty" || current_input_id === "reservesallowed" || current_input_id === "holds_per_day" || current_input_id === "holds_per_record" || current_input_id === "holds_pickup_period" ) { // If the value is not an integer for // - "Current checkouts allowed" // - "Current on-site checkouts allowed" -- 2.34.1