From 49b1df40fc97e0c4354670d80b9deef7d2756b20 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 27 Nov 2017 19:50:04 +0100 Subject: [PATCH] FIX: various issues when editing category permissions This commit also adds multiple tests --- .../components/edit-category-security.hbs | 25 +++-- .../select-kit/components/multi-select.js.es6 | 2 + .../select-kit/components/select-kit.js.es6 | 6 ++ .../components/single-select.js.es6 | 10 +- .../category-edit-security-test.js.es6 | 96 +++++++++++++++++++ 5 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 test/javascripts/acceptance/category-edit-security-test.js.es6 diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-security.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-security.hbs index eb6626fdcf8..fb0a13c213d 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-security.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-security.hbs @@ -9,21 +9,30 @@ {{{i18n "category.can"}}} {{p.permission.description}} {{#if editingPermissions}} - {{d-icon "times-circle"}} + {{d-icon "times-circle"}} {{/if}} {{/each}} {{#if editingPermissions}} - {{combo-box allowInitialValueMutation=true content=category.availableGroups value=selectedGroup}} - {{combo-box class="permission-selector" - nameProperty="description" - content=category.availablePermissions - value=selectedPermission}} - + {{#if category.availableGroups}} + {{combo-box class="available-groups" + allowInitialValueMutation=true + allowsContentReplacement=true + content=category.availableGroups + value=selectedGroup}} + {{combo-box allowInitialValueMutation=true + class="permission-selector" + nameProperty="description" + content=category.availablePermissions + value=selectedPermission}} + + {{/if}} {{else}} {{#unless category.is_special}} - + {{/unless}} {{/if}} diff --git a/app/assets/javascripts/select-kit/components/multi-select.js.es6 b/app/assets/javascripts/select-kit/components/multi-select.js.es6 index 8abfea426a1..279656145a0 100644 --- a/app/assets/javascripts/select-kit/components/multi-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/multi-select.js.es6 @@ -80,6 +80,8 @@ export default SelectKitComponent.extend({ didComputeValues(values) { return values; }, mutateAttributes() { + if (this.get("isDestroyed") || this.get("isDestroying")) return; + Ember.run.next(() => { this.mutateContent(this.get("computedContent")); this.mutateValues(this.get("computedValues")); diff --git a/app/assets/javascripts/select-kit/components/select-kit.js.es6 b/app/assets/javascripts/select-kit/components/select-kit.js.es6 index 29fb333807a..8f76dc045a6 100644 --- a/app/assets/javascripts/select-kit/components/select-kit.js.es6 +++ b/app/assets/javascripts/select-kit/components/select-kit.js.es6 @@ -61,6 +61,7 @@ export default Ember.Component.extend(UtilsMixin, PluginApiMixin, DomHelpersMixi computedContent: null, limitMatches: 100, nameChanges: false, + allowsContentReplacement: false, init() { this._super(); @@ -79,10 +80,15 @@ export default Ember.Component.extend(UtilsMixin, PluginApiMixin, DomHelpersMixi if (this.get("nameChanges")) { this.addObserver(`content.@each.${this.get("nameProperty")}`, this, this._compute); } + + if (this.get("allowsContentReplacement")) { + this.addObserver(`content.[]`, this, this._compute); + } }, willDestroyElement() { this.removeObserver(`content.@each.${this.get("nameProperty")}`, this, this._compute); + this.removeObserver(`content.[]`, this, this._compute); }, willComputeAttributes() {}, diff --git a/app/assets/javascripts/select-kit/components/single-select.js.es6 b/app/assets/javascripts/select-kit/components/single-select.js.es6 index bf9df8207c1..5f539a52688 100644 --- a/app/assets/javascripts/select-kit/components/single-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/single-select.js.es6 @@ -15,7 +15,7 @@ export default SelectKitComponent.extend({ @on("didReceiveAttrs") _compute() { - Ember.run.scheduleOnce("afterRender", () => { + run.scheduleOnce("afterRender", () => { this.willComputeAttributes(); let content = this.willComputeContent(this.get("content") || []); let value = this._beforeWillComputeValue(this.get("value")); @@ -34,6 +34,8 @@ export default SelectKitComponent.extend({ }, mutateAttributes() { + if (this.get("isDestroyed") || this.get("isDestroying")) return; + run.next(() => { this.mutateContent(this.get("computedContent")); this.mutateValue(this.get("computedValue")); @@ -108,7 +110,7 @@ export default SelectKitComponent.extend({ }, autoHighlight() { - Ember.run.schedule("afterRender", () => { + run.schedule("afterRender", () => { if (!isNone(this.get("highlightedValue"))) { return; } const filteredComputedContent = this.get("filteredComputedContent"); @@ -157,14 +159,14 @@ export default SelectKitComponent.extend({ this.willSelect(rowComputedContentItem); this.set("computedValue", rowComputedContentItem.value); this.mutateAttributes(); - Ember.run.schedule("afterRender", () => this.didSelect(rowComputedContentItem)); + run.schedule("afterRender", () => this.didSelect(rowComputedContentItem)); }, onDeselect(rowComputedContentItem) { this.willDeselect(rowComputedContentItem); this.set("computedValue", null); this.mutateAttributes(); - Ember.run.schedule("afterRender", () => this.didDeselect(rowComputedContentItem)); + run.schedule("afterRender", () => this.didDeselect(rowComputedContentItem)); } } }); diff --git a/test/javascripts/acceptance/category-edit-security-test.js.es6 b/test/javascripts/acceptance/category-edit-security-test.js.es6 new file mode 100644 index 00000000000..9d0b6c71295 --- /dev/null +++ b/test/javascripts/acceptance/category-edit-security-test.js.es6 @@ -0,0 +1,96 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("Category Edit - security", { + loggedIn: true +}); + +QUnit.test("default", assert => { + visit("/c/bug"); + + click('.edit-category'); + click('li.edit-category-security a'); + + andThen(() => { + const $permissionListItems = find('.permission-list li'); + + const badgeName = $permissionListItems.eq(0).find('.badge-group').text(); + assert.equal(badgeName, 'everyone'); + + const permission = $permissionListItems.eq(0).find('.permission').text(); + assert.equal(permission, 'Create / Reply / See'); + }); +}); + +QUnit.test("removing a permission", assert => { + visit("/c/bug"); + + click('.edit-category'); + click('li.edit-category-security a'); + click('.edit-category-tab-security .edit-permission'); + expandSelectKit(".available-groups"); + + andThen(() => { + assert.notOk(exists(".available-groups .select-kit-row[data-value='everyone']"), 'everyone is already used and is not in the available groups'); + }); + + click('.edit-category-tab-security .permission-list li:first-of-type .remove-permission'); + expandSelectKit(".available-groups"); + + andThen(() => { + assert.ok(exists(".available-groups .select-kit-row[data-value='everyone']"), 'everyone has been removed and appears in the available groups'); + }); +}); + +QUnit.test("adding a permission", assert => { + visit("/c/bug"); + + click('.edit-category'); + click('li.edit-category-security a'); + click('.edit-category-tab-security .edit-permission'); + expandSelectKit('.available-groups'); + selectKitSelectRow('staff', { selector: '.available-groups' }); + expandSelectKit('.permission-selector'); + selectKitSelectRow('2', { selector: '.permission-selector' }); + click('.edit-category-tab-security .add-permission'); + + andThen(() => { + const $addedPermissionItem = find('.edit-category-tab-security .permission-list li:nth-child(2)'); + + const badgeName = $addedPermissionItem.find('.badge-group').text(); + assert.equal(badgeName, 'staff'); + + const permission = $addedPermissionItem.find('.permission').text(); + assert.equal(permission, 'Reply / See'); + }); +}); + +QUnit.test("adding a previously removed permission", assert => { + visit("/c/bug"); + + click('.edit-category'); + click('li.edit-category-security a'); + click('.edit-category-tab-security .edit-permission'); + expandSelectKit(".available-groups"); + + click('.edit-category-tab-security .permission-list li:first-of-type .remove-permission'); + + andThen(() => { + assert.equal(find(".edit-category-tab-security .permission-list li").length, 0, 'it removes the permission from the list'); + }); + + expandSelectKit('.available-groups'); + selectKitSelectRow('everyone', { selector: '.available-groups' }); + click('.edit-category-tab-security .add-permission'); + + andThen(() => { + assert.equal(find(".edit-category-tab-security .permission-list li").length, 1, 'it adds the permission to the list'); + + const $permissionListItems = find('.permission-list li'); + + const badgeName = $permissionListItems.eq(0).find('.badge-group').text(); + assert.equal(badgeName, 'everyone'); + + const permission = $permissionListItems.eq(0).find('.permission').text(); + assert.equal(permission, 'Create / Reply / See'); + }); +});