diff --git a/Procfile b/Procfile index b4d090db3..6da78f08e 100644 --- a/Procfile +++ b/Procfile @@ -2,3 +2,4 @@ release: ./scripts/release.sh web: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/web.js buildAndSynchronize: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/build-and-synchronize.js screenshotDiff: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/screenshot-diff.js +setBuildReviews: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/create-build-reviews.js diff --git a/apps/backend/db/migrations/20250102150800_build-review.js b/apps/backend/db/migrations/20250102150800_build-review.js new file mode 100644 index 000000000..7a6107905 --- /dev/null +++ b/apps/backend/db/migrations/20250102150800_build-review.js @@ -0,0 +1,22 @@ +/** + * @param {import('knex').Knex} knex + */ +export const up = async (knex) => { + await knex.schema.createTable("build_reviews", (table) => { + table.bigIncrements("id").primary(); + table.dateTime("createdAt").notNullable(); + table.dateTime("updatedAt").notNullable(); + table.bigInteger("userId").index(); + table.foreign("userId").references("users.id"); + table.bigInteger("buildId").index().notNullable(); + table.foreign("buildId").references("builds.id"); + table.enum("state", ["pending", "approved", "rejected"]).notNullable(); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +export const down = async (knex) => { + await knex.schema.dropTable("build_reviews"); +}; diff --git a/apps/backend/db/structure.sql b/apps/backend/db/structure.sql index 14444c14e..dc56b0b04 100644 --- a/apps/backend/db/structure.sql +++ b/apps/backend/db/structure.sql @@ -157,6 +157,44 @@ ALTER TABLE public.build_notifications_id_seq OWNER TO postgres; ALTER SEQUENCE public.build_notifications_id_seq OWNED BY public.build_notifications.id; +-- +-- Name: build_reviews; Type: TABLE; Schema: public; Owner: postgres +-- + +CREATE TABLE public.build_reviews ( + id bigint NOT NULL, + "createdAt" timestamp with time zone NOT NULL, + "updatedAt" timestamp with time zone NOT NULL, + "userId" bigint, + "buildId" bigint NOT NULL, + state text NOT NULL, + CONSTRAINT build_reviews_state_check CHECK ((state = ANY (ARRAY['pending'::text, 'approved'::text, 'rejected'::text]))) +); + + +ALTER TABLE public.build_reviews OWNER TO postgres; + +-- +-- Name: build_reviews_id_seq; Type: SEQUENCE; Schema: public; Owner: postgres +-- + +CREATE SEQUENCE public.build_reviews_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +ALTER TABLE public.build_reviews_id_seq OWNER TO postgres; + +-- +-- Name: build_reviews_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: postgres +-- + +ALTER SEQUENCE public.build_reviews_id_seq OWNED BY public.build_reviews.id; + + -- -- Name: build_shards; Type: TABLE; Schema: public; Owner: postgres -- @@ -1376,6 +1414,13 @@ ALTER TABLE ONLY public.accounts ALTER COLUMN id SET DEFAULT nextval('public.acc ALTER TABLE ONLY public.build_notifications ALTER COLUMN id SET DEFAULT nextval('public.build_notifications_id_seq'::regclass); +-- +-- Name: build_reviews id; Type: DEFAULT; Schema: public; Owner: postgres +-- + +ALTER TABLE ONLY public.build_reviews ALTER COLUMN id SET DEFAULT nextval('public.build_reviews_id_seq'::regclass); + + -- -- Name: build_shards id; Type: DEFAULT; Schema: public; Owner: postgres -- @@ -1619,6 +1664,14 @@ ALTER TABLE ONLY public.build_notifications ADD CONSTRAINT build_notifications_pkey PRIMARY KEY (id); +-- +-- Name: build_reviews build_reviews_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres +-- + +ALTER TABLE ONLY public.build_reviews + ADD CONSTRAINT build_reviews_pkey PRIMARY KEY (id); + + -- -- Name: build_shards build_shards_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres -- @@ -1989,6 +2042,20 @@ CREATE INDEX accounts_userid_index ON public.accounts USING btree ("userId"); CREATE INDEX build_notifications_buildid_index ON public.build_notifications USING btree ("buildId"); +-- +-- Name: build_reviews_buildid_index; Type: INDEX; Schema: public; Owner: postgres +-- + +CREATE INDEX build_reviews_buildid_index ON public.build_reviews USING btree ("buildId"); + + +-- +-- Name: build_reviews_userid_index; Type: INDEX; Schema: public; Owner: postgres +-- + +CREATE INDEX build_reviews_userid_index ON public.build_reviews USING btree ("userId"); + + -- -- Name: build_shards_buildid_index; Type: INDEX; Schema: public; Owner: postgres -- @@ -2451,6 +2518,22 @@ ALTER TABLE ONLY public.build_notifications ADD CONSTRAINT build_notifications_buildid_foreign FOREIGN KEY ("buildId") REFERENCES public.builds(id); +-- +-- Name: build_reviews build_reviews_buildid_foreign; Type: FK CONSTRAINT; Schema: public; Owner: postgres +-- + +ALTER TABLE ONLY public.build_reviews + ADD CONSTRAINT build_reviews_buildid_foreign FOREIGN KEY ("buildId") REFERENCES public.builds(id); + + +-- +-- Name: build_reviews build_reviews_userid_foreign; Type: FK CONSTRAINT; Schema: public; Owner: postgres +-- + +ALTER TABLE ONLY public.build_reviews + ADD CONSTRAINT build_reviews_userid_foreign FOREIGN KEY ("userId") REFERENCES public.users(id); + + -- -- Name: build_shards build_shards_buildid_foreign; Type: FK CONSTRAINT; Schema: public; Owner: postgres -- @@ -2923,4 +3006,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024120 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201210158_google-account.js', 1, NOW()); 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()); \ No newline at end of file +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 diff --git a/apps/backend/src/build/bin/queue-build-reviews.ts b/apps/backend/src/build/bin/queue-build-reviews.ts new file mode 100755 index 000000000..bad1e0cb9 --- /dev/null +++ b/apps/backend/src/build/bin/queue-build-reviews.ts @@ -0,0 +1,40 @@ +#!/usr/bin/env node +import { callbackify } from "node:util"; + +import { Build } from "@/database/models/index.js"; +import logger from "@/logger/index.js"; + +import { buildReviewJob } from "../build-review-job.js"; + +const main = callbackify(async () => { + const batch = 2000; + const totalCount = await Build.query() + .where("conclusion", "changes-detected") + .resultSize(); + + let total = 0; + for (let offset = 0; offset < totalCount; offset += batch) { + const nodes = await Build.query() + .select("id") + .where("conclusion", "changes-detected") + .limit(batch) + .offset(offset) + .orderBy("id", "desc"); + + const ids = nodes.map((node) => node.id); + const percentage = Math.round((total / totalCount) * 100); + logger.info( + `Processing ${total}/${totalCount} (${percentage}%) - Pushing ${ids.length} builds in queue`, + ); + await buildReviewJob.push(...ids); + total += nodes.length; + } + + logger.info(`${total} builds pushed in queue (100% complete)`); +}); + +main((err) => { + if (err) { + throw err; + } +}); diff --git a/apps/backend/src/build/build-review-job.ts b/apps/backend/src/build/build-review-job.ts new file mode 100644 index 000000000..1bb5109bf --- /dev/null +++ b/apps/backend/src/build/build-review-job.ts @@ -0,0 +1,55 @@ +import { assertNever } from "@argos/util/assertNever"; +import { invariant } from "@argos/util/invariant"; + +import { Build } from "@/database/models/Build.js"; +import { BuildReview } from "@/database/models/BuildReview.js"; +import { createJob } from "@/job-core/index.js"; +import logger from "@/logger/index.js"; + +export const buildReviewJob = createJob("build-reviews", { + complete: () => {}, + error: (value, error) => { + console.error("Error while processing build", value, error); + }, + perform: async (buildId: string) => { + logger.info(`[${buildId}] Processing for review`); + const build = await Build.query().findById(buildId).throwIfNotFound(); + const [[status], review] = await Promise.all([ + Build.getReviewStatuses([build]), + BuildReview.query().select("id").where("buildId", build.id).first(), + ]); + if (review) { + logger.info(`[${buildId}] Review already exists`); + return; + } + invariant(status !== undefined, "Status should be defined"); + if (status) { + logger.info(`[${buildId}] Review created`); + const state = (() => { + switch (status) { + case "accepted": + return "approved" as const; + case "rejected": + return "rejected" as const; + default: + assertNever(status); + } + })(); + + // Simulate 10 minutes after build creation + const date = new Date( + new Date(build.createdAt).getTime() + 10 * 60 * 1000, + ).toISOString(); + + await BuildReview.query().insert({ + buildId: build.id, + createdAt: date, + updatedAt: date, + state, + }); + logger.info(`[${buildId}] Review created ${state}`); + } else { + logger.info(`[${buildId}] No review needed`); + } + }, +}); diff --git a/apps/backend/src/database/models/BuildReview.ts b/apps/backend/src/database/models/BuildReview.ts new file mode 100644 index 000000000..e6a7d9708 --- /dev/null +++ b/apps/backend/src/database/models/BuildReview.ts @@ -0,0 +1,47 @@ +import type { RelationMappings } from "objection"; + +import { Model } from "../util/model.js"; +import { mergeSchemas, timestampsSchema } from "../util/schemas.js"; +import { Build } from "./Build.js"; +import { User } from "./User.js"; + +export class BuildReview extends Model { + static override tableName = "build_reviews"; + + static override jsonSchema = mergeSchemas(timestampsSchema, { + required: ["buildId", "state"], + properties: { + buildId: { type: "string" }, + userId: { type: ["string", "null"] }, + state: { type: "string", enum: ["pending", "approved", "rejected"] }, + }, + }); + + buildId!: string; + userId!: string; + state!: "pending" | "approved" | "rejected"; + + static override get relationMappings(): RelationMappings { + return { + build: { + relation: Model.BelongsToOneRelation, + modelClass: Build, + join: { + from: "build_reviews.buildId", + to: "builds.id", + }, + }, + user: { + relation: Model.BelongsToOneRelation, + modelClass: User, + join: { + from: "build_reviews.userId", + to: "users.id", + }, + }, + }; + } + + build?: Build; + user?: User; +} diff --git a/apps/backend/src/database/models/index.ts b/apps/backend/src/database/models/index.ts index f0a5de129..d4a92a4cd 100644 --- a/apps/backend/src/database/models/index.ts +++ b/apps/backend/src/database/models/index.ts @@ -2,6 +2,7 @@ export * from "./Account.js"; export * from "./Build.js"; export * from "./BuildShard.js"; export * from "./BuildNotification.js"; +export * from "./BuildReview.js"; export * from "./Capture.js"; export * from "./Crawl.js"; export * from "./File.js"; diff --git a/apps/backend/src/database/util/model.ts b/apps/backend/src/database/util/model.ts index 949f13413..cfd12d4e0 100644 --- a/apps/backend/src/database/util/model.ts +++ b/apps/backend/src/database/util/model.ts @@ -15,8 +15,9 @@ export class Model extends ObjectionModel { if (!this.createdAt) { this.createdAt = new Date().toISOString(); } - - this.updatedAt = new Date().toISOString(); + if (!this.updatedAt) { + this.updatedAt = new Date().toISOString(); + } } override $beforeUpdate(_opt: ModelOptions, _queryContext: QueryContext) { diff --git a/apps/backend/src/graphql/definitions/Build.ts b/apps/backend/src/graphql/definitions/Build.ts index 72baa84b7..ea1eb98cf 100644 --- a/apps/backend/src/graphql/definitions/Build.ts +++ b/apps/backend/src/graphql/definitions/Build.ts @@ -3,7 +3,8 @@ import { invariant } from "@argos/util/invariant"; import gqlTag from "graphql-tag"; import { pushBuildNotification } from "@/build-notification/index.js"; -import { Build, ScreenshotDiff } from "@/database/models/index.js"; +import { Build, BuildReview, ScreenshotDiff } from "@/database/models/index.js"; +import { transaction } from "@/database/transaction.js"; import { IBaseBranchResolution, @@ -11,7 +12,7 @@ import { type IResolvers, } from "../__generated__/resolver-types.js"; import type { Context } from "../context.js"; -import { forbidden, notFound, unauthenticated } from "../util.js"; +import { badUserInput, forbidden, notFound, unauthenticated } from "../util.js"; import { paginateResult } from "./PageInfo.js"; const { gql } = gqlTag; @@ -283,11 +284,17 @@ export const resolvers: IResolvers = { }, Mutation: { setValidationStatus: async (_root, args, ctx) => { - if (!ctx.auth) { + const { auth } = ctx; + if (!auth) { throw unauthenticated(); } const { buildId, validationStatus } = args; + + if (validationStatus !== "accepted" && validationStatus !== "rejected") { + throw badUserInput("validationStatus must be 'accepted' or 'rejected'"); + } + const build = await Build.query() .findById(buildId) .withGraphFetched("project.account"); @@ -298,15 +305,24 @@ export const resolvers: IResolvers = { invariant(build.project?.account); - const permissions = await build.project.$getPermissions(ctx.auth.user); + const permissions = await build.project.$getPermissions(auth.user); if (!permissions.includes("review")) { throw forbidden("You cannot approve or reject this build"); } - await ScreenshotDiff.query() - .where({ buildId }) - .patch({ validationStatus }); + 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", + }), + ]); + }); // That might be better suited into a $afterUpdate hook. switch (validationStatus) { diff --git a/apps/backend/src/processes/proc/create-build-reviews.ts b/apps/backend/src/processes/proc/create-build-reviews.ts new file mode 100644 index 000000000..9b51bf2dd --- /dev/null +++ b/apps/backend/src/processes/proc/create-build-reviews.ts @@ -0,0 +1,6 @@ +import "../setup.js"; + +import { buildReviewJob } from "@/build/build-review-job.js"; +import { createJobWorker } from "@/job-core/index.js"; + +createJobWorker(buildReviewJob);