diff --git a/apps/backend/db/migrations/20250103090503_remove-validation-status-column.js b/apps/backend/db/migrations/20250103090503_remove-validation-status-column.js new file mode 100644 index 000000000..7d7352650 --- /dev/null +++ b/apps/backend/db/migrations/20250103090503_remove-validation-status-column.js @@ -0,0 +1,17 @@ +/** + * @param {import('knex').Knex} knex + */ +export const up = async (knex) => { + await knex.schema.alterTable("screenshot_diffs", async (table) => { + table.dropColumn("validationStatus"); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +export const down = async (knex) => { + await knex.schema.alterTable("screenshot_diffs", async (table) => { + table.string("validationStatus").notNullable(); + }); +}; diff --git a/apps/backend/db/structure.sql b/apps/backend/db/structure.sql index dc56b0b04..cb8f0b929 100644 --- a/apps/backend/db/structure.sql +++ b/apps/backend/db/structure.sql @@ -1065,7 +1065,6 @@ CREATE TABLE public.screenshot_diffs ( "compareScreenshotId" bigint, score numeric(10,5), "jobStatus" public.job_status, - "validationStatus" character varying(255) NOT NULL, "createdAt" timestamp with time zone NOT NULL, "updatedAt" timestamp with time zone NOT NULL, "s3Id" character varying(255), @@ -3007,4 +3006,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024120 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201222408_gitlab-last-loggedat.js', 1, NOW()); INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241215091232_project-default-role.js', 1, NOW()); INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241231154644_build-status.js', 1, NOW()); -INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20250102150800_build-review.js', 1, NOW()); \ No newline at end of file +INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20250102150800_build-review.js', 1, NOW()); +INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20250103090503_remove-validation-status-column.js', 1, NOW()); \ No newline at end of file diff --git a/apps/backend/src/build/createBuildDiffs.e2e.test.ts b/apps/backend/src/build/createBuildDiffs.e2e.test.ts index 2202e7a98..6b893fdf5 100644 --- a/apps/backend/src/build/createBuildDiffs.e2e.test.ts +++ b/apps/backend/src/build/createBuildDiffs.e2e.test.ts @@ -158,7 +158,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: null, compareScreenshotId: newScreenshot!.id, jobStatus: "complete", - validationStatus: "unknown", stabilityScore: null, }); expect(addDiffWithoutFile).toMatchObject({ @@ -166,7 +165,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: null, compareScreenshotId: newScreenshotWithoutFile!.id, jobStatus: "pending", - validationStatus: "unknown", stabilityScore: null, }); expect(updatedDiff).toMatchObject({ @@ -174,7 +172,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: classicDiffBaseScreenshot!.id, compareScreenshotId: classicDiffCompareScreenshot!.id, jobStatus: "pending", - validationStatus: "unknown", stabilityScore: null, }); expect(removedDiff).toMatchObject({ @@ -182,7 +179,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: removedScreenshot!.id, compareScreenshotId: null, jobStatus: "complete", - validationStatus: "unknown", score: null, }); expect(noFileBaseScreenshotDiff).toMatchObject({ @@ -190,7 +186,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: noFileBaseScreenshotBase!.id, compareScreenshotId: noFileBaseScreenshotCompare!.id, jobStatus: "pending", - validationStatus: "unknown", score: null, stabilityScore: null, }); @@ -199,7 +194,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: noFileCompareScreenshotBase!.id, compareScreenshotId: noFileCompareScreenshotCompare!.id, jobStatus: "pending", - validationStatus: "unknown", score: null, stabilityScore: null, }); @@ -208,7 +202,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: sameFileScreenshotBase!.id, compareScreenshotId: sameFileScreenshotCompare!.id, jobStatus: "complete", - validationStatus: "unknown", stabilityScore: null, }); }); @@ -277,7 +270,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: null, compareScreenshotId: newScreenshot!.id, jobStatus: "complete", - validationStatus: "unknown", stabilityScore: null, }); expect(diffs[1]).toMatchObject({ @@ -285,7 +277,6 @@ describe("#createBuildDiffs", () => { baseScreenshotId: null, compareScreenshotId: newScreenshotWithoutFile!.id, jobStatus: "pending", - validationStatus: "unknown", stabilityScore: null, }); }); diff --git a/apps/backend/src/build/createBuildDiffs.ts b/apps/backend/src/build/createBuildDiffs.ts index d13e4f672..50c666f4f 100644 --- a/apps/backend/src/build/createBuildDiffs.ts +++ b/apps/backend/src/build/createBuildDiffs.ts @@ -152,7 +152,6 @@ export async function createBuildDiffs(build: Build) { compareScreenshot, }), score: sameFileId ? 0 : null, - validationStatus: "unknown" as const, testId: compareScreenshot.testId, }; }); @@ -173,7 +172,6 @@ export async function createBuildDiffs(build: Build) { compareScreenshotId: null, jobStatus: "complete" as const, score: null, - validationStatus: "unknown" as const, })) ?? []; const allInserts = [...inserts, ...removedScreenshots]; diff --git a/apps/backend/src/build/job.e2e.test.ts b/apps/backend/src/build/job.e2e.test.ts index 3805a4fef..acd5e3a2a 100644 --- a/apps/backend/src/build/job.e2e.test.ts +++ b/apps/backend/src/build/job.e2e.test.ts @@ -60,7 +60,6 @@ describe("build", () => { const diffs = await ScreenshotDiff.query().where("buildId", build.id); expect(diffs).toHaveLength(1); expect(diffs[0]).toHaveProperty("jobStatus", "complete"); - expect(diffs[0]).toHaveProperty("validationStatus", "unknown"); expect(diffs[0]).toHaveProperty("score", null); expect(diffs[0]).toHaveProperty("s3Id", null); }); diff --git a/apps/backend/src/database/models/ScreenshotDiff.test.ts b/apps/backend/src/database/models/ScreenshotDiff.test.ts index f256b2b40..838c5d47e 100644 --- a/apps/backend/src/database/models/ScreenshotDiff.test.ts +++ b/apps/backend/src/database/models/ScreenshotDiff.test.ts @@ -7,7 +7,6 @@ const baseData = { baseScreenshotId: "1", compareScreenshotId: "2", jobStatus: "pending", - validationStatus: "unknown", }; describe("models/ScreenshotDiff", () => { diff --git a/apps/backend/src/database/models/ScreenshotDiff.ts b/apps/backend/src/database/models/ScreenshotDiff.ts index 35cb4d5dd..f0183d1a4 100644 --- a/apps/backend/src/database/models/ScreenshotDiff.ts +++ b/apps/backend/src/database/models/ScreenshotDiff.ts @@ -18,12 +18,7 @@ export class ScreenshotDiff extends Model { static override tableName = "screenshot_diffs"; static override jsonSchema = mergeSchemas(timestampsSchema, jobModelSchema, { - required: [ - "buildId", - "baseScreenshotId", - "compareScreenshotId", - "validationStatus", - ], + required: ["buildId", "baseScreenshotId", "compareScreenshotId"], properties: { buildId: { type: "string" }, baseScreenshotId: { type: ["string", "null"] }, @@ -32,10 +27,6 @@ export class ScreenshotDiff extends Model { fileId: { type: ["string", "null"] }, score: { type: ["number", "null"], minimum: 0, maximum: 1 }, stabilityScore: { type: ["number", "null"], minimum: 0, maximum: 100 }, - validationStatus: { - type: "string", - enum: ["unknown", "accepted", "rejected"], - }, testId: { type: ["string", "null"] }, group: { type: ["string", "null"] }, }, @@ -49,7 +40,6 @@ export class ScreenshotDiff extends Model { score!: number | null; stabilityScore!: number | null; jobStatus!: JobStatus; - validationStatus!: "unknown" | "accepted" | "rejected"; testId!: string | null; group!: string | null; diff --git a/apps/backend/src/database/seeds.ts b/apps/backend/src/database/seeds.ts index 4a3e5ecee..3e076c72b 100644 --- a/apps/backend/src/database/seeds.ts +++ b/apps/backend/src/database/seeds.ts @@ -290,7 +290,6 @@ export async function seed() { compareScreenshotId: screenshots[1]!.id, score: null, jobStatus: "complete" as const, - validationStatus: "unknown" as const, s3Id: "penelope-diff-transparent.png", createdAt: now, updatedAt: now, diff --git a/apps/backend/src/database/testing/factory.ts b/apps/backend/src/database/testing/factory.ts index 524db8ed8..a9b76319b 100644 --- a/apps/backend/src/database/testing/factory.ts +++ b/apps/backend/src/database/testing/factory.ts @@ -155,7 +155,6 @@ export const ScreenshotDiff = defineFactory(models.ScreenshotDiff, () => ({ baseScreenshotId: Screenshot.associate("id"), compareScreenshotId: Screenshot.associate("id"), jobStatus: "complete", - validationStatus: "unknown", score: 0, })); diff --git a/apps/backend/src/graphql/definitions/Build.ts b/apps/backend/src/graphql/definitions/Build.ts index ea1eb98cf..bd169bdf9 100644 --- a/apps/backend/src/graphql/definitions/Build.ts +++ b/apps/backend/src/graphql/definitions/Build.ts @@ -4,7 +4,6 @@ import gqlTag from "graphql-tag"; import { pushBuildNotification } from "@/build-notification/index.js"; import { Build, BuildReview, ScreenshotDiff } from "@/database/models/index.js"; -import { transaction } from "@/database/transaction.js"; import { IBaseBranchResolution, @@ -311,17 +310,10 @@ export const resolvers: IResolvers = { throw forbidden("You cannot approve or reject this build"); } - await transaction(async (trx) => { - await Promise.all([ - ScreenshotDiff.query(trx) - .where({ buildId }) - .patch({ validationStatus }), - BuildReview.query(trx).insert({ - buildId, - userId: auth.user.id, - state: validationStatus === "accepted" ? "approved" : "rejected", - }), - ]); + await BuildReview.query().insert({ + buildId, + userId: auth.user.id, + state: validationStatus === "accepted" ? "approved" : "rejected", }); // That might be better suited into a $afterUpdate hook. diff --git a/apps/backend/src/screenshot-diff/computeScreenshotDiff.e2e.test.ts b/apps/backend/src/screenshot-diff/computeScreenshotDiff.e2e.test.ts index 22cc6b48f..0069b9fd2 100644 --- a/apps/backend/src/screenshot-diff/computeScreenshotDiff.e2e.test.ts +++ b/apps/backend/src/screenshot-diff/computeScreenshotDiff.e2e.test.ts @@ -99,7 +99,6 @@ describe("#computeScreenshotDiff", () => { baseScreenshotId: baseScreenshot.id, compareScreenshotId: compareScreenshot.id, jobStatus: "pending", - validationStatus: "unknown", testId: test.id, }); }); @@ -153,7 +152,6 @@ describe("#computeScreenshotDiff", () => { baseScreenshotId: baseScreenshot.id, compareScreenshotId: compareScreenshot.id, jobStatus: "pending", - validationStatus: "unknown", }); });