refactor(#1474): replace approvals lookup 7-tuple with named struct, drop type_complexity allow
This commit is contained in:
parent
b7eb0f3930
commit
f5d9f325c6
1 changed files with 58 additions and 73 deletions
|
|
@ -166,44 +166,20 @@ impl Approvals {
|
||||||
|
|
||||||
/// Mark pending -> approved (or fail if not pending). Returns the (now-updated)
|
/// Mark pending -> approved (or fail if not pending). Returns the (now-updated)
|
||||||
/// approval so the caller can run the action and pass the agent name.
|
/// approval so the caller can run the action and pass the agent name.
|
||||||
#[allow(clippy::type_complexity)]
|
|
||||||
pub fn mark_approved(&self, id: i64) -> Result<Approval> {
|
pub fn mark_approved(&self, id: i64) -> Result<Approval> {
|
||||||
let conn = self.conn.lock().unwrap();
|
let conn = self.conn.lock().unwrap();
|
||||||
// Row shape: (agent, kind, commit_ref, requested_at, status,
|
let row: Option<ApprovalLookup> = conn
|
||||||
// fetched_sha, description).
|
|
||||||
let current: Option<(
|
|
||||||
String,
|
|
||||||
String,
|
|
||||||
String,
|
|
||||||
i64,
|
|
||||||
String,
|
|
||||||
Option<String>,
|
|
||||||
Option<String>,
|
|
||||||
)> = conn
|
|
||||||
.query_row(
|
.query_row(
|
||||||
"SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, description
|
ApprovalLookup::SELECT,
|
||||||
FROM approvals WHERE id = ?1",
|
|
||||||
params![id],
|
params![id],
|
||||||
|row| {
|
ApprovalLookup::from_row,
|
||||||
Ok((
|
|
||||||
row.get(0)?,
|
|
||||||
row.get(1)?,
|
|
||||||
row.get(2)?,
|
|
||||||
row.get(3)?,
|
|
||||||
row.get(4)?,
|
|
||||||
row.get(5)?,
|
|
||||||
row.get(6)?,
|
|
||||||
))
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
.optional()?;
|
.optional()?;
|
||||||
let Some((agent, kind, commit_ref, requested_at, status, fetched_sha, description)) =
|
let Some(row) = row else {
|
||||||
current
|
|
||||||
else {
|
|
||||||
bail!("approval {id} not found");
|
bail!("approval {id} not found");
|
||||||
};
|
};
|
||||||
if status != "pending" {
|
if row.status != "pending" {
|
||||||
bail!("approval {id} is {status}, not pending");
|
bail!("approval {id} is {}, not pending", row.status);
|
||||||
}
|
}
|
||||||
let resolved_at = now_unix();
|
let resolved_at = now_unix();
|
||||||
conn.execute(
|
conn.execute(
|
||||||
|
|
@ -212,15 +188,15 @@ impl Approvals {
|
||||||
)?;
|
)?;
|
||||||
Ok(Approval {
|
Ok(Approval {
|
||||||
id,
|
id,
|
||||||
agent,
|
agent: row.agent,
|
||||||
kind: kind_from_str(&kind)?,
|
kind: kind_from_str(&row.kind)?,
|
||||||
commit_ref,
|
commit_ref: row.commit_ref,
|
||||||
requested_at,
|
requested_at: row.requested_at,
|
||||||
status: ApprovalStatus::Approved,
|
status: ApprovalStatus::Approved,
|
||||||
resolved_at: Some(resolved_at),
|
resolved_at: Some(resolved_at),
|
||||||
note: None,
|
note: None,
|
||||||
fetched_sha,
|
fetched_sha: row.fetched_sha,
|
||||||
description,
|
description: row.description,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -251,45 +227,20 @@ impl Approvals {
|
||||||
/// kind / agent / sha. Errors if the approval isn't pending — once
|
/// kind / agent / sha. Errors if the approval isn't pending — once
|
||||||
/// it's approved/denied/failed/cancelled, the resolution is final.
|
/// it's approved/denied/failed/cancelled, the resolution is final.
|
||||||
pub fn mark_cancelled(&self, id: i64, canceller: &str) -> Result<Approval> {
|
pub fn mark_cancelled(&self, id: i64, canceller: &str) -> Result<Approval> {
|
||||||
// Row-shape alias for the SELECT below so we don't trip
|
|
||||||
// clippy::type_complexity. Order matches the SELECT projection:
|
|
||||||
// agent, kind, commit_ref, requested_at, status, fetched_sha,
|
|
||||||
// description.
|
|
||||||
type CancelLookupRow = (
|
|
||||||
String,
|
|
||||||
String,
|
|
||||||
String,
|
|
||||||
i64,
|
|
||||||
String,
|
|
||||||
Option<String>,
|
|
||||||
Option<String>,
|
|
||||||
);
|
|
||||||
let mut conn = self.conn.lock().unwrap();
|
let mut conn = self.conn.lock().unwrap();
|
||||||
let tx = conn.transaction()?;
|
let tx = conn.transaction()?;
|
||||||
let row: Option<CancelLookupRow> = tx
|
let row: Option<ApprovalLookup> = tx
|
||||||
.query_row(
|
.query_row(
|
||||||
"SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, description
|
ApprovalLookup::SELECT,
|
||||||
FROM approvals WHERE id = ?1",
|
|
||||||
params![id],
|
params![id],
|
||||||
|row| {
|
ApprovalLookup::from_row,
|
||||||
Ok((
|
|
||||||
row.get(0)?,
|
|
||||||
row.get(1)?,
|
|
||||||
row.get(2)?,
|
|
||||||
row.get(3)?,
|
|
||||||
row.get(4)?,
|
|
||||||
row.get(5)?,
|
|
||||||
row.get(6)?,
|
|
||||||
))
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
.optional()?;
|
.optional()?;
|
||||||
let Some((agent, kind, commit_ref, requested_at, status, fetched_sha, description)) = row
|
let Some(row) = row else {
|
||||||
else {
|
|
||||||
bail!("approval {id} not found");
|
bail!("approval {id} not found");
|
||||||
};
|
};
|
||||||
if status != "pending" {
|
if row.status != "pending" {
|
||||||
bail!("approval {id} is {status}, not pending");
|
bail!("approval {id} is {}, not pending", row.status);
|
||||||
}
|
}
|
||||||
let resolved_at = now_unix();
|
let resolved_at = now_unix();
|
||||||
let note = format!("cancelled by {canceller}");
|
let note = format!("cancelled by {canceller}");
|
||||||
|
|
@ -300,15 +251,15 @@ impl Approvals {
|
||||||
tx.commit()?;
|
tx.commit()?;
|
||||||
Ok(Approval {
|
Ok(Approval {
|
||||||
id,
|
id,
|
||||||
agent,
|
agent: row.agent,
|
||||||
kind: kind_from_str(&kind)?,
|
kind: kind_from_str(&row.kind)?,
|
||||||
commit_ref,
|
commit_ref: row.commit_ref,
|
||||||
requested_at,
|
requested_at: row.requested_at,
|
||||||
status: ApprovalStatus::Cancelled,
|
status: ApprovalStatus::Cancelled,
|
||||||
resolved_at: Some(resolved_at),
|
resolved_at: Some(resolved_at),
|
||||||
note: Some(note),
|
note: Some(note),
|
||||||
fetched_sha,
|
fetched_sha: row.fetched_sha,
|
||||||
description,
|
description: row.description,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -325,6 +276,40 @@ impl Approvals {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Columns needed to rebuild an [`Approval`] after a status transition,
|
||||||
|
/// shared by `mark_approved` / `mark_cancelled`. Replaces a 7-field
|
||||||
|
/// tuple that tripped `clippy::type_complexity` and was duplicated
|
||||||
|
/// across both callers (one suppressed the lint, the other aliased the
|
||||||
|
/// tuple) — one named projection + mapper now backs both.
|
||||||
|
struct ApprovalLookup {
|
||||||
|
agent: String,
|
||||||
|
kind: String,
|
||||||
|
commit_ref: String,
|
||||||
|
requested_at: i64,
|
||||||
|
status: String,
|
||||||
|
fetched_sha: Option<String>,
|
||||||
|
description: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ApprovalLookup {
|
||||||
|
/// The single-row lookup by id (`?1`). Column order matches
|
||||||
|
/// [`ApprovalLookup::from_row`].
|
||||||
|
const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, \
|
||||||
|
description FROM approvals WHERE id = ?1";
|
||||||
|
|
||||||
|
fn from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<Self> {
|
||||||
|
Ok(Self {
|
||||||
|
agent: row.get(0)?,
|
||||||
|
kind: row.get(1)?,
|
||||||
|
commit_ref: row.get(2)?,
|
||||||
|
requested_at: row.get(3)?,
|
||||||
|
status: row.get(4)?,
|
||||||
|
fetched_sha: row.get(5)?,
|
||||||
|
description: row.get(6)?,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Collect approval rows, dropping (and logging) any that fail to
|
/// Collect approval rows, dropping (and logging) any that fail to
|
||||||
/// deserialize. A single malformed / unknown-kind row must never blank
|
/// deserialize. A single malformed / unknown-kind row must never blank
|
||||||
/// the whole list: `collect::<Result<Vec>>()` is all-or-nothing, so one
|
/// the whole list: `collect::<Result<Vec>>()` is all-or-nothing, so one
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue