-
Notifications
You must be signed in to change notification settings - Fork 39
Fetch notifications using the current server time #7122
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use a browser extension to set your timezone to be something in the future relative to where you are (using a browser extension like https://change-timezone.pdfwork.com/ (for testing, I used the
Pacific/Apia
locale which has an offset of780
, which is the time zone difference from UTC (in minutes) (with daylight consideration)) - Open the query builder and run a query. After it has been run, click "Export to CSV"
- In
v7
with a time zone set far in the future, you should not receive any notification even after Specify fetches new notifications. When you reload the page, the new notification would be visible then. - In
issue-4566-time
, you should receive a notification once the browser sends a request to fetch notifications. You should be able to either wait for a notification to appear or click on the "Notifications" navigation menu item.
- In
Looks good, I receive the notifications instantly on the issue branch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use a browser extension to set your timezone to be something in the future relative to where you are (using a browser extension like https://change-timezone.pdfwork.com/ (for testing, I used the
Pacific/Apia
locale which has an offset of780
, which is the time zone difference from UTC (in minutes) (with daylight consideration)) - Open the query builder and run a query. After it has been run, click "Export to CSV"
- In
v7
with a time zone set far in the future, you should not receive any notification even after Specify fetches new notifications. When you reload the page, the new notification would be visible then. - In
issue-4566-time
, you should receive a notification once the browser sends a request to fetch notifications. You should be able to either wait for a notification to appear or click on the "Notifications" navigation menu item.
- In
Looks good! I received the notification without needing to refresh.
Triggered by 461a269 on branch refs/heads/issue-4566-time
Triggered by 5bf301f on branch refs/heads/issue-4566-time
Co-authored-by: Copilot <[email protected]>
Triggered by 0935117 on branch refs/heads/issue-4566-time
053c145
to
01c2e83
Compare
Triggered by 06b9d10 on branch refs/heads/issue-4566-time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use a browser extension to set your timezone to be something in the future relative to where you are (using a browser extension like https://change-timezone.pdfwork.com/ (for testing, I used the
Pacific/Apia
locale which has an offset of780
, which is the time zone difference from UTC (in minutes) (with daylight consideration)) - Open the query builder and run a query. After it has been run, click "Export to CSV"
- In
v7
with a time zone set far in the future, you should not receive any notification even after Specify fetches new notifications. When you reload the page, the new notification would be visible then. - In
issue-4566-time
, you should receive a notification once the browser sends a request to fetch notifications. You should be able to either wait for a notification to appear or click on the "Notifications" navigation menu item.
- In
Works as expected!
Triggered by f8ac63a on branch refs/heads/issue-4566-time
Triggered by 328b3d4 on branch refs/heads/issue-4566-time
Fixes #4566
This change fixes an issue where users in timezones with an offset larger than one set for Django (by default,
America/Chicago
) would not receive new notifications due to the fetching logic using the date of the user's browser locale and not the server time (which is used when creating newnotification_message
records), resulting in them not being fetched.This PR adds a new API endpoint to fetch the current server time and integrates it into the notifications polling mechanism to ensure consistent time-based queries.
/context/server_time.json
fetchServerTime
utility to retrieve and parse server timeuseNotificationsFetch
to use server time in its polling URLSince I see you requested a self-review, @melton-jason: please feel free to edit this PR however you see fit!
Checklist
self-explanatory (or properly documented)
Testing instructions
Test the behavior in this PR against the latest tagged release:
Pacific/Apia
locale which has an offset of780
, which is the time zone difference from UTC (in minutes) (with daylight consideration))v7
with a time zone set far in the future, you should not receive any notification even after Specify fetches new notifications. When you reload the page, the new notification would be visible then.issue-4566-time
, you should receive a notification once the browser sends a request to fetch notifications. You should be able to either wait for a notification to appear or click on the "Notifications" navigation menu item.