Attachment 170533 Details for Bug 36716 – Bug 36716: Do special processing based on input id, not column index

[patch] Bug 36716: Do special processing based on input id, not column index

Bug-36716-Do-special-processing-based-on-input-id-.patch (text/plain), 6.39 KB, created by Martin Renvoize (ashimema) on 2024-08-21 08:25:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-08-21 08:25:27 UTC

Size: 6.39 KB

patch

obsolete

>From fbb8610f158e6f007a5557a8532f5f92c348d17a Mon Sep 17 00:00:00 2001
>From: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
>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"
>
>Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> .../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 f4901df8d0b..ad8ec9d168f 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
>@@ -1512,11 +1512,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');
>@@ -1525,7 +1526,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) {
>@@ -1533,7 +1534,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') );
>@@ -1559,7 +1560,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();
>@@ -1570,7 +1570,7 @@
>                             // Remove potential previous input added
>                             $(current_column).find("input").remove();
>                             $(current_column).append("<input type='hidden' name='"+name+"' value='"+val+"' />");
>-                        } 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.46.0

Actions: View | Diff | Splinter Review

Attachments on bug 36716: 170214 | 170233 | 170533