From 6ffdcd8d1f7e40090cb04c80d39bbbc7ce6f2d37 Mon Sep 17 00:00:00 2001 From: tdgao Date: Thu, 29 Oct 2026 17:45:39 -0600 Subject: [PATCH 01/19] feat: remove tech review summary private message and add issue count with verdict --- .../tech-review/TechRevThreadTab.vue | 145 +----------------- .../components/ui/thread/ThreadMessage.vue | 29 +++- ...778827ac0459ce36353a2a70d98a54eb47a8d.json | 28 ---- ...d96e2f0cdebdf44c449ee8199d6b26618ac9a.json | 51 ++++++ apps/labrinth/src/models/v2/threads.rs | 2 +- apps/labrinth/src/models/v3/threads.rs | 4 + .../routes/internal/moderation/tech_review.rs | 36 +++-- .../api-client/src/modules/labrinth/types.ts | 4 + 8 files changed, 114 insertions(+), 185 deletions(-) delete mode 100644 apps/labrinth/.sqlx/query-36899cf74976b8bec1452b071af778827ac0459ce36353a2a70d98a54eb47a8d.json create mode 100644 apps/labrinth/.sqlx/query-7bac4e23534619c79e18b3f832cd96e2f0cdebdf44c449ee8199d6b26618ac9a.json diff --git a/apps/frontend/src/components/ui/moderation/tech-review/TechRevThreadTab.vue b/apps/frontend/src/components/ui/moderation/tech-review/TechRevThreadTab.vue index 18d95256794..a5862cab540 100644 --- a/apps/frontend/src/components/ui/moderation/tech-review/TechRevThreadTab.vue +++ b/apps/frontend/src/components/ui/moderation/tech-review/TechRevThreadTab.vue @@ -19,18 +19,13 @@ import { injectModrinthClient, injectNotificationManager, TeleportOverflowMenu, - useFormatBytes, - useFormatDateTime, useVIntl, } from '@modrinth/ui' -import { capitalizeString, type ThreadMessage, type User } from '@modrinth/utils' -import dayjs from 'dayjs' import { computed, ref } from 'vue' import type { UnsafeFile } from '~/components/ui/moderation/MaliciousSummaryModal.vue' import ThreadView from '~/components/ui/thread/ThreadView.vue' -import { severityOrder } from './helpers' import type { FlattenedFileReport } from './types' import { injectTechReviewDecisions } from './use-tech-review-decisions' @@ -51,23 +46,11 @@ const emit = defineEmits<{ statusChanged: [status: Labrinth.Projects.v2.ProjectStatus] }>() -const auth = useAuthState() const featureFlags = useFeatureFlags() const { formatMessage } = useVIntl() const { addNotification } = injectNotificationManager() const client = injectModrinthClient() const { getDetailDecision } = injectTechReviewDecisions() -const formatBytes = useFormatBytes() - -const formatDateTimeUtc = useFormatDateTime({ - year: 'numeric', - month: 'long', - day: 'numeric', - hour: 'numeric', - minute: '2-digit', - timeZoneName: 'short', - timeZone: 'UTC', -}) const remainingMessageCount = computed(() => { if (!props.thread?.messages) return 0 @@ -168,121 +151,6 @@ const unsafeFiles = computed(() => { })) }) -const reviewSummaryPreview = computed(() => { - const fileDecisions = new Map< - string, - { - fileName: string - fileSize: number - decisions: { - filePath: string - issueType: string - severity: string - decision: 'safe' | 'malware' - }[] - maxSeverity: Labrinth.TechReview.Internal.DelphiSeverity - } - >() - let totalSafe = 0 - let totalUnsafe = 0 - - for (const report of props.reports) { - if (!fileDecisions.has(report.id)) { - fileDecisions.set(report.id, { - fileName: report.file_name, - fileSize: report.file_size, - decisions: [], - maxSeverity: 'low', - }) - } - const fileData = fileDecisions.get(report.id)! - - for (const issue of report.issues) { - for (const detail of issue.details) { - const decision = getDetailDecision(detail.id, detail.status) - if (decision === 'pending') continue - - fileData.decisions.push({ - filePath: detail.file_path, - issueType: issue.issue_type.replace(/_/g, ' '), - severity: detail.severity, - decision, - }) - - if (severityOrder[detail.severity] > severityOrder[fileData.maxSeverity]) { - fileData.maxSeverity = detail.severity - } - - if (decision === 'safe') totalSafe++ - else totalUnsafe++ - } - } - } - - const totalDecisions = totalSafe + totalUnsafe - if (totalDecisions === 0) return '' - - const timestamp = formatDateTimeUtc(dayjs().toDate()) - let markdown = `## Tech Review Summary\n*${timestamp}*\n\n` - markdown += `
\nFile Details (${totalSafe} safe, ${totalUnsafe} unsafe)\n\n` - - for (const [, fileData] of fileDecisions) { - if (fileData.decisions.length === 0) continue - - const fileSafe = fileData.decisions.filter((d) => d.decision === 'safe').length - const fileUnsafe = fileData.decisions.filter((d) => d.decision === 'malware').length - const fileVerdict = fileUnsafe > 0 ? 'Unsafe' : 'Safe' - - markdown += `### ${fileData.fileName}\n` - markdown += `> ${formatBytes(fileData.fileSize)} • ${fileData.decisions.length} issues • Max severity: ${fileData.maxSeverity} • **Verdict:** ${fileVerdict}\n\n` - markdown += `
\nIssues (${fileSafe} safe, ${fileUnsafe} unsafe)\n\n` - markdown += `| Class | Issue Type | Severity | Decision |\n` - markdown += `|-------|------------|----------|----------|\n` - - for (const d of fileData.decisions) { - const decisionText = d.decision === 'safe' ? '✅ Safe' : '❌ Unsafe' - markdown += `| \`${d.filePath}\` | ${d.issueType} | ${capitalizeString(d.severity)} | ${decisionText} |\n` - } - - markdown += `\n
\n\n` - } - - markdown += `
\n\n` - markdown += `---\n\n**Total:** ${totalDecisions} issues reviewed (${totalSafe} safe, ${totalUnsafe} unsafe)\n\n` - - return markdown -}) - -const threadWithPreview = computed(() => { - if (!reviewSummaryPreview.value) return props.thread - - const user = auth.value?.user as User | null - if (!user) return props.thread - - const previewMessage: ThreadMessage & { preview: true } = { - id: 'preview-message', - author_id: user.id, - body: { - type: 'text', - body: reviewSummaryPreview.value, - private: true, - replying_to: null, - associated_images: [], - }, - created: new Date().toISOString(), - hide_identity: false, - preview: true, - } - - return { - ...props.thread, - messages: [...props.thread.messages, previewMessage], - members: props.thread.members.some((m) => m.id === user.id) - ? props.thread.members - : [...props.thread.members, user], - } -}) - const allIssuesResolved = computed(() => { for (const report of props.reports) { for (const issue of report.issues) { @@ -305,19 +173,10 @@ async function handleSubmitReview(verdict: 'safe' | 'unsafe') { hasSubmittedPassReview.value = verdict === 'safe' const editorContent = threadViewRef.value?.getReplyContent() || '' - let message: string | undefined - if (reviewSummaryPreview.value && editorContent) { - message = `${reviewSummaryPreview.value}${editorContent}` - } else if (reviewSummaryPreview.value) { - message = reviewSummaryPreview.value - } else if (editorContent) { - message = editorContent - } - try { await client.labrinth.tech_review_internal.submitProject(props.project.id, { verdict, - message, + message: editorContent || undefined, }) emit('markComplete', props.project.id) addNotification({ @@ -361,7 +220,7 @@ async function handleSubmitReview(verdict: 'safe' | 'unsafe') { @@ -171,13 +171,27 @@ {{ formatMessage(imageMessages.threadReopened) }} - + + + {{ formatMessage(imageMessages.techReviewEntered) }} @@ -193,6 +207,7 @@ v-if="!hasBody" v-tooltip="formatDateTime(message.created)" class="message__date ms-1.5 text-sm text-secondary opacity-80" + :class="{ 'ms-0 mt-1 block text-xs': message.body.type === 'tech_review' }" > {{ timeSincePosted }} @@ -374,6 +389,16 @@ const imageMessages = defineMessages({ id: 'thread.message.thread-reopened', defaultMessage: 'reopened the thread.', }, + techReviewPassed: { + id: 'thread.message.tech-review-passed', + defaultMessage: + 'completed technical review and marked project as passed with {safe, plural, one {# safe issue} other {# safe issues}} reviewed.', + }, + techReviewFailed: { + id: 'thread.message.tech-review-failed', + defaultMessage: + 'completed technical review and marked project as failed with {safe} safe and {unsafe, plural, one {# unsafe issue} other {# unsafe issues}} reviewed.', + }, techReviewCompleted: { id: 'thread.message.tech-review-completed', defaultMessage: 'completed technical review and marked project as .', diff --git a/apps/labrinth/.sqlx/query-36899cf74976b8bec1452b071af778827ac0459ce36353a2a70d98a54eb47a8d.json b/apps/labrinth/.sqlx/query-36899cf74976b8bec1452b071af778827ac0459ce36353a2a70d98a54eb47a8d.json deleted file mode 100644 index 9a8233465fb..00000000000 --- a/apps/labrinth/.sqlx/query-36899cf74976b8bec1452b071af778827ac0459ce36353a2a70d98a54eb47a8d.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT\n didws.id AS \"issue_detail_id!\"\n FROM mods m\n INNER JOIN versions v ON v.mod_id = m.id\n INNER JOIN files f ON f.version_id = v.id\n INNER JOIN delphi_reports dr\n ON dr.file_id = f.id\n AND NOT EXISTS (\n SELECT 1\n FROM delphi_reports newer_dr\n WHERE\n newer_dr.file_id = dr.file_id\n AND newer_dr.delphi_version > dr.delphi_version\n )\n INNER JOIN delphi_report_issues dri ON dri.report_id = dr.id\n INNER JOIN delphi_issue_details_with_statuses didws ON didws.issue_id = dri.id\n WHERE\n m.id = $1\n AND didws.status = 'pending'\n AND didws.severity != 'hidden'\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "issue_detail_id!", - "type_info": "Int8", - "origin": { - "Table": { - "table": "delphi_issue_details_with_statuses", - "name": "id" - } - } - } - ], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [ - true - ] - }, - "hash": "36899cf74976b8bec1452b071af778827ac0459ce36353a2a70d98a54eb47a8d" -} diff --git a/apps/labrinth/.sqlx/query-7bac4e23534619c79e18b3f832cd96e2f0cdebdf44c449ee8199d6b26618ac9a.json b/apps/labrinth/.sqlx/query-7bac4e23534619c79e18b3f832cd96e2f0cdebdf44c449ee8199d6b26618ac9a.json new file mode 100644 index 00000000000..af6cfafd5c0 --- /dev/null +++ b/apps/labrinth/.sqlx/query-7bac4e23534619c79e18b3f832cd96e2f0cdebdf44c449ee8199d6b26618ac9a.json @@ -0,0 +1,51 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT\n didws.id AS \"issue_detail_id!\",\n didws.status AS \"status!: DelphiStatus\"\n FROM mods m\n INNER JOIN versions v ON v.mod_id = m.id\n INNER JOIN files f ON f.version_id = v.id\n INNER JOIN delphi_reports dr\n ON dr.file_id = f.id\n AND NOT EXISTS (\n SELECT 1\n FROM delphi_reports newer_dr\n WHERE\n newer_dr.file_id = dr.file_id\n AND newer_dr.delphi_version > dr.delphi_version\n )\n INNER JOIN delphi_report_issues dri ON dri.report_id = dr.id\n INNER JOIN delphi_issue_details_with_statuses didws ON didws.issue_id = dri.id\n WHERE\n m.id = $1\n AND didws.severity != 'hidden'\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "issue_detail_id!", + "type_info": "Int8", + "origin": { + "Table": { + "table": "delphi_issue_details_with_statuses", + "name": "id" + } + } + }, + { + "ordinal": 1, + "name": "status!: DelphiStatus", + "type_info": { + "Custom": { + "name": "delphi_report_issue_status", + "kind": { + "Enum": [ + "pending", + "safe", + "unsafe" + ] + } + } + }, + "origin": { + "Table": { + "table": "delphi_issue_details_with_statuses", + "name": "status" + } + } + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + true, + true + ] + }, + "hash": "7bac4e23534619c79e18b3f832cd96e2f0cdebdf44c449ee8199d6b26618ac9a" +} diff --git a/apps/labrinth/src/models/v2/threads.rs b/apps/labrinth/src/models/v2/threads.rs index efd87f6cf2c..892dbf6e9d4 100644 --- a/apps/labrinth/src/models/v2/threads.rs +++ b/apps/labrinth/src/models/v2/threads.rs @@ -100,7 +100,7 @@ impl From for LegacyMessageBody { new_status, old_status, }, - crate::models::v3::threads::MessageBody::TechReview { verdict } => { + crate::models::v3::threads::MessageBody::TechReview { verdict, .. } => { LegacyMessageBody::Text { body: format!( "(legacy) Reviewed technical report and gave verdict {verdict:?}" diff --git a/apps/labrinth/src/models/v3/threads.rs b/apps/labrinth/src/models/v3/threads.rs index 5bef100510c..6be4cf458c0 100644 --- a/apps/labrinth/src/models/v3/threads.rs +++ b/apps/labrinth/src/models/v3/threads.rs @@ -54,6 +54,10 @@ pub enum MessageBody { }, TechReview { verdict: DelphiVerdict, + #[serde(default, skip_serializing_if = "Option::is_none")] + safe_issues: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + unsafe_issues: Option, }, TechReviewEntered, TechReviewExited, diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review.rs b/apps/labrinth/src/routes/internal/moderation/tech_review.rs index 8780a4a4943..9f1391c9f7c 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review.rs +++ b/apps/labrinth/src/routes/internal/moderation/tech_review.rs @@ -1138,10 +1138,11 @@ pub async fn submit_report( .wrap_internal_err("locking project for technical review submission")? .wrap_not_found_err("project not found")?; - let pending_issue_details = sqlx::query!( + let issue_details = sqlx::query!( r#" SELECT - didws.id AS "issue_detail_id!" + didws.id AS "issue_detail_id!", + didws.status AS "status!: DelphiStatus" FROM mods m INNER JOIN versions v ON v.mod_id = m.id INNER JOIN files f ON f.version_id = v.id @@ -1158,25 +1159,34 @@ pub async fn submit_report( INNER JOIN delphi_issue_details_with_statuses didws ON didws.issue_id = dri.id WHERE m.id = $1 - AND didws.status = 'pending' AND didws.severity != 'hidden' "#, project_id as _, ) .fetch_all(&mut txn) .await - .wrap_internal_err("failed to fetch pending issues")?; + .wrap_internal_err("failed to fetch issue verdicts")?; + let pending_issue_details = issue_details + .iter() + .filter(|detail| detail.status == DelphiStatus::Pending) + .map(|detail| DelphiReportIssueDetailsId(detail.issue_detail_id)) + .collect_vec(); if !pending_issue_details.is_empty() { - let details = pending_issue_details - .into_iter() - .map(|record| DelphiReportIssueDetailsId(record.issue_detail_id)) - .collect_vec(); return Err(ApiError::Request(eyre::eyre!( - "report still has issue details with no verdict: {details:?}" + "report still has issue details with no verdict: {pending_issue_details:?}" ))); } + let safe_issues = issue_details + .iter() + .filter(|detail| detail.status == DelphiStatus::Safe) + .count() as u64; + let unsafe_issues = issue_details + .iter() + .filter(|detail| detail.status == DelphiStatus::Unsafe) + .count() as u64; + let record = sqlx::query!( r#" SELECT t.id AS "thread_id: DBThreadId" @@ -1210,7 +1220,11 @@ pub async fn submit_report( let verdict = submit_report.verdict; ThreadMessageBuilder { author_id: Some(user.id.into()), - body: MessageBody::TechReview { verdict }, + body: MessageBody::TechReview { + verdict, + safe_issues: Some(safe_issues), + unsafe_issues: Some(unsafe_issues), + }, thread_id: record.thread_id, hide_identity: user.role.is_mod(), } @@ -1943,7 +1957,7 @@ where })?; let verdict = match &last_review.body { - MessageBody::TechReview { verdict } => *verdict, + MessageBody::TechReview { verdict, .. } => *verdict, _ => return None, }; diff --git a/packages/api-client/src/modules/labrinth/types.ts b/packages/api-client/src/modules/labrinth/types.ts index 90800c7448f..06e35a73354 100644 --- a/packages/api-client/src/modules/labrinth/types.ts +++ b/packages/api-client/src/modules/labrinth/types.ts @@ -2292,6 +2292,8 @@ export namespace Labrinth { | { type: 'tech_review' verdict: 'safe' | 'unsafe' + safe_issues?: number + unsafe_issues?: number } | { type: 'tech_review_entered' @@ -3015,6 +3017,8 @@ export namespace Labrinth { | { type: 'tech_review' verdict: 'safe' | 'unsafe' + safe_issues?: number + unsafe_issues?: number } | { type: 'tech_review_entered' From 00511388951d32856ddfe24537084819f9d3e1fb Mon Sep 17 00:00:00 2001 From: tdgao Date: Thu, 29 Oct 2026 17:51:30 -0600 Subject: [PATCH 02/19] remove: local storage caching for file contents --- .../project-review/tech-review/index.vue | 4 - .../tech-review/use-tech-review-sources.ts | 74 ------------------- 2 files changed, 78 deletions(-) diff --git a/apps/frontend/src/components/project-review/tech-review/index.vue b/apps/frontend/src/components/project-review/tech-review/index.vue index 6fc46f8d8bf..50e53b73571 100644 --- a/apps/frontend/src/components/project-review/tech-review/index.vue +++ b/apps/frontend/src/components/project-review/tech-review/index.vue @@ -14,10 +14,6 @@ No technical-review reports are available for this project