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

Call check-in route when popup is opened #37

Merged
merged 1 commit into from
May 8, 2024
Merged

Call check-in route when popup is opened #37

merged 1 commit into from
May 8, 2024

Conversation

jdgarcia
Copy link
Contributor

@jdgarcia jdgarcia commented May 3, 2024

I know this PR conflicts with the event PR but it's a one line conflict, I'd rather avoid starting a PR train.

The route isn't live yet, I'll just hold off on merging this until after it is.

Copy link
Collaborator

@miggy-e miggy-e left a comment

Choose a reason for hiding this comment

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

please update the checkin date tester before merging

Comment on lines +50 to +53
// Don't check in more than once every 12 hours to reduce amount of requests
const { lastCheckin } = await storage.local.get('lastCheckin');
if (lastCheckin && Date.now() - lastCheckin < 1000 * 60 * 60) {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

1: I think this currently checks every hour
2: is there a reason this is different than the gkdev checkin that's once every 24 hours?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.
And I went with 12 instead of 24 for cases such as using it at 10am the first day and 9am the next.

@jdgarcia jdgarcia merged commit 3877545 into main May 8, 2024
1 check passed
@jdgarcia jdgarcia deleted the checkin branch May 8, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants