Bug 30717 - Dates displayed in ISO format when editing items
Summary: Dates displayed in ISO format when editing items
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
: 30361 30659 (view as bug list)
Depends on: 27526 29369
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-09 07:55 UTC by Mengu Yazicioglu
Modified: 2023-12-28 20:42 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:
22.05.00, 21.11.07


Attachments
Bug 30717: Format dates when editing items (2.81 KB, patch)
2022-05-09 08:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30717: Format dates when editing items (2.86 KB, patch)
2022-05-09 23:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 30717: Format dates when editing items (2.95 KB, patch)
2022-05-13 07:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30717: (QA follow-up) Move to module (3.63 KB, patch)
2022-05-13 07:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30717: Fix selenium tests (1.99 KB, patch)
2022-07-12 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mengu Yazicioglu 2022-05-09 07:55:01 UTC
Hi,
After upgrade from 19.05 to 21.11.04 we got a problem with additem section.
dateaccessioned area of additem doesn't seem correctly even we changed dateformat in systempreferences.
I've removed the blank in the line 
var dateformat_pref = "[% Koha.Preference('dateformat') | html %]"; of calendar.inc
Comment 1 Jonathan Druart 2022-05-09 08:30:38 UTC
No data loss, that's not critical.
Comment 2 Jonathan Druart 2022-05-09 08:35:19 UTC
Created attachment 134781 [details] [review]
Bug 30717: Format dates when editing items

This is a result of bug 29369 and bug 27526.
Bug 29369 removed the format of the dates, and bug 27526 assumed that it
was the correct behaviour.

Here we don't want to let the controller know which fields is a date, or
we will have to access the subfield structure to know which subfields
have the "date cataloguing plugin".

This patch suggests to use the altFormat options from flatpickr that
will "Show the user a readable date (as per altFormat), but return
something totally different to the server."

It's actually an option we want to use for all our dates, that will
reduce a lot the overhead in our controllers.

Test plan:
Edit items (additem.pl and in batch) and confirm that the subfields
using the dateaccessioned plugin are displayed according to the
dateformat syspref. Make sure the date is stored correctly.
Comment 3 David Nind 2022-05-09 23:37:25 UTC
Created attachment 134797 [details] [review]
Bug 30717: Format dates when editing items

This is a result of bug 29369 and bug 27526.
Bug 29369 removed the format of the dates, and bug 27526 assumed that it
was the correct behaviour.

Here we don't want to let the controller know which fields is a date, or
we will have to access the subfield structure to know which subfields
have the "date cataloguing plugin".

This patch suggests to use the altFormat options from flatpickr that
will "Show the user a readable date (as per altFormat), but return
something totally different to the server."

It's actually an option we want to use for all our dates, that will
reduce a lot the overhead in our controllers.

Test plan:
Edit items (additem.pl and in batch) and confirm that the subfields
using the dateaccessioned plugin are displayed according to the
dateformat syspref. Make sure the date is stored correctly.

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Marcel de Rooy 2022-05-13 06:42:18 UTC
QAing
Comment 5 Marcel de Rooy 2022-05-13 07:48:05 UTC
Created attachment 134947 [details] [review]
Bug 30717: Format dates when editing items

This is a result of bug 29369 and bug 27526.
Bug 29369 removed the format of the dates, and bug 27526 assumed that it
was the correct behaviour.

Here we don't want to let the controller know which fields is a date, or
we will have to access the subfield structure to know which subfields
have the "date cataloguing plugin".

This patch suggests to use the altFormat options from flatpickr that
will "Show the user a readable date (as per altFormat), but return
something totally different to the server."

It's actually an option we want to use for all our dates, that will
reduce a lot the overhead in our controllers.

Test plan:
Edit items (additem.pl and in batch) and confirm that the subfields
using the dateaccessioned plugin are displayed according to the
dateformat syspref. Make sure the date is stored correctly.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2022-05-13 07:48:10 UTC
Created attachment 134948 [details] [review]
Bug 30717: (QA follow-up) Move to module

We will probably use this a bit more :)
Let's put it in a module (with a trivial test).

Test plan:
Repeat item edit.
Run  t/DateUtils.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2022-05-13 07:49:05 UTC
QA tools gives a false negative:

 FAIL   Koha/DateUtils.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   FAIL   valid
                Subroutine flatpickr_date_form
Comment 8 Marcel de Rooy 2022-05-13 07:52:10 UTC
Took me bit to figure out this qa tools warn, but this is the cause:

perl -cw outputs:
$VAR1 = 'Subroutine dt_from_string redefined at Koha/DateUtils.pm line 58.
Subroutine output_pref redefined at Koha/DateUtils.pm line 269.
Subroutine format_sqldatetime redefined at Koha/DateUtils.pm line 360.
Subroutine flatpickr_date_format redefined at Koha/DateUtils.pm line 392.
Koha/DateUtils.pm syntax OK
';

The line Subroutine flatpickr_date_format redefined at Koha/DateUtils.pm line 392.
goes thru the following regex:
    s/at .* line .*$// for @errors;
And this regex is too generic. It already the last two letters at from the subroutine name!
So it leaves: Subroutine flatpickr_date_form
Which is no longer removed by the redefined regex..
    s/^Subroutine .* redefined $// for @errors;
Since redefined was already eaten.
Comment 9 Fridolin Somers 2022-05-14 01:43:07 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 10 Jonathan Druart 2022-05-18 07:32:31 UTC
(In reply to Marcel de Rooy from comment #8)
> Took me bit to figure out this qa tools warn, but this is the cause:
> 
> perl -cw outputs:
> $VAR1 = 'Subroutine dt_from_string redefined at Koha/DateUtils.pm line 58.
> Subroutine output_pref redefined at Koha/DateUtils.pm line 269.
> Subroutine format_sqldatetime redefined at Koha/DateUtils.pm line 360.
> Subroutine flatpickr_date_format redefined at Koha/DateUtils.pm line 392.
> Koha/DateUtils.pm syntax OK
> ';
> 
> The line Subroutine flatpickr_date_format redefined at Koha/DateUtils.pm
> line 392.
> goes thru the following regex:
>     s/at .* line .*$// for @errors;
> And this regex is too generic. It already the last two letters at from the
> subroutine name!
> So it leaves: Subroutine flatpickr_date_form
> Which is no longer removed by the redefined regex..
>     s/^Subroutine .* redefined $// for @errors;
> Since redefined was already eaten.

Please report when you find bugs :)

See https://gitlab.com/koha-community/qa-test-tools/-/issues/60
Comment 11 Jonathan Druart 2022-05-31 09:42:28 UTC
*** Bug 30659 has been marked as a duplicate of this bug. ***
Comment 12 Katrin Fischer 2022-06-19 11:44:22 UTC
Hi, I think this still needs backporting as it was reported against 21.11.04?
Comment 13 Katrin Fischer 2022-06-19 11:46:14 UTC
*** Bug 30361 has been marked as a duplicate of this bug. ***
Comment 14 Arthur Suzuki 2022-06-22 14:33:05 UTC
pushed to 21.11.x for 21.11.07, thx!
Comment 15 Victor Grousset/tuxayo 2022-06-25 23:51:01 UTC
Missing dependencies for 21.05.x, it shouldn't be affected, no backport.
Comment 16 Jonathan Druart 2022-07-12 09:32:38 UTC
Created attachment 137613 [details] [review]
Bug 30717: Fix selenium tests

Error while executing command: element not interactable: Element <input id="tag_952_subfield_w_810691" class="input_marceditor items.replacementpricedate noEnterSubmit flatpickr-input" name="items.replacementpricedate" type="hidden"> is not reachable by keyboard at /usr/share/perl5/Selenium/Remote/Driver.pm line 411.
Comment 17 Jonathan Druart 2022-07-12 09:33:14 UTC
No idea why this is not failing on 21.11 but git bisect led me to this bug.
Comment 18 Jonathan Druart 2022-07-12 09:34:30 UTC
Hum, we released 22.05 since this. Moving to its own bug report.
Comment 19 Jonathan Druart 2022-07-12 09:37:33 UTC
Selenium fix moved to bug 31139.