From 37a04db82e1155dfe29f316fa3cdc55f04219258 Mon Sep 17 00:00:00 2001 From: Trey t Date: Mon, 22 Dec 2025 14:47:37 -0600 Subject: [PATCH] Fix notification queries to exclude tasks from inactive residences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - task_repo.go: Add is_active filter to residence subquery in UserIDs filter - handler.go: Add is_active filter to daily digest residence join - onboarding_email_service.go: Fix Django table names and task status filter - task_repo_test.go: Add regression tests for inactive residence filtering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/repositories/task_repo.go | 4 +- internal/repositories/task_repo_test.go | 112 ++++++++++++++++++ internal/services/onboarding_email_service.go | 16 +-- internal/worker/jobs/handler.go | 4 +- 4 files changed, 124 insertions(+), 12 deletions(-) diff --git a/internal/repositories/task_repo.go b/internal/repositories/task_repo.go index 0d6b0b1..6335446 100644 --- a/internal/repositories/task_repo.go +++ b/internal/repositories/task_repo.go @@ -62,9 +62,9 @@ func (r *TaskRepository) applyFilterOptions(query *gorm.DB, opts TaskFilterOptio } else if len(opts.ResidenceIDs) > 0 { query = query.Where("task_task.residence_id IN ?", opts.ResidenceIDs) } else if len(opts.UserIDs) > 0 { - // For notifications: tasks assigned to users OR owned by users + // For notifications: tasks assigned to users OR owned by users (only from active residences) query = query.Where( - "(task_task.assigned_to_id IN ? OR task_task.residence_id IN (SELECT id FROM residence_residence WHERE owner_id IN ?))", + "(task_task.assigned_to_id IN ? OR task_task.residence_id IN (SELECT id FROM residence_residence WHERE owner_id IN ? AND is_active = true))", opts.UserIDs, opts.UserIDs, ) } diff --git a/internal/repositories/task_repo_test.go b/internal/repositories/task_repo_test.go index 71f5cc1..3e4b3ff 100644 --- a/internal/repositories/task_repo_test.go +++ b/internal/repositories/task_repo_test.go @@ -1303,6 +1303,118 @@ func TestTaskFilterOptions_UserIDs(t *testing.T) { assert.Equal(t, "User1 Overdue", tasks[0].Title) } +// TestTaskFilterOptions_UserIDs_ExcludesInactiveResidences verifies that tasks +// from inactive residences are excluded when filtering by user IDs. +// This is a regression test for the bug where users received notifications +// for tasks on residences they had deactivated. +func TestTaskFilterOptions_UserIDs_ExcludesInactiveResidences(t *testing.T) { + db := testutil.SetupTestDB(t) + repo := NewTaskRepository(db) + + user := testutil.CreateTestUser(t, db, "owner", "owner@test.com", "password") + + // Create an active residence + activeResidence := testutil.CreateTestResidence(t, db, user.ID, "Active House") + + // Create an inactive residence (manually, since testutil always creates active) + // Note: Must update after create because GORM's default:true overrides struct value + inactiveResidence := &models.Residence{ + OwnerID: user.ID, + Name: "Inactive House", + StreetAddress: "456 Inactive St", + City: "Test City", + StateProvince: "TS", + PostalCode: "12345", + Country: "USA", + IsPrimary: false, + } + err := db.Create(inactiveResidence).Error + require.NoError(t, err) + // Set to inactive after creation to bypass GORM default + db.Model(inactiveResidence).Update("is_active", false) + + now := time.Now().UTC() + pastDue := now.AddDate(0, 0, -5) + + // Create overdue tasks in both residences + db.Create(&models.Task{ + ResidenceID: activeResidence.ID, + CreatedByID: user.ID, + Title: "Overdue in Active Residence", + DueDate: &pastDue, + }) + db.Create(&models.Task{ + ResidenceID: inactiveResidence.ID, + CreatedByID: user.ID, + Title: "Overdue in Inactive Residence", + DueDate: &pastDue, + }) + + // Filter by user ID (used by notification system) + opts := TaskFilterOptions{UserIDs: []uint{user.ID}, IncludeInProgress: true} + tasks, err := repo.GetOverdueTasks(now, opts) + require.NoError(t, err) + + // Should only return task from active residence + assert.Len(t, tasks, 1, "Should only return tasks from active residences") + assert.Equal(t, "Overdue in Active Residence", tasks[0].Title) +} + +// TestTaskFilterOptions_UserIDs_DueSoon_ExcludesInactiveResidences verifies that +// due soon tasks from inactive residences are excluded when filtering by user IDs. +func TestTaskFilterOptions_UserIDs_DueSoon_ExcludesInactiveResidences(t *testing.T) { + db := testutil.SetupTestDB(t) + repo := NewTaskRepository(db) + + user := testutil.CreateTestUser(t, db, "owner", "owner@test.com", "password") + + // Create an active residence + activeResidence := testutil.CreateTestResidence(t, db, user.ID, "Active House") + + // Create an inactive residence + // Note: Must update after create because GORM's default:true overrides struct value + inactiveResidence := &models.Residence{ + OwnerID: user.ID, + Name: "Inactive House", + StreetAddress: "456 Inactive St", + City: "Test City", + StateProvince: "TS", + PostalCode: "12345", + Country: "USA", + IsPrimary: false, + } + err := db.Create(inactiveResidence).Error + require.NoError(t, err) + // Set to inactive after creation to bypass GORM default + db.Model(inactiveResidence).Update("is_active", false) + + now := time.Now().UTC() + dueSoon := now.AddDate(0, 0, 5) // 5 days from now + + // Create due soon tasks in both residences + db.Create(&models.Task{ + ResidenceID: activeResidence.ID, + CreatedByID: user.ID, + Title: "Due Soon in Active Residence", + DueDate: &dueSoon, + }) + db.Create(&models.Task{ + ResidenceID: inactiveResidence.ID, + CreatedByID: user.ID, + Title: "Due Soon in Inactive Residence", + DueDate: &dueSoon, + }) + + // Filter by user ID (used by notification system) + opts := TaskFilterOptions{UserIDs: []uint{user.ID}, IncludeInProgress: true} + tasks, err := repo.GetDueSoonTasks(now, 30, opts) + require.NoError(t, err) + + // Should only return task from active residence + assert.Len(t, tasks, 1, "Should only return tasks from active residences") + assert.Equal(t, "Due Soon in Active Residence", tasks[0].Title) +} + func TestTaskFilterOptions_IncludeArchived(t *testing.T) { db := testutil.SetupTestDB(t) repo := NewTaskRepository(db) diff --git a/internal/services/onboarding_email_service.go b/internal/services/onboarding_email_service.go index 3b81553..28d3801 100644 --- a/internal/services/onboarding_email_service.go +++ b/internal/services/onboarding_email_service.go @@ -168,10 +168,10 @@ func (s *OnboardingEmailService) UsersNeedingNoResidenceEmail() ([]models.User, // 3. Have no residences // 4. Haven't received this email type yet err := s.db.Raw(` - SELECT u.* FROM users u - LEFT JOIN residences r ON r.owner_id = u.id AND r.is_active = true + SELECT u.* FROM auth_user u + LEFT JOIN residence_residence r ON r.owner_id = u.id AND r.is_active = true LEFT JOIN onboarding_emails oe ON oe.user_id = u.id AND oe.email_type = ? - WHERE u.verified = true + WHERE u.is_active = true AND u.date_joined < ? AND r.id IS NULL AND oe.id IS NULL @@ -197,15 +197,15 @@ func (s *OnboardingEmailService) UsersNeedingNoTasksEmail() ([]models.User, erro // 4. Have no tasks across ANY of their residences // 5. Haven't received this email type yet err := s.db.Raw(` - SELECT DISTINCT u.* FROM users u - INNER JOIN residences r ON r.owner_id = u.id AND r.is_active = true - LEFT JOIN tasks t ON t.residence_id IN (SELECT id FROM residences WHERE owner_id = u.id AND is_active = true) AND t.is_active = true + SELECT DISTINCT u.* FROM auth_user u + INNER JOIN residence_residence r ON r.owner_id = u.id AND r.is_active = true + LEFT JOIN task_task t ON t.residence_id IN (SELECT id FROM residence_residence WHERE owner_id = u.id AND is_active = true) AND t.is_cancelled = false AND t.is_archived = false LEFT JOIN onboarding_emails oe ON oe.user_id = u.id AND oe.email_type = ? - WHERE u.verified = true + WHERE u.is_active = true AND t.id IS NULL AND oe.id IS NULL AND EXISTS ( - SELECT 1 FROM residences r2 + SELECT 1 FROM residence_residence r2 WHERE r2.owner_id = u.id AND r2.is_active = true AND r2.created_at < ? diff --git a/internal/worker/jobs/handler.go b/internal/worker/jobs/handler.go index 8f31fbe..9b92d70 100644 --- a/internal/worker/jobs/handler.go +++ b/internal/worker/jobs/handler.go @@ -338,9 +338,9 @@ func (h *Handler) HandleDailyDigest(ctx context.Context, task *asynq.Task) error THEN t.id END) as due_this_week FROM auth_user u - JOIN residence_residence r ON r.owner_id = u.id OR r.id IN ( + JOIN residence_residence r ON (r.owner_id = u.id OR r.id IN ( SELECT residence_id FROM residence_residence_users WHERE user_id = u.id - ) + )) AND r.is_active = true JOIN task_task t ON t.residence_id = r.id AND t.is_cancelled = false AND t.is_archived = false