Skip to content

bug: can't load daily insights #10784

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
Closed
mtojek opened this issue Nov 20, 2023 · 5 comments · Fixed by #10797
Closed

bug: can't load daily insights #10784

mtojek opened this issue Nov 20, 2023 · 5 comments · Fixed by #10797
Assignees
@mafredri @BrunoQuaresma
Labels
s2 Broken use cases or features (with a workaround). Only humans may set this.

Comments

@mtojek
Copy link
Member

mtojek commented Nov 20, 2023

Steps to reproduce

  1. Go to insights.
  2. Select "Daily"
  3. Select a random day range, for example: Nov 1- Nov 20.
  4. Result
Screenshot 2023-11-20 at 10 36 04

I guess that it depends on the user location, but for me backend complains about the future timestamp.

Screenshot 2023-11-20 at 10 47 18
@cdr-bot cdr-bot bot added the bug label Nov 20, 2023
@mtojek mtojek assigned mafredri and BrunoQuaresma and unassigned mafredri Nov 20, 2023
@mafredri
Copy link
Member

Seems the dashboard is requesting the wrong timezone. It's using -02:00 instead of +02:00 for me.

@matifali
Copy link
Member

I noticed this in #10624, but suddenly, it was not reproducible. I guess it happens when 5th November, the DST change date, is within the selected interval.

@matifali matifali added the s2 Broken use cases or features (with a workaround). Only humans may set this. label Nov 20, 2023
@mtojek
Copy link
Member Author

mtojek commented Nov 20, 2023

Also, can we modify the Insights page to show the error somewhere in the UI?

@mafredri
Copy link
Member

mafredri commented Nov 20, 2023 •
edited
Loading

The underlying cause for this bug is the isPositive check added in 645c4bd. The assumption is that positive means we request a positive timezone, but it's actually the other way around, as per:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getTimezoneOffset#negative_values_and_positive_values

@BrunoQuaresma
Copy link
Collaborator

I would rather have the BE taking care of these specific things and rules, is that possible @mafredri ?

@mafredri mafredri mentioned this issue Nov 20, 2023
Merged
mafredri added a commit that referenced this issue Nov 20, 2023
@mafredri
fix(site): correctly interpret timezone based on offset in `formatOff…
6b3f599 
…set` (#10797)

Fixes #10784
ericpaulsen pushed a commit to lbi22/coder that referenced this issue Nov 27, 2023
@mafredri @ericpaulsen
fix(site): correctly interpret timezone based on offset in `formatOff…
e7fe638 
…set` (coder#10797)

Fixes coder#10784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mafredri mafredri

@BrunoQuaresma BrunoQuaresma

Labels
s2 Broken use cases or features (with a workaround). Only humans may set this.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(site): correctly interpret timezone based on offset in formatOffset coder/coder
4 participants
@mafredri @BrunoQuaresma @matifali @mtojek