github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-05-07 03:58:09 -05:00
Code Issues 690 Packages Projects Releases 11 Wiki Activity

[GH-ISSUE #1550] Reminders wrongly get ignored when multiple ones exist in a 26-hour window #6442

New Issue
Closed
opened 2026-04-20 17:02:37 -05:00 by GiteaMirror · 3 comments
GiteaMirror commented 2026-04-20 17:02:37 -05:00
Owner
Copy Link

Originally created by @shibijm on GitHub (Sep 26, 2025).
Original GitHub issue: https://github.com/go-vikunja/vikunja/issues/1550

Description

When a task's reminders are being evaluated, if more than one reminder exists for that task between now-12h and now+1m+14h, only the first reminder from the database is considered, and a flag is set to not consider further reminders, regardless of the validity of the first reminder. If there are subsequent reminders that are valid and due to be sent, they get wrongly ignored.

Scenario:

  • It is currently 5 PM.
  • Two reminders are set for one of your tasks: one at 9 AM (in the past) and one at 5 PM (due now).
  • getTasksWithRemindersDueAndTheirUsers runs at 5 PM through the scheduled cron function that runs every minute.
  • Reminders that exists between now-12h and now+1m+14h are fetched from the database, so both the reminders mentioned above are included in this 26-hour window.
  • While looping through the reminders, the 9 AM reminder is getting processed first since it happens to be the first one that was inserted into the database.
  • While the inner user loop is running, seen[r.TaskID][u.User.ID] gets set to true for the above-mentioned task and your user.
  • After that, an if statement checks whether the 9 AM reminder is within the current 1-minute window. 9 AM is not within the 1-minute window at 5 PM, so it gets ignored correctly.
  • The loop is now processing the 5 PM reminder. However, the key for the above task and user has already been set in the seen map because of the previous 9 AM reminder, despite it being out-of-range. Because of this, the 5 PM reminder doesn't get checked and hence gets dropped silently, even though a notification for it is supposed to be sent right now.

Steps to reproduce:

To reproduce this issue easily, you can create a task and set a reminder at each minute in the next 5 minutes. After those 5 minutes, you would have received only 1 notification for only the 1st reminder that was created, instead of all 5 of them.

Fix:

This issue can be fixed by moving the seen[r.TaskID][u.User.ID] = true line to be inside the if block that checks whether the reminder is within the current 1-minute window. That way, as intended, the relevant taskID+userID combo in the seen map will exist only if a previously-processed reminder was actually in-range and eligible for being sent now.

Alternative fix through other improvements:

I might be missing something or misunderstanding some intended logic so correct me if I'm wrong:

The reminder's time instant stored in the database and parsed by the backend, regardless of its timezone or the server's timezone, already correctly matches the time instant represented by the local time inputted by the user on the frontend, so all the timezone-related code in this function seems unnecessary.

In fact, the timezone-related code doesn't seem to be affecting the behaviour of this function at all:
The tz in the function is being used only once for the actualReminder := r.Reminder.In(tz) line, but this does not affect any further logic, because as mentioned in Go's documentation for time.Time.In, this just sets the timezone/location for display purposes so it doesn't change the underlying time value. Using r.Reminder instead of actualReminder for the 1-minute window check yields the same result.

From Go's docs:

In returns a copy of t representing the same time instant, but with the copy's location information set to loc for display purposes.

Each time has an associated Location. The methods Time.Local, Time.UTC, and Time.In return a Time with a specific Location. Changing the Location of a Time value with these methods does not change the actual instant it represents, only the time zone in which to interpret it.

Removing all the timezone-related code from this function would not affect the current behaviour of it, and the database can be queried for only reminders in the current 1-minute window instead of a 26-hour window. Through this change, in addition to unnecessary overhead getting removed, the main issue of in-range reminders getting ignored will also be fixed.

Vikunja Version

v1.0.0-rc2

Browser and version

Not applicable

Can you reproduce the bug on the Vikunja demo site?

Reminder notifications don't seem to work at all on the demo site

Screenshots

Not applicable

Originally created by @shibijm on GitHub (Sep 26, 2025). Original GitHub issue: https://github.com/go-vikunja/vikunja/issues/1550 ### Description When a task's reminders are being evaluated, if more than one reminder exists for that task between now-12h and now+1m+14h, only the first reminder from the database is considered, and a flag is set to not consider further reminders, regardless of the validity of the first reminder. If there are subsequent reminders that are valid and due to be sent, they get wrongly ignored. #### Scenario: - It is currently 5 PM. - Two reminders are set for one of your tasks: one at 9 AM (in the past) and one at 5 PM (due now). - [`getTasksWithRemindersDueAndTheirUsers`](https://github.com/go-vikunja/vikunja/blob/48d202f3ced4645fe49f20278afe3a48fa4a259e/pkg/models/task_reminder.go#L163) runs at 5 PM through the scheduled cron function that runs every minute. - Reminders that exists between now-12h and now+1m+14h are fetched from the database, so both the reminders mentioned above are included in this 26-hour window. - While [looping through the reminders](https://github.com/go-vikunja/vikunja/blob/48d202f3ced4645fe49f20278afe3a48fa4a259e/pkg/models/task_reminder.go#L217), the 9 AM reminder is getting processed first since it happens to be the first one that was inserted into the database. - While the inner user loop is running, [`seen[r.TaskID][u.User.ID]` gets set to `true`](https://github.com/go-vikunja/vikunja/blob/48d202f3ced4645fe49f20278afe3a48fa4a259e/pkg/models/task_reminder.go#L230) for the above-mentioned task and your user. - After that, an [`if` statement](https://github.com/go-vikunja/vikunja/blob/48d202f3ced4645fe49f20278afe3a48fa4a259e/pkg/models/task_reminder.go#L247) checks whether the 9 AM reminder is within the current 1-minute window. 9 AM is not within the 1-minute window at 5 PM, so it gets ignored correctly. - The loop is now processing the 5 PM reminder. However, the key for the above task and user has already been set in the `seen` map because of the previous 9 AM reminder, despite it being out-of-range. Because of this, the 5 PM reminder [doesn't get checked](https://github.com/go-vikunja/vikunja/blob/48d202f3ced4645fe49f20278afe3a48fa4a259e/pkg/models/task_reminder.go#L227) and hence gets dropped silently, even though a notification for it is supposed to be sent right now. #### Steps to reproduce: To reproduce this issue easily, you can create a task and set a reminder at each minute in the next 5 minutes. After those 5 minutes, you would have received only 1 notification for only the 1st reminder that was created, instead of all 5 of them. #### Fix: This issue can be fixed by moving the `seen[r.TaskID][u.User.ID] = true` line to be inside the `if` block that checks whether the reminder is within the current 1-minute window. That way, as intended, the relevant taskID+userID combo in the `seen` map will exist only if a previously-processed reminder was actually in-range and eligible for being sent now. #### Alternative fix through other improvements: I might be missing something or misunderstanding some intended logic so correct me if I'm wrong: The reminder's time instant stored in the database and parsed by the backend, regardless of its timezone or the server's timezone, already correctly matches the time instant represented by the local time inputted by the user on the frontend, so all the timezone-related code in this function seems unnecessary. In fact, the timezone-related code doesn't seem to be affecting the behaviour of this function at all: The `tz` in the function is being used only once for the `actualReminder := r.Reminder.In(tz)` line, but this does not affect any further logic, because as mentioned in [Go's documentation for `time.Time.In`](https://pkg.go.dev/time#Time.In), this just sets the timezone/location for display purposes so it doesn't change the underlying time value. Using `r.Reminder` instead of `actualReminder` for the 1-minute window check yields the same result. From Go's docs: > In returns a copy of t representing the same time instant, but with the copy's location information set to loc for display purposes. > Each time has an associated Location. The methods Time.Local, Time.UTC, and Time.In return a Time with a specific Location. Changing the Location of a Time value with these methods does not change the actual instant it represents, only the time zone in which to interpret it. Removing all the timezone-related code from this function would not affect the current behaviour of it, and the database can be queried for only reminders in the current 1-minute window instead of a 26-hour window. Through this change, in addition to unnecessary overhead getting removed, the main issue of in-range reminders getting ignored will also be fixed. ### Vikunja Version v1.0.0-rc2 ### Browser and version Not applicable ### Can you reproduce the bug on the Vikunja demo site? Reminder notifications don't seem to work at all on the demo site ### Screenshots Not applicable
GiteaMirror commented 2026-04-20 17:02:38 -05:00
Author
Owner
Copy Link

@kolaente commented on GitHub (Sep 29, 2025):

Thanks for the issue and the detailed analysis. I've opened a draft PR with a fix here: https://github.com/go-vikunja/vikunja/pull/1564

Removing all the timezone-related code from this function would not affect the current behaviour of it, and the database can be queried for only reminders in the current 1-minute window instead of a 26-hour window. Through this change, in addition to unnecessary overhead getting removed, the main issue of in-range reminders getting ignored will also be fixed.

The goal with this logic is to allow users to receive a reminder in their time zone. This means if I set a reminder for 9 AM, every user will get it at 9 AM in their time zone, regardless of what that is.

<!-- gh-comment-id:3345702879 --> @kolaente commented on GitHub (Sep 29, 2025): Thanks for the issue and the detailed analysis. I've opened a draft PR with a fix here: https://github.com/go-vikunja/vikunja/pull/1564 > Removing all the timezone-related code from this function would not affect the current behaviour of it, and the database can be queried for only reminders in the current 1-minute window instead of a 26-hour window. Through this change, in addition to unnecessary overhead getting removed, the main issue of in-range reminders getting ignored will also be fixed. The goal with this logic is to allow users to receive a reminder in their time zone. This means if I set a reminder for 9 AM, every user will get it at 9 AM in their time zone, regardless of what that is.
GiteaMirror commented 2026-04-20 17:02:38 -05:00
Author
Owner
Copy Link

@shibijm commented on GitHub (Sep 29, 2025):

Thank you for the quick fix!

The goal with this logic is to allow users to receive a reminder in their time zone. This means if I set a reminder for 9 AM, every user will get it at 9 AM in their time zone, regardless of what that is.

Hmm, that's not the current behaviour of the function, and I don't think it can be made to work that way without bigger changes.

The reminder field in the task_reminders table is a specific point in time. If my computer's/browser's timezone is UTC+05:30 and I set a reminder at 9 AM, it gets stored in the table as 03:30 UTC (the frontend also transmits it to the backend in UTC). From that UTC time, for the goal you mentioned, we need to determine different points in time that correspond to 9 AM in different timezones. To do that, we need the timezone of the user who created the reminder, to calculate the offsets of other users' timezones relative to that. These offsets can then be applied to the reminder by changing the underlying time value to determine different points in time that match 9 AM in different users' timezones.

The required timezone of the user who created the reminder is not being stored in that table. The ID of the user who created the reminder isn't being stored either, so you can't get the required timezone from the users table. If it's meant to be based on the timezone of the user who created the task, then the created_by_id field in the tasks table can be used to join with users, but nothing like that is being done for this purpose in that function. The frontend would also have to be changed to reflect the reminder's time properly, since currently it displays the local time without any offsets, directly corresponding to the UTC time stored in the database.

I also think that the intended behaviour (which isn't currently the case) might not be desirable for many use cases. For example, if a reminder is set for a specific event that's taking place at a specific time, like an online meeting, that reminder should be sent at that exact specific time for all users without considering their timezones.

Regardless of the intended behaviour, what I was saying in the main issue post is that the current timezone logic in the function is redundant because it's not affecting any behaviour at all. See the example below:

https://go.dev/play/p/FzDT_8gRGVh

Code

package main

import (
	"fmt"
	"time"
)

func main() {
	now := time.Date(2025, 8, 1, 3, 30, 0, 0, time.UTC)

	// Assuming that a reminder fetched from the database is
	// due right now, we'll make it the same as `now`
	reminder := now
	fmt.Println("Reminder:", reminder)

	// UTC+05:30
	user1Tz, _ := time.LoadLocation("Asia/Kolkata")
	user1Reminder := reminder.In(user1Tz)
	fmt.Println("User1 Reminder:", user1Reminder)

	// UTC+09:00
	user2Tz, _ := time.LoadLocation("Asia/Tokyo")
	user2Reminder := reminder.In(user2Tz)
	fmt.Println("User2 Reminder:", user2Reminder)

	// It's apparent from the result of the below loop block
	// that `actualReminder` / `reminder.In(tz)` does not affect
	// any logic at all, it's the same as using `reminder`
	for _, actualReminder := range []time.Time{reminder, user1Reminder, user2Reminder} {
		if (actualReminder.After(now) && actualReminder.Before(now.Add(time.Minute))) || actualReminder.Equal(now) {
			fmt.Println("Check passed for actualReminder", actualReminder)
		}
	}

	// These are all equal, Go's `time.Time.In` doesn't change the underlying time value
	fmt.Println(reminder.Equal(user1Reminder) && reminder.Equal(user2Reminder) && user1Reminder.Equal(user2Reminder))
}

Output

Reminder: 2025-08-01 03:30:00 +0000 UTC
User1 Reminder: 2025-08-01 09:00:00 +0530 IST
User2 Reminder: 2025-08-01 12:30:00 +0900 JST
Check passed for actualReminder 2025-08-01 03:30:00 +0000 UTC
Check passed for actualReminder 2025-08-01 09:00:00 +0530 IST
Check passed for actualReminder 2025-08-01 12:30:00 +0900 JST
true
<!-- gh-comment-id:3348512017 --> @shibijm commented on GitHub (Sep 29, 2025): Thank you for the quick fix! > The goal with this logic is to allow users to receive a reminder in their time zone. This means if I set a reminder for 9 AM, every user will get it at 9 AM in their time zone, regardless of what that is. Hmm, that's not the current behaviour of the function, and I don't think it can be made to work that way without bigger changes. The `reminder` field in the `task_reminders` table is a specific point in time. If my computer's/browser's timezone is UTC+05:30 and I set a reminder at 9 AM, it gets stored in the table as 03:30 UTC (the frontend also transmits it to the backend in UTC). From that UTC time, for the goal you mentioned, we need to determine different points in time that correspond to 9 AM in different timezones. To do that, we need the timezone of the user who created the reminder, to calculate the offsets of other users' timezones relative to that. These offsets can then be applied to the reminder by changing the underlying time value to determine different points in time that match 9 AM in different users' timezones. The required timezone of the user who created the reminder is not being stored in that table. The ID of the user who created the reminder isn't being stored either, so you can't get the required timezone from the `users` table. If it's meant to be based on the timezone of the user who created the task, then the `created_by_id` field in the `tasks` table can be used to join with `users`, but nothing like that is being done for this purpose in that function. The frontend would also have to be changed to reflect the reminder's time properly, since currently it displays the local time without any offsets, directly corresponding to the UTC time stored in the database. I also think that the intended behaviour (which isn't currently the case) might not be desirable for many use cases. For example, if a reminder is set for a specific event that's taking place at a specific time, like an online meeting, that reminder should be sent at that exact specific time for all users without considering their timezones. Regardless of the intended behaviour, what I was saying in the main issue post is that the current timezone logic in the function is redundant because it's not affecting any behaviour at all. See the example below: https://go.dev/play/p/FzDT_8gRGVh #### Code ```go package main import ( "fmt" "time" ) func main() { now := time.Date(2025, 8, 1, 3, 30, 0, 0, time.UTC) // Assuming that a reminder fetched from the database is // due right now, we'll make it the same as `now` reminder := now fmt.Println("Reminder:", reminder) // UTC+05:30 user1Tz, _ := time.LoadLocation("Asia/Kolkata") user1Reminder := reminder.In(user1Tz) fmt.Println("User1 Reminder:", user1Reminder) // UTC+09:00 user2Tz, _ := time.LoadLocation("Asia/Tokyo") user2Reminder := reminder.In(user2Tz) fmt.Println("User2 Reminder:", user2Reminder) // It's apparent from the result of the below loop block // that `actualReminder` / `reminder.In(tz)` does not affect // any logic at all, it's the same as using `reminder` for _, actualReminder := range []time.Time{reminder, user1Reminder, user2Reminder} { if (actualReminder.After(now) && actualReminder.Before(now.Add(time.Minute))) || actualReminder.Equal(now) { fmt.Println("Check passed for actualReminder", actualReminder) } } // These are all equal, Go's `time.Time.In` doesn't change the underlying time value fmt.Println(reminder.Equal(user1Reminder) && reminder.Equal(user2Reminder) && user1Reminder.Equal(user2Reminder)) } ``` #### Output ```text Reminder: 2025-08-01 03:30:00 +0000 UTC User1 Reminder: 2025-08-01 09:00:00 +0530 IST User2 Reminder: 2025-08-01 12:30:00 +0900 JST Check passed for actualReminder 2025-08-01 03:30:00 +0000 UTC Check passed for actualReminder 2025-08-01 09:00:00 +0530 IST Check passed for actualReminder 2025-08-01 12:30:00 +0900 JST true ```
GiteaMirror commented 2026-04-20 17:02:39 -05:00
Author
Owner
Copy Link

@kolaente commented on GitHub (Oct 1, 2025):

Thanks for the explanation! It might be that I missed that - at least what I was wanting to achieve with the feature is what I outlined above. The feature was also one of the first in Vikunja, very likely there are bugs with it.

Note that the idea was for every user who gets the reminder to have it in their time zone, not in the one of the user who created the reminder.

Another use case I want is (and that is working right now) to incorporate daylight savings, especially for repeating tasks. When I set a task to repeat weekly and it has a reminder for 9:00, I want that to happen every time at 9:00.

I think there are a few ways to solve this:

  • Store the time zone of the reminder with the date (and all other dates) and only use that. This would mean to expose this somewhere in the UI, and only use the user's time zone from the settings only as fallback or default. IIRC Todoist does that.
  • Store dates without a time zone and figure out at run time if it is currently that time in the user's time zone. This would come closer to what I wanted to achieve originally.

But you bring up a valid point: if there is a deadline at a certain time, I want that to be the same point in time everywhere, regardless of the time zone.

And the use case "multiple people work on the same thing in different time zones" is probably an edge case anyway. What isn't, are daylight savings, so that should be something we need to pay attention to anyways.

<!-- gh-comment-id:3355959087 --> @kolaente commented on GitHub (Oct 1, 2025): Thanks for the explanation! It might be that I missed that - at least what I was wanting to achieve with the feature is what I outlined above. The feature was also one of the first in Vikunja, very likely there are bugs with it. Note that the idea was _for every user who gets the reminder_ to have it in their time zone, not in the one of the user who created the reminder. Another use case I want is (and that is working right now) to incorporate daylight savings, especially for repeating tasks. When I set a task to repeat weekly and it has a reminder for 9:00, I want that to happen every time at 9:00. I think there are a few ways to solve this: - Store the time zone of the reminder with the date (and all other dates) and only use that. This would mean to expose this somewhere in the UI, and only use the user's time zone from the settings only as fallback or default. IIRC Todoist does that. - Store dates without a time zone and figure out at run time if it is currently that time in the user's time zone. This would come closer to what I wanted to achieve originally. But you bring up a valid point: if there is a deadline at a certain time, I want that to be the same point in time everywhere, regardless of the time zone. And the use case "multiple people work on the same thing in different time zones" is probably an edge case anyway. What isn't, are daylight savings, so that should be something we need to pay attention to anyways.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#6442
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?