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

Custom reports: show server hours in site's timezone #16565

Merged
merged 2 commits into from Oct 19, 2020
Merged

Custom reports: show server hours in site's timezone #16565

merged 2 commits into from Oct 19, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Oct 13, 2020

fix dev-2020

Wasn't sure where to put the method. Initially had it in the formatter but it's not formatting the hour it is converting. Also re the behaviour with period/date being used it might be actually ok to put this into a dimension class.

Once merged I'd also adjust eg in Media Analytics

@tsteur tsteur added the Needs Review PRs that need a code review label Oct 13, 2020
@tsteur tsteur added this to the 4.0.0-RC milestone Oct 13, 2020
@tsteur tsteur changed the title Convert visit hour to site timezone Custom reports: show hours in site's timezone Oct 13, 2020
@tsteur tsteur changed the title Custom reports: show hours in site's timezone Custom reports: show server hours in site's timezone Oct 13, 2020
$siteSearch = 1, $searchKeywordParameters = null,
$searchCategoryParameters = null, $timezone = 'Pacific/Auckland');
$hourConverted = VisitLastActionTime::convertHourToHourInSiteTimezone(5, $idSite);
$this->assertEquals(18, $hourConverted);
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity: Will this test fail when the time is changed due to daylight savings? Guess Auckland has summer/winter time as well, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it will. I reckon it'll be easy to change though then. Do you maybe know a timezone that doesn't have daylight savings? happy to change it

Copy link
Member

Choose a reason for hiding this comment

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

Asia shouldn't have daylight saving afaik. Guess you could use eg Asia/Jakarta (UTC+7)

} catch (\Exception $e) {
// ignore any error eg if values are wrong...
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is it needed to use the date/period if we're just getting the hour later? Could we just use a dummy date or today()?

Copy link
Member Author

Choose a reason for hiding this comment

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

@diosmosis We'll try to use the date in case of daylight savings to make sure it is being considered. If we just used today then it wouldn't always be correct (see method description)

Copy link
Member

Choose a reason for hiding this comment

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

I see, that's unfortunate.

@tsteur tsteur merged commit 2dfee2f into 4.x-dev Oct 19, 2020
@tsteur tsteur deleted the dev2020 branch October 19, 2020 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants