From 2b3e468c3dad621f867af40ba31a228547c7a7e8 Mon Sep 17 00:00:00 2001 From: yelinz Date: Wed, 10 Apr 2024 11:08:35 +0200 Subject: [PATCH] fix(delete): delete concurrently and clean up correctly fixes bug when deleting multiple the last one keeps the side panel open, eventhough it does not exist anymore --- addon/components/document-delete-button.js | 60 +++++++++------- addon/services/alexandria-documents.js | 1 + .../components/document-card-test.js | 4 ++ .../components/document-delete-button-test.js | 70 +++++++++++++++++-- .../single-document-details-test.js | 5 ++ 5 files changed, 109 insertions(+), 31 deletions(-) diff --git a/addon/components/document-delete-button.js b/addon/components/document-delete-button.js index 679c3f2d..460ff8fd 100644 --- a/addon/components/document-delete-button.js +++ b/addon/components/document-delete-button.js @@ -30,33 +30,45 @@ export default class DocumentDeleteButtonComponent extends Component { } } - @task *delete() { - try { - if (this.args.docsToDelete) { - const docs = Array.isArray(this.args.docsToDelete) - ? this.args.docsToDelete - : [this.args.docsToDelete]; // if the supplied argument is not an array we make it one - - for (const doc of docs) { - yield doc.destroyRecord().catch((error) => { - doc.rollbackAttributes(); - throw error; - }); - this.documents.deselectDocument(doc); - } - } - - if (this.args.onConfirm) { - this.args.onConfirm(this.args.docsToDelete); - } + delete = task(async () => { + if (!this.args.docsToDelete) { + return this.hideDialog(); + } + + const docs = Array.isArray(this.args.docsToDelete) + ? this.args.docsToDelete + : [this.args.docsToDelete]; // if the supplied argument is not an array we make it one + + if (this.args.onConfirm) { + this.args.onConfirm(docs); + } + + const deletionStatus = await Promise.allSettled( + docs.map((doc) => { + return doc.destroyRecord(); + }), + ); + + const success = []; + const rejected = []; + deletionStatus.forEach((element) => { + (element.status === "rejected" ? rejected : success).push(element); + }); + success.forEach((_, index) => { + this.documents.deselectDocument(docs[index]); + }); + + rejected.forEach((error, index) => { + docs[index].rollbackAttributes(); + new ErrorHandler(this, error).notify("alexandria.errors.delete-document"); + }); + + if (!rejected.length) { this.notification.success( this.intl.t("alexandria.success.delete-document"), ); - } catch (error) { - new ErrorHandler(this, error).notify("alexandria.errors.delete-document"); - } finally { - this.hideDialog(); } - } + this.hideDialog(); + }); } diff --git a/addon/services/alexandria-documents.js b/addon/services/alexandria-documents.js index 30d502c1..20177aac 100644 --- a/addon/services/alexandria-documents.js +++ b/addon/services/alexandria-documents.js @@ -38,6 +38,7 @@ export default class AlexandriaDocumentsService extends Service { this.selectedDocuments.length === 0 ? undefined : this.selectedDocuments.map((d) => d.id); + this.router.transitionTo(this.router.currentRouteName, { queryParams: { document: docs, diff --git a/tests/integration/components/document-card-test.js b/tests/integration/components/document-card-test.js index c33173ab..8d0b4ab0 100644 --- a/tests/integration/components/document-card-test.js +++ b/tests/integration/components/document-card-test.js @@ -24,6 +24,9 @@ module("Integration | Component | document-card", function (hooks) { }); test("delete file", async function (assert) { + const transitionTo = fake(); + this.owner.lookup("service:router").transitionTo = transitionTo; + const destroy = fake(); this.document = { id: 1, @@ -36,6 +39,7 @@ module("Integration | Component | document-card", function (hooks) { await click("[data-test-delete]"); await click("[data-test-delete-confirm]"); assert.ok(destroy.calledOnce, "destroyRecord was called once"); + assert.ok(transitionTo.calledOnce, "transitionTo was called once"); }); test("thumbnail", async function (assert) { diff --git a/tests/integration/components/document-delete-button-test.js b/tests/integration/components/document-delete-button-test.js index 971975bc..319b288d 100644 --- a/tests/integration/components/document-delete-button-test.js +++ b/tests/integration/components/document-delete-button-test.js @@ -2,15 +2,22 @@ import { render, click } from "@ember/test-helpers"; import { setupRenderingTest } from "dummy/tests/helpers"; import { hbs } from "ember-cli-htmlbars"; import { module, test } from "qunit"; +import { fake } from "sinon"; module("Integration | Component | document-delete-button", function (hooks) { setupRenderingTest(hooks); + hooks.beforeEach(function () { + this.owner.lookup("service:router").transitionTo = fake(); + }); + test("delete document", async function (assert) { this.document = { id: 1, title: "Test", - destroyRecord() {}, + destroyRecord() { + assert.step("destroy test"); + }, }; this.onCancel = () => assert.step("cancel"); @@ -18,16 +25,65 @@ module("Integration | Component | document-delete-button", function (hooks) { await render(hbs` + + + `); + + await click("[data-test-delete]"); + await click("[data-test-delete-cancel]"); + + await click("[data-test-delete]"); + await click("[data-test-delete-confirm]"); + + assert.verifySteps(["cancel", "confirm", "destroy test"]); + }); + + test("delete document multiple", async function (assert) { + this.documents = [ + { + id: 1, + title: "Test", + destroyRecord() { + assert.step("destroy test"); + }, + }, + { + id: 2, + title: "Bar", + destroyRecord() { + assert.step("destroy bar"); + }, + }, + ]; + + this.onCancel = () => assert.step("cancel"); + this.onConfirm = () => assert.step("confirm"); + + await render(hbs` + + {{on "click" showDialog}} + data-test-delete + type="button" + > + Delete + `); @@ -37,6 +93,6 @@ module("Integration | Component | document-delete-button", function (hooks) { await click("[data-test-delete]"); await click("[data-test-delete-confirm]"); - assert.verifySteps(["cancel", "confirm"]); + assert.verifySteps(["cancel", "confirm", "destroy test", "destroy bar"]); }); }); diff --git a/tests/integration/components/single-document-details-test.js b/tests/integration/components/single-document-details-test.js index 7cd1abf2..4565dad3 100644 --- a/tests/integration/components/single-document-details-test.js +++ b/tests/integration/components/single-document-details-test.js @@ -51,12 +51,16 @@ module("Integration | Component | single-document-details", function (hooks) { }); test("delete document", async function (assert) { + const transitionTo = fake(); + this.owner.lookup("service:router").transitionTo = transitionTo; + const destroy = fake(); this.selectedDocument = { id: 1, title: "Test", destroyRecord: async () => destroy(), }; + await render( hbs``, ); @@ -65,6 +69,7 @@ module("Integration | Component | single-document-details", function (hooks) { await click("[data-test-delete-confirm]"); assert.ok(destroy.calledOnce, "destroyRecord was called once"); + assert.ok(transitionTo.calledOnce, "transitionTo was called once"); }); test("edit document title", async function (assert) {