From 25fa3dc02671ff3c7f2d9f28c331bc152c8e69a6 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 30 Jan 2024 19:41:12 -0500 Subject: [PATCH] Improve approving-your-own-doc UX (#469) * Improve approving-your-own-doc UX * Update document.hbs --- web/app/components/document/sidebar.hbs | 7 +- web/app/components/document/sidebar.ts | 25 +++++-- web/mirage/config.ts | 42 ++++++++++-- .../acceptance/authenticated/document-test.ts | 67 +++++++++++++++++++ 4 files changed, 130 insertions(+), 11 deletions(-) diff --git a/web/app/components/document/sidebar.hbs b/web/app/components/document/sidebar.hbs index 6fedc1c7b..d76f4eb56 100644 --- a/web/app/components/document/sidebar.hbs +++ b/web/app/components/document/sidebar.hbs @@ -121,7 +121,11 @@
- +
@@ -374,6 +378,7 @@
{ + approve = task(async (options?: { skipSuccessMessage: boolean }) => { try { await this.fetchSvc.fetch( `/api/${this.configSvc.config.api_version}/approvals/${this.docID}`, @@ -870,7 +870,9 @@ export default class DocumentSidebarComponent extends Component { + changeDocumentStatus = task(async (newStatus: string) => { try { + if ( + newStatus === "Approved" && + this.args.document.approvers?.includes(this.args.profile.email) && + !this.args.document.approvedBy?.includes(this.args.profile.email) + ) { + // If the owner is an approver, process their approval first. + await this.approve.perform({ skipSuccessMessage: true }); + } + await this.patchDocument.perform({ - status: status, + status: newStatus, }); - this.showFlashSuccess("Done!", `Document status changed to "${status}"`); + + this.showFlashSuccess( + "Done!", + `Document status changed to "${newStatus}"`, + ); } catch (error: unknown) { this.maybeShowFlashError( error as Error, diff --git a/web/mirage/config.ts b/web/mirage/config.ts index 5943aa5a4..1d88dcdf1 100644 --- a/web/mirage/config.ts +++ b/web/mirage/config.ts @@ -520,6 +520,28 @@ export default function (mirageConfig) { return new Response(404, {}, {}); }); + /** + * Used when approving a document. + * Adds the user's email to the `approvedBy` array. + */ + this.post("/approvals/:document_id", (schema, request) => { + const document = schema.document.findBy({ + objectID: request.params.document_id, + }); + + if (document) { + if (!document.attrs.approvedBy?.includes(TEST_USER_EMAIL)) { + const approvedBy = document.attrs.approvedBy || []; + document.update({ + approvedBy: [...approvedBy, TEST_USER_EMAIL], + }); + } + return new Response(200, {}, document.attrs); + } + + return new Response(404, {}, {}); + }); + /** * Used by the AuthenticatedUserService to add and remove subscriptions. */ @@ -616,11 +638,6 @@ export default function (mirageConfig) { * Used to confirm that an approver has access to a document. */ this.get("/people", (schema, request) => { - // This allows the test user to view docs they're an approver on. - if (request.queryParams.emails === TEST_USER_EMAIL) { - return new Response(200, {}, []); - } - if (request.queryParams.emails !== "") { const emails = request.queryParams.emails.split(","); @@ -870,6 +887,21 @@ export default function (mirageConfig) { } }); + /** + * Used by the sidebar to update a document, + * e.g., to change a its status. + */ + this.patch("/documents/:document_id", (schema, request) => { + let document = schema.document.findBy({ + objectID: request.params.document_id, + }); + if (document) { + let attrs = JSON.parse(request.requestBody); + document.update(attrs); + return new Response(200, {}, document.attrs); + } + }); + /************************************************************************* * * PUT requests diff --git a/web/tests/acceptance/authenticated/document-test.ts b/web/tests/acceptance/authenticated/document-test.ts index 0fc3481ba..98d76595c 100644 --- a/web/tests/acceptance/authenticated/document-test.ts +++ b/web/tests/acceptance/authenticated/document-test.ts @@ -77,6 +77,9 @@ const CONTINUE_TO_DOCUMENT_BUTTON_SELECTOR = const DOC_PUBLISHED_COPY_URL_BUTTON_SELECTOR = "[data-test-doc-published-copy-url-button]"; +const CHANGE_DOC_STATUS_BUTTON = "[data-test-change-doc-status-button]"; +const DOC_STATUS = "[data-test-doc-status]"; + const CUSTOM_STRING_FIELD_SELECTOR = "[data-test-custom-field-type='string']"; const CUSTOM_PEOPLE_FIELD_SELECTOR = "[data-test-custom-field-type='people']"; const EDITABLE_FIELD_SAVE_BUTTON_SELECTOR = @@ -772,6 +775,70 @@ module("Acceptance | authenticated/document", function (hooks) { assert.dom(CUSTOM_PEOPLE_FIELD_SELECTOR).hasText("None"); }); + test(`you can move a doc into the "approved" status`, async function (this: AuthenticatedDocumentRouteTestContext, assert) { + this.server.create("document", { + objectID: 1, + status: "In-Review", + }); + + await visit("/document/1"); + + assert.dom(DOC_STATUS).hasText("In review"); + + await click(CHANGE_DOC_STATUS_BUTTON); + + assert.dom(DOC_STATUS).hasText("Approved"); + + const doc = this.server.schema.document.first(); + + assert.equal(doc.attrs.status, "Approved"); + }); + + test("you can approve your own doc", async function (this: AuthenticatedDocumentRouteTestContext, assert) { + this.server.create("document", { + objectID: 1, + status: "In-Review", + approvers: [TEST_USER_EMAIL], + }); + + await visit("/document/1"); + + await click(CHANGE_DOC_STATUS_BUTTON); + + const doc = this.server.schema.document.first(); + + assert.true(doc.attrs.approvedBy?.includes(TEST_USER_EMAIL)); + + assert + .dom(`${APPROVERS_SELECTOR} li ${APPROVED_BADGE_SELECTOR}`) + .exists("the approver is badged with a check"); + + assert.equal(doc.attrs.status, "Approved"); + }); + + test("owners can move a doc they previously approved from in-review to approved", async function (this: AuthenticatedDocumentRouteTestContext, assert) { + this.server.create("document", { + objectID: 1, + status: "In-Review", + approvers: [TEST_USER_EMAIL], + owners: [TEST_USER_EMAIL], + approvedBy: [TEST_USER_EMAIL], + }); + + await visit("/document/1"); + + await click(CHANGE_DOC_STATUS_BUTTON); + + const doc = this.server.schema.document.first(); + + assert.equal(doc.attrs.status, "Approved"); + + assert + .dom(FLASH_MESSAGE_SELECTOR) + .exists({ count: 1 }) + .hasAttribute("data-test-flash-notification-type", "success"); + }); + test("approvers who have approved a document are badged with a checkmark", async function (this: AuthenticatedDocumentRouteTestContext, assert) { this.server.create("document", { objectID: 1,