From 62fbd7275e28c61e286e22408d7d9af4fb0ad3c9 Mon Sep 17 00:00:00 2001 From: teernisse Date: Tue, 10 Mar 2026 11:06:53 -0400 Subject: [PATCH] fix(me): show activity on closed/merged items in dashboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activity feed and since-last-check inbox previously filtered to only open items via state = 'opened' checks in the SQL subqueries. This meant comments on merged MRs (post-merge follow-ups, questions) and closed issues were silently dropped from the feed. Remove the state filter from the association checks in both query_activity() and query_since_last_check(). The user-association checks (assigned, authored, reviewing) remain — activity still only appears for items the user is connected to, regardless of state. The simplified subqueries also eliminate unnecessary JOINs to the issues/merge_requests tables that were only needed for the state check, resulting in slightly more efficient index-only scans on issue_assignees and mr_reviewers. Add 4 tests covering: merged MR (authored), closed MR (reviewer), closed issue (assignee), and merged MR in the since-last-check inbox. --- src/cli/commands/me/me_tests.rs | 109 ++++++++++++++++++++++++++++++++ src/cli/commands/me/queries.rs | 28 ++++---- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/src/cli/commands/me/me_tests.rs b/src/cli/commands/me/me_tests.rs index e2b9b5d..99cf109 100644 --- a/src/cli/commands/me/me_tests.rs +++ b/src/cli/commands/me/me_tests.rs @@ -627,6 +627,115 @@ fn activity_is_own_flag() { assert!(results[0].is_own); } +// ─── Activity on Closed/Merged Items ───────────────────────────────────────── + +#[test] +fn activity_note_on_merged_authored_mr() { + let conn = setup_test_db(); + insert_project(&conn, 1, "group/repo"); + insert_mr(&conn, 10, 1, 99, "alice", "merged", false); + + let disc_id = 100; + insert_discussion(&conn, disc_id, 1, Some(10), None); + let t = now_ms() - 1000; + insert_note_at( + &conn, + 200, + disc_id, + 1, + "bob", + false, + "follow-up question", + t, + ); + + let results = query_activity(&conn, "alice", &[], 0).unwrap(); + assert_eq!( + results.len(), + 1, + "should see activity on merged MR authored by user" + ); + assert_eq!(results[0].entity_iid, 99); + assert_eq!(results[0].entity_type, "mr"); +} + +#[test] +fn activity_note_on_closed_mr_as_reviewer() { + let conn = setup_test_db(); + insert_project(&conn, 1, "group/repo"); + insert_mr(&conn, 10, 1, 99, "bob", "closed", false); + insert_reviewer(&conn, 10, "alice"); + + let disc_id = 100; + insert_discussion(&conn, disc_id, 1, Some(10), None); + let t = now_ms() - 1000; + insert_note_at(&conn, 200, disc_id, 1, "bob", false, "can you re-check?", t); + + let results = query_activity(&conn, "alice", &[], 0).unwrap(); + assert_eq!( + results.len(), + 1, + "should see activity on closed MR where user is reviewer" + ); +} + +#[test] +fn activity_note_on_closed_assigned_issue() { + let conn = setup_test_db(); + insert_project(&conn, 1, "group/repo"); + insert_issue_with_state(&conn, 10, 1, 42, "someone", "closed"); + insert_assignee(&conn, 10, "alice"); + + let disc_id = 100; + insert_discussion(&conn, disc_id, 1, None, Some(10)); + let t = now_ms() - 1000; + insert_note_at( + &conn, + 200, + disc_id, + 1, + "bob", + false, + "reopening discussion", + t, + ); + + let results = query_activity(&conn, "alice", &[], 0).unwrap(); + assert_eq!( + results.len(), + 1, + "should see activity on closed issue assigned to user" + ); +} + +#[test] +fn since_last_check_includes_comment_on_merged_mr() { + let conn = setup_test_db(); + insert_project(&conn, 1, "group/repo"); + insert_mr(&conn, 10, 1, 99, "alice", "merged", false); + + let disc_id = 100; + insert_discussion(&conn, disc_id, 1, Some(10), None); + let t = now_ms() - 1000; + insert_note_at( + &conn, + 200, + disc_id, + 1, + "bob", + false, + "post-merge question", + t, + ); + + let groups = query_since_last_check(&conn, "alice", 0).unwrap(); + let total_events: usize = groups.iter().map(|g| g.events.len()).sum(); + assert_eq!( + total_events, 1, + "should see others' comments on merged MR in inbox" + ); +} + // ─── Assignment Detection Tests (Task #12) ───────────────────────────────── #[test] diff --git a/src/cli/commands/me/queries.rs b/src/cli/commands/me/queries.rs index a67e22c..8b72cce 100644 --- a/src/cli/commands/me/queries.rs +++ b/src/cli/commands/me/queries.rs @@ -362,19 +362,18 @@ pub fn query_activity( let project_clause = build_project_clause_at("p.id", project_ids, 3); // Build the "my items" subquery fragments for issue/MR association checks. - // These ensure we only see activity on items CURRENTLY associated with the user - // AND currently open (AC-3.6). Without the state filter, activity would include - // events on closed/merged items that don't appear in the dashboard lists. + // These ensure we only see activity on items associated with the user, + // regardless of state (open, closed, or merged). Comments on merged MRs + // and closed issues are still relevant (follow-up discussions, post-merge + // questions, etc.). let my_issue_check = "EXISTS ( SELECT 1 FROM issue_assignees ia - JOIN issues i2 ON ia.issue_id = i2.id - WHERE ia.issue_id = {entity_issue_id} AND ia.username = ?1 AND i2.state = 'opened' + WHERE ia.issue_id = {entity_issue_id} AND ia.username = ?1 )"; let my_mr_check = "( - EXISTS (SELECT 1 FROM merge_requests mr2 WHERE mr2.id = {entity_mr_id} AND mr2.author_username = ?1 AND mr2.state = 'opened') + EXISTS (SELECT 1 FROM merge_requests mr2 WHERE mr2.id = {entity_mr_id} AND mr2.author_username = ?1) OR EXISTS (SELECT 1 FROM mr_reviewers rv - JOIN merge_requests mr3 ON rv.merge_request_id = mr3.id - WHERE rv.merge_request_id = {entity_mr_id} AND rv.username = ?1 AND mr3.state = 'opened') + WHERE rv.merge_request_id = {entity_mr_id} AND rv.username = ?1) )"; // Source 1: Human comments on my items @@ -574,7 +573,7 @@ struct RawSinceCheckRow { /// Query actionable events from others since `cursor_ms`. /// Returns events from three sources: -/// 1. Others' comments on my open items +/// 1. Others' comments on my items (any state) /// 2. @mentions on any item (not restricted to my items) /// 3. Assignment/review-request system notes mentioning me pub fn query_since_last_check( @@ -583,19 +582,18 @@ pub fn query_since_last_check( cursor_ms: i64, ) -> Result> { // Build the "my items" subquery fragments (reused from activity). + // No state filter: comments on closed/merged items are still actionable. let my_issue_check = "EXISTS ( SELECT 1 FROM issue_assignees ia - JOIN issues i2 ON ia.issue_id = i2.id - WHERE ia.issue_id = {entity_issue_id} AND ia.username = ?1 AND i2.state = 'opened' + WHERE ia.issue_id = {entity_issue_id} AND ia.username = ?1 )"; let my_mr_check = "( - EXISTS (SELECT 1 FROM merge_requests mr2 WHERE mr2.id = {entity_mr_id} AND mr2.author_username = ?1 AND mr2.state = 'opened') + EXISTS (SELECT 1 FROM merge_requests mr2 WHERE mr2.id = {entity_mr_id} AND mr2.author_username = ?1) OR EXISTS (SELECT 1 FROM mr_reviewers rv - JOIN merge_requests mr3 ON rv.merge_request_id = mr3.id - WHERE rv.merge_request_id = {entity_mr_id} AND rv.username = ?1 AND mr3.state = 'opened') + WHERE rv.merge_request_id = {entity_mr_id} AND rv.username = ?1) )"; - // Source 1: Others' comments on my open items + // Source 1: Others' comments on my items (any state) let source1 = format!( "SELECT n.created_at, 'note', CASE WHEN d.issue_id IS NOT NULL THEN 'issue' ELSE 'mr' END,