From 549ff1cf329255f97037ead40ef122a4c50747a3 Mon Sep 17 00:00:00 2001 From: Farzaneh Haghani Date: Fri, 31 May 2024 22:20:54 +0100 Subject: [PATCH] draft changed to status --- client/setupTests.mjs | 6 +-- client/src/pages/Drafts/Drafts.test.jsx | 2 +- client/src/pages/Drafts/index.jsx | 4 +- client/src/services/resourceService.js | 21 +++-------- client/src/services/resourceService.test.js | 14 +++---- e2e/fixtures/oneDraftResource.json | 2 +- e2e/fixtures/onePublishedResource.json | 2 +- e2e/integration/pagination.test.js | 8 ++-- e2e/integration/suggest.test.js | 2 +- server/db.js | 4 -- server/docs/resources.yml | 10 ++--- server/docs/schema.yml | 20 +++++----- server/resources/resources.test.js | 41 +++++++++++---------- server/resources/resourcesController.js | 33 +++++------------ server/resources/resourcesRepository.js | 40 +++++++++----------- server/resources/resourcesService.js | 24 +++++------- 16 files changed, 98 insertions(+), 135 deletions(-) diff --git a/client/setupTests.mjs b/client/setupTests.mjs index eb29b566..122ec90c 100644 --- a/client/setupTests.mjs +++ b/client/setupTests.mjs @@ -9,10 +9,10 @@ global.fetch = whatwgFetch; export const resourceStub = (overrides = {}) => ({ accession: new Date(), description: null, - draft: false, + status: "published", id: randomUUID(), - publication: null, - publisher: null, + statusChangedDate: null, + statusChangedBy: null, source: randomUUID(), title: "", topic: null, diff --git a/client/src/pages/Drafts/Drafts.test.jsx b/client/src/pages/Drafts/Drafts.test.jsx index 3f42bed9..885ee73d 100644 --- a/client/src/pages/Drafts/Drafts.test.jsx +++ b/client/src/pages/Drafts/Drafts.test.jsx @@ -41,7 +41,7 @@ describe("Drafts", () => { }), http.patch("/api/resources/:id", ({ request: req }) => { patchRequest = req; - return HttpResponse.json({ ...resource, draft: false }); + return HttpResponse.json({ ...resource, status: "published" }); }) ); render(); diff --git a/client/src/pages/Drafts/index.jsx b/client/src/pages/Drafts/index.jsx index 9e0e4852..a0554371 100644 --- a/client/src/pages/Drafts/index.jsx +++ b/client/src/pages/Drafts/index.jsx @@ -13,7 +13,7 @@ export default function Drafts() { const publish = useCallback( async (id) => { - await resourceService.publish(id); + await resourceService.action(id, "published"); await refreshDrafts(); }, [refreshDrafts, resourceService] @@ -22,7 +22,7 @@ export default function Drafts() { const reject = useCallback( async (id) => { if (window.confirm("Do you really want to reject?")) { - await resourceService.reject(id); + await resourceService.action(id, "rejected"); await refreshDrafts(); } }, diff --git a/client/src/services/resourceService.js b/client/src/services/resourceService.js index 439cc307..058001ad 100644 --- a/client/src/services/resourceService.js +++ b/client/src/services/resourceService.js @@ -7,7 +7,7 @@ export default class ResourceService { async getDrafts() { const res = await this.fetch( - `${ResourceService.ENDPOINT}?${new URLSearchParams({ draft: true })}` + `${ResourceService.ENDPOINT}?${new URLSearchParams({ status: "drafted" })}` ); if (res.ok) { const { resources } = await res.json(); @@ -30,9 +30,9 @@ export default class ResourceService { } } - async publish(id) { + async action(id, status) { const res = await this.fetch(`${ResourceService.ENDPOINT}/${id}`, { - body: JSON.stringify({ draft: false }), + body: JSON.stringify({ status }), headers: { "Content-Type": "application/json" }, method: "PATCH", }); @@ -41,17 +41,6 @@ export default class ResourceService { } } - async reject(id) { - const res = await this.fetch(`${ResourceService.ENDPOINT}/${id}`, { - body: JSON.stringify({ draft: false }), - headers: { "Content-Type": "application/json" }, - method: "DELETE", - }); - if (res.ok) { - return this._revive(await res.json()); - } - } - async suggest(resource) { const res = await this.fetch(ResourceService.ENDPOINT, { body: JSON.stringify(resource), @@ -68,11 +57,11 @@ export default class ResourceService { } } - _revive({ accession, publication, ...resource }) { + _revive({ accession, statusChangedDate, ...resource }) { return { ...resource, accession: accession && new Date(accession), - publication: publication && new Date(publication), + statusChangedDate: statusChangedDate && new Date(statusChangedDate), }; } } diff --git a/client/src/services/resourceService.test.js b/client/src/services/resourceService.test.js index 8a8acc02..1fc32205 100644 --- a/client/src/services/resourceService.test.js +++ b/client/src/services/resourceService.test.js @@ -17,12 +17,12 @@ describe("ResourceService", () => { }) ); await service.getDrafts(); - expect(new URL(request.url).searchParams.get("draft")).toBe("true"); + expect(new URL(request.url).searchParams.get("status")).toBe("drafted"); }); it("returns resources on success", async () => { - const resources = [true, true, true].map((draft) => - resourceStub({ draft }) + const resources = ["drafted", "drafted", "drafted"].map((status) => + resourceStub({ status }) ); server.use( http.get("/api/resources", () => { @@ -87,12 +87,12 @@ describe("ResourceService", () => { server.use( http.patch("/api/resources/:id", async ({ request: req }) => { request = req; - return HttpResponse.json({ draft: false }); + return HttpResponse.json({ status: "published" }); }) ); - await service.publish(id); + await service.action(id, "published"); expect(new URL(request.url).pathname).toMatch(new RegExp(`/${id}$`)); - await expect(request.json()).resolves.toEqual({ draft: false }); + await expect(request.json()).resolves.toEqual({ status: "published" }); }); }); @@ -102,7 +102,7 @@ describe("ResourceService", () => { const submitted = { title: "foo bar", url: "https://example.com" }; const created = resourceStub({ ...submitted, - draft: true, + status: "drafted", }); server.use( http.post("/api/resources", ({ request: req }) => { diff --git a/e2e/fixtures/oneDraftResource.json b/e2e/fixtures/oneDraftResource.json index f7693e3c..767a977b 100644 --- a/e2e/fixtures/oneDraftResource.json +++ b/e2e/fixtures/oneDraftResource.json @@ -9,7 +9,7 @@ ], "resources": [ { - "draft": true, + "status": "drafted", "source": "2a8e4bb3-6f9c-4602-b084-af343fcbe6f0", "title": "Joi documentation", "url": "https://joi.dev/" diff --git a/e2e/fixtures/onePublishedResource.json b/e2e/fixtures/onePublishedResource.json index 98716ab1..7a0ab994 100644 --- a/e2e/fixtures/onePublishedResource.json +++ b/e2e/fixtures/onePublishedResource.json @@ -10,7 +10,7 @@ "resources": [ { "description": "Part 4 of an ongoing series on test-driven development (TDD) in JavaScript.", - "draft": false, + "status": "published", "source": "2a8e4bb3-6f9c-4602-b084-af343fcbe6f0", "title": "JS TDD Ohm", "url": "https://blog.jonrshar.pe/2023/May/23/js-tdd-ohm.html" diff --git a/e2e/integration/pagination.test.js b/e2e/integration/pagination.test.js index 8d810535..c5669742 100644 --- a/e2e/integration/pagination.test.js +++ b/e2e/integration/pagination.test.js @@ -38,12 +38,12 @@ function createResource(index) { date.setHours(12, 13, 14); const accession = new Date(date.getTime()); date.setHours(14, 15, 16); - const publication = new Date(date.getTime()); + const status_changed_date = new Date(date.getTime()); return { accession, - draft: false, - publication, - publisher: admin.id, + status: "published", + status_changed_date, + status_changed_by: admin.id, source: user.id, title: `Resource ${id}`, url: `https://example.com/resources/${id}`, diff --git a/e2e/integration/suggest.test.js b/e2e/integration/suggest.test.js index e9fef367..1a7fd215 100644 --- a/e2e/integration/suggest.test.js +++ b/e2e/integration/suggest.test.js @@ -31,7 +31,7 @@ it("lets an authenticated user suggest a resource", () => { expect(submitted).to.have.property("title", title); expect(submitted).to.have.property("url", url); cy.request({ - body: { draft: false }, + body: { status: "published" }, headers: { Authorization: `Bearer ${Cypress.env("SUDO_TOKEN")}` }, method: "PATCH", url: `/api/resources/${created.id}`, diff --git a/server/db.js b/server/db.js index ba66c695..17741eb7 100644 --- a/server/db.js +++ b/server/db.js @@ -111,7 +111,3 @@ export function updateQuery(tableName, columns) { .join(", ") ); } - -export function deleteQuery(tableName) { - return format("DELETE FROM %I WHERE id = $1 RETURNING *;", tableName); -} diff --git a/server/docs/resources.yml b/server/docs/resources.yml index 9dc017bf..d0385b67 100644 --- a/server/docs/resources.yml +++ b/server/docs/resources.yml @@ -14,7 +14,7 @@ **Note**: drafts are only available to admin users or with SUDO token. schema: - type: boolean + type: string - in: query name: page description: Which page of resources to fetch. @@ -87,11 +87,11 @@ application/json: schema: type: object - required: [draft] + required: [status] properties: - draft: - type: boolean - example: false + status: + type: string + example: "published" responses: 200: content: diff --git a/server/docs/schema.yml b/server/docs/schema.yml index 8b93afad..45e22293 100644 --- a/server/docs/schema.yml +++ b/server/docs/schema.yml @@ -81,12 +81,12 @@ components: allOf: - $ref: "#/components/schemas/BaseResource" - type: object - required: [draft] + required: [status] properties: - draft: - type: boolean - example: true - publication: + status: + type: string + example: "drafted" + statusChangedDate: type: string example: null nullable: true @@ -102,15 +102,15 @@ components: allOf: - $ref: "#/components/schemas/BaseResource" - type: object - required: [draft, publication] + required: [status, statusChangedDate] properties: - draft: - example: false - publication: + status: + example: "published" + statusChangedDate: type: string format: date-time nullable: false - publisher: + statusChangedBy: description: | The ID of the user who published the resource (or `null`, if the SUDO token was used). type: string diff --git a/server/resources/resources.test.js b/server/resources/resources.test.js index 417a7e69..c8077098 100644 --- a/server/resources/resources.test.js +++ b/server/resources/resources.test.js @@ -23,7 +23,7 @@ describe("/api/resources", () => { expect(body).toMatchObject({ accession: expect.stringMatching(patterns.DATETIME), description: null, - draft: true, + status: "drafted", id: expect.stringMatching(patterns.UUID), source: id, title: resource.title, @@ -160,7 +160,7 @@ describe("/api/resources", () => { const { agent: adminAgent } = await authenticateAs("admin"); const { body: envelope } = await adminAgent .get("/api/resources") - .query({ draft: true }) + .query({ status: "drafted" }) .set("User-Agent", "supertest") .expect(200); expect(envelope).toEqual({ @@ -179,7 +179,7 @@ describe("/api/resources", () => { const { agent: adminAgent } = await authenticateAs("admin"); const { body: envelope } = await adminAgent .get("/api/resources") - .query({ draft: true, page: 2, perPage: 10 }) + .query({ status: "drafted", page: 2, perPage: 10 }) .set("User-Agent", "supertest") .expect(200); expect(envelope).toEqual({ @@ -198,7 +198,7 @@ describe("/api/resources", () => { const { agent } = await authenticateAs("admin"); await agent .get("/api/resources") - .query({ draft: true, page: 0, perPage: "foo" }) + .query({ status: "drafted", page: 0, perPage: "foo" }) .set("User-Agent", "supertest") .expect(400, { page: '"page" must be greater than or equal to 1', @@ -221,7 +221,7 @@ describe("/api/resources", () => { body: { resources }, } = await anonAgent .get("/api/resources") - .query({ draft: true }) + .query({ status: "drafted" }) .set("Authorization", `Bearer ${sudoToken}`) .set("User-Agent", "supertest") .expect(200); @@ -242,7 +242,7 @@ describe("/api/resources", () => { await anonAgent .get("/api/resources") - .query({ draft: true }) + .query({ status: "drafted" }) .set("User-Agent", "supertest") .expect(403, "Forbidden"); }); @@ -269,15 +269,15 @@ describe("/api/resources", () => { const { body: { - resources: [draft], + resources: [status], }, } = await anonAgent .get("/api/resources") - .query({ draft: true }) + .query({ status: "drafted" }) .set("Authorization", `Bearer ${sudoToken}`) .set("User-Agent", "supertest") .expect(200); - expect(draft).toHaveProperty("topic_name", topic.name); + expect(status).toHaveProperty("topic_name", topic.name); }); }); @@ -296,16 +296,16 @@ describe("/api/resources", () => { const { body: updated } = await anonAgent .patch(`/api/resources/${resource.id}`) - .send({ draft: false }) + .send({ status: "published" }) .set("Authorization", `Bearer ${sudoToken}`) .set("User-Agent", "supertest") .expect(200); expect(updated).toEqual({ ...resource, - draft: false, - publication: expect.stringMatching(patterns.DATETIME), - publisher: null, + status: "published", + status_changed_date: expect.stringMatching(patterns.DATETIME), + status_changed_by: null, }); const { @@ -328,11 +328,11 @@ describe("/api/resources", () => { await anonAgent .patch(`/api/resources/${resource.id}`) - .send({ draft: true, title: "Something else" }) + .send({ status: "drafted", title: "Something else" }) .set("Authorization", `Bearer ${sudoToken}`) .set("User-Agent", "supertest") .expect(400, { - draft: '"draft" must be [false]', + status: '"status" must be one of [rejected, published]', title: '"title" is not allowed', }); }); @@ -341,7 +341,7 @@ describe("/api/resources", () => { const { agent } = await authenticateAs("anonymous"); await agent .patch(`/api/resources/${randomUUID()}`) - .send({ draft: false }) + .send({ status: "published" }) .set("Authorization", `Bearer ${sudoToken}`) .set("User-Agent", "supertest") .expect(404); @@ -361,7 +361,7 @@ describe("/api/resources", () => { await anonAgent .patch(`/api/resources/${resource.id}`) - .send({ draft: false }) + .send({ status: "published" }) .set("User-Agent", "supertest") .expect(401); }); @@ -378,11 +378,14 @@ describe("/api/resources", () => { const { body: published } = await adminAgent .patch(`/api/resources/${created.id}`) - .send({ draft: false }) + .send({ status: "published" }) .set("User-Agent", "supertest") .expect(200); - expect(published).toMatchObject({ publisher: admin.id, source: user.id }); + expect(published).toMatchObject({ + status_changed_by: admin.id, + source: user.id, + }); }); }); }); diff --git a/server/resources/resourcesController.js b/server/resources/resourcesController.js index f69fdd18..aa53d68f 100644 --- a/server/resources/resourcesController.js +++ b/server/resources/resourcesController.js @@ -21,17 +21,17 @@ router .get( validated({ query: Joi.object({ - draft: Joi.boolean(), + status: Joi.string(), page: Joi.number().integer().min(1), perPage: Joi.number().integer().min(1), }).unknown(), }), asyncHandler(async (req, res) => { - const { draft, page, perPage } = req.query; - if (draft && !req.superuser && !req.user?.is_admin) { + const { status, page, perPage } = req.query; + if (status && !req.superuser && !req.user?.is_admin) { return res.sendStatus(403); } - res.send(await service.getAll({ draft }, { page, perPage })); + res.send(await service.getAll({ status }, { page, perPage })); }) ) .post( @@ -67,12 +67,15 @@ router sudoOnly, validated({ body: Joi.object({ - draft: Joi.any().valid(false), + status: Joi.any().valid("rejected", "published"), }), }), asyncHandler(async (req, res) => { try { - res.send(await service.publish(req.params.id, req.user?.id ?? null)); + const { status } = req.body; + res.send( + await service.action(req.params.id, status, req.user?.id ?? null) + ); } catch (err) { if (err instanceof service.MissingResource) { logger.info(err.message); @@ -82,24 +85,6 @@ router } }) ) - .delete( - sudoOnly, - validated({ - body: Joi.object({ - draft: Joi.any().valid(false), - }), - }), - asyncHandler(async (req, res) => { - try { - res.send(await service.reject(req.params.id)); - } catch (err) { - if (err instanceof service.MissingResource) { - logger.info(err.message); - return res.sendStatus(404); - } - } - }) - ) .all(methodNotAllowed); export default router; diff --git a/server/resources/resourcesRepository.js b/server/resources/resourcesRepository.js index 567f7032..2d835f11 100644 --- a/server/resources/resourcesRepository.js +++ b/server/resources/resourcesRepository.js @@ -1,10 +1,4 @@ -import db, { - ErrorCodes, - insertQuery, - singleLine, - updateQuery, - deleteQuery, -} from "../db"; +import db, { ErrorCodes, insertQuery, singleLine, updateQuery } from "../db"; import { DuplicateResource } from "./resourcesService"; @@ -17,7 +11,7 @@ const resourceQuery = singleLine` const pagedResourceQuery = singleLine` ${resourceQuery} - WHERE draft = $1 + WHERE status = $1 ORDER BY accession DESC LIMIT $2 OFFSET $3; @@ -46,17 +40,17 @@ export const add = async ({ description, source, title, topic, url }) => { } }; -export const count = async ({ draft }) => { +export const count = async ({ status }) => { const { rows: [{ count }], - } = await db.query("SELECT COUNT(*) FROM resources WHERE draft = $1;", [ - draft, + } = await db.query("SELECT COUNT(*) FROM resources WHERE status = $1;", [ + status, ]); return parseInt(count, 10); }; -export const findAll = async ({ draft, limit, offset }) => { - const { rows } = await db.query(pagedResourceQuery, [draft, limit, offset]); +export const findAll = async ({ status, limit, offset }) => { + const { rows } = await db.query(pagedResourceQuery, [status, limit, offset]); return rows; }; @@ -67,19 +61,19 @@ export const findOne = async (id) => { return resource; }; -export const update = async (id, { draft, publication, publisher }) => { +export const update = async ( + id, + { status, statusChangedDate, statusChangedBy } +) => { const { rows: [updated], } = await db.query( - updateQuery("resources", ["draft", "publication", "publisher"]), - [id, draft, publication, publisher] + updateQuery("resources", [ + "status", + "status_changed_date", + "status_changed_by", + ]), + [id, status, statusChangedDate, statusChangedBy] ); return updated; }; - -export const destroy = async (id) => { - const { - rows: [deleted], - } = await db.query(deleteQuery("resources"), [id]); - return deleted; -}; diff --git a/server/resources/resourcesService.js b/server/resources/resourcesService.js index b109ddbf..049ad598 100644 --- a/server/resources/resourcesService.js +++ b/server/resources/resourcesService.js @@ -17,13 +17,16 @@ export const create = async (resource) => { return await repository.add(resource); }; -export async function getAll({ draft = false }, { page = 1, perPage = 20 }) { +export async function getAll( + { status = "published" }, + { page = 1, perPage = 20 } +) { const resources = await repository.findAll({ - draft, + status, limit: perPage, offset: (page - 1) * perPage, }); - const totalCount = await repository.count({ draft }); + const totalCount = await repository.count({ status }); return { lastPage: Math.ceil(totalCount / perPage) || 1, page, @@ -33,20 +36,13 @@ export async function getAll({ draft = false }, { page = 1, perPage = 20 }) { }; } -export async function publish(resourceId, publisherId) { +export async function action(resourceId, status, publisherId) { if (!(await repository.findOne(resourceId))) { throw new MissingResource(resourceId); } return await repository.update(resourceId, { - draft: false, - publisher: publisherId, - publication: new Date(), + status, + statusChangedBy: publisherId, + statusChangedDate: new Date(), }); } - -export async function reject(resourceId) { - if (!(await repository.findOne(resourceId))) { - throw new MissingResource(resourceId); - } - return await repository.destroy(resourceId); -}