diff --git a/app/assets/javascripts/discourse/components/tag-list.js.es6 b/app/assets/javascripts/discourse/components/tag-list.js.es6 index df2c024cb87..efa958d71a2 100644 --- a/app/assets/javascripts/discourse/components/tag-list.js.es6 +++ b/app/assets/javascripts/discourse/components/tag-list.js.es6 @@ -1,6 +1,7 @@ export default Ember.Component.extend({ classNameBindings: [':tag-list', 'categoryClass'], + isPrivateMessage: false, sortedTags: Ember.computed.sort('tags', 'sortProperties'), title: function() { diff --git a/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6 b/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6 new file mode 100644 index 00000000000..8f2ee7ce79b --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6 @@ -0,0 +1,13 @@ +export default Ember.Controller.extend({ + sortProperties: ['count:desc', 'id'], + + actions: { + sortByCount() { + this.set('sortProperties', ['count:desc', 'id']); + }, + + sortById() { + this.set('sortProperties', ['id']); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/user-private-messages.js.es6 b/app/assets/javascripts/discourse/controllers/user-private-messages.js.es6 index 9f50056d0bb..fa1aaca9421 100644 --- a/app/assets/javascripts/discourse/controllers/user-private-messages.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user-private-messages.js.es6 @@ -12,7 +12,7 @@ export default Ember.Controller.extend({ currentPath: Em.computed.alias('application.currentPath'), selected: Em.computed.alias('userTopicsList.selected'), bulkSelectEnabled: Em.computed.alias('userTopicsList.bulkSelectEnabled'), - pmTags: Em.computed.alias('userTopicsList.model.topic_list.pm_tags'), + showToggleBulkSelect: true, pmTaggingEnabled: Ember.computed.alias('site.can_tag_pms'), tagId: null, diff --git a/app/assets/javascripts/discourse/lib/render-tag.js.es6 b/app/assets/javascripts/discourse/lib/render-tag.js.es6 index 70fa81179f3..ea34505e5bd 100644 --- a/app/assets/javascripts/discourse/lib/render-tag.js.es6 +++ b/app/assets/javascripts/discourse/lib/render-tag.js.es6 @@ -6,7 +6,7 @@ export default function renderTag(tag, params) { let path; if (tagName === "a" && !params.noHref) { const current_user = Discourse.User.current(); - path = params.isPrivateMessage ? `/u/${current_user.username}/messages/tag/${tag}` : `/tags/${tag}`; + path = params.isPrivateMessage ? `/u/${current_user.username}/messages/tags/${tag}` : `/tags/${tag}`; } const href = path ? ` href='${Discourse.getURL(path)}' ` : ""; 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 0696f12ced4..ee447923aa5 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -96,7 +96,8 @@ export default function() { this.route('archive'); this.route('group', { path: 'group/:name'}); this.route('groupArchive', { path: 'group/:name/archive'}); - this.route('tag', { path: 'tag/:id'}); + this.route('tags'); + this.route('tagsShow', { path: 'tags/:id'}); }); this.route('preferences', { resetNamespace: true }, function() { diff --git a/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6 b/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6 index 3db308bbfc2..bb50a91634d 100644 --- a/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6 @@ -35,8 +35,12 @@ export default (viewName, path, channel) => { selected: [] }); - this.controllerFor("user-private-messages").set("archive", false); - this.controllerFor("user-private-messages").set("pmView", viewName); + this.controllerFor("user-private-messages").setProperties({ + archive: false, + pmView: viewName, + showToggleBulkSelect: true + }); + this.searchService.set('contextType', 'private_messages'); }, diff --git a/app/assets/javascripts/discourse/routes/user-private-messages-tag.js.es6 b/app/assets/javascripts/discourse/routes/user-private-messages-tags-show.js.es6 similarity index 67% rename from app/assets/javascripts/discourse/routes/user-private-messages-tag.js.es6 rename to app/assets/javascripts/discourse/routes/user-private-messages-tags-show.js.es6 index 8d6a16975d7..6b2fa60e297 100644 --- a/app/assets/javascripts/discourse/routes/user-private-messages-tag.js.es6 +++ b/app/assets/javascripts/discourse/routes/user-private-messages-tags-show.js.es6 @@ -2,10 +2,9 @@ import createPMRoute from "discourse/routes/build-private-messages-route"; export default createPMRoute('tags', 'private-messages-tags').extend({ model(params) { - this.controllerFor('user-private-messages').set('tagId', params.id); const username = this.modelFor("user").get("username_lower"); return this.store.findFiltered("topicList", { - filter: `topics/private-messages-tag/${username}/${params.id}` + filter: `topics/private-messages-tags/${username}/${params.id}` }); } }); diff --git a/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6 b/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6 new file mode 100644 index 00000000000..4edf278a52d --- /dev/null +++ b/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6 @@ -0,0 +1,22 @@ +import { ajax } from 'discourse/lib/ajax'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +export default Discourse.Route.extend({ + model() { + return ajax('/tags/personal_messages').then(result => { + return result.tags.map(tag => Ember.Object.create(tag)); + }).catch(popupAjaxError); + }, + + titleToken() { + return [I18n.t("tagging.tags"), I18n.t("user.private_messages")]; + }, + + setupController(controller, model) { + this.controllerFor('user-private-messages-tags').setProperties({ + model, + sortProperties: this.siteSettings.tags_sort_alphabetically ? ['id'] : ['count:desc', 'id'] + }); + this.controllerFor("user-private-messages").set("showToggleBulkSelect", false); + } +}); diff --git a/app/assets/javascripts/discourse/templates/components/tag-list.hbs b/app/assets/javascripts/discourse/templates/components/tag-list.hbs index 9c791cf25d7..5d4deec2c2d 100644 --- a/app/assets/javascripts/discourse/templates/components/tag-list.hbs +++ b/app/assets/javascripts/discourse/templates/components/tag-list.hbs @@ -10,7 +10,7 @@ {{#each sortedTags as |tag|}}
{{#if tag.count}} - {{discourse-tag tag.id}} x {{tag.count}} + {{discourse-tag tag.id isPrivateMessage=isPrivateMessage}} x {{tag.count}} {{else}} {{discourse-tag tag.id}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs b/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs new file mode 100644 index 00000000000..0682a895765 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs @@ -0,0 +1,17 @@ +
+
+

{{i18n "tagging.tags"}}

+
+
+ +
+ {{i18n "tagging.sort_by"}} + {{i18n "tagging.sort_by_count"}} + {{i18n "tagging.sort_by_name"}} +
+ +
+ +{{#if model}} + {{tag-list tags=model sortProperties=sortProperties titleKey="tagging.all_tags" isPrivateMessage=true}} +{{/if}} diff --git a/app/assets/javascripts/discourse/templates/user/messages.hbs b/app/assets/javascripts/discourse/templates/user/messages.hbs index 27808fd6983..1a8982ae741 100644 --- a/app/assets/javascripts/discourse/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/templates/user/messages.hbs @@ -23,29 +23,39 @@ {{plugin-outlet name="user-messages-nav" connectorTagName='li' args=(hash model=model)}} {{#each model.groups as |group|}} - {{#if group.has_messages}} -
  • - {{#link-to 'userPrivateMessages.group' group.name}} - {{d-icon "group" class="glyph"}} - {{capitalize-string group.name}} - {{/link-to}} -
  • -
  • - {{#link-to 'userPrivateMessages.groupArchive' group.name}} - {{i18n 'user.messages.archive'}} + {{#if group.has_messages}} +
  • + {{#link-to 'userPrivateMessages.group' group.name}} + {{d-icon "group" class="glyph"}} + {{capitalize-string group.name}} + {{/link-to}} +
  • +
  • + {{#link-to 'userPrivateMessages.groupArchive' group.name}} + {{i18n 'user.messages.archive'}} + {{/link-to}} +
  • + {{/if}} + {{/each}} + + {{#if pmTaggingEnabled}} +
  • + {{#link-to 'userPrivateMessages.tags' model}} + {{i18n 'user.messages.tags'}} {{/link-to}}
  • {{/if}} - {{/each}} {{/mobile-nav}} {{/d-section}}
    - + {{#if showToggleBulkSelect}} + + {{/if}} {{#if site.mobileView}} {{#if showNewPM}} @@ -74,10 +84,6 @@ {{#if isGroup}} {{group-notifications-button value=group.group_user.notification_level group=group user=model}} {{/if}} - - {{#if pmTaggingEnabled}} - {{pm-tag-drop pmTags=pmTags tagId=tagId}} - {{/if}}
    {{outlet}} diff --git a/app/assets/javascripts/select-kit/components/pm-tag-drop.js.es6 b/app/assets/javascripts/select-kit/components/pm-tag-drop.js.es6 deleted file mode 100644 index 23451ed0f62..00000000000 --- a/app/assets/javascripts/select-kit/components/pm-tag-drop.js.es6 +++ /dev/null @@ -1,19 +0,0 @@ -import TagDropComponent from "select-kit/components/tag-drop"; -import DiscourseURL from "discourse/lib/url"; -import { default as computed } from "ember-addons/ember-computed-decorators"; - -export default TagDropComponent.extend({ - @computed - allTagsUrl() { - return `/u/${this.currentUser.username}/messages/`; - }, - - content: Ember.computed.alias("pmTags"), - - actions: { - onSelect(tagId) { - const url = `/u/${this.currentUser.username}/messages/tag/${tagId}`; - DiscourseURL.routeTo(url); - } - } -}); diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 06cc6e2ba14..ba7946b1d32 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -134,7 +134,6 @@ class ListController < ApplicationController def self.generate_message_route(action) define_method("#{action}") do list_opts = build_topic_list_options - list_opts[:show_pm_tags] = true if guardian.can_tag_pms? target_user = fetch_user_from_params({ include_inactive: current_user.try(:staff?) }, [:user_stat, :user_option]) guardian.ensure_can_see_private_messages!(target_user.id) list = generate_list_for(action.to_s, target_user, list_opts) diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index b5d6fadb0c4..6cf3b813ffb 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -19,7 +19,7 @@ class TagsController < ::ApplicationController skip_before_action :check_xhr, only: [:tag_feed, :show, :index] before_action :set_category_from_params, except: [:index, :update, :destroy, - :tag_feed, :search, :notifications, :update_notifications] + :tag_feed, :search, :notifications, :update_notifications, :personal_messages] def index @description_meta = I18n.t("tags.title") @@ -191,6 +191,13 @@ class TagsController < ::ApplicationController render json: { valid: valid_tags } end + def personal_messages + guardian.ensure_can_tag_pms! + pm_tags = Tag.pm_tags(guardian: guardian) + + render json: { tags: pm_tags } + end + private def ensure_tags_enabled diff --git a/app/models/tag.rb b/app/models/tag.rb index 264c123ebe4..c7c67f2625b 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -63,8 +63,8 @@ class Tag < ActiveRecord::Base return [] unless (guardian || Guardian.new).can_tag_pms? limit = limit_arg || SiteSetting.max_tags_in_filter_list - tag_names = Tag.exec_sql <<~SQL - SELECT tags.name AS tag_name + tag_names_with_counts = Tag.exec_sql <<~SQL + SELECT tags.name, COUNT(topics.id) AS topic_count FROM tags INNER JOIN topic_tags ON tags.id = topic_tags.tag_id INNER JOIN topics ON topics.id = topic_tags.topic_id @@ -74,7 +74,7 @@ class Tag < ActiveRecord::Base LIMIT #{limit} SQL - tag_names.values.flatten + tag_names_with_counts.map { |t| { id: t['name'], text: t['name'], count: t['topic_count'] } } end def self.include_tags? diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index 2228f1e56e7..dbb468cb489 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -35,7 +35,6 @@ class TopicList :for_period, :per_page, :top_tags, - :pm_tags, :current_user, :tags @@ -60,15 +59,6 @@ class TopicList Tag.top_tags(opts) end - def pm_tags - guardian = Guardian.new(@current_user) - if @opts[:show_pm_tags] && guardian.can_tag_pms? - Tag.pm_tags(guardian: guardian) - else - [] - end - end - def preload_key if @category "topic_list_#{@category.url.sub(/^\//, '')}/l/#{@filter}" diff --git a/app/serializers/topic_list_serializer.rb b/app/serializers/topic_list_serializer.rb index 6ad24845725..994ea84950c 100644 --- a/app/serializers/topic_list_serializer.rb +++ b/app/serializers/topic_list_serializer.rb @@ -8,7 +8,6 @@ class TopicListSerializer < ApplicationSerializer :for_period, :per_page, :top_tags, - :pm_tags, :tags has_many :topics, serializer: TopicListItemSerializer, embed: :objects @@ -30,10 +29,6 @@ class TopicListSerializer < ApplicationSerializer Tag.include_tags? end - def include_pm_tags? - scope.can_tag_pms? - end - def include_tags? SiteSetting.tagging_enabled && object.tags.present? end diff --git a/config/routes.rb b/config/routes.rb index ee175fc8592..e02bd44f2b5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -368,7 +368,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/messages/:filter" => "user_actions#private_messages", constraints: { username: RouteFormat.username } get "#{root_path}/:username/messages/group/:group_name" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username } get "#{root_path}/:username/messages/group/:group_name/archive" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username } - get "#{root_path}/:username/messages/tag/:tag_id" => "user_actions#private_messages", constraints: StaffConstraint.new + get "#{root_path}/:username/messages/tags/:tag_id" => "user_actions#private_messages", constraints: StaffConstraint.new get "#{root_path}/:username.json" => "users#show", constraints: { username: RouteFormat.username }, defaults: { format: :json } get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username, format: /(json|html)/ } }.merge(index == 1 ? { as: 'user' } : {})) put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username }, defaults: { format: :json } @@ -607,7 +607,7 @@ Discourse::Application.routes.draw do get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent" get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive" get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread" - get "private-messages-tag/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", constraints: StaffConstraint.new + get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", constraints: StaffConstraint.new scope "/private-messages-group/:username", group_name: RouteFormat.username do get ":group_name.json" => "list#private_messages_group", as: "topics_private_messages_group" @@ -732,6 +732,7 @@ Discourse::Application.routes.draw do get '/filter/list' => 'tags#index' get '/filter/search' => 'tags#search' get '/check' => 'tags#check_hashtag' + get '/personal_messages' => 'tags#personal_messages' 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/lib/topic_query.rb b/lib/topic_query.rb index 5075ae43a53..ee65210573e 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -32,8 +32,7 @@ class TopicQuery match_all_tags no_subcategories slow_platform - no_tags - show_pm_tags) + no_tags) end def self.valid_options diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 79442d52e1f..d642d96f70f 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -98,9 +98,8 @@ describe Tag do describe '#pm_tags' do before do - @private_tags = [] personal_message = Fabricate(:private_message_topic) - 2.times { |i| @private_tags << Fabricate(:tag, topics: [personal_message]) } + 2.times { |i| Fabricate(:tag, topics: [personal_message], name: "tag-#{i}") } end it "returns nothing if user is not a staff" do @@ -114,7 +113,10 @@ describe Tag do it "returns all pm tags if user is a staff and pm tagging is enabled" do SiteSetting.allow_staff_to_tag_pms = true - expect(described_class.pm_tags(guardian: Guardian.new(Fabricate(:admin)))).to match_array(@private_tags.map(&:name)) + tags = described_class.pm_tags(guardian: Guardian.new(Fabricate(:admin))) + expect(tags.length).to eq(2) + expect(tags[0][:id]).to eq("tag-0") + expect(tags[1][:text]).to eq("tag-1") end end diff --git a/spec/models/topic_list_spec.rb b/spec/models/topic_list_spec.rb index 3113eecae03..d5022065f79 100644 --- a/spec/models/topic_list_spec.rb +++ b/spec/models/topic_list_spec.rb @@ -78,28 +78,4 @@ describe TopicList do end end end - - describe '#pm_tags' do - let(:admin) { Fabricate(:admin) } - let(:personal_message) { Fabricate(:private_message_topic) } - - before do - SiteSetting.tagging_enabled = true - SiteSetting.allow_staff_to_tag_pms = true - @private_tags = [] - 2.times { |i| @private_tags << Fabricate(:tag, topics: [personal_message]) } - end - - context 'when viewed as normal user' do - it 'returns no tags' do - expect(TopicList.new('liked', personal_message.user, [personal_message], show_pm_tags: true).pm_tags).to be_empty - end - end - - context 'when viewed as admin' do - it 'returns pm tags' do - expect(TopicList.new('liked', admin, [personal_message], show_pm_tags: true).pm_tags).to match_array(@private_tags.map(&:name)) - end - end - end end diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 0d5078bc358..9456e46fbff 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -96,17 +96,15 @@ RSpec.describe ListController do it 'should fail for non-staff users' do sign_in(user) - get "/topics/private-messages-tag/#{user.username}/#{tag.name}.json" + get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" expect(response.status).to eq(404) end it 'should be success for staff users' do [moderator, admin].each do |user| sign_in(user) - get "/topics/private-messages-tag/#{user.username}/#{tag.name}.json" + get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json" expect(response).to be_success - data = JSON.parse(response.body) - expect(data["topic_list"]["pm_tags"].length).to eq(1) end end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index 9bef5d5f7c0..58928384cf5 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -53,4 +53,36 @@ describe TagsController do expect(tag["value"]).to eq('test') end end + + describe '#personal_messages' do + before do + SiteSetting.allow_staff_to_tag_pms = true + personal_message = Fabricate(:private_message_topic) + Fabricate(:tag, topics: [personal_message], name: 'test') + end + + context "as a normal user" do + it "should return the right response" do + get "/tags/personal_messages.json" + + expect(response).not_to be_success + end + end + + context "as an admin" do + before do + admin = Fabricate(:admin) + sign_in(admin) + end + + it "should return the right response" do + get "/tags/personal_messages.json" + + expect(response).to be_success + + tag = JSON.parse(response.body)['tags'] + expect(tag[0]["id"]).to eq('test') + end + end + end end