Bug 35341 - Circulation rule dates are being overwritten
Summary: Circulation rule dates are being overwritten
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 36708
  Show dependency treegraph
 
Reported: 2023-11-15 12:20 UTC by Matt Blenkinsop
Modified: 2024-05-12 03:19 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02,23.05.09


Attachments
Bug 35341: Fix ciruclation rule dates being overwritten (6.49 KB, patch)
2023-11-15 12:23 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35341: Fix ciruclation rule dates being overwritten (6.52 KB, patch)
2023-11-15 19:51 UTC, David Nind
Details | Diff | Splinter Review
Bug 35341: Testing (2.99 KB, patch)
2023-11-17 10:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Fix ciruclation rule dates being overwritten (3.94 KB, patch)
2023-12-05 16:08 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35341: Fix circulation rule dates being overwritten (3.94 KB, patch)
2023-12-05 16:08 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35341: Improve processing hard due date and auto renewal date (4.63 KB, patch)
2023-12-08 09:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Do not reinstantiate flatpickr (2.61 KB, patch)
2024-01-11 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35341: Improve processing hard due date and auto renewal date (4.63 KB, patch)
2024-01-11 13:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Do not reinstantiate flatpickr (2.85 KB, patch)
2024-01-11 13:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Do not reinstantiate flatpickr (2.33 KB, patch)
2024-01-11 14:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Improve processing hard due date and auto renewal date (6.87 KB, patch)
2024-01-12 07:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35341: Improve processing hard due date and auto renewal date (6.90 KB, patch)
2024-01-12 10:58 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2023-11-15 12:20:20 UTC
When setting either the "Hard due date" or the "No automatic renewal after (hard limit)" columns - if only one is amended then the other will be overwritten with the value of the last rule that was in the editor
Comment 1 Matt Blenkinsop 2023-11-15 12:23:15 UTC
Created attachment 158980 [details] [review]
Bug 35341: Fix ciruclation rule dates being overwritten

This patch correctly sets the values of the date fields so that they are not overwritten when only one is amended.

It also amends the collumn indexes being used in the click event as new columns have been added without these being updated

Test plan:
1) Navigate to Administration > Circulation and fine rules
2) Add a new circulation rule for a category of your choice
3) For the "All" rule, add the date 31/12/2023 to both the Hard due date and No automatic renewal after (hard limit) (NARAHL) columns and save
4) Now edit the rule that you created.
5) Change the two columns above to 01/12/2023 and click save
6) Edit the rule again and change the Hard due date to 15/12/2023. Click save
7) Note that the date for your rule in the NARAHL has changed to 31/12/2023 despite not being edited
8) Click edit again, and change the NARAHL date to 15/12/2023 and save.
9) This time the date in Hard due date will have changed to 31/12/2023
10) Apply patch
11) Edit the dates individually a few times and this time your rule should update correctly
Comment 2 Katrin Fischer 2023-11-15 13:25:57 UTC
Hi Matt, 

I noticed we have quite a lot of "bugs" related to circulation rules right now. I didn't have time to dig deeper yet, but could this one relate to one of these as well?

Bug 34151 - Hard due date being ignored in circulation rules in favor of loan period
Bug 32262 - smart-rules: Some rules can be created but are not visible on interface 
Bug 34826 - Circulation rules edit row select elements don't always update when selecting new row for editing

?
Comment 3 Matt Blenkinsop 2023-11-15 13:50:39 UTC
No it doesn't look like it - this is a separate issue spotted by one of our customers

Bug 34826 is the most similar but I think that one relates more to bug 34269
Comment 4 David Nind 2023-11-15 19:51:45 UTC
Created attachment 159005 [details] [review]
Bug 35341: Fix ciruclation rule dates being overwritten

This patch correctly sets the values of the date fields so that they are not overwritten when only one is amended.

It also amends the collumn indexes being used in the click event as new columns have been added without these being updated

Test plan:
1) Navigate to Administration > Circulation and fine rules
2) Add a new circulation rule for a category of your choice
3) For the "All" rule, add the date 31/12/2023 to both the Hard due date and No automatic renewal after (hard limit) (NARAHL) columns and save
4) Now edit the rule that you created.
5) Change the two columns above to 01/12/2023 and click save
6) Edit the rule again and change the Hard due date to 15/12/2023. Click save
7) Note that the date for your rule in the NARAHL has changed to 31/12/2023 despite not being edited
8) Click edit again, and change the NARAHL date to 15/12/2023 and save.
9) This time the date in Hard due date will have changed to 31/12/2023
10) Apply patch
11) Edit the dates individually a few times and this time your rule should update correctly

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Katrin Fischer 2023-11-16 21:44:21 UTC
(In reply to Matt Blenkinsop from comment #3)
> No it doesn't look like it - this is a separate issue spotted by one of our
> customers
> 
> Bug 34826 is the most similar but I think that one relates more to bug 34269

Thanks for checking!
Comment 6 Marcel de Rooy 2023-11-17 07:44:03 UTC
Looking here
Comment 7 Marcel de Rooy 2023-11-17 08:28:01 UTC
This really is a rabbit hole ;)
This = smart rules..
Comment 8 Marcel de Rooy 2023-11-17 10:15:57 UTC
I think that we are not going in the right direction here with the hidden backup inputs. We are trying to solve a flatpickr issue here. We should do that differently.

Seeing several related bugs in the smart rules. Just listing a few:

[1] This is incorrect. The i is the column starting from 0. 
                        } else if ( i == 5 || i == 6 || i == 27 || i == 28 || i == 29 || current_input_id === "holds_pickup_period" ) {
                            // If the value is not an integer for
                            //     - "Current checkouts allowed"
                            //     - "Current on-site checkouts allowed"
                            //     - "Holds allowed (total)"
                            //     - "Holds allowed (daily)"
                            //     - "Holds per record (count)"
                            //     - "Holds pickup period (day)"
                            // The value is "Unlimited" (or an equivalent translated string)
                            // an it should be set to an empty string
Current checkouts allowed should be 4?
Because 6 is listed, it is now cleared. (Loan period should allow a zero?)

[2] There is a check on overwriting the edit rule. But shouldnt there be a check too about warning to overwrite a current rule when saving a blank rule ?
Out of scope.

[3] Flatpickr fields should be cleared in edit rule when saving/submitting.

[4] If you do not click in the flatpickr field, the date is not saved! (This is in my test patch.) The input is still empty. We should trigger flatpickr to fill it before.

Will submit a test patch which is not finished but gives an idea of the solution that I am thinking of.
Comment 9 Marcel de Rooy 2023-11-17 10:18:32 UTC
Created attachment 159077 [details] [review]
Bug 35341: Testing
Comment 10 Matt Blenkinsop 2023-12-05 16:08:05 UTC
Created attachment 159591 [details] [review]
Bug 35341: Fix ciruclation rule dates being overwritten

This patch correctly sets the values of the date fields so that they are not overwritten when only one is amended.

It also amends the collumn indexes being used in the click event as new columns have been added without these being updated

Test plan:
1) Navigate to Administration > Circulation and fine rules
2) Add a new circulation rule for a category of your choice
3) For the "All" rule, add the date 31/12/2023 to both the Hard due date and No automatic renewal after (hard limit) (NARAHL) columns and save
4) Now edit the rule that you created.
5) Change the two columns above to 01/12/2023 and click save
6) Edit the rule again and change the Hard due date to 15/12/2023. Click save
7) Note that the date for your rule in the NARAHL has changed to 31/12/2023 despite not being edited
8) Click edit again, and change the NARAHL date to 15/12/2023 and save.
9) This time the date in Hard due date will have changed to 31/12/2023
10) Apply patch
11) Edit the dates individually a few times and this time your rule should update correctly
Comment 11 Matt Blenkinsop 2023-12-05 16:08:38 UTC
Created attachment 159592 [details] [review]
Bug 35341: Fix circulation rule dates being overwritten

This patch correctly sets the values of the date fields so that they are not overwritten when only one is amended.

It also amends the collumn indexes being used in the click event as new columns have been added without these being updated

Test plan:
1) Navigate to Administration > Circulation and fine rules
2) Add a new circulation rule for a category of your choice
3) For the "All" rule, add the date 31/12/2023 to both the Hard due date and No automatic renewal after (hard limit) (NARAHL) columns and save
4) Now edit the rule that you created.
5) Change the two columns above to 01/12/2023 and click save
6) Edit the rule again and change the Hard due date to 15/12/2023. Click save
7) Note that the date for your rule in the NARAHL has changed to 31/12/2023 despite not being edited
8) Click edit again, and change the NARAHL date to 15/12/2023 and save.
9) This time the date in Hard due date will have changed to 31/12/2023
10) Apply patch
11) Edit the dates individually a few times and this time your rule should update correctly
Comment 12 Matt Blenkinsop 2023-12-05 16:11:26 UTC
I've taken your test patch Marcel and made some changes so that it now only uses flatpickr rather than adding additional hidden inputs to hold the value.

This feels like a major bug so I think we should get a fix in place for this ASAP. Let me know what you think and if you agree with the concept I will address some of the other issues you spotted in follow-up bugs (e.g. incorrect column indexes etc).
Comment 13 Marcel de Rooy 2023-12-06 12:12:03 UTC
(In reply to Matt Blenkinsop from comment #12)
> I've taken your test patch Marcel and made some changes so that it now only
> uses flatpickr rather than adding additional hidden inputs to hold the value.
> 
> This feels like a major bug so I think we should get a fix in place for this
> ASAP. Let me know what you think and if you agree with the concept I will
> address some of the other issues you spotted in follow-up bugs (e.g.
> incorrect column indexes etc).

Okay. Will have a look later this week.
Comment 14 Marcel de Rooy 2023-12-08 07:21:48 UTC
QA: Looking here
Comment 15 Marcel de Rooy 2023-12-08 08:29:51 UTC
Still some issues here:

Template process failed: undef error - The given date (0NaN-aN-aN) does not match the date format (iso) at /usr/share/koha/Koha/DateUtils.pm line 205.
=> Try another date format like dd-mm-yy
=> var formatted_date = flatpickr.formatDate(new Date(itm_text), "Y-m-d") does not look good.

Still looking further
Comment 16 Marcel de Rooy 2023-12-08 09:23:05 UTC
Created attachment 159669 [details] [review]
Bug 35341: Improve processing hard due date and auto renewal date

After quite a struggle, came up with this approach using
flatpickr's parseDate and setDate. Seems to be the best.
The dateformat variable is set in calendar.inc.

Test plan:
Test edit, save, clear rules, focus on both dates.
Toggle all I18N preference date formats.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2023-12-08 09:24:59 UTC
(In reply to Matt Blenkinsop from comment #12)
> I've taken your test patch Marcel and made some changes so that it now only
> uses flatpickr rather than adding additional hidden inputs to hold the value.
> 
> This feels like a major bug so I think we should get a fix in place for this
> ASAP. Let me know what you think and if you agree with the concept I will
> address some of the other issues you spotted in follow-up bugs (e.g.
> incorrect column indexes etc).

Sorry, but it did not work yet. I obsoleted your patch now in favor of adding flatpickr methods on top of the first patch.

Could you add your signoff, Matts? And perhaps let Martin do the final QA?
Comment 18 Marcel de Rooy 2023-12-08 09:29:29 UTC
Note that this patch makes sure that the dates are not kept when saving or clearing the rule in the edit row.
Comment 19 Martin Renvoize 2023-12-14 09:49:58 UTC
Matt is on holiday at the minute, but I'm having a quick look.

I'm afraid I've found some issues, though I'm not sure if they predate the patches or not.

The core issue is fixed.. woop woop!

However;
1. Each time one clicks 'Edit' to edit a line, the flatpickers each get an extra 'clear date' red X added.
2. It appears that there's a link between the two flatpickers where they each highlight the corresponding date picked in the other rule.. that link doesn't update when either date is updated during edit.
Comment 20 Martin Renvoize 2023-12-14 10:00:29 UTC
OK, maybe I'm confused.. looks like it shows the currently selected date prior to change rather than the other pickers date.  My mistake.

First issue about repeated X is still valid however.
Comment 21 Marcel de Rooy 2023-12-20 12:55:17 UTC
Looking here again
Comment 22 Marcel de Rooy 2023-12-20 15:35:28 UTC
Hmm. This comes from flatpickr instantiation etc. Not my favorite waste of time:

Calendar.inc contains:
         onReady: function( selectedDates, dateStr, instance ){
             /* When flatpickr instance is created, automatically append a "clear date" link */

This onReady is called too often.
Flatpickr says that onReady gets triggered once the calendar is in a ready state.
What happens now, is that the edit of a circulation rule triggers this onReady again and keeps adding buttons.

Sounds easy to fix but the automagical duplication of flatpickr inputs somehow interferes.
In the current codebase the second input gets wrapped into the flatpickr_wrapper class.
But if I move the onReady code to document.ready level only, somehow this second input is no longer wrapped into that span? Although the control works, you will see it wrap after clearing a date.

Not sure how to continue here.

Jonathan: do you have a suggestion here?

I tried several things including a filter like:

-            $(instance.input).find('~input.flatpickr:first')
+            $(instance.input)
+                .filter( function () { return $(this).siblings('span').length == 0 } )
+                .find('~input.flatpickr:first')
Comment 23 Marcel de Rooy 2023-12-20 15:37:02 UTC
As a separate function:

+    function add_clear_button (input) {
+        /* Append a "clear date" link */
+        $(input).find('~input.flatpickr:first')
+            /* Add a wrapper element so that we can prevent the clear button from wrapping */
+            .wrap("<span class='flatpickr_wrapper'></span>")
+            .attr("autocomplete", "off")
+            .after( $("<a/>")
+                .attr("href","#")
+                .addClass("clear_date")
+                .on("click", function(e){
+                    e.preventDefault();
+                    $(input).flatpickr().clear();
+                })
+                .addClass("fa fa-fw fa-times")
+                .attr("aria-hidden", true)
+                .attr("aria-label", _("Clear date") )
+            ).keydown(function(e) {
+                var key = (event.keyCode ? event.keyCode : event.which);
+                if ( key == 40 ) {
+                    $(input).flatpickr().set('allowInput',false);
+                }
+            });
+    }
Comment 24 Jonathan Druart 2024-01-11 10:56:54 UTC
Created attachment 160830 [details] [review]
Bug 35341: Do not reinstantiate flatpickr

We do not need to reinstantiate flatpickr, we need to retrieve the
existing instance.
Comment 25 Jonathan Druart 2024-01-11 11:02:39 UTC
Marcel, I think I have fixed the problem you were facing. However I am failing at hiding the error (not a JS Error object, but something appearing in the console) raised by is_valid_date (actually flatpickr.parseDate) when we have "None defined".

I have tried to silent it (with an option) but errorHandler in flatpickr is not documented (and I don't find a way to pass it when using parseDate as a static method).

The only possible solution I found is to temporary override console.warn, but this sounds terribly wrong.
Comment 26 Marcel de Rooy 2024-01-11 12:37:19 UTC
Ah great. I am looking further now. Thanks.
Comment 27 Marcel de Rooy 2024-01-11 13:02:13 UTC
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 e3edc701bc..f3c57ad25e 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,13 @@
                     } else if ( i == 9 ) {
                         // specific processing for Hard due date
                         $(current_column).find("select").val(itm_code);
-                        if (is_valid_date(itm_text)) {
+                        if (itm_text) {
                             input_values = itm_text.split(' ');
                             var fp = document.querySelector("#hardduedate")._flatpickr;
-                            var hardduedate = fp.parseDate( input_values[1], flatpickr_dateformat_string );
-                            if( hardduedate) fp.setDate( hardduedate, 1 );
+                            if (is_valid_date(input_values[1])) {
+                                var hardduedate = fp.parseDate( input_values[1], flatpickr_dateformat_string );
+                                if( hardduedate) fp.setDate( hardduedate, 1 );
+                            }
                         }
                     } else if ( i == 26 ) {
                         // specific processing for No automatic renewal after (hard limit)
Comment 28 Marcel de Rooy 2024-01-11 13:02:55 UTC
Adding the above change since item_text does contain more than a date in the first case. It also includes the Before/On/After text part for the first date.
Comment 29 Marcel de Rooy 2024-01-11 13:03:43 UTC
(In reply to Jonathan Druart from comment #25)
> Marcel, I think I have fixed the problem you were facing. However I am
> failing at hiding the error (not a JS Error object, but something appearing
> in the console) raised by is_valid_date (actually flatpickr.parseDate) when
> we have "None defined".
> 
> I have tried to silent it (with an option) but errorHandler in flatpickr is
> not documented (and I don't find a way to pass it when using parseDate as a
> static method).
> 
> The only possible solution I found is to temporary override console.warn,
> but this sounds terribly wrong.

I was not able to recreate a JS error in the console.
Comment 30 Marcel de Rooy 2024-01-11 13:07:47 UTC
Created attachment 160844 [details] [review]
Bug 35341: Improve processing hard due date and auto renewal date

After quite a struggle, came up with this approach using
flatpickr's parseDate and setDate. Seems to be the best.
The dateformat variable is set in calendar.inc.

Test plan:
Test edit, save, clear rules, focus on both dates.
Toggle all I18N preference date formats.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 31 Marcel de Rooy 2024-01-11 13:07:50 UTC
Created attachment 160845 [details] [review]
Bug 35341: Do not reinstantiate flatpickr

We do not need to reinstantiate flatpickr, we need to retrieve the
existing instance.

Test plan:
See previous patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 32 Marcel de Rooy 2024-01-11 13:09:40 UTC
Martin or Matt, could one of you have a final look? Three members of QA team already did until now :)
Comment 33 Marcel de Rooy 2024-01-11 13:21:47 UTC
Wondering if we really need this btw, but it should not harm either:

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
index f027552053..076820873b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
@@ -200,6 +200,7 @@
                 fp.set("maxDate", now.setMinutes(now.getMinutes() + 1));
             }, 30000);
         }
+        return fp;
     }

     $(document).ready(function(){
Comment 34 Jonathan Druart 2024-01-11 14:09:13 UTC
(In reply to Marcel de Rooy from comment #33)
> Wondering if we really need this btw, but it should not harm either:
> 
> diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
> b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
> index f027552053..076820873b 100644
> --- a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
> +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
> @@ -200,6 +200,7 @@
>                  fp.set("maxDate", now.setMinutes(now.getMinutes() + 1));
>              }, 30000);
>          }
> +        return fp;
>      }
> 
>      $(document).ready(function(){

No we don't, I was trying something else.
Comment 35 Jonathan Druart 2024-01-11 14:09:52 UTC
(In reply to Marcel de Rooy from comment #29)
> (In reply to Jonathan Druart from comment #25)
> > Marcel, I think I have fixed the problem you were facing. However I am
> > failing at hiding the error (not a JS Error object, but something appearing
> > in the console) raised by is_valid_date (actually flatpickr.parseDate) when
> > we have "None defined".
> > 
> > I have tried to silent it (with an option) but errorHandler in flatpickr is
> > not documented (and I don't find a way to pass it when using parseDate as a
> > static method).
> > 
> > The only possible solution I found is to temporary override console.warn,
> > but this sounds terribly wrong.
> 
> I was not able to recreate a JS error in the console.

With the sample data, click "Edit"
=> "Error: Invalid date provided: defined"
Comment 36 Marcel de Rooy 2024-01-11 14:26:03 UTC
(In reply to Jonathan Druart from comment #35)
> (In reply to Marcel de Rooy from comment #29)
> > (In reply to Jonathan Druart from comment #25)
> > > Marcel, I think I have fixed the problem you were facing. However I am
> > > failing at hiding the error (not a JS Error object, but something appearing
> > > in the console) raised by is_valid_date (actually flatpickr.parseDate) when
> > > we have "None defined".
> > > 
> > > I have tried to silent it (with an option) but errorHandler in flatpickr is
> > > not documented (and I don't find a way to pass it when using parseDate as a
> > > static method).
> > > 
> > > The only possible solution I found is to temporary override console.warn,
> > > but this sounds terribly wrong.
> > 
> > I was not able to recreate a JS error in the console.
> 
> With the sample data, click "Edit"
> => "Error: Invalid date provided: defined"

Okay, will refresh db and try.
Comment 37 Marcel de Rooy 2024-01-11 14:36:08 UTC
Created attachment 160850 [details] [review]
Bug 35341: Do not reinstantiate flatpickr

We do not need to reinstantiate flatpickr, we need to retrieve the
existing instance.

Test plan:
See previous patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 38 Marcel de Rooy 2024-01-11 14:37:25 UTC
(In reply to Jonathan Druart from comment #34)
> (In reply to Marcel de Rooy from comment #33)
> > Wondering if we really need this btw, but it should not harm either:
> 
> No we don't, I was trying something else.

Removed the line.
Comment 39 Marcel de Rooy 2024-01-11 14:38:24 UTC
(In reply to Jonathan Druart from comment #35)
> (In reply to Marcel de Rooy from comment #29)
> > I was not able to recreate a JS error in the console.
> 
> With the sample data, click "Edit"
> => "Error: Invalid date provided: defined"

With fresh db still not able to reproduce. Did you try the current patches and did you refresh browser cache?
Comment 40 Marcel de Rooy 2024-01-11 14:39:03 UTC
Will rebuild my container completely.
Comment 41 Marcel de Rooy 2024-01-11 15:36:28 UTC
Back to Assigned. Needs more attention still :)
Comment 42 Marcel de Rooy 2024-01-12 07:36:41 UTC
Created attachment 160896 [details] [review]
Bug 35341: Improve processing hard due date and auto renewal date

After quite a struggle, came up with this approach using
flatpickr's parseDate and setDate. Seems to be the best.
The dateformat variable is set in calendar.inc.

Test plan:
Test edit, save, clear rules, focus on both dates.
Toggle all I18N preference date formats.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 35341: Do not reinstantiate flatpickr

We do not need to reinstantiate flatpickr, we need to retrieve the
existing instance.

Test plan:
See previous patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
12-01-2024 Squashed both patches. Removed the unused hidden inputs for
hardduedatecompare and added a data attribute for the duedate.
The check with is_valid_date seems no longer needed here. Normally
date is empty or valid. A js warning from parseDate should be
exceptional. Made code for both dates a bit more consistent.
Comment 43 Marcel de Rooy 2024-01-12 07:37:03 UTC
Back to Signed Off.
Comment 44 Martin Renvoize 2024-01-12 10:58:08 UTC
Created attachment 160928 [details] [review]
Bug 35341: Improve processing hard due date and auto renewal date

After quite a struggle, came up with this approach using
flatpickr's parseDate and setDate. Seems to be the best.
The dateformat variable is set in calendar.inc.

Test plan:
Test edit, save, clear rules, focus on both dates.
Toggle all I18N preference date formats.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 35341: Do not reinstantiate flatpickr

We do not need to reinstantiate flatpickr, we need to retrieve the
existing instance.

Test plan:
See previous patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
12-01-2024 Squashed both patches. Removed the unused hidden inputs for
hardduedatecompare and added a data attribute for the duedate.
The check with is_valid_date seems no longer needed here. Normally
date is empty or valid. A js warning from parseDate should be
exceptional. Made code for both dates a bit more consistent.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2024-01-12 10:59:23 UTC
Thanks for all your work here Marcel, 

It's all looking much better for me now.. so far I've not managed to break anything :)

Passing QA
Comment 46 Katrin Fischer 2024-01-16 11:09:19 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 47 Fridolin Somers 2024-01-17 09:53:29 UTC
Pushed to 23.11.x for 23.11.02
Comment 48 Lucas Gass 2024-02-02 18:01:49 UTC
Backported to 23.05.x for upcoming 23.05.09