From d89ffbeffdc1d0e528fd07c15c79c8f308b67e10 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 12 Nov 2018 16:24:34 +0000 Subject: [PATCH] FEATURE: Add button to delete unused tags (#6587) This is particularly useful if you have uploaded a CSV file, and wish to bulk-delete all of the tags that you uploaded. --- .../components/tags-admin-dropdown.js.es6 | 9 ++++- .../discourse/controllers/tags-index.js.es6 | 37 ++++++++++++++++++ app/controllers/tags_controller.rb | 13 +++++++ app/models/tag.rb | 2 + config/locales/client.en.yml | 10 +++++ config/routes.rb | 2 + spec/models/tag_spec.rb | 14 +++++++ spec/requests/tags_controller_spec.rb | 39 +++++++++++++++++++ 8 files changed, 125 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/components/tags-admin-dropdown.js.es6 b/app/assets/javascripts/discourse/components/tags-admin-dropdown.js.es6 index 03b5a3c4713..3fdbdf563c7 100644 --- a/app/assets/javascripts/discourse/components/tags-admin-dropdown.js.es6 +++ b/app/assets/javascripts/discourse/components/tags-admin-dropdown.js.es6 @@ -22,6 +22,12 @@ export default DropdownSelectBoxComponent.extend({ name: I18n.t("tagging.upload"), description: I18n.t("tagging.upload_description"), icon: "upload" + }, + { + id: "deleteUnusedTags", + name: I18n.t("tagging.delete_unused"), + description: I18n.t("tagging.delete_unused_description"), + icon: "trash" } ]; @@ -30,7 +36,8 @@ export default DropdownSelectBoxComponent.extend({ actionNames: { manageGroups: "showTagGroups", - uploadTags: "showUploader" + uploadTags: "showUploader", + deleteUnusedTags: "deleteUnused" }, mutateValue(id) { diff --git a/app/assets/javascripts/discourse/controllers/tags-index.js.es6 b/app/assets/javascripts/discourse/controllers/tags-index.js.es6 index 8f4cf5ba29f..71b2c96da23 100644 --- a/app/assets/javascripts/discourse/controllers/tags-index.js.es6 +++ b/app/assets/javascripts/discourse/controllers/tags-index.js.es6 @@ -1,5 +1,7 @@ import computed from "ember-addons/ember-computed-decorators"; import showModal from "discourse/lib/show-modal"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; export default Ember.Controller.extend({ sortProperties: ["totalCount:desc", "id"], @@ -38,6 +40,41 @@ export default Ember.Controller.extend({ showUploader() { showModal("tag-upload"); + }, + + deleteUnused() { + ajax("/tags/unused", { type: "GET" }) + .then(result => { + const displayN = 20; + const tags = result["tags"]; + const tagString = tags.slice(0, displayN).join(", "); + var more = Math.max(0, tags.length - displayN); + const string = + more === 0 + ? I18n.t("tagging.delete_unused_confirmation", { + count: tags.length, + tags: tagString + }) + : I18n.t("tagging.delete_unused_confirmation_more", { + total: tags.length, + tags: tagString, + count: more + }); + + bootbox.confirm( + string, + I18n.t("tagging.cancel_delete_unused"), + I18n.t("tagging.delete_unused"), + proceed => { + if (proceed) { + ajax("/tags/unused", { type: "DELETE" }) + .then(() => this.send("refresh")) + .catch(popupAjaxError); + } + } + ); + }) + .catch(popupAjaxError); } } }); diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 14549413d42..624eff5fb36 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -146,6 +146,19 @@ class TagsController < ::ApplicationController end end + def list_unused + guardian.ensure_can_admin_tags! + render json: { tags: Tag.unused.pluck(:name) } + end + + def destroy_unused + guardian.ensure_can_admin_tags! + tags = Tag.unused + StaffActionLogger.new(current_user).log_custom('deleted_unused_tags', tags: tags.pluck(:name)) + tags.destroy_all + render json: success_json + end + def destroy guardian.ensure_can_admin_tags! tag_name = params[:tag_id] diff --git a/app/models/tag.rb b/app/models/tag.rb index ec42d1a62c5..4e77af16d45 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -9,6 +9,8 @@ class Tag < ActiveRecord::Base where("lower(name) IN (?)", name) end + scope :unused, -> { where(topic_count: 0, pm_topic_count: 0) } + has_many :tag_users # notification settings has_many :topic_tags, dependent: :destroy diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1c557df1956..734f9592a0b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2710,6 +2710,15 @@ en: upload_description: "Upload a text file to create tags in bulk" upload_instructions: "One per line, optionally with a tag group in the format 'tag_name,tag_group'." upload_successful: "Tags uploaded successfully" + delete_unused_confirmation: + one: "1 tag will be deleted: %{tags}" + other: "{{count}} tags will be deleted: %{tags}" + delete_unused_confirmation_more: + one: "{{total}} tags will be deleted: %{tags} and one more" + other: "{{total}} tags will be deleted: %{tags} and %{count} more" + delete_unused: "Delete Unused Tags" + delete_unused_description: "Delete all tags which are not attached to any topics or personal messages" + cancel_delete_unused: "Cancel" filters: without_category: "%{filter} %{tag} topics" with_category: "%{filter} %{tag} topics in %{category}" @@ -3534,6 +3543,7 @@ en: revoke_moderation: "revoke moderation" backup_create: "create backup" deleted_tag: "deleted tag" + deleted_unused_tags: "deleted unused tags" renamed_tag: "renamed tag" revoke_email: "revoke email" lock_trust_level: "lock trust level" diff --git a/config/routes.rb b/config/routes.rb index d3cebd11eed..a9fed0f8914 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -776,6 +776,8 @@ Discourse::Application.routes.draw do get '/check' => 'tags#check_hashtag' get '/personal_messages/:username' => 'tags#personal_messages' post '/upload' => 'tags#upload' + get '/unused' => 'tags#list_unused' + delete '/unused' => 'tags#destroy_unused' constraints(tag_id: /[^\/]+?/, format: /json|rss/) do get '/:tag_id.rss' => 'tags#tag_feed' get '/:tag_id' => 'tags#show', as: 'tag_show' diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 0fcb28e42a9..a48081bf7d6 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -172,4 +172,18 @@ describe Tag do expect(tag.topic_count).to eq(1) end end + + context "unused tags scope" do + let!(:tags) do + [ Fabricate(:tag, name: "used_publically", topic_count: 2, pm_topic_count: 0), + Fabricate(:tag, name: "used_privately", topic_count: 0, pm_topic_count: 3), + Fabricate(:tag, name: "used_everywhere", topic_count: 0, pm_topic_count: 3), + Fabricate(:tag, name: "unused1", topic_count: 0, pm_topic_count: 0), + Fabricate(:tag, name: "unused2", topic_count: 0, pm_topic_count: 0)] + end + + it "returns the correct tags" do + expect(Tag.unused.pluck(:name)).to contain_exactly("unused1", "unused2") + end + end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index 9166a4442a2..740e628dcf0 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -389,6 +389,45 @@ describe TagsController do end end + describe '#unused' do + it "fails if you can't manage tags" do + sign_in(Fabricate(:user)) + get "/tags/unused.json" + expect(response.status).to eq(403) + delete "/tags/unused.json" + expect(response.status).to eq(403) + end + + context 'logged in' do + before do + sign_in(Fabricate(:admin)) + end + + context 'with some tags' do + let!(:tags) { [ + Fabricate(:tag, name: "used_publically", topic_count: 2, pm_topic_count: 0), + Fabricate(:tag, name: "used_privately", topic_count: 0, pm_topic_count: 3), + Fabricate(:tag, name: "used_everywhere", topic_count: 0, pm_topic_count: 3), + Fabricate(:tag, name: "unused1", topic_count: 0, pm_topic_count: 0), + Fabricate(:tag, name: "unused2", topic_count: 0, pm_topic_count: 0) + ]} + + it 'returns the correct unused tags' do + get "/tags/unused.json" + expect(response.status).to eq(200) + json = ::JSON.parse(response.body) + expect(json["tags"]).to contain_exactly("unused1", "unused2") + end + + it 'deletes the correct tags' do + expect { delete "/tags/unused.json" }.to change { Tag.count }.by(-2) & change { UserHistory.count }.by(1) + expect(Tag.pluck(:name)).to contain_exactly("used_publically", "used_privately", "used_everywhere") + end + end + + end + end + context '#upload_csv' do it 'requires you to be logged in' do post "/tags/upload.json"