mirror of
https://github.com/modrinth/code.git
synced 2026-08-25 00:55:25 +00:00
feat: global trace database frontend + improvements to tech review status sync (#6671)
* wip * ui edits * better logic for syncing tech review state in a single centralized place * never automatically finish a tech review report * fix css * fix clippy * prepr
This commit is contained in:
@@ -254,6 +254,10 @@ pub struct ReportIssueDetail {
|
||||
pub data: HashMap<String, serde_json::Value>,
|
||||
/// How important is this issue, as flagged by Delphi?
|
||||
pub severity: DelphiSeverity,
|
||||
/// Project-local verdict for this detail, if one exists.
|
||||
pub local_status: Option<DelphiStatus>,
|
||||
/// Global verdict for this detail's key, if one exists.
|
||||
pub global_status: Option<DelphiStatus>,
|
||||
/// Has this issue detail been marked as safe or unsafe?
|
||||
pub status: DelphiStatus,
|
||||
}
|
||||
|
||||
@@ -114,6 +114,14 @@ impl From<crate::models::v3::threads::MessageBody> for LegacyMessageBody {
|
||||
associated_images: Vec::new(),
|
||||
}
|
||||
}
|
||||
crate::models::v3::threads::MessageBody::TechReviewExited => {
|
||||
LegacyMessageBody::Text {
|
||||
body: "(legacy) Exited technical review".into(),
|
||||
private: true,
|
||||
replying_to: None,
|
||||
associated_images: Vec::new(),
|
||||
}
|
||||
}
|
||||
crate::models::v3::threads::MessageBody::TechReviewExitFileDeleted => {
|
||||
LegacyMessageBody::Text {
|
||||
body: "(legacy) Exited technical review because file was deleted".into(),
|
||||
|
||||
@@ -28,8 +28,11 @@ pub struct ThreadMessage {
|
||||
pub hide_identity: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)]
|
||||
#[derive(
|
||||
Debug, Clone, Serialize, Deserialize, utoipa::ToSchema, strum::AsRefStr,
|
||||
)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
pub enum MessageBody {
|
||||
Text {
|
||||
body: String,
|
||||
@@ -47,6 +50,7 @@ pub enum MessageBody {
|
||||
verdict: DelphiVerdict,
|
||||
},
|
||||
TechReviewEntered,
|
||||
TechReviewExited,
|
||||
TechReviewExitFileDeleted,
|
||||
ThreadClosure,
|
||||
ThreadReopen,
|
||||
@@ -62,6 +66,7 @@ impl MessageBody {
|
||||
Self::Text { private, .. } | Self::Deleted { private } => *private,
|
||||
Self::TechReview { .. }
|
||||
| Self::TechReviewEntered
|
||||
| Self::TechReviewExited
|
||||
| Self::TechReviewExitFileDeleted => true,
|
||||
Self::StatusChange { .. }
|
||||
| Self::ThreadClosure
|
||||
|
||||
@@ -13,20 +13,18 @@ use crate::{
|
||||
auth::check_is_moderator_from_headers,
|
||||
database::{
|
||||
models::{
|
||||
DBFileId, DBProjectId, DBThreadId, DelphiReportId,
|
||||
DelphiReportIssueDetailsId, DelphiReportIssueId,
|
||||
DBFileId, DBProjectId, DelphiReportId, DelphiReportIssueDetailsId,
|
||||
DelphiReportIssueId,
|
||||
delphi_report_item::{
|
||||
DBDelphiReport, DBDelphiReportIssue, DelphiSeverity,
|
||||
DelphiStatus, ReportIssueDetail,
|
||||
},
|
||||
thread_item::ThreadMessageBuilder,
|
||||
},
|
||||
redis::RedisPool,
|
||||
},
|
||||
models::{
|
||||
ids::{ProjectId, VersionId},
|
||||
pats::Scopes,
|
||||
threads::MessageBody,
|
||||
},
|
||||
queue::session::AuthQueue,
|
||||
routes::ApiError,
|
||||
@@ -34,6 +32,7 @@ use crate::{
|
||||
};
|
||||
|
||||
pub mod rescan;
|
||||
pub mod tech_review_sync;
|
||||
|
||||
pub fn config(cfg: &mut actix_web::web::ServiceConfig) {
|
||||
cfg.service(
|
||||
@@ -211,107 +210,6 @@ async fn ingest_report_deserialized(
|
||||
"Delphi found issues in file",
|
||||
);
|
||||
|
||||
let record = sqlx::query!(
|
||||
r#"
|
||||
SELECT
|
||||
EXISTS(
|
||||
SELECT 1 FROM delphi_issue_details_with_statuses didws
|
||||
WHERE didws.project_id = $1 AND didws.status = 'pending'
|
||||
) AS "pending_issue_details_exist!",
|
||||
t.id AS "thread_id: DBThreadId"
|
||||
FROM mods m
|
||||
INNER JOIN threads t ON t.mod_id = $1
|
||||
"#,
|
||||
DBProjectId::from(report.project_id) as _,
|
||||
)
|
||||
.fetch_one(&mut transaction)
|
||||
.await
|
||||
.wrap_internal_err("failed to check if pending issue details exist")?;
|
||||
|
||||
let issue_detail_keys = report
|
||||
.issues
|
||||
.values()
|
||||
.flatten()
|
||||
.map(|issue_detail| issue_detail.key.0.clone())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let has_unflagged_issue_details = sqlx::query!(
|
||||
r#"
|
||||
SELECT EXISTS(
|
||||
SELECT 1
|
||||
FROM unnest($2::text[]) AS incoming(detail_key)
|
||||
LEFT JOIN delphi_global_detail_verdicts dgdv
|
||||
ON dgdv.detail_key = incoming.detail_key
|
||||
LEFT JOIN delphi_issue_detail_verdicts didv
|
||||
ON didv.project_id = $1 AND didv.detail_key = incoming.detail_key
|
||||
WHERE dgdv.detail_key IS NULL AND didv.project_id IS NULL
|
||||
) AS "has_unflagged_issue_details!"
|
||||
"#,
|
||||
DBProjectId::from(report.project_id) as _,
|
||||
&issue_detail_keys
|
||||
)
|
||||
.fetch_one(&mut transaction)
|
||||
.await
|
||||
.wrap_internal_err("failed to check if report has unflagged issue details")?;
|
||||
|
||||
let should_enter_tech_review = !record.pending_issue_details_exist
|
||||
&& has_unflagged_issue_details.has_unflagged_issue_details;
|
||||
|
||||
if should_enter_tech_review {
|
||||
info!("File's project is entering tech review queue");
|
||||
|
||||
ThreadMessageBuilder {
|
||||
author_id: None,
|
||||
body: MessageBody::TechReviewEntered,
|
||||
thread_id: record.thread_id,
|
||||
hide_identity: false,
|
||||
}
|
||||
.insert(&mut transaction)
|
||||
.await
|
||||
.wrap_internal_err("failed to add entering tech review message")?;
|
||||
} else {
|
||||
info!(
|
||||
"File's project is not entering tech review queue (already pending or no new unflagged issue details)"
|
||||
);
|
||||
}
|
||||
|
||||
// TODO: Currently, the way we determine if an issue is in tech review or not
|
||||
// is if it has any issue details which are pending.
|
||||
// If you mark all issue details are safe or not safe - even if you don't
|
||||
// submit the final report - the project will be taken out of tech review
|
||||
// queue, and into moderation queue.
|
||||
//
|
||||
// This is undesirable, but we can't rework the database schema to fix it
|
||||
// right now. As a hack, we add a dummy report issue which blocks the
|
||||
// project from exiting the tech review queue.
|
||||
if should_enter_tech_review {
|
||||
let dummy_issue_id = DBDelphiReportIssue {
|
||||
id: DelphiReportIssueId(0), // This will be set by the database
|
||||
report_id,
|
||||
issue_type: "__dummy".into(),
|
||||
}
|
||||
.upsert(&mut transaction)
|
||||
.await
|
||||
.wrap_internal_err("failed to upsert dummy Delphi report issue")?;
|
||||
|
||||
ReportIssueDetail {
|
||||
id: DelphiReportIssueDetailsId(0), // This will be set by the database
|
||||
issue_id: dummy_issue_id,
|
||||
key: "".into(),
|
||||
jar: None,
|
||||
file_path: "".into(),
|
||||
decompiled_source: None,
|
||||
data: HashMap::new(),
|
||||
severity: DelphiSeverity::Low,
|
||||
status: DelphiStatus::Pending,
|
||||
}
|
||||
.insert(&mut transaction)
|
||||
.await
|
||||
.wrap_internal_err(
|
||||
"failed to insert dummy Delphi report issue detail",
|
||||
)?;
|
||||
}
|
||||
|
||||
for (issue_type, issue_details) in report.issues {
|
||||
let issue_id = DBDelphiReportIssue {
|
||||
id: DelphiReportIssueId(0), // This will be set by the database
|
||||
@@ -340,6 +238,8 @@ async fn ingest_report_deserialized(
|
||||
decompiled_source: decompiled_source.cloned().flatten(),
|
||||
data: issue_detail.data,
|
||||
severity: issue_detail.severity,
|
||||
local_status: None,
|
||||
global_status: None,
|
||||
status: DelphiStatus::Pending,
|
||||
}
|
||||
.insert(&mut transaction)
|
||||
@@ -348,6 +248,13 @@ async fn ingest_report_deserialized(
|
||||
}
|
||||
}
|
||||
|
||||
tech_review_sync::sync_project_tech_review_state(
|
||||
&[DBProjectId::from(report.project_id)],
|
||||
tech_review_sync::TechReviewExitReason::Resolved,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
|
||||
transaction
|
||||
.commit()
|
||||
.await
|
||||
@@ -397,83 +304,6 @@ pub async fn run(
|
||||
Ok(HttpResponse::NoContent().finish())
|
||||
}
|
||||
|
||||
pub async fn is_project_in_tech_review(
|
||||
project_id: DBProjectId,
|
||||
exec: impl crate::database::Executor<'_, Database = sqlx::Postgres>,
|
||||
) -> Result<bool, ApiError> {
|
||||
let row = sqlx::query!(
|
||||
r#"
|
||||
SELECT EXISTS(
|
||||
SELECT 1
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id
|
||||
WHERE
|
||||
didws.project_id = $1
|
||||
AND didws.status = 'pending'
|
||||
-- see delphi.rs todo comment
|
||||
AND dri.issue_type != '__dummy'
|
||||
) AS "is_in_tech_review!"
|
||||
"#,
|
||||
project_id as _,
|
||||
)
|
||||
.fetch_one(exec)
|
||||
.await
|
||||
.wrap_internal_err("failed to fetch project tech review state")?;
|
||||
|
||||
Ok(row.is_in_tech_review)
|
||||
}
|
||||
|
||||
pub async fn send_tech_review_exit_file_deleted_message(
|
||||
project_id: DBProjectId,
|
||||
txn: &mut crate::database::PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let thread = sqlx::query!(
|
||||
r#"
|
||||
SELECT id AS "thread_id: DBThreadId"
|
||||
FROM threads
|
||||
WHERE mod_id = $1
|
||||
LIMIT 1
|
||||
"#,
|
||||
project_id as _,
|
||||
)
|
||||
.fetch_optional(&mut *txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to fetch thread for tech review exit message")?;
|
||||
|
||||
if let Some(thread) = thread {
|
||||
ThreadMessageBuilder {
|
||||
author_id: None,
|
||||
body: MessageBody::TechReviewExitFileDeleted,
|
||||
thread_id: thread.thread_id,
|
||||
hide_identity: false,
|
||||
}
|
||||
.insert(txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to add tech review exit message")?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn send_tech_review_exit_file_deleted_message_if_exited(
|
||||
project_id: DBProjectId,
|
||||
was_in_tech_review: bool,
|
||||
txn: &mut crate::database::PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
if !was_in_tech_review {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let is_still_in_tech_review =
|
||||
is_project_in_tech_review(project_id, &mut *txn).await?;
|
||||
|
||||
if !is_still_in_tech_review {
|
||||
send_tech_review_exit_file_deleted_message(project_id, txn).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Run Delphi.
|
||||
#[utoipa::path(
|
||||
context_path = "/delphi",
|
||||
|
||||
@@ -0,0 +1,411 @@
|
||||
//! Synchronizes moderation thread messages and dummy queue blockers with the
|
||||
//! current computed tech review state of affected projects.
|
||||
//!
|
||||
//! When a project has a Delphi report submitted for it, or when a moderator
|
||||
//! updates one of its issue details' rows (like flagging a detail as *globally*
|
||||
//! safe or unsafe), we will need to recheck if the project still belongs in the
|
||||
//! tech review queue or if it needs to exit now.
|
||||
//!
|
||||
//! Side-note: "entering the queue" or "exiting the queue" right now just means
|
||||
//! adding a new message to the project's moderation thread which indicates if
|
||||
//! it entered/exited. In the future this should be replaced with a more proper
|
||||
//! audit log table, or "is project currently in tech review" table.
|
||||
//!
|
||||
//! A project is considered to need tech review when it has at least one
|
||||
//! non-dummy issue detail whose effective status is pending or unsafe, or when
|
||||
//! it already has a dummy pending detail blocking the final review submission.
|
||||
//! Effective status is just the local detail's verdict (from
|
||||
//! `delphi_issue_detail_verdicts`), or if it's null then the global verdict for
|
||||
//! the same `drid.key` (from `delphi_global_detail_verdicts`).
|
||||
//!
|
||||
//! Some examples of how this behavior manifests: let's assume you have projects
|
||||
//! _A_ and _B_ currently in tech review. They each have one (unresolved) issue
|
||||
//! detail with key _K_.
|
||||
//! - If you mark _K_ on _A_ as locally safe/unsafe, then _A_ is fully resolved,
|
||||
//! but we still have the `__dummy` detail, which means it's still in the
|
||||
//! queue until the moderator submits the actual report. _B_ is entirely
|
||||
//! unaffected.
|
||||
//! - If you mark _K_ on _A_ as globally safe, then _A_ and _B_ both get fully
|
||||
//! resolved, but both still have the `__dummy` detail, so they also still
|
||||
//! need the final report to be submitted by the moderator.
|
||||
//!
|
||||
//! In practice, this means that some projects may have e.g. "100/100 traces
|
||||
//! are safe" reported, but they will just be waiting for final moderator
|
||||
//! approval.
|
||||
//!
|
||||
//! The logic for checking whether a project is now in tech review or not, and
|
||||
//! sending the appropriate message, is complex! That's why this module exists:
|
||||
//! to act as a single chokepoint which (correctly) syncs all the state, instead
|
||||
//! of having each mutation run its own ad-hoc update logic.
|
||||
|
||||
use itertools::Itertools;
|
||||
|
||||
use crate::{
|
||||
database::{
|
||||
PgTransaction,
|
||||
models::{
|
||||
DBProjectId, DBThreadId, DelphiReportId,
|
||||
delphi_report_item::DelphiVerdict,
|
||||
thread_item::ThreadMessageBuilder,
|
||||
},
|
||||
},
|
||||
models::threads::MessageBody,
|
||||
routes::ApiError,
|
||||
util::error::Context,
|
||||
};
|
||||
|
||||
const DUMMY_ISSUE_TYPE: &str = "__dummy";
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub enum TechReviewExitReason {
|
||||
Resolved,
|
||||
FileDeleted,
|
||||
}
|
||||
|
||||
struct ProjectTechReviewState {
|
||||
has_pending_detail: bool,
|
||||
has_unsafe_detail: bool,
|
||||
has_dummy: bool,
|
||||
thread_id: Option<DBThreadId>,
|
||||
report_id: Option<DelphiReportId>,
|
||||
last_tech_review_message_type: Option<String>,
|
||||
}
|
||||
|
||||
pub async fn sync_project_tech_review_state(
|
||||
project_ids: &[DBProjectId],
|
||||
exit_reason: TechReviewExitReason,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let project_ids = project_ids.iter().copied().unique().collect::<Vec<_>>();
|
||||
if project_ids.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let project_ids_raw = project_ids.iter().map(|id| id.0).collect::<Vec<_>>();
|
||||
let tech_review_message_types = tech_review_message_types();
|
||||
|
||||
let rows = sqlx::query!(
|
||||
r#"
|
||||
WITH project_ids AS (
|
||||
SELECT unnest($1::bigint[]) AS project_id
|
||||
)
|
||||
SELECT
|
||||
p.project_id AS "project_id!: DBProjectId",
|
||||
EXISTS(
|
||||
SELECT 1
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id
|
||||
WHERE
|
||||
didws.project_id = p.project_id
|
||||
AND didws.status = 'pending'
|
||||
AND dri.issue_type != $3
|
||||
) AS "has_pending_detail!",
|
||||
EXISTS(
|
||||
SELECT 1
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id
|
||||
WHERE
|
||||
didws.project_id = p.project_id
|
||||
AND didws.status = 'unsafe'
|
||||
AND dri.issue_type != $3
|
||||
) AS "has_unsafe_detail!",
|
||||
EXISTS(
|
||||
SELECT 1
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id
|
||||
WHERE
|
||||
didws.project_id = p.project_id
|
||||
AND didws.status = 'pending'
|
||||
AND dri.issue_type = $3
|
||||
) AS "has_dummy!",
|
||||
(
|
||||
SELECT t.id
|
||||
FROM threads t
|
||||
WHERE t.mod_id = p.project_id
|
||||
ORDER BY t.id
|
||||
LIMIT 1
|
||||
) AS "thread_id: DBThreadId",
|
||||
(
|
||||
SELECT tm.body->>'type'
|
||||
FROM threads t
|
||||
INNER JOIN threads_messages tm ON tm.thread_id = t.id
|
||||
WHERE
|
||||
t.mod_id = p.project_id
|
||||
AND tm.body->>'type' = ANY($2::text[])
|
||||
ORDER BY tm.created DESC, tm.id DESC
|
||||
LIMIT 1
|
||||
) AS "last_tech_review_message_type",
|
||||
(
|
||||
SELECT dr.id
|
||||
FROM versions v
|
||||
INNER JOIN files f ON f.version_id = v.id
|
||||
INNER JOIN delphi_reports dr ON dr.file_id = f.id
|
||||
WHERE v.mod_id = p.project_id
|
||||
ORDER BY dr.created DESC, dr.id DESC
|
||||
LIMIT 1
|
||||
) AS "report_id: DelphiReportId"
|
||||
FROM project_ids p
|
||||
"#,
|
||||
&project_ids_raw,
|
||||
&tech_review_message_types,
|
||||
DUMMY_ISSUE_TYPE,
|
||||
)
|
||||
.fetch_all(&mut *txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to fetch project tech review state")?;
|
||||
|
||||
for row in rows {
|
||||
let state = ProjectTechReviewState {
|
||||
has_pending_detail: row.has_pending_detail,
|
||||
has_unsafe_detail: row.has_unsafe_detail,
|
||||
has_dummy: row.has_dummy,
|
||||
thread_id: row.thread_id,
|
||||
report_id: row.report_id,
|
||||
last_tech_review_message_type: row.last_tech_review_message_type,
|
||||
};
|
||||
|
||||
sync_one_project_tech_review_state(state, exit_reason, txn).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn sync_detail_key_tech_review_state(
|
||||
detail_keys: &[String],
|
||||
exit_reason: TechReviewExitReason,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let detail_keys = detail_keys.iter().cloned().unique().collect::<Vec<_>>();
|
||||
|
||||
if detail_keys.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let rows = sqlx::query!(
|
||||
r#"
|
||||
SELECT DISTINCT didws.project_id AS "project_id!: DBProjectId"
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
WHERE didws.key = ANY($1::text[])
|
||||
"#,
|
||||
&detail_keys,
|
||||
)
|
||||
.fetch_all(&mut *txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to fetch projects affected by detail keys")?;
|
||||
|
||||
let project_ids = rows
|
||||
.into_iter()
|
||||
.map(|row| row.project_id)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
sync_project_tech_review_state(&project_ids, exit_reason, txn).await
|
||||
}
|
||||
|
||||
pub async fn sync_deleted_project_tech_review_exit(
|
||||
project_id: DBProjectId,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let tech_review_message_types = tech_review_message_types();
|
||||
|
||||
let row = sqlx::query!(
|
||||
r#"
|
||||
SELECT
|
||||
(
|
||||
SELECT t.id
|
||||
FROM threads t
|
||||
WHERE t.mod_id = $1
|
||||
ORDER BY t.id
|
||||
LIMIT 1
|
||||
) AS "thread_id: DBThreadId",
|
||||
(
|
||||
SELECT tm.body->>'type'
|
||||
FROM threads t
|
||||
INNER JOIN threads_messages tm ON tm.thread_id = t.id
|
||||
WHERE
|
||||
t.mod_id = $1
|
||||
AND tm.body->>'type' = ANY($2::text[])
|
||||
ORDER BY tm.created DESC, tm.id DESC
|
||||
LIMIT 1
|
||||
) AS "last_tech_review_message_type"
|
||||
"#,
|
||||
project_id as DBProjectId,
|
||||
&tech_review_message_types,
|
||||
)
|
||||
.fetch_one(&mut *txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to fetch deleted project tech review state")?;
|
||||
|
||||
if let Some(thread_id) = row.thread_id
|
||||
&& should_send_exit(row.last_tech_review_message_type.as_deref())
|
||||
{
|
||||
insert_exit_message(thread_id, TechReviewExitReason::FileDeleted, txn)
|
||||
.await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn sync_one_project_tech_review_state(
|
||||
state: ProjectTechReviewState,
|
||||
exit_reason: TechReviewExitReason,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let needs_tech_review =
|
||||
state.has_pending_detail || state.has_unsafe_detail || state.has_dummy;
|
||||
|
||||
if needs_tech_review {
|
||||
if (state.has_pending_detail || state.has_unsafe_detail)
|
||||
&& !state.has_dummy
|
||||
&& let Some(report_id) = state.report_id
|
||||
{
|
||||
// TODO: Currently, the queue query determines whether a project is
|
||||
// in tech review by checking whether it has any pending issue
|
||||
// details. If all visible issue details are marked safe or unsafe
|
||||
// before the final report is submitted, the project would otherwise
|
||||
// leave the tech review queue without a final tech review verdict
|
||||
// message.
|
||||
//
|
||||
// This should be replaced with explicit tech review state, such as
|
||||
// an append-only project tech review event table where the latest
|
||||
// enter/exit event is the current state. Until then, this dummy
|
||||
// issue detail acts as the pending queue blocker.
|
||||
ensure_dummy_issue_detail(report_id, txn).await?;
|
||||
}
|
||||
|
||||
if let Some(thread_id) = state.thread_id
|
||||
&& state.last_tech_review_message_type.as_deref()
|
||||
!= Some(MessageBody::TechReviewEntered.as_ref())
|
||||
{
|
||||
ThreadMessageBuilder {
|
||||
author_id: None,
|
||||
body: MessageBody::TechReviewEntered,
|
||||
thread_id,
|
||||
hide_identity: false,
|
||||
}
|
||||
.insert(txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to add entering tech review message")?;
|
||||
}
|
||||
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if matches!(exit_reason, TechReviewExitReason::Resolved)
|
||||
&& state.last_tech_review_message_type.as_deref()
|
||||
== Some(MessageBody::TechReviewEntered.as_ref())
|
||||
{
|
||||
if let Some(report_id) = state.report_id {
|
||||
ensure_dummy_issue_detail(report_id, txn).await?;
|
||||
}
|
||||
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if let Some(thread_id) = state.thread_id
|
||||
&& should_send_exit(state.last_tech_review_message_type.as_deref())
|
||||
{
|
||||
insert_exit_message(thread_id, exit_reason, txn).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn should_send_exit(last_tech_review_message_type: Option<&str>) -> bool {
|
||||
matches!(last_tech_review_message_type, Some(message_type) if !matches!(
|
||||
message_type,
|
||||
message_type if message_type == MessageBody::TechReviewExited.as_ref()
|
||||
|| message_type == MessageBody::TechReviewExitFileDeleted.as_ref()
|
||||
|| message_type == tech_review_completed_message_type()
|
||||
))
|
||||
}
|
||||
|
||||
async fn insert_exit_message(
|
||||
thread_id: DBThreadId,
|
||||
exit_reason: TechReviewExitReason,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
let body = match exit_reason {
|
||||
TechReviewExitReason::Resolved => MessageBody::TechReviewExited,
|
||||
TechReviewExitReason::FileDeleted => {
|
||||
MessageBody::TechReviewExitFileDeleted
|
||||
}
|
||||
};
|
||||
|
||||
ThreadMessageBuilder {
|
||||
author_id: None,
|
||||
body,
|
||||
thread_id,
|
||||
hide_identity: false,
|
||||
}
|
||||
.insert(txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to add exiting tech review message")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn ensure_dummy_issue_detail(
|
||||
report_id: DelphiReportId,
|
||||
txn: &mut PgTransaction<'_>,
|
||||
) -> Result<(), ApiError> {
|
||||
sqlx::query!(
|
||||
r#"
|
||||
WITH dummy_issue AS (
|
||||
INSERT INTO delphi_report_issues (report_id, issue_type)
|
||||
VALUES ($1, $2)
|
||||
ON CONFLICT (report_id, issue_type)
|
||||
DO UPDATE SET issue_type = EXCLUDED.issue_type
|
||||
RETURNING id
|
||||
)
|
||||
INSERT INTO delphi_report_issue_details (
|
||||
issue_id,
|
||||
key,
|
||||
jar,
|
||||
file_path,
|
||||
decompiled_source,
|
||||
data,
|
||||
severity
|
||||
)
|
||||
SELECT
|
||||
id,
|
||||
'',
|
||||
NULL,
|
||||
'',
|
||||
NULL,
|
||||
'{}'::jsonb,
|
||||
'low'::delphi_severity
|
||||
FROM dummy_issue
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM delphi_report_issue_details drid
|
||||
WHERE drid.issue_id = dummy_issue.id
|
||||
)
|
||||
"#,
|
||||
report_id as DelphiReportId,
|
||||
DUMMY_ISSUE_TYPE,
|
||||
)
|
||||
.execute(&mut *txn)
|
||||
.await
|
||||
.wrap_internal_err("failed to ensure dummy Delphi report issue detail")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn tech_review_message_types() -> Vec<String> {
|
||||
[
|
||||
MessageBody::TechReviewEntered.as_ref(),
|
||||
MessageBody::TechReviewExited.as_ref(),
|
||||
MessageBody::TechReviewExitFileDeleted.as_ref(),
|
||||
tech_review_completed_message_type(),
|
||||
]
|
||||
.into_iter()
|
||||
.map(|message_type| message_type.to_string())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn tech_review_completed_message_type() -> &'static str {
|
||||
MessageBody::TechReview {
|
||||
verdict: DelphiVerdict::Safe,
|
||||
}
|
||||
.as_ref()
|
||||
}
|
||||
@@ -31,7 +31,13 @@ use crate::{
|
||||
threads::{MessageBody, Thread},
|
||||
},
|
||||
queue::session::AuthQueue,
|
||||
routes::{ApiError, internal::moderation::Ownership},
|
||||
routes::{
|
||||
ApiError,
|
||||
internal::{
|
||||
delphi::tech_review_sync::{self, TechReviewExitReason},
|
||||
moderation::Ownership,
|
||||
},
|
||||
},
|
||||
search::SearchState,
|
||||
util::error::Context,
|
||||
};
|
||||
@@ -238,6 +244,8 @@ pub async fn get_issue(
|
||||
'decompiled_source', didws.decompiled_source,
|
||||
'data', didws.data,
|
||||
'severity', didws.severity,
|
||||
'local_status', didws.local_status,
|
||||
'global_status', didws.global_status,
|
||||
'status', didws.status
|
||||
)
|
||||
), '[]'::jsonb)
|
||||
@@ -313,6 +321,8 @@ pub async fn get_report(
|
||||
'decompiled_source', didws.decompiled_source,
|
||||
'data', didws.data,
|
||||
'severity', didws.severity,
|
||||
'local_status', didws.local_status,
|
||||
'global_status', didws.global_status,
|
||||
'status', didws.status
|
||||
)
|
||||
), '[]'::jsonb)
|
||||
@@ -513,6 +523,8 @@ async fn fetch_project_reports(
|
||||
didws.file_path AS "file_path!: String",
|
||||
didws.data AS "data!: sqlx::types::Json<HashMap<String, serde_json::Value>>",
|
||||
didws.severity AS "severity!: DelphiSeverity",
|
||||
didws.local_status AS "local_status?: DelphiStatus",
|
||||
didws.global_status AS "global_status?: DelphiStatus",
|
||||
didws.status AS "status!: DelphiStatus"
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
WHERE didws.issue_id = ANY($1::bigint[])
|
||||
@@ -571,6 +583,8 @@ async fn fetch_project_reports(
|
||||
decompiled_source: None,
|
||||
data: d.data.0,
|
||||
severity: d.severity,
|
||||
local_status: d.local_status,
|
||||
global_status: d.global_status,
|
||||
status: d.status,
|
||||
})
|
||||
.into_group_map_by(|d| d.issue_id);
|
||||
@@ -1176,7 +1190,9 @@ pub struct UpdateGlobalIssue {
|
||||
/// Key of the issue detail to update globally.
|
||||
pub detail_key: String,
|
||||
/// What the moderator has decided the outcome of this issue is globally.
|
||||
pub verdict: DelphiVerdict,
|
||||
///
|
||||
/// `pending` removes the global verdict for this issue detail key.
|
||||
pub verdict: DelphiStatus,
|
||||
}
|
||||
|
||||
/// Update technical review issue details.
|
||||
@@ -1296,6 +1312,35 @@ pub async fn update_issue_details(
|
||||
return Err(ApiError::Request(eyre!("issue detail does not exist")));
|
||||
}
|
||||
|
||||
let affected_projects = sqlx::query!(
|
||||
r#"
|
||||
SELECT DISTINCT didws.project_id AS "project_id!: DBProjectId"
|
||||
FROM delphi_issue_details_with_statuses didws
|
||||
INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id
|
||||
WHERE
|
||||
didws.id = ANY($1::bigint[])
|
||||
AND dri.issue_type != '__dummy'
|
||||
"#,
|
||||
&detail_ids,
|
||||
)
|
||||
.fetch_all(&mut txn)
|
||||
.await
|
||||
.wrap_internal_err(
|
||||
"failed to fetch projects affected by issue detail updates",
|
||||
)?;
|
||||
|
||||
let affected_project_ids = affected_projects
|
||||
.into_iter()
|
||||
.map(|row| row.project_id)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
tech_review_sync::sync_project_tech_review_state(
|
||||
&affected_project_ids,
|
||||
TechReviewExitReason::Resolved,
|
||||
&mut txn,
|
||||
)
|
||||
.await?;
|
||||
|
||||
txn.commit()
|
||||
.await
|
||||
.wrap_internal_err("failed to commit transaction")?;
|
||||
@@ -1305,7 +1350,8 @@ pub async fn update_issue_details(
|
||||
|
||||
/// Update global technical review issue detail verdicts.
|
||||
///
|
||||
/// This marks every issue detail with a matching key as safe or unsafe.
|
||||
/// This marks every issue detail with a matching key as safe or unsafe, or
|
||||
/// unsets the global verdict with `pending`.
|
||||
#[utoipa::path(
|
||||
context_path = "/moderation/tech-review",
|
||||
tag = "moderation",
|
||||
@@ -1346,8 +1392,9 @@ pub async fn update_global_issue_details(
|
||||
let verdicts = updates
|
||||
.iter()
|
||||
.map(|u| match u.verdict {
|
||||
DelphiVerdict::Safe => "safe".to_string(),
|
||||
DelphiVerdict::Unsafe => "unsafe".to_string(),
|
||||
DelphiStatus::Safe => "safe".to_string(),
|
||||
DelphiStatus::Unsafe => "unsafe".to_string(),
|
||||
DelphiStatus::Pending => "pending".to_string(),
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -1362,16 +1409,31 @@ pub async fn update_global_issue_details(
|
||||
SELECT *
|
||||
FROM unnest($1::text[], $2::text[]) WITH ORDINALITY
|
||||
AS u(detail_key, verdict, ord)
|
||||
),
|
||||
latest AS (
|
||||
SELECT DISTINCT ON (detail_key)
|
||||
detail_key,
|
||||
verdict
|
||||
FROM incoming
|
||||
ORDER BY detail_key, ord DESC
|
||||
),
|
||||
deleted AS (
|
||||
DELETE FROM delphi_global_detail_verdicts dgdv
|
||||
USING latest
|
||||
WHERE
|
||||
dgdv.detail_key = latest.detail_key
|
||||
AND latest.verdict = 'pending'
|
||||
RETURNING 1
|
||||
)
|
||||
INSERT INTO delphi_global_detail_verdicts (
|
||||
detail_key,
|
||||
verdict
|
||||
)
|
||||
SELECT DISTINCT ON (detail_key)
|
||||
SELECT
|
||||
detail_key,
|
||||
verdict::delphi_report_issue_status
|
||||
FROM incoming
|
||||
ORDER BY detail_key, ord DESC
|
||||
FROM latest
|
||||
WHERE verdict != 'pending'
|
||||
ON CONFLICT (detail_key)
|
||||
DO UPDATE SET verdict = EXCLUDED.verdict
|
||||
"#,
|
||||
@@ -1382,6 +1444,13 @@ pub async fn update_global_issue_details(
|
||||
.await
|
||||
.wrap_internal_err("failed to update global issue details")?;
|
||||
|
||||
tech_review_sync::sync_detail_key_tech_review_state(
|
||||
&detail_keys,
|
||||
TechReviewExitReason::Resolved,
|
||||
&mut txn,
|
||||
)
|
||||
.await?;
|
||||
|
||||
txn.commit()
|
||||
.await
|
||||
.wrap_internal_err("failed to commit transaction")?;
|
||||
|
||||
@@ -2746,17 +2746,11 @@ pub async fn project_delete_internal(
|
||||
.begin()
|
||||
.await
|
||||
.wrap_internal_err("failed to start transaction")?;
|
||||
let was_in_tech_review =
|
||||
delphi::is_project_in_tech_review(project.inner.id, &mut transaction)
|
||||
.await?;
|
||||
|
||||
if was_in_tech_review {
|
||||
delphi::send_tech_review_exit_file_deleted_message(
|
||||
project.inner.id,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
delphi::tech_review_sync::sync_deleted_project_tech_review_exit(
|
||||
project.inner.id,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let context = ImageContext::Project {
|
||||
project_id: Some(project.inner.id.into()),
|
||||
|
||||
@@ -910,9 +910,6 @@ pub async fn delete_file(
|
||||
}
|
||||
|
||||
let mut transaction = pool.begin().await?;
|
||||
let was_in_tech_review =
|
||||
delphi::is_project_in_tech_review(row.project_id, &mut transaction)
|
||||
.await?;
|
||||
|
||||
sqlx::query!(
|
||||
"
|
||||
@@ -937,9 +934,9 @@ pub async fn delete_file(
|
||||
database::models::version_item::cleanup_unused_attribution_files_and_groups(&mut transaction)
|
||||
.await?;
|
||||
|
||||
delphi::send_tech_review_exit_file_deleted_message_if_exited(
|
||||
row.project_id,
|
||||
was_in_tech_review,
|
||||
delphi::tech_review_sync::sync_project_tech_review_state(
|
||||
&[row.project_id],
|
||||
delphi::tech_review_sync::TechReviewExitReason::FileDeleted,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1217,11 +1217,6 @@ pub async fn version_delete(
|
||||
}
|
||||
|
||||
let mut transaction = pool.begin().await?;
|
||||
let was_in_tech_review = delphi::is_project_in_tech_review(
|
||||
version.inner.project_id,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let context = ImageContext::Version {
|
||||
version_id: Some(version.inner.id.into()),
|
||||
@@ -1242,9 +1237,9 @@ pub async fn version_delete(
|
||||
)
|
||||
.await?;
|
||||
|
||||
delphi::send_tech_review_exit_file_deleted_message_if_exited(
|
||||
version.inner.project_id,
|
||||
was_in_tech_review,
|
||||
delphi::tech_review_sync::sync_project_tech_review_state(
|
||||
&[version.inner.project_id],
|
||||
delphi::tech_review_sync::TechReviewExitReason::FileDeleted,
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
|
||||
Reference in New Issue
Block a user