Skip to content
This repository was archived by the owner on Feb 8, 2023. It is now read-only.

Conversation

@aysiu
Copy link
Contributor

@aysiu aysiu commented Jan 21, 2020

The code as it's written now will never update last_seen unless it's blank. So it will be set once, and then never updated again. That effectively breaks the days_between_notification preference, because the span between last_seen and now will get only bigger as time goes by, and so the notifications will keep popping up every half hour.

This PR eliminates the check for whether the last_seen pref is blank, and just updates last_seen regardless.

Since right before this snippet of code, there is an exit 0 if the days_between_notifications setting is respected, last_seen will not be updated if it's been less than the number of days between notifications.

@aysiu aysiu requested a review from erikng January 22, 2020 03:10
@aysiu
Copy link
Contributor Author

aysiu commented Jan 22, 2020

In looking at the code again, I'm actually thinking it'd be better to have last_seen get set here:
https://github.com/erikng/nudge/blob/master/payload/Library/nudge/Resources/nudge#L691

That's when the actual notification pops up.

Based on my testing, this seems the best place to put it.

@aysiu
Copy link
Contributor Author

aysiu commented Jan 22, 2020

Closing for now. Not 100% certain this fixes the issue. Have to do more testing and debugging.

@aysiu aysiu closed this Jan 22, 2020
@aysiu aysiu deleted the patch-3 branch March 12, 2020 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant