Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle client count timezone #15167

Merged
merged 2 commits into from
May 20, 2022
Merged

Handle client count timezone #15167

merged 2 commits into from
May 20, 2022

Conversation

arnav28
Copy link
Contributor

@arnav28 arnav28 commented Apr 25, 2022

  • Backend convert the timezone to UTC, to mitigate it's impact sending
    start and end date other than 1. Chose 10 and 20 randomly.

- Backend convert the timezone to UTC, to mitigate it's impact sending
  start and end date other than 1. Chose 10 and 20 randomly.
@arnav28 arnav28 added the ui label Apr 25, 2022
}
if (end_time) {
if (Array.isArray(end_time)) {
let endYear = Number(end_time[0]);
let endMonth = Number(end_time[1]);
end_time = formatRFC3339(new Date(endYear, endMonth));
end_time = formatRFC3339(new Date(endYear, endMonth, 20));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing did the API always return with the last day of the month? Might be worth having backend take a look (or maybe you already have) to see if sending an arbitrary middle of the month day has any implications we're unaware of.

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my original question still applies here - I think a backend dev reviewing would be helpful! Just to make sure this won't introduce anything unexpected. Otherwise thanks for tackling! Nice to have this finally merged :)

@arnav28
Copy link
Contributor Author

arnav28 commented May 20, 2022

I think my original question still applies here - I think a backend dev reviewing would be helpful! Just to make sure this won't introduce anything unexpected. Otherwise thanks for tackling! Nice to have this finally merged :)

Yes, confirmed with backend that they have startOfMonth, endOfMonth for start and end params. I tested it as well. 👍
Sorry forgot to mention during our discussion.

@arnav28 arnav28 merged commit 2ceb100 into main May 20, 2022
arnav28 added a commit that referenced this pull request Jun 1, 2022
Client count went through major refactoring in 1.10 and as a result
straightforward backport was not possible.
arnav28 added a commit that referenced this pull request Jun 1, 2022
* This is the backport for #15167
Client count went through major refactoring in 1.10 and as a result
straightforward backport was not possible.

* Added changelog

* Fix test
Gabrielopesantos pushed a commit to Gabrielopesantos/vault that referenced this pull request Jun 6, 2022
* Handle client count timezone

- Backend convert the timezone to UTC, to mitigate it's impact sending
  start and end date other than 1. Chose 10 and 20 randomly.

* Added changelog
@arnav28 arnav28 deleted the client-count-timezone-fix branch June 15, 2022 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants