From 080e899b8c742524dd2a40eec26b1096597a9ef9 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 30 Oct 2019 16:57:13 +0100 Subject: [PATCH] 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 * Update app/assets/javascripts/discourse/components/tag-groups-form.js.es6 Co-Authored-By: Joffrey JAFFEUX * Update app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6 Co-Authored-By: Joffrey JAFFEUX --- .../admin/adapters/tag-group.js.es6 | 5 ++ .../discourse/components/radio-button.js.es6 | 11 ++- .../components/tag-groups-form.js.es6 | 69 +++++++++++++++++ .../controllers/tag-groups-edit.js.es6 | 15 ++++ .../controllers/tag-groups-new.js.es6 | 14 ++++ .../controllers/tag-groups-show.js.es6 | 28 ------- .../discourse/controllers/tag-groups.js.es6 | 18 +---- .../discourse/models/tag-group.js.es6 | 75 ++---------------- .../discourse/routes/app-route-map.js.es6 | 3 +- ...ups-show.js.es6 => tag-groups-edit.js.es6} | 4 + .../discourse/routes/tag-groups-new.js.es6 | 17 ++++ .../templates/components/tag-groups-form.hbs | 77 +++++++++++++++++++ .../discourse/templates/tag-groups-edit.hbs | 1 + .../discourse/templates/tag-groups-new.hbs | 1 + .../discourse/templates/tag-groups-show.hbs | 50 ------------ .../discourse/templates/tag-groups.hbs | 11 ++- app/controllers/tag_groups_controller.rb | 17 +++- config/routes.rb | 2 +- 18 files changed, 246 insertions(+), 172 deletions(-) create mode 100644 app/assets/javascripts/admin/adapters/tag-group.js.es6 create mode 100644 app/assets/javascripts/discourse/components/tag-groups-form.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/tag-groups-new.js.es6 delete mode 100644 app/assets/javascripts/discourse/controllers/tag-groups-show.js.es6 rename app/assets/javascripts/discourse/routes/{tag-groups-show.js.es6 => tag-groups-edit.js.es6} (73%) create mode 100644 app/assets/javascripts/discourse/routes/tag-groups-new.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs create mode 100644 app/assets/javascripts/discourse/templates/tag-groups-edit.hbs create mode 100644 app/assets/javascripts/discourse/templates/tag-groups-new.hbs delete mode 100644 app/assets/javascripts/discourse/templates/tag-groups-show.hbs diff --git a/app/assets/javascripts/admin/adapters/tag-group.js.es6 b/app/assets/javascripts/admin/adapters/tag-group.js.es6 new file mode 100644 index 00000000000..2e950aa2c12 --- /dev/null +++ b/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 +}); diff --git a/app/assets/javascripts/discourse/components/radio-button.js.es6 b/app/assets/javascripts/discourse/components/radio-button.js.es6 index 3f6d5a61090..b6d44b4a990 100644 --- a/app/assets/javascripts/discourse/components/radio-button.js.es6 +++ b/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.selection === value) { - this.set("selection", undefined); + + if (this.onChange) { + this.onChange(value); + } else { + if (this.selection === value) { + this.set("selection", undefined); + } + this.set("selection", value); } - this.set("selection", value); }, @computed("value", "selection") diff --git a/app/assets/javascripts/discourse/components/tag-groups-form.js.es6 b/app/assets/javascripts/discourse/components/tag-groups-form.js.es6 new file mode 100644 index 00000000000..4d932e1d69b --- /dev/null +++ b/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(); + } + }); + } + ); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6 b/app/assets/javascripts/discourse/controllers/tag-groups-edit.js.es6 new file mode 100644 index 00000000000..067a2f615b8 --- /dev/null +++ b/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"); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/tag-groups-new.js.es6 b/app/assets/javascripts/discourse/controllers/tag-groups-new.js.es6 new file mode 100644 index 00000000000..e4a3a0187ee --- /dev/null +++ b/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); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/tag-groups-show.js.es6 b/app/assets/javascripts/discourse/controllers/tag-groups-show.js.es6 deleted file mode 100644 index 9ebdef5d92e..00000000000 --- a/app/assets/javascripts/discourse/controllers/tag-groups-show.js.es6 +++ /dev/null @@ -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"); - }); - } - } - ); - } - } -}); diff --git a/app/assets/javascripts/discourse/controllers/tag-groups.js.es6 b/app/assets/javascripts/discourse/controllers/tag-groups.js.es6 index cfde9214ea8..725f8846f1e 100644 --- a/app/assets/javascripts/discourse/controllers/tag-groups.js.es6 +++ b/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({ - id: "new", - name: I18n.t("tagging.groups.new_name") - }); - this.model.pushObject(newTagGroup); - this.send("selectTagGroup", newTagGroup); + this.transitionToRoute("tagGroups.new"); } } }); diff --git a/app/assets/javascripts/discourse/models/tag-group.js.es6 b/app/assets/javascripts/discourse/models/tag-group.js.es6 index 6d0579a1b96..e42ccee9dda 100644 --- a/app/assets/javascripts/discourse/models/tag-group.js.es6 +++ b/app/assets/javascripts/discourse/models/tag-group.js.es6 @@ -1,77 +1,18 @@ -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: { - get(permissions) { - if (!permissions) return "public"; + permissionName(permissions) { + if (!permissions) return "public"; - if (permissions["everyone"] === PermissionType.FULL) { - return "public"; - } else if (permissions["everyone"] === PermissionType.READONLY) { - return "visible"; - } 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 }); - } + if (permissions["everyone"] === PermissionType.FULL) { + return "public"; + } else if (permissions["everyone"] === PermissionType.READONLY) { + return "visible"; + } else { + return "private"; } - }, - - 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" }); } }); diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index d29d77463b0..2bc476425c5 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/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"); } ); diff --git a/app/assets/javascripts/discourse/routes/tag-groups-show.js.es6 b/app/assets/javascripts/discourse/routes/tag-groups-edit.js.es6 similarity index 73% rename from app/assets/javascripts/discourse/routes/tag-groups-show.js.es6 rename to app/assets/javascripts/discourse/routes/tag-groups-edit.js.es6 index 7593f364bed..a923282cce2 100644 --- a/app/assets/javascripts/discourse/routes/tag-groups-show.js.es6 +++ b/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); } }); diff --git a/app/assets/javascripts/discourse/routes/tag-groups-new.js.es6 b/app/assets/javascripts/discourse/routes/tag-groups-new.js.es6 new file mode 100644 index 00000000000..64c460f5121 --- /dev/null +++ b/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") + }); + } +}); diff --git a/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs b/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs new file mode 100644 index 00000000000..b6b724f05e2 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/tag-groups-form.hbs @@ -0,0 +1,77 @@ +
+

{{text-field value=buffered.name}}

+
+ +
+
+ {{tag-chooser + tags=buffered.tag_names + everyTag=true + allowCreate=true + filterPlaceholder="tagging.groups.tags_placeholder" + unlimitedTagCount=true}} +
+ +
+ + {{tag-chooser + tags=buffered.parent_tag_name + everyTag=true + maximum=1 + allowCreate=true + filterPlaceholder="tagging.groups.parent_tag_placeholder"}} + {{i18n 'tagging.groups.parent_tag_description'}} +
+ +
+ +
+ +
+
+ {{radio-button + class="tag-permissions-choice" + name="tag-permissions-choice" + value="public" + id="public-permission" + selection=buffered.permissionName + onChange=(action "setPermissions")}} + + +
+
+ {{radio-button + class="tag-permissions-choice" + name="tag-permissions-choice" + value="visible" + id="visible-permission" + selection=buffered.permissionName + onChange=(action "setPermissions")}} + + +
+
+ {{radio-button + class="tag-permissions-choice" + name="tag-permissions-choice" + value="private" + id="private-permission" + selection=buffered.permissionName + onChange=(action "setPermissions")}} + + +
+
+ + + +
diff --git a/app/assets/javascripts/discourse/templates/tag-groups-edit.hbs b/app/assets/javascripts/discourse/templates/tag-groups-edit.hbs new file mode 100644 index 00000000000..b6b0e9adb10 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/tag-groups-edit.hbs @@ -0,0 +1 @@ +{{tag-groups-form model=model onDestroy=(action "onDestroy")}} diff --git a/app/assets/javascripts/discourse/templates/tag-groups-new.hbs b/app/assets/javascripts/discourse/templates/tag-groups-new.hbs new file mode 100644 index 00000000000..172d8c77482 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/tag-groups-new.hbs @@ -0,0 +1 @@ +{{tag-groups-form model=model onSave=(action "onSave")}} diff --git a/app/assets/javascripts/discourse/templates/tag-groups-show.hbs b/app/assets/javascripts/discourse/templates/tag-groups-show.hbs deleted file mode 100644 index 76953050399..00000000000 --- a/app/assets/javascripts/discourse/templates/tag-groups-show.hbs +++ /dev/null @@ -1,50 +0,0 @@ -
-

{{text-field value=model.name}}

-
-
-
- {{tag-chooser - tags=model.tag_names - everyTag=true - allowCreate=true - filterPlaceholder="tagging.groups.tags_placeholder" - unlimitedTagCount=true}} -
- -
- - {{tag-chooser - tags=model.parent_tag_name - everyTag=true - maximum=1 - allowCreate=true - filterPlaceholder="tagging.groups.parent_tag_placeholder"}} - {{i18n 'tagging.groups.parent_tag_description'}} -
- -
- -
- -
-
- {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="public" id="public-permission" selection=model.permissionName}} - -
-
- {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="visible" id="visible-permission" selection=model.permissionName}} - -
-
- {{radio-button class="tag-permissions-choice" name="tag-permissions-choice" value="private" id="private-permission" selection=model.permissionName}} - -
-
- - - - {{model.savingStatus}} -
diff --git a/app/assets/javascripts/discourse/templates/tag-groups.hbs b/app/assets/javascripts/discourse/templates/tag-groups.hbs index 6c49ad8aafc..bbb1d78c3e9 100644 --- a/app/assets/javascripts/discourse/templates/tag-groups.hbs +++ b/app/assets/javascripts/discourse/templates/tag-groups.hbs @@ -4,10 +4,17 @@
    {{#each model as |tagGroup|}} -
  • {{tagGroup.name}}
    • +
  • + {{#link-to "tagGroups.edit" tagGroup}} + {{tagGroup.name}} + {{/link-to}} +
    • {{/each}}
- + + {{#if this.siteSettings.tagging_enabled}} + + {{/if}}
{{outlet}} diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb index f0677769c45..6b87032fa77 100644 --- a/app/controllers/tag_groups_controller.rb +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index 7e5fd73d085..44594aa6ac3 100644 --- a/config/routes.rb +++ b/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