✨ feat(board): Add client moves and review feedback
This commit is contained in:
parent
feb3aa7571
commit
d011ac6214
10 changed files with 600 additions and 28 deletions
|
|
@ -1,15 +1,17 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { createCard } from "@/app/actions/cards";
|
||||
import { acceptCard, createCard } from "@/app/actions/cards";
|
||||
import { requireSession } from "@/lib/auth/session";
|
||||
import { getServerPb } from "@/lib/pb/server";
|
||||
import { resolveWorkArea } from "@/lib/work-areas/server";
|
||||
import { relinkProject } from "@/lib/linking/engine";
|
||||
|
||||
vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
|
||||
vi.mock("@/lib/auth/session", () => ({ requireSession: vi.fn() }));
|
||||
vi.mock("@/lib/pb/server", () => ({ getServerPb: vi.fn() }));
|
||||
vi.mock("@/lib/work-areas/server", () => ({ resolveWorkArea: vi.fn() }));
|
||||
vi.mock("@/app/actions/activity", () => ({ logActivity: vi.fn() }));
|
||||
vi.mock("@/lib/linking/engine", () => ({ relinkProject: vi.fn() }));
|
||||
|
||||
describe("owner card creation", () => {
|
||||
beforeEach(() => {
|
||||
|
|
@ -48,3 +50,44 @@ describe("owner card creation", () => {
|
|||
}));
|
||||
});
|
||||
});
|
||||
|
||||
describe("accepting a client request", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(requireSession).mockResolvedValue({ userId: "owner-1", role: "owner", clientId: "" });
|
||||
vi.mocked(relinkProject).mockResolvedValue({} as never);
|
||||
});
|
||||
|
||||
function setup(column: string) {
|
||||
const update = vi.fn(async (_id: string, _patch: Record<string, unknown>) => ({}));
|
||||
const getFirstListItem = vi.fn(async () => ({ position: 2000 }));
|
||||
const card = { id: "card-1", project: "project-1", column, start_date: "", accepted: false };
|
||||
const columns = [
|
||||
{ id: "inbox", project: "project-1", is_inbox: true, name: "Inbox" },
|
||||
{ id: "backlog", project: "project-1", is_inbox: false, name: "Backlog" },
|
||||
{ id: "todo", project: "project-1", is_inbox: false, name: "To Do" },
|
||||
];
|
||||
vi.mocked(getServerPb).mockResolvedValue({
|
||||
collection: (name: string) => name === "cards"
|
||||
? { getOne: async () => card, getFirstListItem, update }
|
||||
: { getFullList: async () => columns },
|
||||
} as never);
|
||||
return { update, getFirstListItem };
|
||||
}
|
||||
|
||||
it("keeps To Do when the client already chose it", async () => {
|
||||
const { update, getFirstListItem } = setup("todo");
|
||||
await acceptCard({ cardId: "card-1", clientSlug: "client", projectSlug: "project" });
|
||||
expect(update).toHaveBeenCalledWith("card-1", expect.objectContaining({ accepted: true, start_date: expect.any(String) }));
|
||||
expect(update.mock.calls[0][1]).not.toHaveProperty("column");
|
||||
expect(getFirstListItem).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still moves an Inbox request to Backlog", async () => {
|
||||
const { update } = setup("inbox");
|
||||
await acceptCard({ cardId: "card-1", clientSlug: "client", projectSlug: "project" });
|
||||
expect(update).toHaveBeenCalledWith("card-1", expect.objectContaining({
|
||||
accepted: true, column: "backlog", position: 3000,
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -148,9 +148,9 @@ export async function updateCard(input: {
|
|||
* Phase 2 uses it to bound which commits a scope rule may claim, so a new card
|
||||
* cannot retroactively swallow a year of old commits sharing its scope.
|
||||
*
|
||||
* The card moves to the column immediately after the Inbox — Backlog in the
|
||||
* default layout — rather than to a hardcoded name, so a renamed board still
|
||||
* works.
|
||||
* An Inbox card moves to the column immediately after Inbox. If the client
|
||||
* already placed their request in Backlog or To Do, accepting it keeps the
|
||||
* chosen lane while still recording the owner's acceptance and start date.
|
||||
*/
|
||||
export async function acceptCard(input: {
|
||||
cardId: string;
|
||||
|
|
@ -168,23 +168,28 @@ export async function acceptCard(input: {
|
|||
});
|
||||
|
||||
const inboxIndex = columns.findIndex((c) => c.is_inbox);
|
||||
const destination = columns[inboxIndex + 1] ?? columns[inboxIndex];
|
||||
const source = columns.find((c) => c.id === card.column);
|
||||
if (!source) throw new Error("This card has no column in its project");
|
||||
const destination = source.is_inbox
|
||||
? columns[inboxIndex + 1] ?? source
|
||||
: source;
|
||||
if (!destination) throw new Error("This project has no column to accept into");
|
||||
|
||||
const last = await pb
|
||||
.collection("cards")
|
||||
.getFirstListItem<CardRecord>(`column = "${destination.id}"`, {
|
||||
sort: "-position",
|
||||
})
|
||||
.catch(() => null);
|
||||
const last = source.is_inbox
|
||||
? await pb.collection("cards")
|
||||
.getFirstListItem<CardRecord>(`column = "${destination.id}"`, { sort: "-position" })
|
||||
.catch(() => null)
|
||||
: null;
|
||||
|
||||
const today = new Date().toISOString().slice(0, 10);
|
||||
|
||||
await pb.collection("cards").update(input.cardId, {
|
||||
accepted: true,
|
||||
start_date: card.start_date || today,
|
||||
column: destination.id,
|
||||
position: positionBetween(last ? last.position : null, null),
|
||||
...(source.is_inbox ? {
|
||||
column: destination.id,
|
||||
position: positionBetween(last ? last.position : null, null),
|
||||
} : {}),
|
||||
});
|
||||
|
||||
await relinkProject(card.project);
|
||||
|
|
|
|||
125
src/app/api/client/cards/[id]/move/route.ts
Normal file
125
src/app/api/client/cards/[id]/move/route.ts
Normal file
|
|
@ -0,0 +1,125 @@
|
|||
import { revalidatePath } from "next/cache";
|
||||
|
||||
import { getAdminPb } from "@/lib/pb/admin";
|
||||
import { requireSession } from "@/lib/auth/session";
|
||||
import { AccessError, assertClientOwnsProject, canWriteClientFields } from "@/lib/auth/access";
|
||||
import { clientMoveTargets, isClientReviewReturn } from "@/lib/board/client-moves";
|
||||
import { auditRow, writeCardWithAudit } from "@/lib/cards/write-with-audit";
|
||||
import { jsonError, jsonOk } from "@/lib/http/json";
|
||||
import { positionBetween } from "@/lib/ordering/fractional";
|
||||
import type { CardRecord, ColumnRecord, ProjectRecord } from "@/lib/pb/types";
|
||||
|
||||
/** Clients may arrange queued work and resolve Review, but never start work for the owner. */
|
||||
export async function PATCH(
|
||||
request: Request,
|
||||
context: { params: Promise<{ id: string }> }
|
||||
): Promise<Response> {
|
||||
try {
|
||||
const session = await requireSession();
|
||||
const { id } = await context.params;
|
||||
|
||||
let body: unknown;
|
||||
try {
|
||||
body = await request.json();
|
||||
} catch {
|
||||
return jsonError("Invalid request body", 400);
|
||||
}
|
||||
if (!body || typeof body !== "object" || Array.isArray(body)) {
|
||||
return jsonError("Invalid request body", 400);
|
||||
}
|
||||
const input = body as Record<string, unknown>;
|
||||
if (Object.keys(input).some((key) => key !== "column" && key !== "comment")) {
|
||||
return jsonError("Unsupported field in request", 400);
|
||||
}
|
||||
if (typeof input.column !== "string" || !input.column) {
|
||||
return jsonError("A destination column is required", 400);
|
||||
}
|
||||
if (input.comment !== undefined && typeof input.comment !== "string") {
|
||||
return jsonError("Invalid feedback", 400);
|
||||
}
|
||||
|
||||
const pb = await getAdminPb();
|
||||
const card = await pb.collection("cards").getOne<CardRecord>(id).catch(() => null);
|
||||
if (!card || card.archived) return jsonError("Card not found", 404);
|
||||
const project = await pb.collection("projects").getOne<ProjectRecord>(card.project).catch(() => null);
|
||||
if (!project) return jsonError("Card not found", 404);
|
||||
assertClientOwnsProject(session, project);
|
||||
if (!canWriteClientFields(session, project.client)) return jsonError("Card not found", 404);
|
||||
|
||||
const columns = await pb.collection("columns").getFullList<ColumnRecord>({
|
||||
filter: `project = "${project.id}"`,
|
||||
sort: "position",
|
||||
});
|
||||
const source = columns.find((column) => column.id === card.column);
|
||||
if (!source) return jsonError("Card is not in a movable column", 409);
|
||||
const destination = clientMoveTargets(columns, source.id)
|
||||
.find((column) => column.id === input.column);
|
||||
if (!destination) return jsonError("That move is not allowed", 409);
|
||||
|
||||
const reviewReturn = isClientReviewReturn(source, destination);
|
||||
const comment = (input.comment as string | undefined)?.trim().slice(0, 20000) ?? "";
|
||||
if (comment && !reviewReturn) return jsonError("Feedback is only available when sending Review back", 400);
|
||||
|
||||
const last = await pb.collection("cards").getList<Pick<CardRecord, "position">>(1, 1, {
|
||||
filter: `column = "${destination.id}" && archived = false`,
|
||||
sort: "-position",
|
||||
fields: "position",
|
||||
});
|
||||
const position = positionBetween(last.items[0]?.position ?? null, null);
|
||||
const updated = await writeCardWithAudit(pb, {
|
||||
cardId: card.id,
|
||||
patch: { column: destination.id, position },
|
||||
rows: [auditRow({
|
||||
project: card.project,
|
||||
card: card.id,
|
||||
actor: session.userId,
|
||||
action: "column_changed",
|
||||
field: "column",
|
||||
oldValue: source.name,
|
||||
newValue: destination.name,
|
||||
})],
|
||||
});
|
||||
|
||||
let warning = "";
|
||||
if (comment) {
|
||||
try {
|
||||
await pb.collection("comments").create({
|
||||
card: card.id,
|
||||
author: session.userId,
|
||||
body: comment,
|
||||
});
|
||||
} catch (error) {
|
||||
console.error("[client-move] feedback failed after move", error);
|
||||
warning = "Card moved, but feedback could not be saved. Please add it on the card.";
|
||||
}
|
||||
if (!warning) {
|
||||
await pb.collection("activity").create(auditRow({
|
||||
project: card.project,
|
||||
card: card.id,
|
||||
actor: session.userId,
|
||||
action: "comment_added",
|
||||
field: "",
|
||||
oldValue: "",
|
||||
newValue: "",
|
||||
})).catch((error) => {
|
||||
console.error("[client-move] feedback saved but activity failed", error);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
revalidatePath("/");
|
||||
revalidatePath("/plan");
|
||||
return jsonOk({
|
||||
card: {
|
||||
id: card.id,
|
||||
column: updated?.column ?? destination.id,
|
||||
position: updated?.position ?? position,
|
||||
},
|
||||
warning,
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof AccessError) return jsonError(error.message, error.status);
|
||||
console.error("[client-move] unexpected error", error);
|
||||
return jsonError("Could not move the card", 500);
|
||||
}
|
||||
}
|
||||
127
src/app/api/client/cards/client-move-route.test.ts
Normal file
127
src/app/api/client/cards/client-move-route.test.ts
Normal file
|
|
@ -0,0 +1,127 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { PATCH } from "@/app/api/client/cards/[id]/move/route";
|
||||
import { requireSession } from "@/lib/auth/session";
|
||||
import { getAdminPb } from "@/lib/pb/admin";
|
||||
|
||||
vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
|
||||
vi.mock("@/lib/auth/session", () => ({ requireSession: vi.fn() }));
|
||||
vi.mock("@/lib/pb/admin", () => ({ getAdminPb: vi.fn() }));
|
||||
|
||||
const columns = [
|
||||
{ id: "inbox", project: "p1", name: "Inbox", is_inbox: true, is_done: false },
|
||||
{ id: "backlog", project: "p1", name: "Backlog", is_inbox: false, is_done: false },
|
||||
{ id: "todo", project: "p1", name: "To Do", is_inbox: false, is_done: false },
|
||||
{ id: "progress", project: "p1", name: "In Progress", is_inbox: false, is_done: false },
|
||||
{ id: "review", project: "p1", name: "Review", is_inbox: false, is_done: false },
|
||||
{ id: "done", project: "p1", name: "Done", is_inbox: false, is_done: true },
|
||||
];
|
||||
|
||||
function setup(source: string) {
|
||||
let card = { id: "card-1", project: "p1", column: source, archived: false, accepted: false, position: 1000 };
|
||||
const update = vi.fn(async (_id: string, patch: Record<string, unknown>) => {
|
||||
card = { ...card, ...patch } as typeof card;
|
||||
return card;
|
||||
});
|
||||
const commentCreate = vi.fn(async () => ({ id: "comment-1" }));
|
||||
const activityCreate = vi.fn(async () => ({ id: "activity-1" }));
|
||||
const pb = {
|
||||
collection: vi.fn((name: string) => {
|
||||
if (name === "cards") return {
|
||||
getOne: vi.fn(async () => card),
|
||||
getList: vi.fn(async () => ({ items: [] })),
|
||||
update,
|
||||
};
|
||||
if (name === "projects") return { getOne: vi.fn(async () => ({ id: "p1", client: "c1" })) };
|
||||
if (name === "columns") return { getFullList: vi.fn(async () => columns) };
|
||||
if (name === "comments") return { create: commentCreate };
|
||||
if (name === "activity") return { create: activityCreate };
|
||||
throw new Error(`Unexpected collection ${name}`);
|
||||
}),
|
||||
};
|
||||
vi.mocked(getAdminPb).mockResolvedValue(pb as never);
|
||||
return { update, commentCreate, activityCreate };
|
||||
}
|
||||
|
||||
function move(column: string, comment?: string, extras: Record<string, unknown> = {}) {
|
||||
return PATCH(new Request("http://localhost/api/client/cards/card-1/move", {
|
||||
method: "PATCH",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ column, ...(comment === undefined ? {} : { comment }), ...extras }),
|
||||
}), { params: Promise.resolve({ id: "card-1" }) });
|
||||
}
|
||||
|
||||
describe("client card move route", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(requireSession).mockResolvedValue({ userId: "client-user", role: "client_manager", clientId: "c1" });
|
||||
});
|
||||
|
||||
it("moves a request to To Do without claiming the owner accepted it", async () => {
|
||||
const { update, activityCreate } = setup("inbox");
|
||||
const response = await move("todo");
|
||||
expect(response.status).toBe(200);
|
||||
expect(update).toHaveBeenCalledWith("card-1", { column: "todo", position: 1000 });
|
||||
expect(activityCreate).toHaveBeenCalledWith(expect.objectContaining({
|
||||
action: "column_changed", old_value: "Inbox", new_value: "To Do",
|
||||
}));
|
||||
});
|
||||
|
||||
it("saves feedback when a client sends Review back", async () => {
|
||||
const { commentCreate, activityCreate } = setup("review");
|
||||
const response = await move("backlog", " Please adjust the layout ");
|
||||
expect(response.status).toBe(200);
|
||||
expect(commentCreate).toHaveBeenCalledWith({
|
||||
card: "card-1", author: "client-user", body: "Please adjust the layout",
|
||||
});
|
||||
expect(activityCreate).toHaveBeenCalledWith(expect.objectContaining({ action: "comment_added" }));
|
||||
});
|
||||
|
||||
it("keeps a successful move when optional feedback fails to save and reports it", async () => {
|
||||
const { update, commentCreate } = setup("review");
|
||||
commentCreate.mockRejectedValueOnce(new Error("storage unavailable"));
|
||||
const log = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
const response = await move("todo", "Please adjust the layout");
|
||||
expect(response.status).toBe(200);
|
||||
expect(update).toHaveBeenCalledWith("card-1", expect.objectContaining({ column: "todo" }));
|
||||
expect((await response.json()).warning).toMatch(/feedback could not be saved/);
|
||||
} finally {
|
||||
log.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("allows Review to Done but rejects Done from the queue and entry into developer lanes", async () => {
|
||||
setup("review");
|
||||
expect((await move("done")).status).toBe(200);
|
||||
const { update } = setup("todo");
|
||||
expect((await move("done")).status).toBe(409);
|
||||
expect((await move("progress")).status).toBe(409);
|
||||
expect((await move("review")).status).toBe(409);
|
||||
setup("progress");
|
||||
expect((await move("backlog")).status).toBe(409);
|
||||
expect(update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not attach review feedback to an unrelated queue move", async () => {
|
||||
const { update, commentCreate } = setup("inbox");
|
||||
expect((await move("backlog", "Please change this")).status).toBe(400);
|
||||
expect(update).not.toHaveBeenCalled();
|
||||
expect(commentCreate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects cross-client and owner requests before writing", async () => {
|
||||
const { update } = setup("inbox");
|
||||
vi.mocked(requireSession).mockResolvedValue({ userId: "other-client", role: "client_manager", clientId: "c2" });
|
||||
expect((await move("backlog")).status).toBe(404);
|
||||
vi.mocked(requireSession).mockResolvedValue({ userId: "owner", role: "owner", clientId: "" });
|
||||
expect((await move("backlog")).status).toBe(404);
|
||||
expect(update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not accept a client-supplied position or other card fields", async () => {
|
||||
const { update } = setup("inbox");
|
||||
expect((await move("backlog", undefined, { position: 999999 })).status).toBe(400);
|
||||
expect(update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,8 +1,10 @@
|
|||
import { getServerPb } from "@/lib/pb/server";
|
||||
import { requireSession } from "@/lib/auth/session";
|
||||
import { logActivity } from "@/app/actions/activity";
|
||||
import { AccessError } from "@/lib/auth/access";
|
||||
import { jsonError, jsonOk } from "@/lib/http/json";
|
||||
import type { CardRecord } from "@/lib/pb/types";
|
||||
import { relinkProject } from "@/lib/linking/engine";
|
||||
import type { CardRecord, ColumnRecord } from "@/lib/pb/types";
|
||||
|
||||
/**
|
||||
* PATCH /api/owner/cards/:id/move — the owner drops a card in a new place.
|
||||
|
|
@ -12,9 +14,9 @@ import type { CardRecord } from "@/lib/pb/types";
|
|||
* authorises the update, so a client manager reaching this route is refused by
|
||||
* the database even though the handler itself is simple.
|
||||
*
|
||||
* That is why there is no field whitelist here: PocketBase is already
|
||||
* enforcing who may write. Only the two positional fields are forwarded
|
||||
* regardless, so a stray body field cannot ride along.
|
||||
* Only the two positional fields come from the request. When the owner moves
|
||||
* a pending request into work, the server also records acceptance and its
|
||||
* start date so a client-selected To Do position does not skip that step.
|
||||
*/
|
||||
export async function PATCH(
|
||||
request: Request,
|
||||
|
|
@ -47,16 +49,44 @@ export async function PATCH(
|
|||
}
|
||||
|
||||
const pb = await getServerPb();
|
||||
const card = await pb.collection("cards").getOne<CardRecord>(id).catch(() => null);
|
||||
if (!card || card.archived) return jsonError("Card not found", 404);
|
||||
const destination = await pb.collection("columns").getOne<ColumnRecord>(column).catch(() => null);
|
||||
if (!destination || destination.project !== card.project) {
|
||||
return jsonError("Destination column not found", 400);
|
||||
}
|
||||
const accepting = !card.accepted &&
|
||||
card.column !== destination.id && !destination.is_inbox;
|
||||
|
||||
const updated = await pb
|
||||
.collection("cards")
|
||||
.update<CardRecord>(id, { column, position });
|
||||
.update<CardRecord>(id, {
|
||||
column,
|
||||
position,
|
||||
...(accepting ? { accepted: true, start_date: card.start_date || new Date().toISOString().slice(0, 10) } : {}),
|
||||
});
|
||||
if (accepting) {
|
||||
await logActivity({
|
||||
project: card.project,
|
||||
card: card.id,
|
||||
actor: session.userId,
|
||||
action: "card_accepted",
|
||||
newValue: destination.name,
|
||||
}).catch((error) => {
|
||||
console.error("[owner-move] accepted card but activity failed", error);
|
||||
});
|
||||
await relinkProject(card.project).catch((error) => {
|
||||
console.error("[owner-move] accepted card but relinking failed", error);
|
||||
});
|
||||
}
|
||||
|
||||
return jsonOk({
|
||||
card: {
|
||||
id: updated.id,
|
||||
column: updated.column,
|
||||
position: updated.position,
|
||||
accepted: updated.accepted,
|
||||
start_date: updated.start_date,
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
|
|
@ -66,4 +96,4 @@ export async function PATCH(
|
|||
console.error("[owner-move] failed", error);
|
||||
return jsonError("Could not move the card", 500);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
58
src/app/api/owner/cards/owner-move.test.ts
Normal file
58
src/app/api/owner/cards/owner-move.test.ts
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { PATCH } from "@/app/api/owner/cards/[id]/move/route";
|
||||
import { requireSession } from "@/lib/auth/session";
|
||||
import { getServerPb } from "@/lib/pb/server";
|
||||
import { relinkProject } from "@/lib/linking/engine";
|
||||
import { logActivity } from "@/app/actions/activity";
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ requireSession: vi.fn() }));
|
||||
vi.mock("@/lib/pb/server", () => ({ getServerPb: vi.fn() }));
|
||||
vi.mock("@/lib/linking/engine", () => ({ relinkProject: vi.fn() }));
|
||||
vi.mock("@/app/actions/activity", () => ({ logActivity: vi.fn() }));
|
||||
|
||||
function setup(destinationProject = "p1") {
|
||||
const card = { id: "card-1", project: "p1", column: "todo", position: 1000,
|
||||
accepted: false, start_date: "", archived: false, created_by_role: "client" };
|
||||
const update = vi.fn(async (_id: string, patch: Record<string, unknown>) => ({ ...card, ...patch }));
|
||||
vi.mocked(getServerPb).mockResolvedValue({ collection: (name: string) => name === "cards"
|
||||
? { getOne: async () => card, update }
|
||||
: { getOne: async () => ({ id: "progress", project: destinationProject, name: "In Progress", is_inbox: false }) },
|
||||
} as never);
|
||||
return { update };
|
||||
}
|
||||
|
||||
function move() {
|
||||
return PATCH(new Request("http://localhost/api/owner/cards/card-1/move", {
|
||||
method: "PATCH", headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ column: "progress", position: 2000 }),
|
||||
}), { params: Promise.resolve({ id: "card-1" }) });
|
||||
}
|
||||
|
||||
describe("owner board moves", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(requireSession).mockResolvedValue({ userId: "owner", role: "owner", clientId: "" });
|
||||
vi.mocked(relinkProject).mockResolvedValue({} as never);
|
||||
vi.mocked(logActivity).mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("accepts a client-requested card when the owner advances it to work", async () => {
|
||||
const { update } = setup();
|
||||
const response = await move();
|
||||
expect(response.status).toBe(200);
|
||||
expect(update).toHaveBeenCalledWith("card-1", expect.objectContaining({
|
||||
column: "progress", position: 2000, accepted: true, start_date: expect.any(String),
|
||||
}));
|
||||
expect(relinkProject).toHaveBeenCalledWith("p1");
|
||||
expect(logActivity).toHaveBeenCalledWith(expect.objectContaining({
|
||||
project: "p1", card: "card-1", action: "card_accepted", newValue: "In Progress",
|
||||
}));
|
||||
});
|
||||
|
||||
it("rejects a destination from another project", async () => {
|
||||
const { update } = setup("p2");
|
||||
expect((await move()).status).toBe(400);
|
||||
expect(update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -34,6 +34,7 @@ function SortableCard({
|
|||
card,
|
||||
area,
|
||||
role,
|
||||
columns,
|
||||
clientSlug,
|
||||
projectSlug,
|
||||
onPatched,
|
||||
|
|
@ -41,6 +42,7 @@ function SortableCard({
|
|||
card: CardRecord;
|
||||
area: WorkAreaRecord | null;
|
||||
role: Role;
|
||||
columns: ColumnRecord[];
|
||||
clientSlug: string;
|
||||
projectSlug: string;
|
||||
onPatched: (card: CardRecord) => void;
|
||||
|
|
@ -61,8 +63,8 @@ function SortableCard({
|
|||
{...(draggable ? attributes : {})}
|
||||
{...(draggable ? listeners : {})}
|
||||
>
|
||||
{role === "client_manager" && (
|
||||
<ClientCardControls card={card} onPatched={onPatched} />
|
||||
{role === "client_manager" && !columns.find((column) => column.id === card.column)?.is_done && (
|
||||
<ClientCardControls card={card} columns={columns} onPatched={onPatched} />
|
||||
)}
|
||||
</CardTile>
|
||||
);
|
||||
|
|
@ -243,6 +245,10 @@ export function Board({
|
|||
console.error("[board] move failed", response.status);
|
||||
setCards(previous);
|
||||
} else {
|
||||
const result = await response.json();
|
||||
if (result.card) {
|
||||
setCards((current) => current.map((item) => item.id === card.id ? { ...item, ...result.card } : item));
|
||||
}
|
||||
// No router.refresh(): it raced with the optimistic state and could
|
||||
// revert the move before the user saw it land. The next page visit
|
||||
// loads the fresh snapshot from loadBoard.
|
||||
|
|
@ -281,6 +287,7 @@ export function Board({
|
|||
card={card}
|
||||
area={workAreaForCard(card, areas)}
|
||||
role={role}
|
||||
columns={columns}
|
||||
clientSlug={clientSlug}
|
||||
projectSlug={projectSlug}
|
||||
onPatched={onPatched}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
"use client";
|
||||
|
||||
import { useState } from "react";
|
||||
import { ChevronDown } from "lucide-react";
|
||||
|
||||
import {
|
||||
Dropdown,
|
||||
|
|
@ -9,7 +10,9 @@ import {
|
|||
DropdownLabel,
|
||||
DropdownTrigger,
|
||||
} from "@/components/ui/dropdown";
|
||||
import type { CardRecord, Priority } from "@/lib/pb/types";
|
||||
import { useToast } from "@/components/ui/toast";
|
||||
import { clientMoveTargets, isClientReviewReturn } from "@/lib/board/client-moves";
|
||||
import type { CardRecord, ColumnRecord, Priority } from "@/lib/pb/types";
|
||||
|
||||
const PRIORITIES: Array<{ value: Priority; label: string }> = [
|
||||
{ value: "critical", label: "Critical" },
|
||||
|
|
@ -19,21 +22,28 @@ const PRIORITIES: Array<{ value: Priority; label: string }> = [
|
|||
];
|
||||
|
||||
/**
|
||||
* The only two fields a client manager may change.
|
||||
* Clients can edit priority and date, and move cards through the guarded
|
||||
* queue/review workflow. Every write goes through a client API route.
|
||||
*
|
||||
* Both go through PATCH /api/client/cards/:id, never straight to PocketBase.
|
||||
* Their token cannot write to the cards collection at all — see Plan 1a — so
|
||||
* a direct SDK call from here would simply fail. The route is the path.
|
||||
* Their token cannot write to the cards collection at all, so neither route
|
||||
* writes directly from the browser.
|
||||
*/
|
||||
export function ClientCardControls({
|
||||
card,
|
||||
columns,
|
||||
onPatched,
|
||||
}: {
|
||||
card: CardRecord;
|
||||
columns: ColumnRecord[];
|
||||
onPatched: (card: CardRecord) => void;
|
||||
}) {
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [returnTo, setReturnTo] = useState<ColumnRecord | null>(null);
|
||||
const [feedback, setFeedback] = useState("");
|
||||
const { showError, showToast } = useToast();
|
||||
const source = columns.find((column) => column.id === card.column);
|
||||
const destinations = clientMoveTargets(columns, card.column);
|
||||
|
||||
async function patch(body: Record<string, unknown>) {
|
||||
setSaving(true);
|
||||
|
|
@ -60,11 +70,70 @@ export function ClientCardControls({
|
|||
onPatched({ ...card, ...updated } as CardRecord);
|
||||
}
|
||||
|
||||
async function move(destination: ColumnRecord, comment = "") {
|
||||
if (saving) return;
|
||||
setSaving(true);
|
||||
setError(null);
|
||||
try {
|
||||
const response = await fetch(`/api/client/cards/${card.id}/move`, {
|
||||
method: "PATCH",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ column: destination.id, comment }),
|
||||
});
|
||||
if (!response.ok) {
|
||||
showError("Could not move this card. Please try again.");
|
||||
return;
|
||||
}
|
||||
const result = await response.json();
|
||||
onPatched({ ...card, ...result.card } as CardRecord);
|
||||
setReturnTo(null);
|
||||
setFeedback("");
|
||||
if (result.warning) showError(result.warning);
|
||||
else showToast(destination.is_done ? "Work approved and moved to Done." : `Moved to ${destination.name}.`);
|
||||
} catch {
|
||||
showError("Could not move this card. Please try again.");
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
}
|
||||
|
||||
const currentPriority =
|
||||
PRIORITIES.find((p) => p.value === card.priority) ?? PRIORITIES[2];
|
||||
|
||||
return (
|
||||
<div className="mt-3 flex items-center gap-2 border-t border-border-subtle pt-2.5">
|
||||
<div className="mt-3 flex flex-wrap items-center gap-2 border-t border-border-subtle pt-2.5">
|
||||
{destinations.length > 0 && (
|
||||
<Dropdown>
|
||||
<DropdownTrigger asChild>
|
||||
<button
|
||||
type="button"
|
||||
disabled={saving}
|
||||
aria-label="Move card"
|
||||
className="inline-flex items-center gap-1 rounded-md bg-brand-soft px-2 py-1 text-[11px] font-medium text-brand hover:bg-surface-raised disabled:opacity-50"
|
||||
>
|
||||
Move <ChevronDown size={12} aria-hidden="true" />
|
||||
</button>
|
||||
</DropdownTrigger>
|
||||
<DropdownContent>
|
||||
<DropdownLabel>{destinations.some((destination) => destination.is_done) ? "Review decision" : "Move card to"}</DropdownLabel>
|
||||
{destinations.map((destination) => (
|
||||
<DropdownItem
|
||||
key={destination.id}
|
||||
onSelect={() => {
|
||||
if (source && isClientReviewReturn(source, destination)) {
|
||||
setFeedback("");
|
||||
setReturnTo(destination);
|
||||
} else {
|
||||
void move(destination);
|
||||
}
|
||||
}}
|
||||
>
|
||||
{destination.is_done ? "Approve → Done" : destination.name}
|
||||
</DropdownItem>
|
||||
))}
|
||||
</DropdownContent>
|
||||
</Dropdown>
|
||||
)}
|
||||
<Dropdown>
|
||||
<DropdownTrigger asChild>
|
||||
<button
|
||||
|
|
@ -107,6 +176,41 @@ export function ClientCardControls({
|
|||
{error}
|
||||
</span>
|
||||
)}
|
||||
{returnTo && (
|
||||
<div className="w-full space-y-2 rounded-lg border border-border-subtle bg-surface p-2.5">
|
||||
<p className="text-xs font-medium text-text-primary">Send back to {returnTo.name}</p>
|
||||
<label className="block space-y-1">
|
||||
<span className="text-[11px] text-text-muted">Feedback for the developer (optional)</span>
|
||||
<textarea
|
||||
value={feedback}
|
||||
onChange={(event) => setFeedback(event.target.value)}
|
||||
rows={2}
|
||||
maxLength={20000}
|
||||
disabled={saving}
|
||||
placeholder="What should change?"
|
||||
className="w-full rounded-md border border-border-subtle bg-panel px-2 py-1.5 text-xs text-text-primary outline-none placeholder:text-text-muted focus:border-brand"
|
||||
/>
|
||||
</label>
|
||||
<div className="flex items-center gap-2">
|
||||
<button
|
||||
type="button"
|
||||
disabled={saving}
|
||||
onClick={() => void move(returnTo, feedback)}
|
||||
className="rounded-md bg-brand px-2.5 py-1.5 text-xs font-medium text-brand-fg disabled:opacity-50"
|
||||
>
|
||||
{saving ? "Sending…" : "Send back"}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
disabled={saving}
|
||||
onClick={() => setReturnTo(null)}
|
||||
className="px-1 text-xs text-text-muted hover:text-text-secondary"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
40
src/lib/board/client-moves.test.ts
Normal file
40
src/lib/board/client-moves.test.ts
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { clientMoveTargets } from "@/lib/board/client-moves";
|
||||
import type { ColumnRecord } from "@/lib/pb/types";
|
||||
|
||||
const columns = [
|
||||
{ id: "inbox", project: "project-1", name: "Inbox", is_inbox: true, is_done: false },
|
||||
{ id: "backlog", project: "project-1", name: "Backlog", is_inbox: false, is_done: false },
|
||||
{ id: "todo", project: "project-1", name: "To Do", is_inbox: false, is_done: false },
|
||||
{ id: "progress", project: "project-1", name: "In Progress", is_inbox: false, is_done: false },
|
||||
{ id: "review", project: "project-1", name: "Review", is_inbox: false, is_done: false },
|
||||
{ id: "done", project: "project-1", name: "Done", is_inbox: false, is_done: true },
|
||||
] as ColumnRecord[];
|
||||
|
||||
describe("client board moves", () => {
|
||||
it("lets clients arrange the Inbox, Backlog and To Do queue", () => {
|
||||
expect(clientMoveTargets(columns, "inbox").map((column) => column.id)).toEqual(["backlog", "todo"]);
|
||||
expect(clientMoveTargets(columns, "backlog").map((column) => column.id)).toEqual(["inbox", "todo"]);
|
||||
expect(clientMoveTargets(columns, "todo").map((column) => column.id)).toEqual(["inbox", "backlog"]);
|
||||
});
|
||||
|
||||
it("lets clients approve Review or send it back for changes", () => {
|
||||
expect(clientMoveTargets(columns, "review").map((column) => column.id)).toEqual(["backlog", "todo", "done"]);
|
||||
});
|
||||
|
||||
it("never lets clients move work into In Progress or Review, or move work already in progress or done", () => {
|
||||
expect(clientMoveTargets(columns, "progress")).toEqual([]);
|
||||
expect(clientMoveTargets(columns, "done")).toEqual([]);
|
||||
expect(clientMoveTargets(columns, "unknown")).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not treat another project's column or an extra lane as a destination", () => {
|
||||
const extended = [
|
||||
...columns,
|
||||
{ id: "extra", project: "project-1", name: "Blocked", is_inbox: false, is_done: false },
|
||||
{ id: "foreign", project: "project-2", name: "Backlog", is_inbox: false, is_done: false },
|
||||
] as ColumnRecord[];
|
||||
expect(clientMoveTargets(extended, "inbox").map((column) => column.id)).toEqual(["backlog", "todo"]);
|
||||
});
|
||||
});
|
||||
33
src/lib/board/client-moves.ts
Normal file
33
src/lib/board/client-moves.ts
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
import type { ColumnRecord } from "@/lib/pb/types";
|
||||
|
||||
type ClientLane = "inbox" | "backlog" | "todo" | "review" | "done" | "other";
|
||||
|
||||
function lane(column: ColumnRecord): ClientLane {
|
||||
if (column.is_inbox) return "inbox";
|
||||
if (column.is_done) return "done";
|
||||
const name = column.name.trim().toLowerCase().replace(/[\s-]+/g, "");
|
||||
if (name === "backlog") return "backlog";
|
||||
if (name === "todo") return "todo";
|
||||
if (name === "review") return "review";
|
||||
return "other";
|
||||
}
|
||||
|
||||
export function isClientReviewReturn(source: ColumnRecord, destination: ColumnRecord): boolean {
|
||||
return lane(source) === "review" && (lane(destination) === "backlog" || lane(destination) === "todo");
|
||||
}
|
||||
|
||||
/** Shared by the board control and the server route; the route is authoritative. */
|
||||
export function clientMoveTargets(columns: ColumnRecord[], sourceId: string): ColumnRecord[] {
|
||||
const source = columns.find((column) => column.id === sourceId);
|
||||
if (!source) return [];
|
||||
const sourceLane = lane(source);
|
||||
const queue = sourceLane === "inbox" || sourceLane === "backlog" || sourceLane === "todo";
|
||||
if (!queue && sourceLane !== "review") return [];
|
||||
|
||||
return columns.filter((column) => {
|
||||
if (column.id === sourceId || column.project !== source.project) return false;
|
||||
const destinationLane = lane(column);
|
||||
if (queue) return destinationLane === "inbox" || destinationLane === "backlog" || destinationLane === "todo";
|
||||
return destinationLane === "backlog" || destinationLane === "todo" || destinationLane === "done";
|
||||
});
|
||||
}
|
||||
Loading…
Reference in a new issue