Bug 30059 - Add a JS equivalent to Koha::Patron->get_age
Summary: Add a JS equivalent to Koha::Patron->get_age
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 30055
  Show dependency treegraph
 
Reported: 2022-02-09 12:50 UTC by Jonathan Druart
Modified: 2022-12-12 21:23 UTC (History)
5 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


Attachments
Bug 30059: Add JS equivalent for Koha::Patron->get_age (1.63 KB, patch)
2022-02-09 13:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30059: Add JS equivalent for Koha::Patron->get_age (1.62 KB, patch)
2022-02-16 12:51 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30059: Deal with timezones (1.79 KB, patch)
2022-03-03 09:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30059: Add JS equivalent for Koha::Patron->get_age (1.68 KB, patch)
2022-03-14 18:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30059: Deal with timezones (1.84 KB, patch)
2022-03-14 18:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30059: Add JS equivalent for Koha::Patron->get_age (1.74 KB, patch)
2022-03-24 15:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30059: Deal with timezones (1.89 KB, patch)
2022-03-24 15:27 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-02-09 12:50:02 UTC
To display the age of patrons in a table we will need to do the calculation in JS
Comment 1 Jonathan Druart 2022-02-09 13:05:43 UTC
Created attachment 130358 [details] [review]
Bug 30059: Add JS equivalent for Koha::Patron->get_age

On bug 30055 we are going to use the REST API to display the patron
search result, we will then need to calculate patron's age client-side.
This is moved to its own bug report in case we need to reuse it
somewhere else.

Test plan:
Copy/paste the JS function in your browser's console then call it and
confirm that the result is correct
For instance:
  $get_age('2000-01-01')
Comment 2 Owen Leonard 2022-02-16 12:51:18 UTC
Created attachment 130665 [details] [review]
Bug 30059: Add JS equivalent for Koha::Patron->get_age

On bug 30055 we are going to use the REST API to display the patron
search result, we will then need to calculate patron's age client-side.
This is moved to its own bug report in case we need to reuse it
somewhere else.

Test plan:
Copy/paste the JS function in your browser's console then call it and
confirm that the result is correct
For instance:
  $get_age('2000-01-01')
Comment 3 Nick Clemens 2022-02-18 16:51:33 UTC
get_age('1979-02-17') = 43
get_age('1979-02-18') = 43
get_age('1979-02-19') = 43 XXX Today is 2022-02-18 - Not birthday yet! Stop calling me old!!!
get_age('1979-02-20') = 42
Comment 4 Martin Renvoize 2022-02-18 18:58:41 UTC
Might be an unpopular view.. but don't we already pull in moment.js for other date manipulations.. so couldn't we use https://momentjs.com/docs/#/displaying/difference/ to get a rather accurate result?
Comment 5 Jonathan Druart 2022-02-21 09:39:38 UTC
(In reply to Nick Clemens from comment #3)
> get_age('1979-02-17') = 43
> get_age('1979-02-18') = 43
> get_age('1979-02-19') = 43 XXX Today is 2022-02-18 - Not birthday yet! Stop
> calling me old!!!
> get_age('1979-02-20') = 42

Are you sure?

Today is 2022-02-21:
$get_age("1979-02-19") = 43
$get_age("1979-02-20") = 43
$get_age("1979-02-21") = 43 # today
$get_age("1979-02-22") = 42

So I modified the function with "var today = new Date("2022-02-18");"

$get_age('1979-02-17') = 43
$get_age('1979-02-18') = 43 # "today"
$get_age('1979-02-19') = 42
$get_age('1979-02-20') = 42
Comment 6 Nick Clemens 2022-02-22 12:48:04 UTC
(In reply to Jonathan Druart from comment #5) 
> Are you sure?

Yes, tested again this morning, same results, I believe time portion causes problems:

let dob = new Date('2022-02-23');
dob: Date Thu Feb 22 1979 19:00:00 GMT-0500 (Eastern Standard Time)
Comment 7 Tomás Cohen Arazi 2022-02-22 12:52:33 UTC
(In reply to Martin Renvoize from comment #4)
> Might be an unpopular view.. but don't we already pull in moment.js for
> other date manipulations.. so couldn't we use
> https://momentjs.com/docs/#/displaying/difference/ to get a rather accurate
> result?

I'd agree with this. I also wonder... if the browser date should be used, or the server's... I guess it is not that important. But if we think the UI could make a decision, based on the age, and place a request, this could be rejected because the server could have its own age check, leading to inconsistent behavior.

In that case, I'd just allow embedding the age like in:

GET /patrons
x-koha-emebed: age

(it would require renaming the ->get_age method)
Comment 8 Jonathan Druart 2022-03-03 09:05:16 UTC
The problem is the timezone, not the calculation. I don't think we need a lib for  that.
(In reply to Martin Renvoize from comment #4)
> Might be an unpopular view.. but don't we already pull in moment.js for
> other date manipulations.. so couldn't we use
> https://momentjs.com/docs/#/displaying/difference/ to get a rather accurate
> result?

I didn't manage to make it work, I get "tz is not a function". I've tried with moment-timezone as well, and both with the latest version.
Also worth noting: "Moment.js is a legacy project, now in maintenance mode. In most cases, you should choose a different library.", from https://github.com/moment/moment

(In reply to Tomás Cohen Arazi from comment #7)
> In that case, I'd just allow embedding the age like in:
> 
> GET /patrons
> x-koha-emebed: age
> 
> (it would require renaming the ->get_age method)

I had that concern some months ago (we will have twice the same business logic), but we agreed on providing a "JS equivalent" (bug 28874).

Here it's over-complicating things anyway, we just want to subtract two dates. We can also rely on the client to make extra processing, which is good for server perfs.
Comment 9 Jonathan Druart 2022-03-03 09:22:24 UTC
Created attachment 131309 [details] [review]
Bug 30059: Deal with timezones
Comment 10 Jonathan Druart 2022-03-03 09:23:53 UTC
This patches fixes the problem.

Test plan:
open the console and define a timezone:
let tz = "UTC" or "America/Buenos_Aires"
then call the function.
Comment 11 Tomás Cohen Arazi 2022-03-14 18:13:43 UTC
Created attachment 131672 [details] [review]
Bug 30059: Add JS equivalent for Koha::Patron->get_age

On bug 30055 we are going to use the REST API to display the patron
search result, we will then need to calculate patron's age client-side.
This is moved to its own bug report in case we need to reuse it
somewhere else.

Test plan:
Copy/paste the JS function in your browser's console then call it and
confirm that the result is correct
For instance:
  $get_age('2000-01-01')

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2022-03-14 18:13:48 UTC
Created attachment 131673 [details] [review]
Bug 30059: Deal with timezones

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Fridolin Somers 2022-03-22 19:33:35 UTC
Does Bug 30061 have any impact here ?
Comment 14 Tomás Cohen Arazi 2022-03-22 19:44:33 UTC
(In reply to Fridolin Somers from comment #13)
> Does Bug 30061 have any impact here ?

Unrelated.
Comment 15 Nick Clemens 2022-03-24 15:27:14 UTC
Created attachment 132178 [details] [review]
Bug 30059: Add JS equivalent for Koha::Patron->get_age

On bug 30055 we are going to use the REST API to display the patron
search result, we will then need to calculate patron's age client-side.
This is moved to its own bug report in case we need to reuse it
somewhere else.

Test plan:
Copy/paste the JS function in your browser's console then call it and
confirm that the result is correct
For instance:
  $get_age('2000-01-01')

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2022-03-24 15:27:18 UTC
Created attachment 132179 [details] [review]
Bug 30059: Deal with timezones

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Fridolin Somers 2022-03-25 00:25:30 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄