UX: Revamp category security tab (#11273)

2020-11-20 10:44:34 -05:00
parent dbcf722ab9
commit 7539c2ed7f
15 changed files with 454 additions and 199 deletions
@@ -0,0 +1,123 @@
+import I18n from "I18n";
+import Component from "@ember/component";
+import discourseComputed, { observes } from "discourse-common/utils/decorators";
+import PermissionType from "discourse/models/permission-type";
+import { equal, alias } from "@ember/object/computed";
+
+const EVERYONE = "everyone";
+
+export default Component.extend({
+  classNames: ["permission-row", "row-body"],
+  canCreate: equal("type", PermissionType.FULL),
+  everyonePermissionType: alias("everyonePermission.permission_type"),
+
+  @discourseComputed("type")
+  canReply(value) {
+    return (
+      value === PermissionType.CREATE_POST || value === PermissionType.FULL
+    );
+  },
+
+  @discourseComputed("type")
+  canReplyIcon() {
+    return this.canReply ? "check-square" : "far-square";
+  },
+
+  @discourseComputed("type")
+  canCreateIcon() {
+    return this.canCreate ? "check-square" : "far-square";
+  },
+
+  @discourseComputed("type")
+  replyGranted() {
+    return this.type <= PermissionType.CREATE_POST ? "reply-granted" : "";
+  },
+
+  @discourseComputed("type")
+  createGranted() {
+    return this.type === PermissionType.FULL ? "create-granted" : "";
+  },
+
+  @observes("everyonePermissionType")
+  inheritFromEveryone() {
+    if (this.group_name === EVERYONE) {
+      return;
+    }
+
+    // groups cannot have a lesser permission than "everyone"
+    if (this.everyonePermissionType < this.type) {
+      this.updatePermission(this.everyonePermissionType);
+    }
+  },
+
+  @discourseComputed("everyonePermissionType", "type")
+  replyDisabled(everyonePermissionType) {
+    if (
+      this.group_name !== EVERYONE &&
+      everyonePermissionType &&
+      everyonePermissionType <= PermissionType.CREATE_POST
+    ) {
+      return true;
+    }
+    return false;
+  },
+
+  @discourseComputed("replyDisabled")
+  replyTooltip() {
+    return this.replyDisabled
+      ? I18n.t("category.permissions.inherited")
+      : I18n.t("category.permissions.toggle_reply");
+  },
+
+  @discourseComputed("everyonePermissionType", "type")
+  createDisabled(everyonePermissionType) {
+    if (
+      this.group_name !== EVERYONE &&
+      everyonePermissionType &&
+      everyonePermissionType === PermissionType.FULL
+    ) {
+      return true;
+    }
+    return false;
+  },
+
+  @discourseComputed("createDisabled")
+  createTooltip() {
+    return this.createDisabled
+      ? I18n.t("category.permissions.inherited")
+      : I18n.t("category.permissions.toggle_full");
+  },
+
+  updatePermission(type) {
+    this.category.updatePermission(this.group_name, type);
+  },
+
+  actions: {
+    removeRow() {
+      this.category.removePermission(this.group_name);
+    },
+
+    setPermissionReply() {
+      if (this.type <= PermissionType.CREATE_POST) {
+        this.updatePermission(PermissionType.READONLY);
+      } else {
+        this.updatePermission(PermissionType.CREATE_POST);
+      }
+    },
+
+    setPermissionFull() {
+      if (
+        this.group_name !== EVERYONE &&
+        this.everyonePermissionType === PermissionType.FULL
+      ) {
+        return;
+      }
+
+      if (this.type === PermissionType.FULL) {
+        this.updatePermission(PermissionType.CREATE_POST);
+      } else {
+        this.updatePermission(PermissionType.FULL);
+      }
+    },
+  },
+});
@@ -1,78 +1,39 @@
 import { buildCategoryPanel } from "discourse/components/edit-category-panel";
 import PermissionType from "discourse/models/permission-type";
-import { on } from "discourse-common/utils/decorators";
+import discourseComputed from "discourse-common/utils/decorators";
+
+import { not } from "@ember/object/computed";

 export default buildCategoryPanel("security", {
-  editingPermissions: false,
  selectedGroup: null,
-  selectedPermission: null,
-  showPendingGroupChangesAlert: false,
-  interactedWithDropdowns: false,
+  noGroupSelected: not("selectedGroup"),

-  @on("init")
-  _setup() {
-    this.setProperties({
-      selectedGroup: this.get("category.availableGroups.firstObject"),
-      selectedPermission: this.get(
-        "category.availablePermissions.firstObject.id"
-      ),
-    });
+  @discourseComputed("category.permissions.@each.permission_type")
+  everyonePermission(permissions) {
+    return permissions.findBy("group_name", "everyone");
  },

-  @on("init")
-  _registerValidator() {
-    this.registerValidator(() => {
-      if (
-        !this.showPendingGroupChangesAlert &&
-        this.interactedWithDropdowns &&
-        this.activeTab
-      ) {
-        this.set("showPendingGroupChangesAlert", true);
-        return true;
-      }
-    });
+  @discourseComputed("category.permissions.@each.permission_type")
+  everyoneGrantedFull() {
+    return (
+      this.everyonePermission &&
+      this.everyonePermission.permission_type === PermissionType.FULL
+    );
+  },
+
+  @discourseComputed("everyonePermission")
+  minimumPermission(everyonePermission) {
+    return everyonePermission
+      ? everyonePermission.permission_type
+      : PermissionType.READONLY;
  },

  actions: {
-    onSelectGroup(selectedGroup) {
-      this.setProperties({
-        interactedWithDropdowns: true,
-        selectedGroup,
+    onSelectGroup(group_name) {
+      this.category.addPermission({
+        group_name,
+        permission_type: this.minimumPermission,
      });
    },
-
-    onSelectPermission(selectedPermission) {
-      this.setProperties({
-        interactedWithDropdowns: true,
-        selectedPermission,
-      });
-    },
-
-    editPermissions() {
-      if (!this.get("category.is_special")) {
-        this.set("editingPermissions", true);
-      }
-    },
-
-    addPermission(group, id) {
-      if (!this.get("category.is_special")) {
-        this.category.addPermission({
-          group_name: group + "",
-          permission: PermissionType.create({ id: parseInt(id, 10) }),
-        });
-      }
-
-      this.setProperties({
-        selectedGroup: this.get("category.availableGroups.firstObject"),
-        showPendingGroupChangesAlert: false,
-        interactedWithDropdowns: false,
-      });
-    },
-
-    removePermission(permission) {
-      if (!this.get("category.is_special")) {
-        this.category.removePermission(permission);
-      }
-    },
  },
 });
@@ -6,6 +6,7 @@ import { propertyEqual } from "discourse/lib/computed";
 import getURL from "discourse-common/lib/get-url";
 import { empty } from "@ember/object/computed";
 import DiscourseURL from "discourse/lib/url";
+import { underscore } from "@ember/string";

 export default Component.extend({
  tagName: "li",
@@ -21,7 +22,7 @@ export default Component.extend({

  @discourseComputed("tab")
  title(tab) {
-    return I18n.t("category." + tab.replace("-", "_"));
+    return I18n.t(`category.${underscore(tab)}`);
  },

  didInsertElement() {
@@ -7,6 +7,7 @@ import DiscourseURL from "discourse/lib/url";
 import { readOnly } from "@ember/object/computed";
 import PermissionType from "discourse/models/permission-type";
 import { NotificationLevels } from "discourse/lib/notification-levels";
+import { underscore } from "@ember/string";

 export default Controller.extend({
  selectedTab: "general",
@@ -69,6 +70,11 @@ export default Controller.extend({
      : I18n.t("category.create");
  },

+  @discourseComputed("selectedTab")
+  selectedTabTitle(tab) {
+    return I18n.t(`category.${underscore(tab)}`);
+  },
+
  actions: {
    registerValidator(validator) {
      this.validators.push(validator);
@@ -10,6 +10,8 @@ import Site from "discourse/models/site";
 import User from "discourse/models/user";
 import { getOwner } from "discourse-common/lib/get-owner";

+const STAFF_GROUP_NAME = "staff";
+
 const Category = RestModel.extend({
  permissions: null,

@@ -22,15 +24,13 @@ const Category = RestModel.extend({
    this.set("availableGroups", availableGroups);

    const groupPermissions = this.group_permissions;
+
    if (groupPermissions) {
      this.set(
        "permissions",
        groupPermissions.map((elem) => {
          availableGroups.removeObject(elem.group_name);
-          return {
-            group_name: elem.group_name,
-            permission: PermissionType.create({ id: elem.permission_type }),
-          };
+          return elem;
        })
      );
    }
@@ -231,7 +231,12 @@ const Category = RestModel.extend({
  _permissionsForUpdate() {
    const permissions = this.permissions;
    let rval = {};
-    permissions.forEach((p) => (rval[p.group_name] = p.permission.id));
+    if (permissions.length) {
+      permissions.forEach((p) => (rval[p.group_name] = p.permission_type));
+    } else {
+      // empty permissions => staff-only access
+      rval[STAFF_GROUP_NAME] = PermissionType.FULL;
+    }
    return rval;
  },

@@ -246,9 +251,20 @@ const Category = RestModel.extend({
    this.availableGroups.removeObject(permission.group_name);
  },

-  removePermission(permission) {
-    this.permissions.removeObject(permission);
-    this.availableGroups.addObject(permission.group_name);
+  removePermission(group_name) {
+    const permission = this.permissions.findBy("group_name", group_name);
+    if (permission) {
+      this.permissions.removeObject(permission);
+      this.availableGroups.addObject(group_name);
+    }
+  },
+
+  updatePermission(group_name, type) {
+    this.permissions.forEach((p, i) => {
+      if (p.group_name === group_name) {
+        this.set(`permissions.${i}.permission_type`, type);
+      }
+    });
  },

  @discourseComputed("topics")
@@ -0,0 +1,29 @@
+<span class="group-name">
+  <span class="group-name-label">{{group_name}}</span>
+  <a class="remove-permission" href {{action "removeRow"}}>
+    {{d-icon "far-trash-alt"}}
+  </a>
+</span>
+<span class="options actionable">
+  {{d-button
+    icon="check-square"
+    class="btn btn-flat see"
+    disabled=true
+  }}
+
+  {{d-button
+    icon=canReplyIcon
+    action=(action "setPermissionReply")
+    translatedTitle=replyTooltip
+    class=(concat "btn btn-flat reply-toggle " replyGranted)
+    disabled=replyDisabled
+  }}
+
+  {{d-button
+    icon=canCreateIcon
+    action=(action "setPermissionFull")
+    translatedTitle=createTooltip
+    class=(concat "btn btn-flat create-toggle " createGranted)
+    disabled=createDisabled
+  }}
+</span>
@@ -12,14 +12,16 @@
    <section class="field">
      <label>{{i18n "category.parent"}}</label>
      {{category-chooser
-        rootNone=true
        value=category.parent_category_id
-        excludeCategoryId=category.id
        categories=parentCategories
        allowSubCategories=true
        allowUncategorized=false
        allowRestrictedCategories=true
        onChange=(action (mut category.parent_category_id))
+        options=(hash
+          excludeCategoryId=category.id
+          none=true
+        )
      }}
    </section>
  {{/if}}
@@ -6,62 +6,50 @@
      <p class="warning">{{i18n "category.special_warning"}}</p>
    {{/if}}
  {{/if}}
-  {{#unless category.isUncategorizedCategory}}
-    <ul class="permission-list">
+
+  {{#unless category.is_special}}
+    <div class="category-permissions-table">
+      <div class="permission-row row-header">
+        <span class="group-name">{{i18n "category.permissions.group"}}</span>
+        <span class="options">
+          <span class="cell">{{i18n "category.permissions.see"}}</span>
+          <span class="cell">{{i18n "category.permissions.reply"}}</span>
+          <span class="cell">{{i18n "category.permissions.create"}}</span>
+        </span>
+      </div>
      {{#each category.permissions as |p|}}
-        <li>
-          <span class="name"><span class="badge-group">{{p.group_name}}</span></span>
-          {{html-safe (i18n "category.can")}}
-          <span class="permission">{{p.permission.description}}</span>
-          {{#if editingPermissions}}
-            <a class="remove-permission" href {{action "removePermission" p}}>{{d-icon "times-circle"}}</a>
-          {{/if}}
-        </li>
+        {{category-permission-row group_name=p.group_name type=p.permission_type category=category everyonePermission=everyonePermission}}
      {{/each}}
-    </ul>
-  {{/unless}}
-  {{#if editingPermissions}}
-    {{#if category.availableGroups}}
-      {{combo-box
-        class="available-groups"
-        content=category.availableGroups
-        onChange=(action "onSelectGroup")
-        value=selectedGroup
-        valueProperty=null
-        nameProperty=null
-        options=(hash
-          placementStrategy="absolute"
-        )
-      }}
-      {{combo-box
-        class="permission-selector"
-        nameProperty="description"
-        content=category.availablePermissions
-        onChange=(action "onSelectPermission")
-        value=selectedPermission
-        options=(hash
-          placementStrategy="absolute"
-        )
-      }}
-      {{d-button
-        action=(action "addPermission" selectedGroup selectedPermission)
-        class="btn-primary add-permission"
-        icon="plus"}}
-      {{#if showPendingGroupChangesAlert}}
-        <div class="pending-permission-change-alert">
-          <div class="arrow-div"></div>
-          {{i18n "category.pending_permission_change_alert" group=selectedGroup}}
+
+      {{#unless category.permissions}}
+        <div class="permission-row row-empty">
+          {{i18n "category.permissions.no_groups_selected"}}
+        </div>
+      {{/unless}}
+
+      {{#if category.availableGroups}}
+        <div class="add-group">
+          <span class="group-name">
+            {{combo-box
+              class="available-groups"
+              content=category.availableGroups
+              onChange=(action "onSelectGroup")
+              value=null
+              valueProperty=null
+              nameProperty=null
+              options=(hash
+                none="category.security_add_group"
+              )
+            }}
+          </span>
        </div>
      {{/if}}
+    </div>
+
+    {{#if everyoneGrantedFull}}
+      <p class="warning">{{i18n "category.permissions.everyone_has_access"}}</p>
    {{/if}}
-  {{else}}
-    {{#unless category.is_special}}
-      {{d-button
-        action=(action "editPermissions")
-        class="btn-default edit-permission"
-        label="category.edit_permissions"}}
-    {{/unless}}
-  {{/if}}
+  {{/unless}}
 </section>

 {{plugin-outlet name="category-custom-security" args=(hash category=category) connectorTagName="" tagName="section"}}
@@ -1,6 +1,4 @@
 <section>
-  <h3>{{i18n "category.settings_sections.general"}}</h3>
-
  {{#if showPositionInput}}
    <section class="field position-fields">
      <label for="category-position">
@@ -1,2 +1 @@
-<label>{{i18n "category.topic_template"}}</label>
 {{d-editor value=category.topic_template showLink=showInsertLinkButton}}
@@ -26,9 +26,13 @@
    </ul>
  </div>

-  {{#each panels as |tab|}}
-    {{component tab selectedTab=selectedTab category=model registerValidator=(action "registerValidator")}}
-  {{/each}}
+  <div class="edit-category-content">
+    <h3>{{selectedTabTitle}}</h3>
+
+    {{#each panels as |tab|}}
+      {{component tab selectedTab=selectedTab category=model registerValidator=(action "registerValidator")}}
+    {{/each}}
+  </div>

  <div class="edit-category-footer">
    {{d-button id="save-category" class="btn-primary" disabled=disabled action=(action "saveCategory") label=saveLabel}}
@@ -3,6 +3,7 @@ import { click, visit } from "@ember/test-helpers";
 import { test } from "qunit";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+import I18n from "I18n";

 acceptance("Category Edit - security", function (needs) {
  needs.user();
@@ -10,13 +11,12 @@ acceptance("Category Edit - security", function (needs) {
  test("default", async function (assert) {
    await visit("/c/bug/edit/security");

-    const $firstItem = queryAll(".permission-list li:eq(0)");
-
-    const badgeName = $firstItem.find(".badge-group").text();
+    const firstRow = queryAll(".row-body").first();
+    const badgeName = firstRow.find(".group-name-label").text();
    assert.equal(badgeName, "everyone");

-    const permission = $firstItem.find(".permission").text();
-    assert.equal(permission, "Create / Reply / See");
+    const permission = firstRow.find(".d-icon-check-square");
+    assert.equal(permission.length, 3);
  });

  test("removing a permission", async function (assert) {
@@ -24,47 +24,42 @@ acceptance("Category Edit - security", function (needs) {

    await visit("/c/bug/edit/security");

-    await click(".edit-category-tab-security .edit-permission");
    await availableGroups.expand();
-
    assert.notOk(
      availableGroups.rowByValue("everyone").exists(),
      "everyone is already used and is not in the available groups"
    );

-    await click(
-      ".edit-category-tab-security .permission-list li:first-of-type .remove-permission"
-    );
+    await click(".row-body .remove-permission");
    await availableGroups.expand();

    assert.ok(
      availableGroups.rowByValue("everyone").exists(),
      "everyone has been removed and appears in the available groups"
    );
+    assert.ok(
+      queryAll(".row-empty").text(),
+      I18n.t("category.permissions.no_groups_selected"),
+      "shows message when no groups are selected"
+    );
  });

  test("adding a permission", async function (assert) {
    const availableGroups = selectKit(".available-groups");
-    const permissionSelector = selectKit(".permission-selector");

    await visit("/c/bug/edit/security");

-    await click(".edit-category-tab-security .edit-permission");
    await availableGroups.expand();
    await availableGroups.selectRowByValue("staff");
-    await permissionSelector.expand();
-    await permissionSelector.selectRowByValue("2");
-    await click(".edit-category-tab-security .add-permission");

-    const $addedPermissionItem = queryAll(
-      ".edit-category-tab-security .permission-list li:nth-child(2)"
+    const addedRow = queryAll(".row-body").last();
+
+    assert.equal(addedRow.find(".group-name-label").text(), "staff");
+    assert.equal(
+      addedRow.find(".d-icon-check-square").length,
+      3,
+      "new row permissions match default 'everyone' permissions"
    );
-
-    const badgeName = $addedPermissionItem.find(".badge-group").text();
-    assert.equal(badgeName, "staff");
-
-    const permission = $addedPermissionItem.find(".permission").text();
-    assert.equal(permission, "Reply / See");
  });

  test("adding a previously removed permission", async function (assert) {
@@ -72,33 +67,103 @@ acceptance("Category Edit - security", function (needs) {

    await visit("/c/bug/edit/security");

-    await click(".edit-category-tab-security .edit-permission");
-    await click(
-      ".edit-category-tab-security .permission-list li:first-of-type .remove-permission"
-    );
+    await click(".row-body .remove-permission");

    assert.equal(
-      queryAll(".edit-category-tab-security .permission-list li").length,
+      queryAll(".row-body").length,
      0,
-      "it removes the permission from the list"
+      "removes the permission from the list"
    );

    await availableGroups.expand();
    await availableGroups.selectRowByValue("everyone");
-    await click(".edit-category-tab-security .add-permission");

    assert.equal(
-      queryAll(".edit-category-tab-security .permission-list li").length,
+      queryAll(".row-body").length,
      1,
-      "it adds the permission to the list"
+      "adds back the permission tp the list"
    );

-    const $firstItem = queryAll(".permission-list li:eq(0)");
+    const firstRow = queryAll(".row-body").first();

-    const badgeName = $firstItem.find(".badge-group").text();
-    assert.equal(badgeName, "everyone");
+    assert.equal(firstRow.find(".group-name-label").text(), "everyone");
+    assert.equal(
+      firstRow.find(".d-icon-check-square").length,
+      1,
+      "adds only 'See' permission for a new row"
+    );
+  });

-    const permission = $firstItem.find(".permission").text();
-    assert.equal(permission, "Create / Reply / See");
+  test("editing permissions", async function (assert) {
+    const availableGroups = selectKit(".available-groups");
+
+    await visit("/c/bug/edit/security");
+
+    const everyoneRow = queryAll(".row-body").first();
+
+    assert.equal(
+      everyoneRow.find(".reply-granted, .create-granted").length,
+      2,
+      "everyone has full permissions by default"
+    );
+
+    await availableGroups.expand();
+    await availableGroups.selectRowByValue("staff");
+
+    const staffRow = queryAll(".row-body").last();
+
+    assert.equal(
+      staffRow.find(".reply-granted, .create-granted").length,
+      2,
+      "staff group also has full permissions"
+    );
+
+    await click(everyoneRow.find(".reply-toggle"));
+
+    assert.equal(
+      everyoneRow.find(".reply-granted, .create-granted").length,
+      0,
+      "everyone does not have reply or create"
+    );
+
+    assert.equal(
+      staffRow.find(".reply-granted, .create-granted").length,
+      2,
+      "staff group still has full permissions"
+    );
+
+    await click(staffRow.find(".reply-toggle"));
+
+    assert.equal(
+      everyoneRow.find(".reply-granted, .create-granted").length,
+      0,
+      "everyone permission unchanged"
+    );
+
+    assert.equal(
+      staffRow.find(".reply-granted").length,
+      0,
+      "staff does not have reply permission"
+    );
+
+    assert.equal(
+      staffRow.find(".create-granted").length,
+      0,
+      "staff does not have create permission"
+    );
+
+    await click(everyoneRow.find(".create-toggle"));
+
+    assert.equal(
+      everyoneRow.find(".reply-granted, .create-granted").length,
+      2,
+      "everyone has full permissions"
+    );
+
+    assert.equal(
+      staffRow.find(".reply-granted, .create-granted").length,
+      2,
+      "staff group has full permissions (inherited from everyone)"
+    );
  });
 });