[Bug 35341] New: Circulation rule dates are being overwritten
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Bug ID: 35341 Summary: Circulation rule dates are being overwritten Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: matt.blenkinsop@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor Assignee|koha-bugs@lists.koha-commun |matt.blenkinsop@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #1 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Created attachment 158980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158980&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> 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=35341 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #3 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158980|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 159005 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159005&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (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! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This really is a rabbit hole ;) This = smart rules.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159077 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159077&action=edit Bug 35341: Testing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |major CC| |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=35341 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159005|0 |1 is obsolete| | --- Comment #10 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Created attachment 159591 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159591&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #11 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Created attachment 159592 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159592&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159591|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #12 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- 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). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159077|0 |1 is obsolete| | Attachment #159592|0 |1 is obsolete| | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 159669 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159669&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|m.de.rooy@rijksmuseum.nl |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=35341 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|matt.blenkinsop@ptfs-europe |m.de.rooy@rijksmuseum.nl |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Note that this patch makes sure that the dates are not kept when saving or clearing the rule in the edit row. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here again -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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') -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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); + } + }); + } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 160830 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160830&action=edit Bug 35341: Do not reinstantiate flatpickr We do not need to reinstantiate flatpickr, we need to retrieve the existing instance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.derscheid@lmscloud.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Ah great. I am looking further now. Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159669|0 |1 is obsolete| | Attachment #160830|0 |1 is obsolete| | --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 160844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160844&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 160845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160845&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Martin or Matt, could one of you have a final look? Three members of QA team already did until now :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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(){ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #34 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #36 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160845|0 |1 is obsolete| | --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 160850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160850&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #39 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will rebuild my container completely. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Back to Assigned. Needs more attention still :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160844|0 |1 is obsolete| | Attachment #160850|0 |1 is obsolete| | --- Comment #42 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 160896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160896&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #43 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Back to Signed Off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160896|0 |1 is obsolete| | --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 160928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160928&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 --- Comment #46 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! 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=35341 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|24.05.00 |24.05.00,23.11.02 released in| | --- Comment #47 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |lucas@bywatersolutions.com Version(s)|24.05.00,23.11.02 |24.05.00,23.11.02,23.05.09 released in| | --- Comment #48 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36708 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36708 [Bug 36708] Problems editing circ rules when 'Holds allowed (total)' value is greater than or equal to 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35341 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|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=35341 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED --- Comment #49 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org