DEV: Tag group improvements (#8252)

* DEV: Add the actual "tag_groups/new" route

Allows refreshing the "new" page without an error.

* DEV: Prevent attempts to create group tags if tagging is disabled

* DEV: Refactor the tag-groups controller

Gets rid of `selectedItem`, `selected`, and `selectTagGroup` action.

* DEV: Rename tag-groups-show to tag-groups-edit

* DEV: Refactor tag-groups form

* Extracted the tag-groups-form that's used by tag-groups-new and tag-groups-edit
* The model is now a buffered property
* Serialization relies more heavily on RestAdapter now
* Data is sent as JSON
* Payload is now namespaced ("tag_group")

* Update app/assets/javascripts/discourse/controllers/tag-groups-new.js.es6

Co-Authored-By: Joffrey JAFFEUX <j.jaffeux@gmail.com>

* Update app/assets/javascripts/discourse/components/tag-groups-form.js.es6

Co-Authored-By: Joffrey JAFFEUX <j.jaffeux@gmail.com>

* Update app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6

Co-Authored-By: Joffrey JAFFEUX <j.jaffeux@gmail.com>

app/assets/javascripts/admin/adapters/tag-group.js.es6

@@ -0,0 +1,5 @@
+import RestAdapter from "discourse/adapters/rest";
+
+export default RestAdapter.extend({
+  jsonMode: true
+});

app/assets/javascripts/discourse/components/radio-button.js.es6

@@ -14,10 +14,15 @@ export default Component.extend({
 
   click() {
     const value = $(this.element).val();
+
+    if (this.onChange) {
+      this.onChange(value);
+    } else {
       if (this.selection === value) {
         this.set("selection", undefined);
       }
       this.set("selection", value);
+    }
   },
 
   @computed("value", "selection")

app/assets/javascripts/discourse/components/tag-groups-form.js.es6

@@ -0,0 +1,69 @@
+import Component from "@ember/component";
+import computed from "ember-addons/ember-computed-decorators";
+import { bufferedProperty } from "discourse/mixins/buffered-content";
+import PermissionType from "discourse/models/permission-type";
+
+export default Component.extend(bufferedProperty("model"), {
+  tagName: "",
+
+  @computed("buffered.isSaving", "buffered.name", "buffered.tag_names")
+  savingDisabled(isSaving, name, tagNames) {
+    return isSaving || Ember.isEmpty(name) || Ember.isEmpty(tagNames);
+  },
+
+  actions: {
+    setPermissions(permissionName) {
+      if (permissionName === "private") {
+        this.buffered.set("permissions", {
+          staff: PermissionType.FULL
+        });
+      } else if (permissionName === "visible") {
+        this.buffered.set("permissions", {
+          staff: PermissionType.FULL,
+          everyone: PermissionType.READONLY
+        });
+      } else {
+        this.buffered.set("permissions", {
+          everyone: PermissionType.FULL
+        });
+      }
+    },
+
+    save() {
+      const attrs = this.buffered.getProperties(
+        "name",
+        "tag_names",
+        "parent_tag_name",
+        "one_per_topic",
+        "permissions"
+      );
+
+      this.model.save(attrs).then(() => {
+        this.commitBuffer();
+
+        if (this.onSave) {
+          this.onSave();
+        }
+      });
+    },
+
+    destroy() {
+      return bootbox.confirm(
+        I18n.t("tagging.groups.confirm_delete"),
+        I18n.t("no_value"),
+        I18n.t("yes_value"),
+        destroy => {
+          if (!destroy) {
+            return;
+          }
+
+          this.model.destroyRecord().then(() => {
+            if (this.onDestroy) {
+              this.onDestroy();
+            }
+          });
+        }
+      );
+    }
+  }
+});

app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6

@@ -0,0 +1,15 @@
+import { inject } from "@ember/controller";
+import Controller from "@ember/controller";
+
+export default Controller.extend({
+  tagGroups: inject(),
+
+  actions: {
+    onDestroy() {
+      const tagGroups = this.tagGroups.model;
+      tagGroups.removeObject(this.model);
+
+      this.transitionToRoute("tagGroups.index");
+    }
+  }
+});

app/assets/javascripts/discourse/controllers/tag-groups-new.js.es6

@@ -0,0 +1,14 @@
+import Controller from "@ember/controller";
+
+export default Controller.extend({
+  tagGroups: Ember.inject.controller(),
+
+  actions: {
+    onSave() {
+      const tagGroups = this.tagGroups.model;
+      tagGroups.pushObject(this.model);
+
+      this.transitionToRoute("tagGroups.edit", this.model);
+    }
+  }
+});

app/assets/javascripts/discourse/controllers/tag-groups-show.js.es6

@@ -1,28 +0,0 @@
-import { inject } from "@ember/controller";
-import Controller from "@ember/controller";
-export default Controller.extend({
-  tagGroups: inject(),
-
-  actions: {
-    save() {
-      this.model.save();
-    },
-
-    destroy() {
-      return bootbox.confirm(
-        I18n.t("tagging.groups.confirm_delete"),
-        I18n.t("no_value"),
-        I18n.t("yes_value"),
-        destroy => {
-          if (destroy) {
-            const c = this.get("tagGroups.model");
-            return this.model.destroy().then(() => {
-              c.removeObject(this.model);
-              this.transitionToRoute("tagGroups");
-            });
-          }
-        }
-      );
-    }
-  }
-});

app/assets/javascripts/discourse/controllers/tag-groups.js.es6

@@ -1,25 +1,9 @@
 import Controller from "@ember/controller";
-import TagGroup from "discourse/models/tag-group";
 
 export default Controller.extend({
   actions: {
-    selectTagGroup(tagGroup) {
-      if (this.selectedItem) {
-        this.selectedItem.set("selected", false);
-      }
-      this.set("selectedItem", tagGroup);
-      tagGroup.set("selected", true);
-      tagGroup.set("savingStatus", null);
-      this.transitionToRoute("tagGroups.show", tagGroup);
-    },
-
     newTagGroup() {
-      const newTagGroup = TagGroup.create({
+      this.transitionToRoute("tagGroups.new");
-        id: "new",
-        name: I18n.t("tagging.groups.new_name")
-      });
-      this.model.pushObject(newTagGroup);
-      this.send("selectTagGroup", newTagGroup);
     }
   }
 });

app/assets/javascripts/discourse/models/tag-group.js.es6

@@ -1,22 +1,10 @@
-import { ajax } from "discourse/lib/ajax";
 import RestModel from "discourse/models/rest";
 import computed from "ember-addons/ember-computed-decorators";
 import PermissionType from "discourse/models/permission-type";
 
 export default RestModel.extend({
-  @computed("name", "tag_names", "saving")
-  disableSave(name, tagNames, saving) {
-    return saving || Ember.isEmpty(name) || Ember.isEmpty(tagNames);
-  },
-
-  @computed("id")
-  disableDelete(id) {
-    return !parseInt(id);
-  },
-
   @computed("permissions")
-  permissionName: {
+  permissionName(permissions) {
-    get(permissions) {
     if (!permissions) return "public";
 
     if (permissions["everyone"] === PermissionType.FULL) {
@@ -26,52 +14,5 @@ export default RestModel.extend({
     } else {
       return "private";
     }
-    },
-
-    set(value) {
-      if (value === "private") {
-        this.set("permissions", { staff: PermissionType.FULL });
-      } else if (value === "visible") {
-        this.set("permissions", {
-          staff: PermissionType.FULL,
-          everyone: PermissionType.READONLY
-        });
-      } else {
-        this.set("permissions", { everyone: PermissionType.FULL });
-      }
-    }
-  },
-
-  save() {
-    this.set("savingStatus", I18n.t("saving"));
-    this.set("saving", true);
-
-    const isNew = this.id === "new";
-    const url = isNew ? "/tag_groups" : `/tag_groups/${this.id}`;
-    const data = this.getProperties(
-      "name",
-      "tag_names",
-      "parent_tag_name",
-      "one_per_topic",
-      "permissions"
-    );
-
-    return ajax(url, {
-      data,
-      type: isNew ? "POST" : "PUT"
-    })
-      .then(result => {
-        if (result.tag_group && result.tag_group.id) {
-          this.set("id", result.tag_group.id);
-        }
-      })
-      .finally(() => {
-        this.set("savingStatus", I18n.t("saved"));
-        this.set("saving", false);
-      });
-  },
-
-  destroy() {
-    return ajax(`/tag_groups/${this.id}`, { type: "DELETE" });
   }
 });

app/assets/javascripts/discourse/routes/app-route-map.js.es6

@@ -223,7 +223,8 @@ export default function() {
     "tagGroups",
     { path: "/tag_groups", resetNamespace: true },
     function() {
-      this.route("show", { path: "/:id" });
+      this.route("edit", { path: "/:id" });
+      this.route("new");
     }
   );
 

app/assets/javascripts/discourse/routes/tag-groups-edit.js.es6

@@ -5,5 +5,9 @@ export default DiscourseRoute.extend({
 
   model(params) {
     return this.store.find("tagGroup", params.id);
+  },
+
+  afterModel(tagGroup) {
+    tagGroup.set("savingStatus", null);
   }
 });

app/assets/javascripts/discourse/routes/tag-groups-new.js.es6

@@ -0,0 +1,17 @@
+import DiscourseRoute from "discourse/routes/discourse";
+
+export default DiscourseRoute.extend({
+  showFooter: true,
+
+  beforeModel() {
+    if (!this.siteSettings.tagging_enabled) {
+      this.transitionTo("tagGroups");
+    }
+  },
+
+  model() {
+    return this.store.createRecord("tagGroup", {
+      name: I18n.t("tagging.groups.new_name")
+    });
+  }
+});

app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs

@@ -0,0 +1,77 @@
+<div class="tag-group-content">
+  <h1>{{text-field value=buffered.name}}</h1>
+  <br/>
+
+  <section class="group-tags-list">
+    <label>{{i18n 'tagging.groups.tags_label'}}</label><br/>
+    {{tag-chooser
+      tags=buffered.tag_names
+      everyTag=true
+      allowCreate=true
+      filterPlaceholder="tagging.groups.tags_placeholder"
+      unlimitedTagCount=true}}
+  </section>
+
+  <section class="parent-tag-section">
+    <label>{{i18n 'tagging.groups.parent_tag_label'}}</label>
+    {{tag-chooser
+      tags=buffered.parent_tag_name
+      everyTag=true
+      maximum=1
+      allowCreate=true
+      filterPlaceholder="tagging.groups.parent_tag_placeholder"}}
+    <span class="description">{{i18n 'tagging.groups.parent_tag_description'}}</span>
+  </section>
+
+  <section class="group-one-per-topic">
+    <label>
+      {{input type="checkbox" checked=buffered.one_per_topic name="onepertopic"}}
+      {{i18n 'tagging.groups.one_per_topic_label'}}
+    </label>
+  </section>
+
+  <section class="group-visibility">
+    <div>
+      {{radio-button
+        class="tag-permissions-choice"
+        name="tag-permissions-choice"
+        value="public"
+        id="public-permission"
+        selection=buffered.permissionName
+        onChange=(action "setPermissions")}}
+
+      <label class="radio" for="public-permission">
+        {{i18n 'tagging.groups.everyone_can_use'}}
+      </label>
+    </div>
+    <div>
+      {{radio-button
+        class="tag-permissions-choice"
+        name="tag-permissions-choice"
+        value="visible"
+        id="visible-permission"
+        selection=buffered.permissionName
+        onChange=(action "setPermissions")}}
+
+      <label class="radio" for="visible-permission">
+        {{i18n 'tagging.groups.usable_only_by_staff'}}
+      </label>
+    </div>
+    <div>
+      {{radio-button
+        class="tag-permissions-choice"
+        name="tag-permissions-choice"
+        value="private"
+        id="private-permission"
+        selection=buffered.permissionName
+        onChange=(action "setPermissions")}}
+
+      <label class="radio" for="private-permission">
+        {{i18n 'tagging.groups.visible_only_to_staff'}}
+      </label>
+    </div>
+  </section>
+
+  <button {{action "save"}} disabled={{savingDisabled}} class='btn btn-default'>{{i18n 'tagging.groups.save'}}</button>
+  <button {{action "destroy"}} disabled={{buffered.isNew}} class='btn btn-danger'>{{d-icon "far-trash-alt"}} {{i18n 'tagging.groups.delete'}}</button>
+</div>

app/assets/javascripts/discourse/templates/tag-groups-edit.hbs

@@ -0,0 +1 @@
+{{tag-groups-form model=model onDestroy=(action "onDestroy")}}

app/assets/javascripts/discourse/templates/tag-groups-new.hbs

@@ -0,0 +1 @@
+{{tag-groups-form model=model onSave=(action "onSave")}}

app/assets/javascripts/discourse/templates/tag-groups-show.hbs

@@ -1,50 +0,0 @@
-<div class="tag-group-content">
-  <h1>{{text-field value=model.name}}</h1>
-  <br/>
-  <section class="group-tags-list">
-    <label>{{i18n 'tagging.groups.tags_label'}}</label><br/>
-    {{tag-chooser
-      tags=model.tag_names
-      everyTag=true
-      allowCreate=true
-      filterPlaceholder="tagging.groups.tags_placeholder"
-      unlimitedTagCount=true}}
-  </section>
-
-  <section class="parent-tag-section">
-    <label>{{i18n 'tagging.groups.parent_tag_label'}}</label>
-    {{tag-chooser
-      tags=model.parent_tag_name
-      everyTag=true
-      maximum=1
-      allowCreate=true
-      filterPlaceholder="tagging.groups.parent_tag_placeholder"}}
-    <span class="description">{{i18n 'tagging.groups.parent_tag_description'}}</span>
-  </section>
-
-  <section class="group-one-per-topic">
-    <label>
-      {{input type="checkbox" checked=model.one_per_topic name="onepertopic"}}
-      {{i18n 'tagging.groups.one_per_topic_label'}}
-    </label>
-  </section>
-
-  <section class="group-visibility">
-    <div>
-      {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="public" id="public-permission" selection=model.permissionName}}
-      <label class="radio" for="public-permission">{{i18n 'tagging.groups.everyone_can_use'}}</label>
-    </div>
-    <div>
-      {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="visible" id="visible-permission" selection=model.permissionName}}
-      <label class="radio" for="visible-permission">{{i18n 'tagging.groups.usable_only_by_staff'}}</label>
-    </div>
-    <div>
-      {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="private" id="private-permission" selection=model.permissionName}}
-      <label class="radio" for="private-permission">{{i18n 'tagging.groups.visible_only_to_staff'}}</label>
-    </div>
-  </section>
-
-  <button {{action "save"}} disabled={{model.disableSave}} class='btn btn-default'>{{i18n 'tagging.groups.save'}}</button>
-  <button {{action "destroy"}} disabled={{model.disableDelete}} class='btn btn-danger'>{{d-icon "far-trash-alt"}} {{i18n 'tagging.groups.delete'}}</button>
-  <span class="saving {{unless model.savingStatus 'hidden'}}">{{model.savingStatus}}</span>
-</div>

app/assets/javascripts/discourse/templates/tag-groups.hbs

@@ -4,10 +4,17 @@
   <div class='content-list'>
     <ul>
       {{#each model as |tagGroup|}}
-        <li><a {{action "selectTagGroup" tagGroup}} class="{{if tagGroup.selected 'active'}}">{{tagGroup.name}}</a></li>
+        <li>
+          {{#link-to "tagGroups.edit" tagGroup}}
+            {{tagGroup.name}}
+          {{/link-to}}
+        </li>
       {{/each}}
     </ul>
+
+    {{#if this.siteSettings.tagging_enabled}}
       <button {{action "newTagGroup"}} class='btn btn-default'>{{d-icon "plus"}}{{i18n 'tagging.groups.new'}}</button>
+    {{/if}}
   </div>
 
   {{outlet}}

app/controllers/tag_groups_controller.rb

@@ -1,11 +1,10 @@
 # frozen_string_literal: true
 
 class TagGroupsController < ApplicationController
-
   requires_login
   before_action :ensure_staff
 
-  skip_before_action :check_xhr, only: [:index, :show]
+  skip_before_action :check_xhr, only: [:index, :show, :new]
   before_action :fetch_tag_group, only: [:show, :update, :destroy]
 
   def index
@@ -31,6 +30,13 @@ class TagGroupsController < ApplicationController
     end
   end
 
+  def new
+    tag_groups = TagGroup.order('name ASC').includes(:parent_tag).preload(:tags).all
+    serializer = ActiveModel::ArraySerializer.new(tag_groups, each_serializer: TagGroupSerializer, root: 'tag_groups')
+    store_preloaded "tagGroup", MultiJson.dump(serializer)
+    render "default/empty"
+  end
+
   def create
     guardian.ensure_can_admin_tag_groups!
     @tag_group = TagGroup.new(tag_groups_params)
@@ -73,6 +79,9 @@ class TagGroupsController < ApplicationController
   end
 
   def tag_groups_params
+    tag_group = params.delete(:tag_group)
+    params.merge!(tag_group.permit!) if tag_group
+
     if permissions = params[:permissions]
       permissions.each do |k, v|
         permissions[k] = v.to_i
@@ -87,9 +96,11 @@ class TagGroupsController < ApplicationController
       parent_tag_name: [],
       permissions: permissions&.keys,
     )
+
     result[:tag_names] ||= []
     result[:parent_tag_name] ||= []
-    result[:one_per_topic] = (params[:one_per_topic] == "true")
+    result[:one_per_topic] = params[:one_per_topic].in?([true, "true"])
+
     result
   end
 end

config/routes.rb

@@ -845,7 +845,7 @@ Discourse::Application.routes.draw do
     end
   end
 
-  resources :tag_groups, constraints: StaffConstraint.new, except: [:new, :edit] do
+  resources :tag_groups, constraints: StaffConstraint.new, except: [:edit] do
     collection do
       get '/filter/search' => 'tag_groups#search'
     end