theirongolddev/gitlore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(who): filter unresolved discussions to open entities only

Browse Source 
Workload and active modes now exclude discussions on closed issues and
merged/closed MRs by default. Adds --include-closed flag to restore
the previous behavior when needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
teernisse
2026-02-18 10:27:24 -05:00
parent 714c8c2623
commit 63bd58c9b4
4 changed files with 216 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
src/cli/commands/who.rs
View File

@@ -344,7 +344,14 @@ pub fn run_who(config: &Config, args: &WhoArgs) -> Result<WhoRun> {
.map(resolve_since_required)
.transpose()?;
let limit = usize::from(args.limit);
let result = query_workload(&conn, username, project_id, since_ms, limit)?;
let result = query_workload(
&conn,
username,
project_id,
since_ms,
limit,
args.include_closed,
)?;
Ok(WhoRun {
resolved_input: WhoResolvedInput {
mode: "workload".to_string(),
@@ -377,7 +384,7 @@ pub fn run_who(config: &Config, args: &WhoArgs) -> Result<WhoRun> {
WhoMode::Active => {
let since_ms = resolve_since(args.since.as_deref(), "7d")?;
let limit = usize::from(args.limit);
let result = query_active(&conn, project_id, since_ms, limit)?;
let result = query_active(&conn, project_id, since_ms, limit, args.include_closed)?;
Ok(WhoRun {
resolved_input: WhoResolvedInput {
mode: "active".to_string(),
@@ -1149,6 +1156,7 @@ fn query_workload(
project_id: Option<i64>,
since_ms: Option<i64>,
limit: usize,
include_closed: bool,
) -> Result<WorkloadResult> {
let limit_plus_one = (limit + 1) as i64;
@@ -1245,7 +1253,14 @@ fn query_workload(
.collect::<std::result::Result<Vec<_>, _>>()?;
// Query 4: Unresolved discussions where user participated
let disc_sql = "SELECT d.noteable_type,
let state_filter = if include_closed {
""
} else {
" AND (i.id IS NULL OR i.state = 'opened')
AND (m.id IS NULL OR m.state = 'opened')"
};
let disc_sql = format!(
"SELECT d.noteable_type,
COALESCE(i.iid, m.iid) AS entity_iid,
(p.path_with_namespace ||
CASE WHEN d.noteable_type = 'MergeRequest' THEN '!' ELSE '#' END ||
@@ -1266,10 +1281,12 @@ fn query_workload(
)
AND (?2 IS NULL OR d.project_id = ?2)
AND (?3 IS NULL OR d.last_note_at >= ?3)
{state_filter}
ORDER BY d.last_note_at DESC
LIMIT ?4";
LIMIT ?4"
);
let mut stmt = conn.prepare_cached(disc_sql)?;
let mut stmt = conn.prepare_cached(&disc_sql)?;
let unresolved_discussions: Vec<WorkloadDiscussion> = stmt
.query_map(
rusqlite::params![username, project_id, since_ms, limit_plus_one],
@@ -1451,35 +1468,63 @@ fn query_active(
project_id: Option<i64>,
since_ms: i64,
limit: usize,
include_closed: bool,
) -> Result<ActiveResult> {
let limit_plus_one = (limit + 1) as i64;
// Total unresolved count -- two static variants
let total_sql_global = "SELECT COUNT(*) FROM discussions d
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1";
let total_sql_scoped = "SELECT COUNT(*) FROM discussions d
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1
AND d.project_id = ?2";
let total_unresolved_in_window: u32 = match project_id {
None => conn.query_row(total_sql_global, rusqlite::params![since_ms], |row| {
row.get(0)
})?,
Some(pid) => conn.query_row(total_sql_scoped, rusqlite::params![since_ms, pid], |row| {
row.get(0)
})?,
// State filter for open-entities-only (default behavior)
let state_joins = if include_closed {
""
} else {
" LEFT JOIN issues i ON d.issue_id = i.id
LEFT JOIN merge_requests m ON d.merge_request_id = m.id"
};
let state_filter = if include_closed {
""
} else {
" AND (i.id IS NULL OR i.state = 'opened')
AND (m.id IS NULL OR m.state = 'opened')"
};
// Active discussions with context -- two static SQL variants
let sql_global = "
// Total unresolved count -- conditionally built
let total_sql_global = format!(
"SELECT COUNT(*) FROM discussions d
{state_joins}
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1
{state_filter}"
);
let total_sql_scoped = format!(
"SELECT COUNT(*) FROM discussions d
{state_joins}
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1
AND d.project_id = ?2
{state_filter}"
);
let total_unresolved_in_window: u32 = match project_id {
None => conn.query_row(&total_sql_global, rusqlite::params![since_ms], |row| {
row.get(0)
})?,
Some(pid) => {
conn.query_row(&total_sql_scoped, rusqlite::params![since_ms, pid], |row| {
row.get(0)
})?
}
};
// Active discussions with context -- conditionally built SQL
let sql_global = format!(
"
WITH picked AS (
SELECT d.id, d.noteable_type, d.issue_id, d.merge_request_id,
d.project_id, d.last_note_at
FROM discussions d
{state_joins}
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1
{state_filter}
ORDER BY d.last_note_at DESC
LIMIT ?2
),
@@ -1520,16 +1565,20 @@ fn query_active(
LEFT JOIN note_counts nc ON nc.discussion_id = p.id
LEFT JOIN participants pa ON pa.discussion_id = p.id
ORDER BY p.last_note_at DESC
";
"
);
let sql_scoped = "
let sql_scoped = format!(
"
WITH picked AS (
SELECT d.id, d.noteable_type, d.issue_id, d.merge_request_id,
d.project_id, d.last_note_at
FROM discussions d
{state_joins}
WHERE d.resolvable = 1 AND d.resolved = 0
AND d.last_note_at >= ?1
AND d.project_id = ?2
{state_filter}
ORDER BY d.last_note_at DESC
LIMIT ?3
),
@@ -1570,7 +1619,8 @@ fn query_active(
LEFT JOIN note_counts nc ON nc.discussion_id = p.id
LEFT JOIN participants pa ON pa.discussion_id = p.id
ORDER BY p.last_note_at DESC
";
"
);
// Row-mapping closure shared between both variants
let map_row = |row: &rusqlite::Row| -> rusqlite::Result<ActiveDiscussion> {
@@ -1613,12 +1663,12 @@ fn query_active(
// Select variant first, then prepare exactly one statement
let discussions: Vec<ActiveDiscussion> = match project_id {
None => {
let mut stmt = conn.prepare_cached(sql_global)?;
let mut stmt = conn.prepare_cached(&sql_global)?;
stmt.query_map(rusqlite::params![since_ms, limit_plus_one], &map_row)?
.collect::<std::result::Result<Vec<_>, _>>()?
}
Some(pid) => {
let mut stmt = conn.prepare_cached(sql_scoped)?;
let mut stmt = conn.prepare_cached(&sql_scoped)?;
stmt.query_map(rusqlite::params![since_ms, pid, limit_plus_one], &map_row)?
.collect::<std::result::Result<Vec<_>, _>>()?
}

137
src/cli/commands/who_tests.rs
View File

@@ -54,15 +54,27 @@ fn insert_mr(conn: &Connection, id: i64, project_id: i64, iid: i64, author: &str
}
fn insert_issue(conn: &Connection, id: i64, project_id: i64, iid: i64, author: &str) {
insert_issue_with_state(conn, id, project_id, iid, author, "opened");
}
fn insert_issue_with_state(
conn: &Connection,
id: i64,
project_id: i64,
iid: i64,
author: &str,
state: &str,
) {
conn.execute(
"INSERT INTO issues (id, gitlab_id, project_id, iid, title, state, author_username, created_at, updated_at, last_seen_at)
VALUES (?1, ?2, ?3, ?4, ?5, 'opened', ?6, ?7, ?8, ?9)",
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)",
rusqlite::params![
id,
id * 10,
project_id,
iid,
format!("Issue {iid}"),
state,
author,
now_ms(),
now_ms(),
@@ -134,6 +146,24 @@ fn insert_diffnote(
.unwrap();
}
fn insert_note(conn: &Connection, id: i64, discussion_id: i64, project_id: i64, author: &str) {
conn.execute(
"INSERT INTO notes (id, gitlab_id, discussion_id, project_id, note_type, is_system, author_username, body, created_at, updated_at, last_seen_at)
VALUES (?1, ?2, ?3, ?4, 'DiscussionNote', 0, ?5, 'comment', ?6, ?7, ?8)",
rusqlite::params![
id,
id * 10,
discussion_id,
project_id,
author,
now_ms(),
now_ms(),
now_ms()
],
)
.unwrap();
}
fn insert_assignee(conn: &Connection, issue_id: i64, username: &str) {
conn.execute(
"INSERT INTO issue_assignees (issue_id, username) VALUES (?1, ?2)",
@@ -263,6 +293,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -286,6 +317,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -309,6 +341,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -332,6 +365,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -355,6 +389,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -378,6 +413,7 @@ fn test_is_file_path_discrimination() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
})
.unwrap(),
@@ -402,6 +438,7 @@ fn test_detail_rejected_outside_expert_mode() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
};
let mode = resolve_mode(&args).unwrap();
@@ -430,6 +467,7 @@ fn test_detail_allowed_in_expert_mode() {
as_of: None,
explain_score: false,
include_bots: false,
include_closed: false,
all_history: false,
};
let mode = resolve_mode(&args).unwrap();
@@ -579,7 +617,7 @@ fn test_workload_query() {
insert_assignee(&conn, 1, "dev_a");
insert_mr(&conn, 1, 1, 100, "dev_a", "opened");
let result = query_workload(&conn, "dev_a", None, None, 20).unwrap();
let result = query_workload(&conn, "dev_a", None, None, 20, true).unwrap();
assert_eq!(result.assigned_issues.len(), 1);
assert_eq!(result.authored_mrs.len(), 1);
}
@@ -626,7 +664,7 @@ fn test_active_query() {
// Second note by same participant -- note_count should be 2, participants still ["reviewer_b"]
insert_diffnote(&conn, 2, 1, 1, "reviewer_b", "src/foo.rs", "follow-up");
let result = query_active(&conn, None, 0, 20).unwrap();
let result = query_active(&conn, None, 0, 20, true).unwrap();
assert_eq!(result.total_unresolved_in_window, 1);
assert_eq!(result.discussions.len(), 1);
assert_eq!(result.discussions[0].participants, vec!["reviewer_b"]);
@@ -878,7 +916,7 @@ fn test_active_participants_sorted() {
insert_diffnote(&conn, 1, 1, 1, "zebra_user", "src/foo.rs", "note 1");
insert_diffnote(&conn, 2, 1, 1, "alpha_user", "src/foo.rs", "note 2");
let result = query_active(&conn, None, 0, 20).unwrap();
let result = query_active(&conn, None, 0, 20, true).unwrap();
assert_eq!(
result.discussions[0].participants,
vec!["alpha_user", "zebra_user"]
@@ -3265,3 +3303,94 @@ fn test_deterministic_accumulation_order() {
);
}
}
// ─── Tests: include_closed filter ────────────────────────────────────────────
#[test]
fn workload_excludes_closed_entity_discussions() {
let conn = setup_test_db();
insert_project(&conn, 1, "group/repo");
// Open issue with unresolved discussion
insert_issue_with_state(&conn, 10, 1, 10, "someone", "opened");
insert_discussion(&conn, 100, 1, None, Some(10), true, false);
insert_note(&conn, 1000, 100, 1, "alice");
// Closed issue with unresolved discussion
insert_issue_with_state(&conn, 20, 1, 20, "someone", "closed");
insert_discussion(&conn, 200, 1, None, Some(20), true, false);
insert_note(&conn, 2000, 200, 1, "alice");
// Default: exclude closed
let result = query_workload(&conn, "alice", None, None, 50, false).unwrap();
assert_eq!(result.unresolved_discussions.len(), 1);
assert_eq!(result.unresolved_discussions[0].entity_iid, 10);
}
#[test]
fn workload_include_closed_flag_shows_all() {
let conn = setup_test_db();
insert_project(&conn, 1, "group/repo");
insert_issue_with_state(&conn, 10, 1, 10, "someone", "opened");
insert_discussion(&conn, 100, 1, None, Some(10), true, false);
insert_note(&conn, 1000, 100, 1, "alice");
insert_issue_with_state(&conn, 20, 1, 20, "someone", "closed");
insert_discussion(&conn, 200, 1, None, Some(20), true, false);
insert_note(&conn, 2000, 200, 1, "alice");
let result = query_workload(&conn, "alice", None, None, 50, true).unwrap();
assert_eq!(result.unresolved_discussions.len(), 2);
}
#[test]
fn workload_excludes_merged_mr_discussions() {
let conn = setup_test_db();
insert_project(&conn, 1, "group/repo");
// Open MR with unresolved discussion
insert_mr(&conn, 10, 1, 10, "someone", "opened");
insert_discussion(&conn, 100, 1, Some(10), None, true, false);
insert_note(&conn, 1000, 100, 1, "alice");
// Merged MR with unresolved discussion
insert_mr(&conn, 20, 1, 20, "someone", "merged");
insert_discussion(&conn, 200, 1, Some(20), None, true, false);
insert_note(&conn, 2000, 200, 1, "alice");
let result = query_workload(&conn, "alice", None, None, 50, false).unwrap();
assert_eq!(result.unresolved_discussions.len(), 1);
assert_eq!(result.unresolved_discussions[0].entity_iid, 10);
// include_closed shows both
let result = query_workload(&conn, "alice", None, None, 50, true).unwrap();
assert_eq!(result.unresolved_discussions.len(), 2);
}
#[test]
fn active_excludes_closed_entity_discussions() {
let conn = setup_test_db();
insert_project(&conn, 1, "group/repo");
// Open issue with unresolved discussion
insert_issue_with_state(&conn, 10, 1, 10, "someone", "opened");
insert_discussion(&conn, 100, 1, None, Some(10), true, false);
insert_note(&conn, 1000, 100, 1, "alice");
// Closed issue with unresolved discussion
insert_issue_with_state(&conn, 20, 1, 20, "someone", "closed");
insert_discussion(&conn, 200, 1, None, Some(20), true, false);
insert_note(&conn, 2000, 200, 1, "alice");
// Default: exclude closed
let result = query_active(&conn, None, 0, 50, false).unwrap();
assert_eq!(result.discussions.len(), 1);
assert_eq!(result.discussions[0].entity_iid, 10);
assert_eq!(result.total_unresolved_in_window, 1);
// include_closed shows both
let result = query_active(&conn, None, 0, 50, true).unwrap();
assert_eq!(result.discussions.len(), 2);
assert_eq!(result.total_unresolved_in_window, 2);
}