From 54ad8ebd6d449e0438feb00939017a38e82418a0 Mon Sep 17 00:00:00 2001 From: Jonas Metzener Date: Mon, 11 Dec 2023 18:04:46 +0100 Subject: [PATCH] fix(errors): rollback changes after backend returns an error --- addon/components/document-delete-button.js | 9 ++++++--- addon/components/single-document-details.js | 1 + addon/services/tags.js | 12 ++++++++---- .../integration/components/document-card-test.js | 8 +++----- .../components/single-document-details-test.js | 15 +++++++-------- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/addon/components/document-delete-button.js b/addon/components/document-delete-button.js index f75ac617..9f19af9e 100644 --- a/addon/components/document-delete-button.js +++ b/addon/components/document-delete-button.js @@ -38,7 +38,10 @@ export default class DocumentDeleteButtonComponent extends Component { : [this.args.docsToDelete]; // if the supplied argument is not an array we make it one for (const doc of docs) { - yield doc.destroyRecord(); + yield doc.destroyRecord().catch((error) => { + doc.rollbackAttributes(); + throw error; + }); this.documents.deselectDocument(doc); } } @@ -47,13 +50,13 @@ export default class DocumentDeleteButtonComponent extends Component { this.args.onConfirm(this.args.docsToDelete); } - this.hideDialog(); - this.notification.success( this.intl.t("alexandria.success.delete-document"), ); } catch (error) { new ErrorHandler(this, error).notify("alexandria.errors.delete-document"); + } finally { + this.hideDialog(); } } } diff --git a/addon/components/single-document-details.js b/addon/components/single-document-details.js index b2e89af3..512a794f 100644 --- a/addon/components/single-document-details.js +++ b/addon/components/single-document-details.js @@ -71,6 +71,7 @@ export default class SingleDocumentDetailsComponent extends DocumentCard { this.resetState(); this.notification.success(this.intl.t("alexandria.success.update")); } catch (error) { + this.args.document.rollbackAttributes(); new ErrorHandler(this, error).notify("alexandria.errors.update"); } } diff --git a/addon/services/tags.js b/addon/services/tags.js index bac42c12..4a928eda 100644 --- a/addon/services/tags.js +++ b/addon/services/tags.js @@ -34,6 +34,8 @@ export default class TagsService extends Service { */ @action async add(document, tagInput) { let tag = tagInput; + const tags = [...(await document.tags)]; + try { if (typeof tagInput === "string") { const tagId = dasherize(tagInput.trim()); @@ -56,15 +58,14 @@ export default class TagsService extends Service { } } - const tags = await document.tags; - if (tags.find((t) => t.id === tag.id)) { return tag; } - tags.push(tag); + document.tags = [...tags, tag]; await document.save(); } catch (error) { + document.tags = tags; new ErrorHandler(this, error).notify("alexandria.errors.update"); } @@ -84,11 +85,14 @@ export default class TagsService extends Service { tag = this.store.peekRecord("tag", tag); } - document.tags = (await document.tags).filter((t) => t !== tag); + const tags = [...(await document.tags)]; + + document.tags = tags.filter((t) => t !== tag); try { await document.save(); } catch (error) { + document.tags = tags; new ErrorHandler(this, error).notify("alexandria.errors.update"); } diff --git a/tests/integration/components/document-card-test.js b/tests/integration/components/document-card-test.js index 8b1620c1..c789493d 100644 --- a/tests/integration/components/document-card-test.js +++ b/tests/integration/components/document-card-test.js @@ -62,20 +62,18 @@ module("Integration | Component | document-card", function (hooks) { }); test("delete file", async function (assert) { + const destroy = fake(); this.document = { id: 1, marks: [], - destroyRecord: fake(), + destroyRecord: async () => destroy(), }; await render(hbs``); await click("[data-test-context-menu-trigger]"); await click("[data-test-delete]"); await click("[data-test-delete-confirm]"); - assert.ok( - this.document.destroyRecord.calledOnce, - "destroyRecord was called once", - ); + assert.ok(destroy.calledOnce, "destroyRecord was called once"); }); test("thumbnail", async function (assert) { diff --git a/tests/integration/components/single-document-details-test.js b/tests/integration/components/single-document-details-test.js index d9066df6..236725a0 100644 --- a/tests/integration/components/single-document-details-test.js +++ b/tests/integration/components/single-document-details-test.js @@ -1,3 +1,4 @@ +/* eslint-disable import/no-named-as-default-member */ import Service from "@ember/service"; import { render, click, fillIn, waitFor } from "@ember/test-helpers"; import { tracked } from "@glimmer/tracking"; @@ -6,7 +7,7 @@ import { hbs } from "ember-cli-htmlbars"; import { setupMirage } from "ember-cli-mirage/test-support"; import { setFlatpickrDate } from "ember-flatpickr/test-support/helpers"; import { module, test } from "qunit"; -import { fake } from "sinon"; +import sinon from "sinon"; const mockDocumentsService = class DocumentsService extends Service { deselectDocument() { @@ -63,10 +64,11 @@ module("Integration | Component | single-document-details", function (hooks) { }); test("delete document", async function (assert) { + const destroy = sinon.fake(); this.selectedDocument = { id: 1, title: "Test", - destroyRecord: fake(), + destroyRecord: async () => destroy(), }; await render( hbs``, @@ -75,10 +77,7 @@ module("Integration | Component | single-document-details", function (hooks) { await click("[data-test-delete]"); await click("[data-test-delete-confirm]"); - assert.ok( - this.selectedDocument.destroyRecord.calledOnce, - "destroyRecord was called once", - ); + assert.ok(destroy.calledOnce, "destroyRecord was called once"); }); test("edit document title", async function (assert) { @@ -86,7 +85,7 @@ module("Integration | Component | single-document-details", function (hooks) { // Error: Assertion Failed: You attempted to update [object Object].title to "edited", but it is being tracked by a tracking context, such as a template, computed property, or observer. In order to make sure the context updates properly, you must invalidate the property when updating it. You can mark the property as `@tracked`, or use `@ember/object#set` to do this. class Document { @tracked title = "unedited"; - save = fake(); + save = sinon.fake(); } this.selectedDocument = new Document(); await render( @@ -121,7 +120,7 @@ module("Integration | Component | single-document-details", function (hooks) { test("edit document date", async function (assert) { class Document { @tracked date = "2023-01-01"; - save = fake(); + save = sinon.fake(); } this.selectedDocument = new Document();