From 7a51880192dfd7517d786f6a12a1bb76364af989 Mon Sep 17 00:00:00 2001 From: Jakub Czajkowski Date: Sun, 31 May 2026 17:46:54 +0200 Subject: [PATCH 1/3] refactor(contributors): move order to pivot table --- app/models/contributor.ts | 8 +- app/models/milestone.ts | 8 +- ...6590400_move_contributor_order_to_pivot.ts | 91 +++++++ .../contributor_milestone_order.spec.ts | 224 ++++++++++++++++++ 4 files changed, 326 insertions(+), 5 deletions(-) create mode 100644 database/migrations/1778436590400_move_contributor_order_to_pivot.ts create mode 100644 tests/functional/contributor_milestone_order.spec.ts diff --git a/app/models/contributor.ts b/app/models/contributor.ts index fb8843b1..717662fc 100644 --- a/app/models/contributor.ts +++ b/app/models/contributor.ts @@ -28,9 +28,6 @@ export default class Contributor extends BaseModel { @typedColumn({ foreignKeyOf: () => FileEntry, optional: true }) declare photoKey: string | null; - @typedColumn({ type: "number", hasDefault: true }) - declare order: number; - @typedColumn.dateTime({ autoCreate: true }) declare createdAt: DateTime; @@ -55,7 +52,10 @@ export default class Contributor extends BaseModel { @typedManyToMany(() => Milestone, { pivotTable: "contributor_roles", - pivotColumns: { role_id: { type: "integer", detachFilter: true } }, + pivotColumns: { + role_id: { type: "integer", detachFilter: true }, + order: { type: "number", hasDefault: true }, + }, pivotTimestamps: true, }) declare milestones: ManyToMany; diff --git a/app/models/milestone.ts b/app/models/milestone.ts index 5e445da1..f09ff72a 100644 --- a/app/models/milestone.ts +++ b/app/models/milestone.ts @@ -26,8 +26,14 @@ export default class Milestone extends BaseModel { @typedManyToMany(() => Contributor, { pivotTable: "contributor_roles", - pivotColumns: { role_id: { type: "integer", detachFilter: true } }, + pivotColumns: { + role_id: { type: "integer", detachFilter: true }, + order: { type: "number", hasDefault: true }, + }, pivotTimestamps: true, + onQuery: (query) => { + return query.orderBy("contributor_roles.order", "asc"); + }, }) declare contributors: ManyToMany; diff --git a/database/migrations/1778436590400_move_contributor_order_to_pivot.ts b/database/migrations/1778436590400_move_contributor_order_to_pivot.ts new file mode 100644 index 00000000..ba28d28e --- /dev/null +++ b/database/migrations/1778436590400_move_contributor_order_to_pivot.ts @@ -0,0 +1,91 @@ +import { BaseSchema } from "@adonisjs/lucid/schema"; + +export default class extends BaseSchema { + async up() { + // 1. Remove trigger from contributors (function stays – used by guide_articles and guide_questions) + this.schema.raw( + `DROP TRIGGER IF EXISTS order_autocompute ON "contributors"`, + ); + + // 2. Remove order column from contributors + this.schema.alterTable("contributors", (table) => { + table.dropColumn("order"); + }); + + // 3. Add order to contributor_roles pivot with per-milestone auto-compute trigger + this.schema.raw(` + ALTER TABLE "contributor_roles" + ADD COLUMN "order" REAL NULL; + + WITH ranked AS ( + SELECT DISTINCT contributor_id, milestone_id, + ROW_NUMBER() OVER (PARTITION BY milestone_id ORDER BY contributor_id) AS rn + FROM contributor_roles + ) + UPDATE contributor_roles cr + SET "order" = r.rn + FROM ranked r + WHERE cr.contributor_id = r.contributor_id AND cr.milestone_id = r.milestone_id; + + ALTER TABLE "contributor_roles" + ALTER COLUMN "order" SET NOT NULL; + + CREATE FUNCTION contributor_roles_order_autocompute() RETURNS trigger AS $$ + BEGIN + IF NEW.order IS NULL THEN + SELECT "order" INTO NEW.order + FROM contributor_roles + WHERE milestone_id = NEW.milestone_id AND contributor_id = NEW.contributor_id + LIMIT 1; + + IF NEW.order IS NULL THEN + SELECT MAX("order") + 1 INTO STRICT NEW.order + FROM contributor_roles + WHERE milestone_id = NEW.milestone_id; + + IF NEW.order IS NULL THEN + NEW.order := 1; + END IF; + END IF; + END IF; + + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + + CREATE TRIGGER contributor_roles_order_autocompute + BEFORE INSERT ON "contributor_roles" + FOR EACH ROW + EXECUTE FUNCTION contributor_roles_order_autocompute(); + `); + } + + async down() { + // Remove pivot order trigger and column + this.schema.raw(` + DROP TRIGGER IF EXISTS contributor_roles_order_autocompute ON "contributor_roles"; + DROP FUNCTION IF EXISTS contributor_roles_order_autocompute(); + `); + + this.schema.alterTable("contributor_roles", (table) => { + table.dropColumn("order"); + }); + + // Restore order on contributors + this.schema.raw(` + ALTER TABLE "contributors" + ADD COLUMN "order" REAL NULL; + + UPDATE "contributors" + SET "order" = "id"; + + ALTER TABLE "contributors" + ALTER COLUMN "order" SET NOT NULL; + + CREATE TRIGGER order_autocompute + BEFORE INSERT ON "contributors" + FOR EACH ROW + EXECUTE FUNCTION order_autocompute(); + `); + } +} diff --git a/tests/functional/contributor_milestone_order.spec.ts b/tests/functional/contributor_milestone_order.spec.ts new file mode 100644 index 00000000..fb06b3ee --- /dev/null +++ b/tests/functional/contributor_milestone_order.spec.ts @@ -0,0 +1,224 @@ +import { test } from "@japa/runner"; + +import testUtils from "@adonisjs/core/services/test_utils"; +import db from "@adonisjs/lucid/services/db"; + +import Contributor from "#models/contributor"; +import Milestone from "#models/milestone"; +import Role from "#models/role"; + +test.group("Contributor per-milestone order", (group) => { + group.setup(async () => { + await testUtils.db().migrate(); + }); + group.teardown(async () => { + await testUtils.db().truncate(); + }); + + test("GET /contributors does not include order field", async ({ + client, + assert, + }) => { + await Contributor.create({ name: "OrderTest Alice" }); + + const res = await client.get("/api/v1/contributors"); + res.assertStatus(200); + const body = res.body() as { data: Record[] }; + const contributor = body.data.find((c) => c.name === "OrderTest Alice"); + assert.exists(contributor); + assert.notProperty(contributor ?? {}, "order"); + }); + + test("GET /milestones/:id/contributors returns pivot_order and is sorted asc", async ({ + client, + assert, + }) => { + const role = await Role.create({ name: "OrderTest Role" }); + const c1 = await Contributor.create({ name: "OrderTest Bob" }); + const c2 = await Contributor.create({ name: "OrderTest Carol" }); + const c3 = await Contributor.create({ name: "OrderTest Dave" }); + const milestone = await Milestone.create({ name: "OrderTest Milestone A" }); + + await db + .knexQuery() + .table("contributor_roles") + .insert([ + { + contributor_id: c1.id, + role_id: role.id, + milestone_id: milestone.id, + order: 10, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c2.id, + role_id: role.id, + milestone_id: milestone.id, + order: 5, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c3.id, + role_id: role.id, + milestone_id: milestone.id, + order: 7, + created_at: new Date(), + updated_at: new Date(), + }, + ]); + + const res = await client.get( + `/api/v1/milestones/${milestone.id}/contributors`, + ); + res.assertStatus(200); + const body = res.body() as { + data: { id: number; meta: { pivot_order: number } }[]; + }; + const ids = body.data.map((c) => c.id); + assert.deepEqual(ids, [c2.id, c3.id, c1.id]); + assert.property(body.data[0].meta, "pivot_order"); + assert.equal(body.data[0].meta.pivot_order, 5); + assert.equal(body.data[1].meta.pivot_order, 7); + assert.equal(body.data[2].meta.pivot_order, 10); + }); + + test("order is independent per milestone", async ({ client, assert }) => { + const role = await Role.create({ name: "OrderTest Role 2" }); + const c1 = await Contributor.create({ name: "OrderTest Eve" }); + const c2 = await Contributor.create({ name: "OrderTest Frank" }); + const m1 = await Milestone.create({ name: "OrderTest Milestone B" }); + const m2 = await Milestone.create({ name: "OrderTest Milestone C" }); + + await db + .knexQuery() + .table("contributor_roles") + .insert([ + { + contributor_id: c1.id, + role_id: role.id, + milestone_id: m1.id, + order: 1, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c2.id, + role_id: role.id, + milestone_id: m1.id, + order: 2, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c1.id, + role_id: role.id, + milestone_id: m2.id, + order: 99, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c2.id, + role_id: role.id, + milestone_id: m2.id, + order: 1, + created_at: new Date(), + updated_at: new Date(), + }, + ]); + + const resM1 = await client.get(`/api/v1/milestones/${m1.id}/contributors`); + resM1.assertStatus(200); + const bodyM1 = resM1.body() as { data: { id: number }[] }; + assert.deepEqual( + bodyM1.data.map((c) => c.id), + [c1.id, c2.id], + ); + + const resM2 = await client.get(`/api/v1/milestones/${m2.id}/contributors`); + resM2.assertStatus(200); + const bodyM2 = resM2.body() as { data: { id: number }[] }; + assert.deepEqual( + bodyM2.data.map((c) => c.id), + [c2.id, c1.id], + ); + }); + + test("trigger auto-assigns order per milestone when not provided", async ({ + assert, + }) => { + const role = await Role.create({ name: "OrderTest Role 3" }); + const c1 = await Contributor.create({ name: "OrderTest Grace" }); + const c2 = await Contributor.create({ name: "OrderTest Heidi" }); + const milestone = await Milestone.create({ name: "OrderTest Milestone D" }); + + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: c1.id, + role_id: role.id, + milestone_id: milestone.id, + created_at: new Date(), + updated_at: new Date(), + }); + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: c2.id, + role_id: role.id, + milestone_id: milestone.id, + created_at: new Date(), + updated_at: new Date(), + }); + + const rows = (await db + .knexQuery() + .table("contributor_roles") + .where("milestone_id", milestone.id) + .orderBy("order")) as { contributor_id: number; order: number }[]; + + assert.lengthOf(rows, 2); + assert.equal(rows[0].contributor_id, c1.id); + assert.equal(rows[0].order, 1); + assert.equal(rows[1].contributor_id, c2.id); + assert.equal(rows[1].order, 2); + }); + + test("trigger reuses order for same contributor+milestone with different role", async ({ + assert, + }) => { + const role1 = await Role.create({ name: "OrderTest Role 4a" }); + const role2 = await Role.create({ name: "OrderTest Role 4b" }); + const contributor = await Contributor.create({ + name: "OrderTest Ivan", + }); + const milestone = await Milestone.create({ name: "OrderTest Milestone E" }); + + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: contributor.id, + role_id: role1.id, + milestone_id: milestone.id, + order: 3, + created_at: new Date(), + updated_at: new Date(), + }); + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: contributor.id, + role_id: role2.id, + milestone_id: milestone.id, + created_at: new Date(), + updated_at: new Date(), + }); + + const rows = (await db + .knexQuery() + .table("contributor_roles") + .where({ + contributor_id: contributor.id, + milestone_id: milestone.id, + }) + .orderBy("role_id")) as { role_id: number; order: number }[]; + + assert.lengthOf(rows, 2); + assert.equal(rows[0].order, 3); + assert.equal(rows[1].order, 3); + }); +}); From d06beba85bf76b4461732af6f3bda61f784ee24a Mon Sep 17 00:00:00 2001 From: Jakub Czajkowski Date: Tue, 2 Jun 2026 15:32:47 +0200 Subject: [PATCH 2/3] feat: implement many-to-many relation pivot update functionality --- app/controllers/auto_crud_controller.ts | 133 ++++++++++- app/controllers/v1/drafts.ts | 2 + app/utils/model_autogen.ts | 97 ++++++++ ...512860_move_contributor_order_to_pivot.ts} | 0 .../contributor_milestone_order.spec.ts | 209 ++++++++++++++++++ tests/functional/permissions.spec.ts | 24 ++ 6 files changed, 463 insertions(+), 2 deletions(-) rename database/migrations/{1778436590400_move_contributor_order_to_pivot.ts => 1780074512860_move_contributor_order_to_pivot.ts} (100%) diff --git a/app/controllers/auto_crud_controller.ts b/app/controllers/auto_crud_controller.ts index c41dda61..21e5b7e3 100644 --- a/app/controllers/auto_crud_controller.ts +++ b/app/controllers/auto_crud_controller.ts @@ -37,6 +37,7 @@ import { InternalControllerValidationError, } from "#exceptions/base_controller_errors"; import { + BadRequestException, ConflictException, ForbiddenException, NotFoundException, @@ -45,7 +46,12 @@ import type { preloadRelations } from "#scopes/preload_helper"; import type { handleSearchQuery } from "#scopes/search_helper"; import type { handleSortQuery } from "#scopes/sort_helper"; import "#utils/maps"; -import { AutogenCacheEntry, relationValidator } from "#utils/model_autogen"; +import { + AutogenCacheEntry, + hasUpdatablePivotColumns, + relationValidator, + splitPivotUpdateBody, +} from "#utils/model_autogen"; import type { AnyValidator, PrimaryKeyFieldDescriptor, @@ -140,7 +146,8 @@ export type ControllerAction = | "oneToOneRelationStore" | "oneToManyRelationStore" | "manyToManyRelationAttach" - | "manyToManyRelationDetach"; + | "manyToManyRelationDetach" + | "manyToManyRelationUpdatePivot"; // Use the same Constructor type as other controllers (e.g., mobile_config_controller) @@ -188,6 +195,7 @@ export default abstract class AutoCrudController< * - oneToManyRelationStore -> "create" * - manyToManyRelationAttach -> "update" * - manyToManyRelationDetach -> "update" + * - manyToManyRelationUpdatePivot -> "update" * - index/show -> none (public by default) * * @param action The controller action being performed @@ -210,6 +218,7 @@ export default abstract class AutoCrudController< return "create"; case "manyToManyRelationAttach": case "manyToManyRelationDetach": + case "manyToManyRelationUpdatePivot": return "update"; case "index": case "show": @@ -434,6 +443,10 @@ export default abstract class AutoCrudController< return this.modelCacheEntry.manyToManyIdsValidator(relationName); } + protected updatePivotValidator(relationName: string): AnyValidator { + return this.modelCacheEntry.updatePivotValidator(relationName); + } + /** * The actual self-validation function, does not cache! */ @@ -638,6 +651,14 @@ export default abstract class AutoCrudController< "manyToManyRelationDetach", ]) .as(`relation.${relationName}.detach`); + if (hasUpdatablePivotColumns(relation)) { + router + .patch(`/:localId/${snakeCaseName}/:relatedId`, [ + controller, + "manyToManyRelationUpdatePivot", + ]) + .as(`relation.${relationName}.updatePivot`); + } } } } else { @@ -1269,4 +1290,112 @@ export default abstract class AutoCrudController< return { success: true, numDetached: result }; } + + async manyToManyRelationUpdatePivot(httpCtx: HttpContext): Promise { + const { request, route, auth } = httpCtx; + if (!auth.isAuthenticated) { + await auth.authenticate(); + } + await this.selfValidate(); + const relationName = this.relationNameFromRoute(route); + await this.authenticate( + httpCtx, + "manyToManyRelationUpdatePivot", + relationName, + ); + + const { + params: { localId, relatedId }, + } = (await request.validateUsing( + this.manyToManyIdsValidator(relationName), + )) as { params: { localId: string | number; relatedId: string | number } }; + const body = (await request.validateUsing( + this.updatePivotValidator(relationName), + )) as Record; + + await this.authorizeById(httpCtx, "manyToManyRelationUpdatePivot", { + localId, + relatedId, + relationName, + }); + + const relation = this.model.$relationsDefinitions.get(relationName); + if (relation === undefined) { + throw new InternalControllerError( + `Relation '${relationName}' does not exist on model '${this.model.name}'`, + ); + } + if (relation.type !== "manyToMany") { + throw new InternalControllerError( + `Relation '${relationName}' of model '${this.model.name}' was passed into the 'manyToManyRelationUpdatePivot' method, ` + + `which only supports 'manyToMany' relations, but this relation is of type '${relation.type}'!`, + ); + } + if (!validateTypedManyToManyRelation(relation)) { + throw new InternalControllerError( + `Relation '${relationName}' isn't properly typed!`, + ); + } + if (!relation.booted) { + relation.boot(); + } + + const { pivotKey, pivotUpdate } = splitPivotUpdateBody(relation, body); + + if (Object.keys(pivotUpdate).length === 0) { + throw new BadRequestException( + "At least one updatable pivot field must be provided", + ); + } + + for (const [name, field] of Object.entries( + relation.options.meta.declaredColumnTypes, + )) { + if (field.detachFilter && !(name in pivotKey)) { + throw new BadRequestException( + `Missing required pivot key field: ${name}`, + ); + } + } + + if (this.authorizeRecord !== AutoCrudController.prototype.authorizeRecord) { + const mainInstance = await this.getFirstOrFail(localId); + await this.authorizeRecord( + httpCtx, + "manyToManyRelationUpdatePivot", + mainInstance, + ); + } + + let result; + try { + result = await db + .knexQuery() + .table(relation.pivotTable) + .where({ + ...pivotKey, + [relation.pivotForeignKey]: localId, + [relation.pivotRelatedForeignKey]: relatedId, + }) + .update({ + ...pivotUpdate, + ...("pivotTimestamps" in relation.options && + relation.options.pivotTimestamps === true + ? { updated_at: new Date() } + : {}), + }); + } catch (err) { + throw new BaseError("Failed to update pivot row", { + cause: err, + code: "E_DB_ERROR", + status: 500, + }); + } + + if (result === 0) { + throw new NotFoundException("No relation attachments matched your query"); + } + + return { success: true }; + } } diff --git a/app/controllers/v1/drafts.ts b/app/controllers/v1/drafts.ts index c407ea61..3c3a29fb 100644 --- a/app/controllers/v1/drafts.ts +++ b/app/controllers/v1/drafts.ts @@ -182,6 +182,7 @@ export abstract class GenericDraftController< case "oneToManyRelationStore": case "manyToManyRelationAttach": case "manyToManyRelationDetach": + case "manyToManyRelationUpdatePivot": return "authOnly"; // other actions case "index": @@ -243,6 +244,7 @@ export abstract class GenericDraftController< oneToManyRelationStore: "update", manyToManyRelationAttach: "update", manyToManyRelationDetach: "update", + manyToManyRelationUpdatePivot: "update", }; const slug = slugMap[action]; diff --git a/app/utils/model_autogen.ts b/app/utils/model_autogen.ts index a53f8d2d..56a7f9a6 100644 --- a/app/utils/model_autogen.ts +++ b/app/utils/model_autogen.ts @@ -7,6 +7,7 @@ import type { LucidModel, ModelColumnOptions, } from "@adonisjs/lucid/types/model"; +import type { ManyToManyRelationContract } from "@adonisjs/lucid/types/relations"; import { validateColumnDef, @@ -43,6 +44,7 @@ export class AutogenCacheEntry { #relationStoreValidators = new Map(); #relationAttachValidators = new Map(); #relationDetachValidators = new Map(); + #relationUpdatePivotValidators = new Map(); #manyToManyIdValidators = new Map(); private constructor(model: LucidModel) { @@ -303,6 +305,57 @@ export class AutogenCacheEntry { }); } + public updatePivotValidator(relationName: string): AnyValidator { + return this.#relationUpdatePivotValidators.getOrInsertWith( + relationName, + () => { + const relation = this.model.$relationsDefinitions.get(relationName); + if (relation === undefined) { + throw new InvalidModelDefinition( + `Relation '${relationName}' does not exist on model '${this.model.name}'`, + ); + } + if (relation.type !== "manyToMany") { + throw new InvalidModelDefinition( + `Relation '${relationName}' is not a manyToMany relation!`, + ); + } + if (!validateTypedManyToManyRelation(relation)) { + throw new InvalidModelDefinition( + `Relation '${relationName}' isn't properly typed!`, + ); + } + if (!relation.booted) { + relation.boot(); + } + return vine.compile( + vine.object( + Object.fromEntries( + Object.entries(relation.options.meta.declaredColumnTypes) + .map(([name, field]) => { + if (field.autoGenerated) { + return undefined; + } + let validator = field.validator; + if (field.detachFilter) { + return [name, validator]; + } + if ( + "optional" in validator && + typeof validator.optional === "function" + ) { + validator = (validator.optional as () => SchemaTypes)(); + } + return [name, validator]; + }) + .filter((e) => e !== undefined), + ), + ), + ); + }, + ); + } + public manyToManyIdsValidator(relationName: string): AnyValidator { return this.#manyToManyIdValidators.getOrInsertWith(relationName, () => { const relation = this.model.$relationsDefinitions.get(relationName); @@ -353,6 +406,50 @@ export class AutogenCacheEntry { const relationValidatorCache = new Map(); const modelAutogenCache = new Map(); +export function hasUpdatablePivotColumns( + relation: ManyToManyRelationContract, +): boolean { + if (!validateTypedManyToManyRelation(relation)) { + return false; + } + if (!relation.booted) { + relation.boot(); + } + return Object.values(relation.options.meta.declaredColumnTypes).some( + (field) => !field.autoGenerated && !field.detachFilter, + ); +} + +export function splitPivotUpdateBody( + relation: ManyToManyRelationContract, + body: Record, +): { + pivotKey: Record; + pivotUpdate: Record; +} { + if (!validateTypedManyToManyRelation(relation)) { + throw new InvalidModelDefinition("Relation isn't properly typed!"); + } + if (!relation.booted) { + relation.boot(); + } + const pivotKey: Record = {}; + const pivotUpdate: Record = {}; + for (const [name, field] of Object.entries( + relation.options.meta.declaredColumnTypes, + )) { + if (field.autoGenerated || !(name in body)) { + continue; + } + if (field.detachFilter) { + pivotKey[name] = body[name]; + } else { + pivotUpdate[name] = body[name]; + } + } + return { pivotKey, pivotUpdate }; +} + export function relationValidator(relations: string[]): RelationValidator { return relationValidatorCache.getOrInsertWith(JSON.stringify(relations), () => vine.compile( diff --git a/database/migrations/1778436590400_move_contributor_order_to_pivot.ts b/database/migrations/1780074512860_move_contributor_order_to_pivot.ts similarity index 100% rename from database/migrations/1778436590400_move_contributor_order_to_pivot.ts rename to database/migrations/1780074512860_move_contributor_order_to_pivot.ts diff --git a/tests/functional/contributor_milestone_order.spec.ts b/tests/functional/contributor_milestone_order.spec.ts index fb06b3ee..4ba63868 100644 --- a/tests/functional/contributor_milestone_order.spec.ts +++ b/tests/functional/contributor_milestone_order.spec.ts @@ -1,3 +1,6 @@ +import jwt from "jsonwebtoken"; +import crypto from "node:crypto"; + import { test } from "@japa/runner"; import testUtils from "@adonisjs/core/services/test_utils"; @@ -6,6 +9,96 @@ import db from "@adonisjs/lucid/services/db"; import Contributor from "#models/contributor"; import Milestone from "#models/milestone"; import Role from "#models/role"; +import User from "#models/user"; +import env from "#start/env"; + +function uniqueEmail(prefix: string) { + const id = crypto.randomUUID().slice(0, 8); + return `${prefix}-${id}@order.test`; +} + +async function makeToken(user: User): Promise { + const ACCESS_SECRET = env.get("ACCESS_SECRET"); + const AUDIENCE = "admin.topwr.solvro.pl"; + const ISSUER = "admin.topwr.solvro.pl"; + const ACCESS_EXPIRES_IN_MS = Number.parseInt( + env.get("ACCESS_EXPIRES_IN_MS", "3600000"), + ); + + return jwt.sign({ isRefresh: false }, ACCESS_SECRET, { + subject: user.id.toString(), + audience: AUDIENCE, + issuer: ISSUER, + expiresIn: ACCESS_EXPIRES_IN_MS, + algorithm: "HS256", + allowInsecureKeySizes: false, + allowInvalidAsymmetricKeyTypes: false, + }); +} + +async function ensureSolvroAdminRoleId(): Promise { + const existing: unknown = await db + .knexQuery() + .table("access_roles") + .where({ slug: "solvro_admin" }) + .first(); + if (existing !== null && existing !== undefined) { + return Number((existing as { id: number | string }).id); + } + + const idNum = await db + .knexQuery() + .table("access_roles") + .insert({ + slug: "solvro_admin", + created_at: new Date(), + updated_at: new Date(), + }) + .returning("id") + .then((result: unknown) => { + if (Array.isArray(result)) { + const first = result[0] as unknown; + if (typeof first === "object" && first !== null && "id" in first) { + return Number((first as { id: number | string }).id); + } + return Number(first); + } + if (typeof result === "object" && result !== null && "id" in result) { + return Number((result as { id: number | string }).id); + } + return Number(result); + }); + + return idNum; +} + +async function assignSolvroAdmin(user: User): Promise { + const roleId = await ensureSolvroAdminRoleId(); + const existing: unknown = await db + .knexQuery() + .table("model_roles") + .where({ model_type: "users", model_id: user.id, role_id: roleId }) + .first(); + if (existing === null || existing === undefined) { + await db.knexQuery().table("model_roles").insert({ + model_type: "users", + model_id: user.id, + role_id: roleId, + created_at: new Date(), + updated_at: new Date(), + }); + } +} + +async function adminAuthHeader(): Promise { + const admin = await User.create({ + email: uniqueEmail("order-admin"), + password: "Passw0rd!", + fullName: "Order Test Admin", + }); + await assignSolvroAdmin(admin); + return `Bearer ${await makeToken(admin)}`; +} test.group("Contributor per-milestone order", (group) => { group.setup(async () => { @@ -221,4 +314,120 @@ test.group("Contributor per-milestone order", (group) => { assert.equal(rows[0].order, 3); assert.equal(rows[1].order, 3); }); + + test("PATCH /milestones/:id/contributors/:contributorId updates pivot order", async ({ + client, + assert, + }) => { + const auth = await adminAuthHeader(); + const role = await Role.create({ name: "OrderTest Role PATCH" }); + const c1 = await Contributor.create({ name: "OrderTest Patch A" }); + const c2 = await Contributor.create({ name: "OrderTest Patch B" }); + const milestone = await Milestone.create({ + name: "OrderTest Milestone PATCH", + }); + + await db + .knexQuery() + .table("contributor_roles") + .insert([ + { + contributor_id: c1.id, + role_id: role.id, + milestone_id: milestone.id, + order: 10, + created_at: new Date(), + updated_at: new Date(), + }, + { + contributor_id: c2.id, + role_id: role.id, + milestone_id: milestone.id, + order: 5, + created_at: new Date(), + updated_at: new Date(), + }, + ]); + + const patchRes = await client + .patch(`/api/v1/milestones/${milestone.id}/contributors/${c1.id}`) + .header("Authorization", auth) + .json({ role_id: role.id, order: 1 }); + patchRes.assertStatus(200); + const patchBody = patchRes.body() as { success?: boolean }; + assert.equal(patchBody.success, true); + + const res = await client.get( + `/api/v1/milestones/${milestone.id}/contributors`, + ); + res.assertStatus(200); + const body = res.body() as { + data: { id: number; meta: { pivot_order: number } }[]; + }; + assert.deepEqual( + body.data.map((c) => c.id), + [c1.id, c2.id], + ); + assert.equal(body.data[0].meta.pivot_order, 1); + assert.equal(body.data[1].meta.pivot_order, 5); + }); + + test("PATCH pivot update returns 404 when no row matches", async ({ + client, + }) => { + const auth = await adminAuthHeader(); + const role = await Role.create({ name: "OrderTest Role PATCH 404" }); + const contributor = await Contributor.create({ + name: "OrderTest Patch 404", + }); + const milestone = await Milestone.create({ + name: "OrderTest Milestone PATCH 404", + }); + + const res = await client + .patch( + `/api/v1/milestones/${milestone.id}/contributors/${contributor.id}`, + ) + .header("Authorization", auth) + .json({ role_id: role.id, order: 1 }); + res.assertStatus(404); + }); + + test("PATCH pivot update validates required pivot key and updatable fields", async ({ + client, + }) => { + const auth = await adminAuthHeader(); + const role = await Role.create({ name: "OrderTest Role PATCH 422" }); + const contributor = await Contributor.create({ + name: "OrderTest Patch 422", + }); + const milestone = await Milestone.create({ + name: "OrderTest Milestone PATCH 422", + }); + + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: contributor.id, + role_id: role.id, + milestone_id: milestone.id, + order: 1, + created_at: new Date(), + updated_at: new Date(), + }); + + const missingRole = await client + .patch( + `/api/v1/milestones/${milestone.id}/contributors/${contributor.id}`, + ) + .header("Authorization", auth) + .json({ order: 2 }); + missingRole.assertStatus(422); + + const missingOrder = await client + .patch( + `/api/v1/milestones/${milestone.id}/contributors/${contributor.id}`, + ) + .header("Authorization", auth) + .json({ role_id: role.id }); + missingOrder.assertStatus(400); + }); }); diff --git a/tests/functional/permissions.spec.ts b/tests/functional/permissions.spec.ts index 448e80df..76ce919a 100644 --- a/tests/functional/permissions.spec.ts +++ b/tests/functional/permissions.spec.ts @@ -329,6 +329,30 @@ test.group("Permissions", (group) => { okDetach.assertStatus(200); const okDetachBody = okDetach.body() as unknown as { success?: boolean }; assert.equal(okDetachBody.success, true); + + // Re-attach for pivot update permission checks (milestones have updatable pivot order) + await db.knexQuery().table("contributor_roles").insert({ + contributor_id: person.id, + role_id: role.id, + milestone_id: milestone.id, + order: 1, + created_at: new Date(), + updated_at: new Date(), + }); + + const badPatch = await client + .patch(`/api/v1/milestones/${milestone.id}/contributors/${person.id}`) + .header("Authorization", `Bearer ${userToken}`) + .json({ role_id: role.id, order: 2 }); + badPatch.assertStatus(403); + + const okPatch = await client + .patch(`/api/v1/milestones/${milestone.id}/contributors/${person.id}`) + .header("Authorization", `Bearer ${adminToken}`) + .json({ role_id: role.id, order: 2 }); + okPatch.assertStatus(200); + const okPatchBody = okPatch.body() as unknown as { success?: boolean }; + assert.equal(okPatchBody.success, true); }); test("destroy blocked without permission; allowed for solvro_admin", async ({ From 1ef83e41a337ea895dd45d2cd7895d25ca8c15d1 Mon Sep 17 00:00:00 2001 From: Jakub Czajkowski Date: Wed, 10 Jun 2026 22:41:13 +0200 Subject: [PATCH 3/3] feat(contributor_roles): enforce unique role per contributor and milestone --- ...or_milestone_to_contributor_roles_table.ts | 28 +++++++++++++++ .../contributor_milestone_order.spec.ts | 34 +++++++++---------- 2 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 database/migrations/1780074512861_create_add_unique_contributor_milestone_to_contributor_roles_table.ts diff --git a/database/migrations/1780074512861_create_add_unique_contributor_milestone_to_contributor_roles_table.ts b/database/migrations/1780074512861_create_add_unique_contributor_milestone_to_contributor_roles_table.ts new file mode 100644 index 00000000..2761ddaf --- /dev/null +++ b/database/migrations/1780074512861_create_add_unique_contributor_milestone_to_contributor_roles_table.ts @@ -0,0 +1,28 @@ +import { BaseSchema } from "@adonisjs/lucid/schema"; + +export default class extends BaseSchema { + protected tableName = "contributor_roles"; + + async up() { + // Deduplicate before adding the constraint: keep one role per + // (contributor_id, milestone_id) pair (the lowest role_id). + this.schema.raw(` + DELETE FROM contributor_roles cr + USING contributor_roles keep + WHERE cr.contributor_id = keep.contributor_id + AND cr.milestone_id = keep.milestone_id + AND cr.role_id > keep.role_id; + `); + + // Enforce: a contributor can only have one role per milestone (version). + this.schema.alterTable(this.tableName, (table) => { + table.unique(["contributor_id", "milestone_id"]); + }); + } + + async down() { + this.schema.alterTable(this.tableName, (table) => { + table.dropUnique(["contributor_id", "milestone_id"]); + }); + } +} diff --git a/tests/functional/contributor_milestone_order.spec.ts b/tests/functional/contributor_milestone_order.spec.ts index 4ba63868..b411ec57 100644 --- a/tests/functional/contributor_milestone_order.spec.ts +++ b/tests/functional/contributor_milestone_order.spec.ts @@ -275,7 +275,7 @@ test.group("Contributor per-milestone order", (group) => { assert.equal(rows[1].order, 2); }); - test("trigger reuses order for same contributor+milestone with different role", async ({ + test("a contributor can only have one role per milestone", async ({ assert, }) => { const role1 = await Role.create({ name: "OrderTest Role 4a" }); @@ -293,26 +293,26 @@ test.group("Contributor per-milestone order", (group) => { created_at: new Date(), updated_at: new Date(), }); - await db.knexQuery().table("contributor_roles").insert({ - contributor_id: contributor.id, - role_id: role2.id, - milestone_id: milestone.id, - created_at: new Date(), - updated_at: new Date(), - }); - const rows = (await db - .knexQuery() - .table("contributor_roles") - .where({ + // A second role for the same (contributor, milestone) violates the + // unique constraint and must be rejected. + await assert.rejects(async () => { + await db.knexQuery().table("contributor_roles").insert({ contributor_id: contributor.id, + role_id: role2.id, milestone_id: milestone.id, - }) - .orderBy("role_id")) as { role_id: number; order: number }[]; + created_at: new Date(), + updated_at: new Date(), + }); + }); - assert.lengthOf(rows, 2); - assert.equal(rows[0].order, 3); - assert.equal(rows[1].order, 3); + const rows = (await db.knexQuery().table("contributor_roles").where({ + contributor_id: contributor.id, + milestone_id: milestone.id, + })) as { role_id: number; order: number }[]; + + assert.lengthOf(rows, 1); + assert.equal(rows[0].role_id, role1.id); }); test("PATCH /milestones/:id/contributors/:contributorId updates pivot order", async ({