Moved HUI client over from kat-usage-reports
#9
Conversation
…te so that whatever is passed to that parameter cannot be mutated. Also now usage momentJS to format the date for use in the query string.
| // moments are mutable. | ||
| const adjustedStartDate = startDate.clone().startOf(aggregation === 'week' ? 'isoweek' : aggregation).subtract(1, aggregation); | ||
|
|
||
| const dateFormat = aggregation === 'week' ? '[wk]W/GGGG' : '[m]M/GGGG'; |
There was a problem hiding this comment.
This looks good but, previously, the week start/end dates were created based on isoWeek (and year). Not sure if this results in the same dates.
https://github.com/Financial-Times/kat-usage-report/blob/master/server/lib/hui.js#L66
There was a problem hiding this comment.
Yeah, saw that as well. This code uses the ISO Week (the capital W in the dateFormat) and Year (GGGG), as per the MomentJS docs: https://momentjs.com/docs/#/displaying/, so it should be ok.
lib/huiClient.js
Outdated
| @@ -0,0 +1,47 @@ | |||
| const log = require('@financial-times/n-logger').default; | |||
|
|
|||
| const huiBasePath = process.env.HUI_BASE_PATH; | |||
There was a problem hiding this comment.
Maybe get these vars from the config.
|
|
||
| log.debug({operation: 'getUsage', licenceId, aggregation, start, end, url}); | ||
|
|
||
| return fetch(url, options) |
There was a problem hiding this comment.
All the other client proxies use fetch-retry-or-die as fetch. This doesn't seem like it does.
lib/huiClient.js
Outdated
|
|
||
| const huiBasePath = process.env.HUI_BASE_PATH; | ||
| const licencePath = '/licences/'; | ||
| const options = { |
There was a problem hiding this comment.
If you're going to use fetch-retry-or-die (as mentioned below). You might want to extend config.fetchOptions (as all the other client proxies do - https://github.com/Financial-Times/kat-client-proxies/blob/master/lib/accessLicenceClient.js#L11)
lib/huiClient.js
Outdated
| log.debug({operation: 'getUsage', licenceId, aggregation, start, end, url}); | ||
|
|
||
| return fetch(url, options) | ||
| .then((data) => data.json()) |
There was a problem hiding this comment.
The other proxies use helpers.parseJsonRes in order to parse the response.
No description provided.