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.

Sign up for GitHub

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

Jump to bottom

Time Range: Using relative time takes timezone into account #74013

Merged
merged 18 commits into from
Sep 4, 2023
Merged

Time Range: Using relative time takes timezone into account #74013

merged 18 commits into from
Sep 4, 2023

Conversation

ashharrison90
Copy link
Contributor

@ashharrison90 ashharrison90 commented Aug 29, 2023 •
edited
Loading

What is this feature?

  • handles timezone correctly in dateTimeForTimeZone
  • passes the timezone through correctly in applyPanelTimeOverrides
  • adds an e2e selector for the tooltip container
  • adds an e2e selector for the table body
  • adds a dev dashboard and e2e test to prevent regressing this behaviour

Why do we need this feature?

  • so timezones are properly handled when using relative time overrides

Who is this feature for?

  • everyone! 🙌

Which issue(s) does this PR fix?:

Fixes #59665
FIxes https://github.com/grafana/support-escalations/issues/7227

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@ashharrison90
account for timezone when using relative times
01a3078 
Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>
@ashharrison90
keep as it was originally for now
295125e
@ashharrison90
add e2e test for relative time zone overrides
e869656
@ashharrison90 ashharrison90 added type/bug add to changelog mob session Something solved during a mob session no-backport Skip backport of PR labels Aug 29, 2023
@ashharrison90 ashharrison90 self-assigned this Aug 29, 2023
@ashharrison90 ashharrison90 requested review from grafanabot and a team as code owners August 29, 2023 13:45
@ashharrison90 ashharrison90 requested review from dprokop, ivanortegaalba, joshhunt, L-M-K-B and mckn and removed request for a team August 29, 2023 13:45
@ashharrison90 @eledobleefe
@JoaoSilvaGrafana @L-M-K-B @joshhunt @tskarhed
empty commit to add coauthors
f74bd3a 
Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.2.x milestone Aug 29, 2023
L-M-K-B
L-M-K-B approved these changes Aug 29, 2023
View reviewed changes
Copy link
Contributor

@L-M-K-B L-M-K-B left a comment

Choose a reason for hiding this comment

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

Great mob session and thanks for adding the e2e test. 🤩

@ashharrison90 ashharrison90 requested review from a team and removed request for a team August 29, 2023 15:18
mckn
mckn approved these changes Aug 31, 2023
View reviewed changes
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

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

LGTM!

@ashharrison90 ashharrison90 merged commit e2724f3 into main Sep 4, 2023
18 checks passed
@ashharrison90 ashharrison90 deleted the ash/59665 branch September 4, 2023 09:38
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@ashharrison90 @eledobleefe
@JoaoSilvaGrafana @L-M-K-B @joshhunt @tskarhed @chauchausoup
Time Range: Using relative time takes timezone into account (grafana#…
c85b79c 
…74013)

* account for timezone when using relative times
Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>

* keep as it was originally for now

* add e2e test for relative time zone overrides

* empty commit to add coauthors

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>

* fix types

* switch to using table panel in e2e test

* use regex for partial text match

* actually go to the dashboard...

* use include.text

* check for visibility first

* try waiting on backend request to complete

* CI driven development is fun

* make sure we're waiting for both data query calls

* open dashboard instead

* kick drone

* Revert "open dashboard instead"

This reverts commit bab9c77.

* check timezone second

* refactor to avoid detached elements

---------

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>
rwwiv pushed a commit that referenced this pull request Oct 2, 2023
@ashharrison90 @eledobleefe
@JoaoSilvaGrafana @L-M-K-B @joshhunt @tskarhed @rwwiv
Time Range: Using relative time takes timezone into account (#74013)
1cb710a 
* account for timezone when using relative times
Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>

* keep as it was originally for now

* add e2e test for relative time zone overrides

* empty commit to add coauthors

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>

* fix types

* switch to using table panel in e2e test

* use regex for partial text match

* actually go to the dashboard...

* use include.text

* check for visibility first

* try waiting on backend request to complete

* CI driven development is fun

* make sure we're waiting for both data query calls

* open dashboard instead

* kick drone

* Revert "open dashboard instead"

This reverts commit bab9c77.

* check timezone second

* refactor to avoid detached elements

---------

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Joao Silva <joao.silva@grafana.com>
Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: tskarhed <1438972+tskarhed@users.noreply.github.com>
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
@dnhn dnhn mentioned this pull request Oct 24, 2023
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mckn mckn mckn approved these changes

@L-M-K-B L-M-K-B L-M-K-B approved these changes

@grafanabot grafanabot Awaiting requested review from grafanabot

@dprokop dprokop Awaiting requested review from dprokop dprokop is a code owner automatically assigned from grafana/dashboards-squad

@ivanortegaalba ivanortegaalba Awaiting requested review from ivanortegaalba ivanortegaalba is a code owner automatically assigned from grafana/dashboards-squad

@joshhunt joshhunt Awaiting requested review from joshhunt joshhunt is a code owner automatically assigned from grafana/grafana-frontend-platform

@mdvictor mdvictor Awaiting requested review from mdvictor

@sjd210 sjd210 Awaiting requested review from sjd210

Assignees

@ashharrison90 ashharrison90

Labels
add to changelog area/frontend mob session Something solved during a mob session no-backport Skip backport of PR type/bug
Projects
None yet
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

Timerange: Using "relative time" does not take timezone into account
4 participants
@ashharrison90 @mckn @L-M-K-B @zerok