FIX: various issues when editing category permissions

This commit also adds multiple tests

app/assets/javascripts/discourse/templates/components/edit-category-security.hbs

@@ -9,21 +9,30 @@
         {{{i18n "category.can"}}}
         <span class="permission">{{p.permission.description}}</span>
         {{#if editingPermissions}}
-        <a href {{action "removePermission" p}}>{{d-icon "times-circle"}}</a>
+        <a class="remove-permission" href {{action "removePermission" p}}>{{d-icon "times-circle"}}</a>
         {{/if}}
       </li>
     {{/each}}
   </ul>
   {{#if editingPermissions}}
-    {{combo-box allowInitialValueMutation=true content=category.availableGroups value=selectedGroup}}
+    {{#if category.availableGroups}}
-    {{combo-box class="permission-selector"
+      {{combo-box class="available-groups"
-                nameProperty="description"
+                  allowInitialValueMutation=true
-                content=category.availablePermissions
+                  allowsContentReplacement=true
-                value=selectedPermission}}
+                  content=category.availableGroups
-    <button {{action "addPermission" selectedGroup selectedPermission}} class="btn btn-small">{{i18n 'category.add_permission'}}</button>
+                  value=selectedGroup}}
+      {{combo-box allowInitialValueMutation=true
+                  class="permission-selector"
+                  nameProperty="description"
+                  content=category.availablePermissions
+                  value=selectedPermission}}
+      <button {{action "addPermission" selectedGroup selectedPermission}} class="btn btn-small add-permission">
+        {{i18n 'category.add_permission'}}
+      </button>
+    {{/if}}
   {{else}}
     {{#unless category.is_special}}
-      <button {{action "editPermissions"}} class="btn btn-small">{{i18n 'category.edit_permissions'}}</button>
+      <button {{action "editPermissions"}} class="btn btn-small edit-permission">{{i18n 'category.edit_permissions'}}</button>
     {{/unless}}
   {{/if}}
 </section>

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"));

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() {},

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));
     }
   }
 });

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');
+  });
+});