Bug 26261 - Split calendar.inc into include file and JavaScript file
Summary: Split calendar.inc into include file and JavaScript file
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 21156
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-20 18:58 UTC by Owen Leonard
Modified: 2020-10-27 04:28 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Manual merge request:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.05


Attachments
Bug 26261: Split calendar.inc into include file and JavaScript file (18.34 KB, patch)
2020-08-20 20:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26261: Split calendar.inc into include file and JavaScript file (18.39 KB, patch)
2020-09-26 23:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 26261: Split calendar.inc into include file and JavaScript file (18.44 KB, patch)
2020-09-27 12:15 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2020-08-20 18:58:03 UTC
Now that we have JavaScript internationalization there's no need to have strings defined in an include file like calendar.inc. I propose that we trim calendar.inc down to only those things which require template processing and move the rest of the JavaScript into its own JS file. This would allow us to do proper linting and allow browsers to cache the file.
Comment 1 Owen Leonard 2020-08-20 20:19:01 UTC
Created attachment 108758 [details] [review]
Bug 26261: Split calendar.inc into include file and JavaScript file

This patch moves most of the JavaScript out of calendar.inc and into a
new file, calendar.js. English strings in calendar.js are now wrapped in
the double-underscore function to allow for translation.

The patch shouldn't affect datepicker functionality in any way.

To test, apply the patch and test the datepicker in various contexts:

- Reports -> Statistics wizards -> Patrons
- Date of birth "from" and "to" should be linked so that the former
  must come before the latter.
- Patrons -> Add or edit patron
- The date of birth field datepicker shouldn't allow you to pick a
  future year from the dropdown.
- Tools -> Calendar.
- Test that the calendar tool works correctly.
- Test with various settings of the dateformat system preference.

TESTING TRANSLATABILITY

- Update a translation, e.g. fr-FR:

  > cd misc/translator
  > perl translate update fr-FR

- Open the corresponding .po file for JavaScript strings, e.g.
  misc/translator/po/fr-FR-messages-js.po
- Locate strings pulled from
  koha-tmpl/intranet-tmpl/prog/en/js/calendar.js for translation,
  e.g.:

  msgid "August"
  msgstr ""

- Edit the "msgstr" string however you want (it's just for testing).
- Install the updated translation:

  > perl translate install fr-FR

- Switch to your newly translated language in the staff client
  and test the datepicker again. Translated strings like days of
  the week and months should be correct.
Comment 2 David Nind 2020-09-26 23:35:14 UTC
Created attachment 110809 [details] [review]
Bug 26261: Split calendar.inc into include file and JavaScript file

This patch moves most of the JavaScript out of calendar.inc and into a
new file, calendar.js. English strings in calendar.js are now wrapped in
the double-underscore function to allow for translation.

The patch shouldn't affect datepicker functionality in any way.

To test, apply the patch and test the datepicker in various contexts:

- Reports -> Statistics wizards -> Patrons
- Date of birth "from" and "to" should be linked so that the former
  must come before the latter.
- Patrons -> Add or edit patron
- The date of birth field datepicker shouldn't allow you to pick a
  future year from the dropdown.
- Tools -> Calendar.
- Test that the calendar tool works correctly.
- Test with various settings of the dateformat system preference.

TESTING TRANSLATABILITY

- Update a translation, e.g. fr-FR:

  > cd misc/translator
  > perl translate update fr-FR

- Open the corresponding .po file for JavaScript strings, e.g.
  misc/translator/po/fr-FR-messages-js.po
- Locate strings pulled from
  koha-tmpl/intranet-tmpl/prog/en/js/calendar.js for translation,
  e.g.:

  msgid "August"
  msgstr ""

- Edit the "msgstr" string however you want (it's just for testing).
- Install the updated translation:

  > perl translate install fr-FR

- Switch to your newly translated language in the staff client
  and test the datepicker again. Translated strings like days of
  the week and months should be correct.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2020-09-27 00:54:30 UTC
There may be something fishy going on with the changing date formats and the age calculation on the patron add/edit form.

Before the patch is applied (current master) either when editing or adding, if you change the date format, refresh the patron add/edit page and change the date the age shown is not correct:
1. Date format set to mm/dd/yyyy
2. Add new patron (Patrons > New patron)
3. Make birth date 08/01/2020 (1 August 2020), age is shown as 1 month
4. Change the date format to dd/mm/yyyy
5. Add new patron (Patrons > New patron) (refresh the page as well with SHIFT refresh)
6. Make birth date 08/01/2020 (1 August 2020), age is shown as 8 months
7. Repeat from step 4 AFTER flush_memcached and restart_all
8. Works as expected

I can get it to work correctly by using flush_memcached and restart_all (koha-testing-docker). However, before the patch was applied this was not required.
Comment 4 Katrin Fischer 2020-09-27 12:15:48 UTC
Created attachment 110830 [details] [review]
Bug 26261: Split calendar.inc into include file and JavaScript file

This patch moves most of the JavaScript out of calendar.inc and into a
new file, calendar.js. English strings in calendar.js are now wrapped in
the double-underscore function to allow for translation.

The patch shouldn't affect datepicker functionality in any way.

To test, apply the patch and test the datepicker in various contexts:

- Reports -> Statistics wizards -> Patrons
- Date of birth "from" and "to" should be linked so that the former
  must come before the latter.
- Patrons -> Add or edit patron
- The date of birth field datepicker shouldn't allow you to pick a
  future year from the dropdown.
- Tools -> Calendar.
- Test that the calendar tool works correctly.
- Test with various settings of the dateformat system preference.

TESTING TRANSLATABILITY

- Update a translation, e.g. fr-FR:

  > cd misc/translator
  > perl translate update fr-FR

- Open the corresponding .po file for JavaScript strings, e.g.
  misc/translator/po/fr-FR-messages-js.po
- Locate strings pulled from
  koha-tmpl/intranet-tmpl/prog/en/js/calendar.js for translation,
  e.g.:

  msgid "August"
  msgstr ""

- Edit the "msgstr" string however you want (it's just for testing).
- Install the updated translation:

  > perl translate install fr-FR

- Switch to your newly translated language in the staff client
  and test the datepicker again. Translated strings like days of
  the week and months should be correct.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Jonathan Druart 2020-09-29 12:31:38 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 6 Lucas Gass 2020-10-20 17:09:30 UTC
backported to 20.05.x for 20.05.05
Comment 7 Aleisha Amohia 2020-10-27 04:28:50 UTC
enhancement, not backported to 19.11.x