Skip to content

Commit 2b3e468

Browse files
committed
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
1 parent 94e2406 commit 2b3e468

File tree

5 files changed

+109
-31
lines changed

5 files changed

+109
-31
lines changed

addon/components/document-delete-button.js

+36-24
Original file line numberDiff line numberDiff line change
@@ -30,33 +30,45 @@ export default class DocumentDeleteButtonComponent extends Component {
3030
}
3131
}
3232

33-
@task *delete() {
34-
try {
35-
if (this.args.docsToDelete) {
36-
const docs = Array.isArray(this.args.docsToDelete)
37-
? this.args.docsToDelete
38-
: [this.args.docsToDelete]; // if the supplied argument is not an array we make it one
39-
40-
for (const doc of docs) {
41-
yield doc.destroyRecord().catch((error) => {
42-
doc.rollbackAttributes();
43-
throw error;
44-
});
45-
this.documents.deselectDocument(doc);
46-
}
47-
}
48-
49-
if (this.args.onConfirm) {
50-
this.args.onConfirm(this.args.docsToDelete);
51-
}
33+
delete = task(async () => {
34+
if (!this.args.docsToDelete) {
35+
return this.hideDialog();
36+
}
37+
38+
const docs = Array.isArray(this.args.docsToDelete)
39+
? this.args.docsToDelete
40+
: [this.args.docsToDelete]; // if the supplied argument is not an array we make it one
41+
42+
if (this.args.onConfirm) {
43+
this.args.onConfirm(docs);
44+
}
45+
46+
const deletionStatus = await Promise.allSettled(
47+
docs.map((doc) => {
48+
return doc.destroyRecord();
49+
}),
50+
);
51+
52+
const success = [];
53+
const rejected = [];
54+
deletionStatus.forEach((element) => {
55+
(element.status === "rejected" ? rejected : success).push(element);
56+
});
5257

58+
success.forEach((_, index) => {
59+
this.documents.deselectDocument(docs[index]);
60+
});
61+
62+
rejected.forEach((error, index) => {
63+
docs[index].rollbackAttributes();
64+
new ErrorHandler(this, error).notify("alexandria.errors.delete-document");
65+
});
66+
67+
if (!rejected.length) {
5368
this.notification.success(
5469
this.intl.t("alexandria.success.delete-document"),
5570
);
56-
} catch (error) {
57-
new ErrorHandler(this, error).notify("alexandria.errors.delete-document");
58-
} finally {
59-
this.hideDialog();
6071
}
61-
}
72+
this.hideDialog();
73+
});
6274
}

addon/services/alexandria-documents.js

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export default class AlexandriaDocumentsService extends Service {
3838
this.selectedDocuments.length === 0
3939
? undefined
4040
: this.selectedDocuments.map((d) => d.id);
41+
4142
this.router.transitionTo(this.router.currentRouteName, {
4243
queryParams: {
4344
document: docs,

tests/integration/components/document-card-test.js

+4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ module("Integration | Component | document-card", function (hooks) {
2424
});
2525

2626
test("delete file", async function (assert) {
27+
const transitionTo = fake();
28+
this.owner.lookup("service:router").transitionTo = transitionTo;
29+
2730
const destroy = fake();
2831
this.document = {
2932
id: 1,
@@ -36,6 +39,7 @@ module("Integration | Component | document-card", function (hooks) {
3639
await click("[data-test-delete]");
3740
await click("[data-test-delete-confirm]");
3841
assert.ok(destroy.calledOnce, "destroyRecord was called once");
42+
assert.ok(transitionTo.calledOnce, "transitionTo was called once");
3943
});
4044

4145
test("thumbnail", async function (assert) {

tests/integration/components/document-delete-button-test.js

+63-7
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,88 @@ import { render, click } from "@ember/test-helpers";
22
import { setupRenderingTest } from "dummy/tests/helpers";
33
import { hbs } from "ember-cli-htmlbars";
44
import { module, test } from "qunit";
5+
import { fake } from "sinon";
56

67
module("Integration | Component | document-delete-button", function (hooks) {
78
setupRenderingTest(hooks);
89

10+
hooks.beforeEach(function () {
11+
this.owner.lookup("service:router").transitionTo = fake();
12+
});
13+
914
test("delete document", async function (assert) {
1015
this.document = {
1116
id: 1,
1217
title: "Test",
13-
destroyRecord() {},
18+
destroyRecord() {
19+
assert.step("destroy test");
20+
},
1421
};
1522

1623
this.onCancel = () => assert.step("cancel");
1724
this.onConfirm = () => assert.step("confirm");
1825

1926
await render(hbs`
2027
<DocumentDeleteButton
21-
@document={{this.document}}
28+
@docsToDelete={{this.document}}
29+
@onConfirm={{this.onConfirm}}
30+
@onCancel={{this.onCancel}}
31+
as |showDialog|
32+
>
33+
<button
34+
{{on "click" showDialog}}
35+
data-test-delete
36+
type="button"
37+
>
38+
Delete
39+
</button>
40+
</DocumentDeleteButton>
41+
`);
42+
43+
await click("[data-test-delete]");
44+
await click("[data-test-delete-cancel]");
45+
46+
await click("[data-test-delete]");
47+
await click("[data-test-delete-confirm]");
48+
49+
assert.verifySteps(["cancel", "confirm", "destroy test"]);
50+
});
51+
52+
test("delete document multiple", async function (assert) {
53+
this.documents = [
54+
{
55+
id: 1,
56+
title: "Test",
57+
destroyRecord() {
58+
assert.step("destroy test");
59+
},
60+
},
61+
{
62+
id: 2,
63+
title: "Bar",
64+
destroyRecord() {
65+
assert.step("destroy bar");
66+
},
67+
},
68+
];
69+
70+
this.onCancel = () => assert.step("cancel");
71+
this.onConfirm = () => assert.step("confirm");
72+
73+
await render(hbs`
74+
<DocumentDeleteButton
75+
@docsToDelete={{this.documents}}
2276
@onConfirm={{this.onConfirm}}
2377
@onCancel={{this.onCancel}}
2478
as |showDialog|
2579
>
2680
<button
27-
{{on "click" showDialog}}
28-
data-test-delete
29-
type="button"
30-
>Delete</button>
81+
{{on "click" showDialog}}
82+
data-test-delete
83+
type="button"
84+
>
85+
Delete
86+
</button>
3187
</DocumentDeleteButton>
3288
`);
3389

@@ -37,6 +93,6 @@ module("Integration | Component | document-delete-button", function (hooks) {
3793
await click("[data-test-delete]");
3894
await click("[data-test-delete-confirm]");
3995

40-
assert.verifySteps(["cancel", "confirm"]);
96+
assert.verifySteps(["cancel", "confirm", "destroy test", "destroy bar"]);
4197
});
4298
});

tests/integration/components/single-document-details-test.js

+5
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,16 @@ module("Integration | Component | single-document-details", function (hooks) {
5151
});
5252

5353
test("delete document", async function (assert) {
54+
const transitionTo = fake();
55+
this.owner.lookup("service:router").transitionTo = transitionTo;
56+
5457
const destroy = fake();
5558
this.selectedDocument = {
5659
id: 1,
5760
title: "Test",
5861
destroyRecord: async () => destroy(),
5962
};
63+
6064
await render(
6165
hbs`<SingleDocumentDetails @document={{this.selectedDocument}}/>`,
6266
);
@@ -65,6 +69,7 @@ module("Integration | Component | single-document-details", function (hooks) {
6569
await click("[data-test-delete-confirm]");
6670

6771
assert.ok(destroy.calledOnce, "destroyRecord was called once");
72+
assert.ok(transitionTo.calledOnce, "transitionTo was called once");
6873
});
6974

7075
test("edit document title", async function (assert) {

0 commit comments

Comments
 (0)