From f13470b96bcae5a3fd1dd4baa943cc730a922f31 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 4 May 2016 14:02:47 -0400 Subject: [PATCH] Use db schema for tags instead of plugin store and custom fields --- .../discourse/controllers/rename-tag.js.es6 | 13 +- app/controllers/tags_controller.rb | 65 +++++---- app/helpers/topics_helper.rb | 5 +- app/jobs/onceoff/migrate_tagging_plugin.rb | 16 +++ app/models/tag.rb | 18 +++ app/models/tag_user.rb | 95 +++++++++++++ app/models/topic.rb | 8 +- app/models/topic_tag.rb | 4 + app/models/topic_user.rb | 5 +- app/models/user.rb | 1 + app/serializers/post_revision_serializer.rb | 9 ++ app/serializers/site_serializer.rb | 2 +- app/serializers/topic_list_item_serializer.rb | 2 +- app/serializers/topic_list_serializer.rb | 2 +- app/serializers/topic_view_serializer.rb | 13 +- db/migrate/20160503205953_create_tags.rb | 27 ++++ lib/discourse_tagging.rb | 114 +++++++--------- lib/post_creator.rb | 12 -- lib/post_revisor.rb | 50 +------ lib/pretty_text.rb | 4 +- lib/search.rb | 8 +- lib/topic_creator.rb | 9 ++ lib/topic_query.rb | 15 +++ lib/topics_bulk_action.rb | 7 +- spec/components/post_creator_spec.rb | 53 ++++++++ spec/components/post_revisor_spec.rb | 126 ++++++++++++++++++ spec/components/pretty_text_spec.rb | 6 +- spec/components/search_spec.rb | 10 ++ spec/components/topic_query_spec.rb | 39 ++++++ spec/components/topics_bulk_action_spec.rb | 52 ++++++++ spec/fabricators/tag_fabricator.rb | 3 + spec/models/tag_spec.rb | 32 +++++ spec/models/tag_user_spec.rb | 87 ++++++++++++ 33 files changed, 726 insertions(+), 186 deletions(-) create mode 100644 app/jobs/onceoff/migrate_tagging_plugin.rb create mode 100644 app/models/tag.rb create mode 100644 app/models/tag_user.rb create mode 100644 app/models/topic_tag.rb create mode 100644 db/migrate/20160503205953_create_tags.rb create mode 100644 spec/fabricators/tag_fabricator.rb create mode 100644 spec/models/tag_spec.rb create mode 100644 spec/models/tag_user_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/rename-tag.js.es6 b/app/assets/javascripts/discourse/controllers/rename-tag.js.es6 index 3c5a7bc2921..0585d4a6cc1 100644 --- a/app/assets/javascripts/discourse/controllers/rename-tag.js.es6 +++ b/app/assets/javascripts/discourse/controllers/rename-tag.js.es6 @@ -1,5 +1,6 @@ import ModalFunctionality from 'discourse/mixins/modal-functionality'; import BufferedContent from 'discourse/mixins/buffered-content'; +import { extractError } from 'discourse/lib/ajax-error'; export default Ember.Controller.extend(ModalFunctionality, BufferedContent, { @@ -14,11 +15,15 @@ export default Ember.Controller.extend(ModalFunctionality, BufferedContent, { performRename() { const tag = this.get('model'), self = this; - tag.update({ id: this.get('buffered.id') }).then(function() { + tag.update({ id: this.get('buffered.id') }).then(function(result) { self.send('closeModal'); - self.transitionToRoute('tags.show', tag.get('id')); - }).catch(function() { - self.flash(I18n.t('generic_error'), 'error'); + if (result.responseJson.tag) { + self.transitionToRoute('tags.show', result.responseJson.tag.id); + } else { + self.flash(extractError(result.responseJson.errors[0]), 'error'); + } + }).catch(function(error) { + self.flash(extractError(error), 'error'); }); } } diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 6b08a54f3b0..23eac88860a 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -29,14 +29,11 @@ class TagsController < ::ApplicationController define_method("show_#{filter}") do @tag_id = DiscourseTagging.clean_tag(params[:tag_id]) - # TODO PERF: doesn't scale: - topics_tagged = TopicCustomField.where(name: DiscourseTagging::TAGS_FIELD_NAME, value: @tag_id).pluck(:topic_id) - page = params[:page].to_i query = TopicQuery.new(current_user, build_topic_list_options) - results = query.send("#{filter}_results").where(id: topics_tagged) + results = query.send("#{filter}_results") if @filter_on_category category_ids = [@filter_on_category.id] + @filter_on_category.subcategories.pluck(:id) @@ -52,8 +49,7 @@ class TagsController < ::ApplicationController @list.more_topics_url = list_by_tag_path(tag_id: @tag_id, page: page + 1) @rss = "tag" - - if @list.topics.size == 0 && !TopicCustomField.where(name: DiscourseTagging::TAGS_FIELD_NAME, value: @tag_id).exists? + if @list.topics.size == 0 && !Tag.where(name: @tag_id).exists? raise Discourse::NotFound else respond_with_list(@list) @@ -68,20 +64,25 @@ class TagsController < ::ApplicationController def update guardian.ensure_can_admin_tags! - new_tag_id = DiscourseTagging.clean_tag(params[:tag][:id]) - if current_user.staff? - DiscourseTagging.rename_tag(current_user, params[:tag_id], new_tag_id) + tag = Tag.find_by_name(params[:tag_id]) + raise Discourse::NotFound if tag.nil? + + new_tag_name = DiscourseTagging.clean_tag(params[:tag][:id]) + tag.name = new_tag_name + if tag.save + StaffActionLogger.new(current_user).log_custom('renamed_tag', previous_value: params[:tag_id], new_value: new_tag_name) + render json: { tag: { id: new_tag_name }} + else + render_json_error tag.errors.full_messages end - render json: { tag: { id: new_tag_id }} end def destroy guardian.ensure_can_admin_tags! - tag_id = params[:tag_id] + tag_name = params[:tag_id] TopicCustomField.transaction do - TopicCustomField.where(name: DiscourseTagging::TAGS_FIELD_NAME, value: tag_id).delete_all - UserCustomField.delete_all(name: ::DiscourseTagging.notification_key(tag_id)) - StaffActionLogger.new(current_user).log_custom('deleted_tag', subject: tag_id) + Tag.find_by_name(tag_name).destroy + StaffActionLogger.new(current_user).log_custom('deleted_tag', subject: tag_name) end render json: success_json end @@ -89,45 +90,45 @@ class TagsController < ::ApplicationController def tag_feed discourse_expires_in 1.minute - tag_id = ::DiscourseTagging.clean_tag(params[:tag_id]) + tag_id = DiscourseTagging.clean_tag(params[:tag_id]) @link = "#{Discourse.base_url}/tags/#{tag_id}" @description = I18n.t("rss_by_tag", tag: tag_id) @title = "#{SiteSetting.title} - #{@description}" @atom_link = "#{Discourse.base_url}/tags/#{tag_id}.rss" - query = TopicQuery.new(current_user) - topics_tagged = TopicCustomField.where(name: DiscourseTagging::TAGS_FIELD_NAME, value: tag_id).pluck(:topic_id) - latest_results = query.latest_results.where(id: topics_tagged) + query = TopicQuery.new(current_user, {tags: [tag_id]}) + latest_results = query.latest_results @topic_list = query.create_list(:by_tag, {}, latest_results) render 'list/list', formats: [:rss] end def search - tags = self.class.tags_by_count(guardian, params.slice(:limit)) + query = self.class.tags_by_count(guardian, params.slice(:limit)) term = params[:q] if term.present? term.gsub!(/[^a-z0-9\.\-\_]*/, '') term.gsub!("_", "\\_") - tags = tags.where('value like ?', "%#{term}%") + query = query.where('tags.name like ?', "%#{term}%") end - tags = tags.count(:value).map {|t, c| { id: t, text: t, count: c } } + tags = query.count.map {|t, c| { id: t, text: t, count: c } } render json: { results: tags } end def notifications - level = current_user.custom_fields[::DiscourseTagging.notification_key(params[:tag_id])] || 1 + tag = Tag.find_by_name(params[:tag_id]) + raise Discourse::NotFound unless tag + level = tag.tag_users.where(user: current_user).first.try(:notification_level) || TagUser.notification_levels[:regular] render json: { tag_notification: { id: params[:tag_id], notification_level: level.to_i } } end def update_notifications + tag = Tag.find_by_name(params[:tag_id]) + raise Discourse::NotFound unless tag level = params[:tag_notification][:notification_level].to_i - - current_user.custom_fields[::DiscourseTagging.notification_key(params[:tag_id])] = level - current_user.save_custom_fields - + TagUser.change(current_user.id, tag.id, level) render json: {notification_level: level} end @@ -147,15 +148,8 @@ class TagsController < ::ApplicationController raise Discourse::NotFound unless SiteSetting.tagging_enabled? end - def self.tags_by_count(guardian, opts=nil) - opts = opts || {} - result = TopicCustomField.where(name: DiscourseTagging::TAGS_FIELD_NAME) - .joins(:topic) - .group(:value) - .limit(opts[:limit] || 5) - .order('COUNT(topic_custom_fields.value) DESC') - - guardian.filter_allowed_categories(result) + def self.tags_by_count(guardian, opts={}) + guardian.filter_allowed_categories(Tag.tags_by_count_query(opts)) end def set_category_from_params @@ -182,6 +176,7 @@ class TagsController < ::ApplicationController topic_ids: param_to_integer_list(:topic_ids), exclude_category_ids: params[:exclude_category_ids], category: params[:category], + tags: [params[:tag_id]], order: params[:order], ascending: params[:ascending], min_posts: params[:min_posts], diff --git a/app/helpers/topics_helper.rb b/app/helpers/topics_helper.rb index a92617e80f6..857e1fc9005 100644 --- a/app/helpers/topics_helper.rb +++ b/app/helpers/topics_helper.rb @@ -17,9 +17,8 @@ module TopicsHelper if (tags = topic.tags).present? tags.each do |tag| - tag_id = DiscourseTagging.clean_tag(tag) - url = "#{Discourse.base_url}/tags/#{tag_id}" - breadcrumb << {url: url, name: tag} + url = "#{Discourse.base_url}/tags/#{tag.name}" + breadcrumb << {url: url, name: tag.name} end end diff --git a/app/jobs/onceoff/migrate_tagging_plugin.rb b/app/jobs/onceoff/migrate_tagging_plugin.rb new file mode 100644 index 00000000000..f0bb91e7e49 --- /dev/null +++ b/app/jobs/onceoff/migrate_tagging_plugin.rb @@ -0,0 +1,16 @@ +module Jobs + + class MigrateTaggingPlugin < Jobs::Onceoff + + def execute_onceoff(args) + all_tags = TopicCustomField.where(name: "tags").select('DISTINCT value').all.map(&:value) + tag_id_lookup = Tag.create(all_tags.map { |tag_name| {name: tag_name} }).inject({}) { |h,v| h[v.name] = v.id; h } + + TopicCustomField.where(name: "tags").find_each do |tcf| + TopicTag.create(topic_id: tcf.topic_id, tag_id: tag_id_lookup[tcf.value] || Tag.find_by_name(tcf.value).try(:id)) + end + end + + end + +end diff --git a/app/models/tag.rb b/app/models/tag.rb new file mode 100644 index 00000000000..bb628d38f9e --- /dev/null +++ b/app/models/tag.rb @@ -0,0 +1,18 @@ +class Tag < ActiveRecord::Base + validates :name, presence: true, uniqueness: true + has_many :topic_tags, dependent: :destroy + has_many :topics, through: :topic_tags + has_many :tag_users + + def self.tags_by_count_query(opts={}) + q = TopicTag.joins(:tag, :topic).group("topic_tags.tag_id, tags.name").order('count_all DESC') + q = q.limit(opts[:limit]) if opts[:limit] + q + end + + def self.top_tags(limit_arg=nil) + self.tags_by_count_query(limit: limit_arg || SiteSetting.max_tags_in_filter_list) + .count + .map {|name, count| name} + end +end diff --git a/app/models/tag_user.rb b/app/models/tag_user.rb new file mode 100644 index 00000000000..1abd68ea0ec --- /dev/null +++ b/app/models/tag_user.rb @@ -0,0 +1,95 @@ +class TagUser < ActiveRecord::Base + belongs_to :tag + belongs_to :user + + def self.notification_levels + TopicUser.notification_levels + end + + def self.change(user_id, tag_id, level) + tag_id = tag_id.id if tag_id.is_a?(::Tag) + user_id = user_id.id if user_id.is_a?(::User) + + tag_id = tag_id.to_i + user_id = user_id.to_i + + tag_user = TagUser.where(user_id: user_id, tag_id: tag_id).first + + if tag_user + return tag_user if tag_user.notification_level == level + tag_user.notification_level = level + tag_user.save + else + tag_user = TagUser.create(user_id: user_id, tag_id: tag_id, notification_level: level) + end + + tag_user + rescue ActiveRecord::RecordNotUnique + # In case of a race condition to insert, do nothing + end + + %w{watch track}.each do |s| + define_singleton_method("auto_#{s}_new_topic") do |topic, new_tags=nil| + tag_ids = topic.tags.pluck(:id) + if !new_tags.nil? && topic.created_at && topic.created_at > 5.days.ago + tag_ids = new_tags.map(&:id) + remove_default_from_topic( topic.id, tag_ids, + TopicUser.notification_levels[:"#{s}ing"], + TopicUser.notification_reasons[:"auto_#{s}_tag"] ) + end + + apply_default_to_topic( topic.id, tag_ids, + TopicUser.notification_levels[:"#{s}ing"], + TopicUser.notification_reasons[:"auto_#{s}_tag"]) + end + end + + def self.apply_default_to_topic(topic_id, tag_ids, level, reason) + sql = <<-SQL + INSERT INTO topic_users(user_id, topic_id, notification_level, notifications_reason_id) + SELECT user_id, :topic_id, :level, :reason + FROM tag_users + WHERE notification_level = :level + AND tag_id in (:tag_ids) + AND NOT EXISTS(SELECT 1 FROM topic_users WHERE topic_id = :topic_id AND user_id = tag_users.user_id) + LIMIT 1 + SQL + + exec_sql(sql, + topic_id: topic_id, + tag_ids: tag_ids, + level: level, + reason: reason + ) + end + + def self.remove_default_from_topic(topic_id, tag_ids, level, reason) + sql = <<-SQL + DELETE FROM topic_users + WHERE topic_id = :topic_id + AND notifications_changed_at IS NULL + AND notification_level = :level + AND notifications_reason_id = :reason + SQL + + if !tag_ids.empty? + sql << <<-SQL + AND NOT EXISTS( + SELECT 1 + FROM tag_users + WHERE tag_users.tag_id in (:tag_ids) + AND tag_users.notification_level = :level + AND tag_users.user_id = topic_users.user_id) + SQL + end + + exec_sql(sql, + topic_id: topic_id, + level: level, + reason: reason, + tag_ids: tag_ids + ) + end + + private_class_method :apply_default_to_topic, :remove_default_from_topic +end diff --git a/app/models/topic.rb b/app/models/topic.rb index 2fc44840516..5694d3c0c0e 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -92,6 +92,9 @@ class Topic < ActiveRecord::Base has_many :allowed_users, through: :topic_allowed_users, source: :user has_many :queued_posts + has_many :topic_tags, dependent: :destroy + has_many :tags, through: :topic_tags + has_one :top_topic belongs_to :user belongs_to :last_poster, class_name: 'User', foreign_key: :last_post_user_id @@ -1042,11 +1045,6 @@ SQL builder.exec.first["count"].to_i end - def tags - result = custom_fields[DiscourseTagging::TAGS_FIELD_NAME] - [result].flatten unless result.blank? - end - def convert_to_public_topic(user) public_topic = TopicConverter.new(self, user).convert_to_public_topic add_small_action(user, "public_topic") if public_topic diff --git a/app/models/topic_tag.rb b/app/models/topic_tag.rb new file mode 100644 index 00000000000..9882ddf2450 --- /dev/null +++ b/app/models/topic_tag.rb @@ -0,0 +1,4 @@ +class TopicTag < ActiveRecord::Base + belongs_to :topic + belongs_to :tag, counter_cache: "topic_count" +end diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index 5d0919de823..78480319c07 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -32,7 +32,10 @@ class TopicUser < ActiveRecord::Base auto_watch_category: 6, auto_mute_category: 7, auto_track_category: 8, - plugin_changed: 9) + plugin_changed: 9, + auto_watch_tag: 10, + auto_mute_tag: 11, + auto_track_tag: 12) end def auto_track(user_id, topic_id, reason) diff --git a/app/models/user.rb b/app/models/user.rb index a410a71161b..4b20b97187d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,6 +18,7 @@ class User < ActiveRecord::Base has_many :notifications, dependent: :destroy has_many :topic_users, dependent: :destroy has_many :category_users, dependent: :destroy + has_many :tag_users, dependent: :destroy has_many :topics has_many :user_open_ids, dependent: :destroy has_many :user_actions, dependent: :destroy diff --git a/app/serializers/post_revision_serializer.rb b/app/serializers/post_revision_serializer.rb index 383d4142e98..d1c93541150 100644 --- a/app/serializers/post_revision_serializer.rb +++ b/app/serializers/post_revision_serializer.rb @@ -41,6 +41,7 @@ class PostRevisionSerializer < ApplicationSerializer end add_compared_field :wiki + add_compared_field :tags def previous_hidden previous["hidden"] @@ -149,6 +150,10 @@ class PostRevisionSerializer < ApplicationSerializer } end + def include_tags_changes? + SiteSetting.tagging_enabled + end + protected def post @@ -184,6 +189,10 @@ class PostRevisionSerializer < ApplicationSerializer end end + if SiteSetting.tagging_enabled + latest_modifications["tags"] = post.topic.tags.map(&:name) + end + post_revisions << PostRevision.new( number: post_revisions.last.number + 1, hidden: post.hidden, diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index d49257b1458..74b4929a387 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -106,7 +106,7 @@ class SiteSerializer < ApplicationSerializer SiteSetting.tagging_enabled && SiteSetting.show_filter_by_tag end def top_tags - DiscourseTagging.top_tags + Tag.top_tags end end diff --git a/app/serializers/topic_list_item_serializer.rb b/app/serializers/topic_list_item_serializer.rb index 5a8ac8d3797..d3763cef165 100644 --- a/app/serializers/topic_list_item_serializer.rb +++ b/app/serializers/topic_list_item_serializer.rb @@ -68,7 +68,7 @@ class TopicListItemSerializer < ListableTopicSerializer SiteSetting.tagging_enabled end def tags - object.tags + object.tags.map(&:name) end end diff --git a/app/serializers/topic_list_serializer.rb b/app/serializers/topic_list_serializer.rb index f932b644d14..c0321c7058b 100644 --- a/app/serializers/topic_list_serializer.rb +++ b/app/serializers/topic_list_serializer.rb @@ -27,7 +27,7 @@ class TopicListSerializer < ApplicationSerializer SiteSetting.tagging_enabled && SiteSetting.show_filter_by_tag end def tags - DiscourseTagging.top_tags + Tag.top_tags end end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index b30d0c9e6e0..c44d44cacc0 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -33,8 +33,7 @@ class TopicViewSerializer < ApplicationSerializer :word_count, :deleted_at, :pending_posts_count, - :user_id, - :tags + :user_id attributes :draft, :draft_key, @@ -55,7 +54,8 @@ class TopicViewSerializer < ApplicationSerializer :is_warning, :chunk_size, :bookmarked, - :message_archived + :message_archived, + :tags # TODO: Split off into proper object / serializer def details @@ -231,4 +231,11 @@ class TopicViewSerializer < ApplicationSerializer scope.is_staff? && NewPostManager.queue_enabled? end + def include_tags? + SiteSetting.tagging_enabled + end + def tags + object.topic.tags.map(&:name) + end + end diff --git a/db/migrate/20160503205953_create_tags.rb b/db/migrate/20160503205953_create_tags.rb new file mode 100644 index 00000000000..6af853f7343 --- /dev/null +++ b/db/migrate/20160503205953_create_tags.rb @@ -0,0 +1,27 @@ +class CreateTags < ActiveRecord::Migration + def change + create_table :tags do |t| + t.string :name, null: false + t.integer :topic_count, null: false, default: 0 + t.timestamps + end + + create_table :topic_tags do |t| + t.references :topic, null: false + t.references :tag, null: false + t.timestamps + end + + create_table :tag_users do |t| + t.references :tag, null: false + t.references :user, null: false + t.integer :notification_level, null: false + t.timestamps + end + + add_index :tags, :name, unique: true + add_index :topic_tags, [:topic_id, :tag_id], unique: true + add_index :tag_users, [:user_id, :tag_id, :notification_level], name: "idx_tag_users_ix1", unique: true + add_index :tag_users, [:tag_id, :user_id, :notification_level], name: "idx_tag_users_ix2", unique: true + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 16d2625c0b0..cb5ce1d7c26 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -8,6 +8,55 @@ module DiscourseTagging # isolate_namespace DiscourseTagging # end + def self.tag_topic_by_names(topic, guardian, tag_names_arg) + if SiteSetting.tagging_enabled + tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || [] + + old_tag_names = topic.tags.map(&:name) || [] + new_tag_names = tag_names - old_tag_names + removed_tag_names = old_tag_names - tag_names + + # Protect staff-only tags + unless guardian.is_staff? + staff_tags = DiscourseTagging.staff_only_tags(new_tag_names) + if staff_tags.present? + topic.errors[:base] << I18n.t("tags.staff_tag_disallowed", tag: staff_tags.join(" ")) + return false + end + + staff_tags = DiscourseTagging.staff_only_tags(removed_tag_names) + if staff_tags.present? + topic.errors[:base] << I18n.t("tags.staff_tag_remove_disallowed", tag: staff_tags.join(" ")) + return false + end + end + + if tag_names.present? + tags = Tag.where(name: tag_names).all + if tags.size < tag_names.size + existing_names = tags.map(&:name) + tag_names.each do |name| + next if existing_names.include?(name) + tags << Tag.create(name: name) + end + end + + auto_notify_for(tags, topic) + + topic.tags = tags + else + auto_notify_for([], topic) + topic.tags = [] + end + end + true + end + + def self.auto_notify_for(tags, topic) + TagUser.auto_watch_new_topic(topic, tags) + TagUser.auto_track_new_topic(topic, tags) + end + def self.clean_tag(tag) tag.downcase.strip[0...SiteSetting.max_tag_length].gsub(TAGS_FILTER_REGEXP, '') end @@ -36,8 +85,7 @@ module DiscourseTagging # If the user can't create tags, remove any tags that don't already exist # TODO: this is doing a full count, it should just check first or use a cache unless guardian.can_create_tag? - tag_count = TopicCustomField.where(name: TAGS_FIELD_NAME, value: tags).group(:value).count - tags.delete_if {|t| !tag_count.has_key?(t) } + tags = Tag.where(name: tags).pluck(:name) end return tags[0...SiteSetting.max_tags_per_topic] @@ -47,68 +95,6 @@ module DiscourseTagging "tags_notification:#{tag_id}" end - def self.auto_notify_for(tags, topic) - # This insert will run up to SiteSetting.max_tags_per_topic times - tags.each do |tag| - key_name_sql = ActiveRecord::Base.sql_fragment("('#{notification_key(tag)}')", tag) - - sql = <<-SQL - INSERT INTO topic_users(user_id, topic_id, notification_level, notifications_reason_id) - SELECT ucf.user_id, - #{topic.id.to_i}, - CAST(ucf.value AS INTEGER), - #{TopicUser.notification_reasons[:plugin_changed]} - FROM user_custom_fields AS ucf - WHERE ucf.name IN #{key_name_sql} - AND NOT EXISTS(SELECT 1 FROM topic_users WHERE topic_id = #{topic.id.to_i} AND user_id = ucf.user_id) - AND CAST(ucf.value AS INTEGER) <> #{TopicUser.notification_levels[:regular]} - SQL - - ActiveRecord::Base.exec_sql(sql) - end - end - - def self.rename_tag(current_user, old_id, new_id) - sql = <<-SQL - UPDATE topic_custom_fields AS tcf - SET value = :new_id - WHERE value = :old_id - AND name = :tags_field_name - AND NOT EXISTS(SELECT 1 - FROM topic_custom_fields - WHERE value = :new_id AND name = :tags_field_name AND topic_id = tcf.topic_id) - SQL - - user_sql = <<-SQL - UPDATE user_custom_fields - SET name = :new_user_tag_id - WHERE name = :old_user_tag_id - AND NOT EXISTS(SELECT 1 - FROM user_custom_fields - WHERE name = :new_user_tag_id) - SQL - - ActiveRecord::Base.transaction do - ActiveRecord::Base.exec_sql(sql, new_id: new_id, old_id: old_id, tags_field_name: TAGS_FIELD_NAME) - TopicCustomField.delete_all(name: TAGS_FIELD_NAME, value: old_id) - ActiveRecord::Base.exec_sql(user_sql, new_user_tag_id: notification_key(new_id), - old_user_tag_id: notification_key(old_id)) - UserCustomField.delete_all(name: notification_key(old_id)) - StaffActionLogger.new(current_user).log_custom('renamed_tag', previous_value: old_id, new_value: new_id) - end - end - - def self.top_tags(limit_arg=nil) - # TODO: cache - # TODO: need an index for this (name,value) - TopicCustomField.where(name: TAGS_FIELD_NAME) - .group(:value) - .limit(limit_arg || SiteSetting.max_tags_in_filter_list) - .order('COUNT(value) DESC') - .count - .map {|name, count| name} - end - def self.muted_tags(user) return [] unless user UserCustomField.where(user_id: user.id, value: TopicUser.notification_levels[:muted]).pluck(:name).map { |x| x[0,17] == "tags_notification" ? x[18..-1] : nil}.compact diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 005932a8e59..47d1540379a 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -147,7 +147,6 @@ class PostCreator track_latest_on_category enqueue_jobs BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post) - auto_notify_for_tags trigger_after_events(@post) @@ -439,17 +438,6 @@ class PostCreator PostJobsEnqueuer.new(@post, @topic, new_topic?, {import_mode: @opts[:import_mode]}).enqueue_jobs end - def auto_notify_for_tags - if SiteSetting.tagging_enabled - tags = DiscourseTagging.tags_for_saving(@opts[:tags], @guardian) - if tags.present? - @topic.custom_fields.update(DiscourseTagging::TAGS_FIELD_NAME => tags) - @topic.save - DiscourseTagging.auto_notify_for(tags, @topic) - end - end - end - def new_topic? @opts[:topic_id].blank? end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index d1fb4bcea8b..ec78713495b 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -72,51 +72,15 @@ class PostRevisor tc.check_result(tc.topic.change_category_to_id(category_id)) end - track_topic_field(:tags_empty_array) do |tc, val| - if val.present? - unless tc.guardian.is_staff? - old_tags = tc.topic.tags || [] - staff_tags = DiscourseTagging.staff_only_tags(old_tags) - if staff_tags.present? - tc.topic.errors[:base] << I18n.t("tags.staff_tag_remove_disallowed", tag: staff_tags.join(" ")) - tc.check_result(false) - next - end - end - - tc.record_change(DiscourseTagging::TAGS_FIELD_NAME, tc.topic.custom_fields[DiscourseTagging::TAGS_FIELD_NAME], nil) - tc.topic.custom_fields.delete(DiscourseTagging::TAGS_FIELD_NAME) - end - end - track_topic_field(:tags) do |tc, tags| - if tags.present? && tc.guardian.can_tag_topics? - tags = DiscourseTagging.tags_for_saving(tags, tc.guardian) - old_tags = tc.topic.tags || [] - - new_tags = tags - old_tags - removed_tags = old_tags - tags - - unless tc.guardian.is_staff? - staff_tags = DiscourseTagging.staff_only_tags(new_tags) - if staff_tags.present? - tc.topic.errors[:base] << I18n.t("tags.staff_tag_disallowed", tag: staff_tags.join(" ")) - tc.check_result(false) - next - end - - staff_tags = DiscourseTagging.staff_only_tags(removed_tags) - if staff_tags.present? - tc.topic.errors[:base] << I18n.t("tags.staff_tag_remove_disallowed", tag: staff_tags.join(" ")) - tc.check_result(false) - next - end + if tc.guardian.can_tag_topics? + prev_tags = tc.topic.tags.map(&:name) + next if tags.blank? && prev_tags.blank? + if !DiscourseTagging.tag_topic_by_names(tc.topic, tc.guardian, tags) + tc.check_result(false) + next end - - tc.record_change(DiscourseTagging::TAGS_FIELD_NAME, tc.topic.custom_fields[DiscourseTagging::TAGS_FIELD_NAME], tags) - tc.topic.custom_fields.update(DiscourseTagging::TAGS_FIELD_NAME => tags) - - DiscourseTagging.auto_notify_for(new_tags, tc.topic) if new_tags.present? + tc.record_change('tags', prev_tags, tags) end end diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index e1341006f38..9551a41972c 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -77,8 +77,8 @@ module PrettyText if !is_tag && category = Category.query_from_hashtag_slug(text) [category.url_with_id, text] - elsif is_tag && tag = TopicCustomField.find_by(name: DiscourseTagging::TAGS_FIELD_NAME, value: text.gsub!("#{tag_postfix}", '')) - ["#{Discourse.base_url}/tags/#{tag.value}", text] + elsif is_tag && tag = Tag.find_by_name(text.gsub!("#{tag_postfix}", '')) + ["#{Discourse.base_url}/tags/#{tag.name}", text] else nil end diff --git a/lib/search.rb b/lib/search.rb index 22cb3f79c0c..4de9c8a528b 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -348,10 +348,10 @@ class Search tags = match.split(",") posts.where("topics.id IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{DiscourseTagging::TAGS_FIELD_NAME}' AND - tc.value in (?) + SELECT DISTINCT(tt.topic_id) + FROM topic_tags tt, tags + WHERE tt.tag_id = tags.id + AND tags.name in (?) )", tags) end diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 53409a1837f..1e683f5b447 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -39,6 +39,8 @@ class TopicCreator def create topic = Topic.new(setup_topic_params) + setup_tags(topic) + DiscourseEvent.trigger(:before_create_topic, topic, self) setup_auto_close_time(topic) @@ -90,8 +92,11 @@ class TopicCreator end unless topic.private_message? + # In order of importance: CategoryUser.auto_watch_new_topic(topic) CategoryUser.auto_track_new_topic(topic) + TagUser.auto_watch_new_topic(topic) + TagUser.auto_track_new_topic(topic) end end @@ -141,6 +146,10 @@ class TopicCreator end end + def setup_tags(topic) + DiscourseTagging.tag_topic_by_names(topic, @guardian, @opts[:tags]) + end + def setup_auto_close_time(topic) return unless @opts[:auto_close_time].present? return unless @guardian.can_moderate?(topic) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 8c9ba0d1a34..65696d64c8a 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -20,6 +20,7 @@ class TopicQuery topic_ids visible category + tags order ascending no_subcategories @@ -451,6 +452,20 @@ class TopicQuery result = result.references(:categories) end + # ALL TAGS: something like this? + # Topic.joins(:tags).where('tags.name in (?)', @options[:tags]).group('topic_id').having('count(*)=?', @options[:tags].size).select('topic_id') + + if @options[:tags] && @options[:tags].size > 0 + result = result.joins(:tags).preload(:tags) + + # ANY of the given tags: + if @options[:tags][0].is_a?(Integer) + result = result.where("tags.id in (?)", @options[:tags]) + else + result = result.where("tags.name in (?)", @options[:tags]) + end + end + result = apply_ordering(result, options) result = result.listable_topics.includes(:category) diff --git a/lib/topics_bulk_action.rb b/lib/topics_bulk_action.rb index 36c17f228ee..2b370911309 100644 --- a/lib/topics_bulk_action.rb +++ b/lib/topics_bulk_action.rb @@ -137,12 +137,11 @@ class TopicsBulkAction topics.each do |t| if guardian.can_edit?(t) if tags.present? - t.custom_fields.update(DiscourseTagging::TAGS_FIELD_NAME => tags) - t.save - DiscourseTagging.auto_notify_for(tags, t) + DiscourseTagging.tag_topic_by_names(t, guardian, tags) else - t.custom_fields.delete(DiscourseTagging::TAGS_FIELD_NAME) + t.tags = [] end + @changed_ids << t.id end end end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 1b3c59399ad..1c647fef7a8 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -265,6 +265,59 @@ describe PostCreator do end + context "tags" do + let(:tag_names) { ['art', 'science', 'dance'] } + let(:creator_with_tags) { PostCreator.new(user, basic_topic_params.merge(tags: tag_names)) } + + context "tagging disabled" do + before do + SiteSetting.tagging_enabled = false + end + + it "doesn't create tags" do + expect { @post = creator_with_tags.create }.to change { Tag.count }.by(0) + expect(@post.topic.tags.size).to eq(0) + end + end + + context "tagging enabled" do + before do + SiteSetting.tagging_enabled = true + end + + context "can create tags" do + before do + SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + it "can create all tags if none exist" do + expect { @post = creator_with_tags.create }.to change { Tag.count }.by( tag_names.size ) + expect(@post.topic.tags.map(&:name).sort).to eq(tag_names.sort) + end + + it "creates missing tags if some exist" do + existing_tag1 = Fabricate(:tag, name: tag_names[0]) + existing_tag1 = Fabricate(:tag, name: tag_names[1]) + expect { @post = creator_with_tags.create }.to change { Tag.count }.by( tag_names.size - 2 ) + expect(@post.topic.tags.map(&:name).sort).to eq(tag_names.sort) + end + end + + context "cannot create tags" do + before do + SiteSetting.min_trust_to_create_tag = 4 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + it "only uses existing tags" do + existing_tag1 = Fabricate(:tag, name: tag_names[1]) + expect { @post = creator_with_tags.create }.to change { Tag.count }.by(0) + expect(@post.topic.tags.map(&:name)).to eq([existing_tag1.name]) + end + end + end + end end context 'when auto-close param is given' do diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index c88e7e78e41..9cf5b3ee594 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -362,5 +362,131 @@ describe PostRevisor do expect(payload[:reload_topic]).to eq(true) end end + + context "tagging" do + context "tagging disabled" do + before do + SiteSetting.tagging_enabled = false + end + + it "doesn't add the tags" do + result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['totally', 'update'] }) + expect(result).to eq(true) + post.reload + expect(post.topic.tags.size).to eq(0) + end + end + + context "tagging enabled" do + before do + SiteSetting.tagging_enabled = true + end + + context "can create tags" do + before do + SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + it "can create all tags if none exist" do + expect { + @result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['totally', 'update'] }) + }.to change { Tag.count }.by(2) + expect(@result).to eq(true) + post.reload + expect(post.topic.tags.map(&:name).sort).to eq(['totally', 'update']) + end + + it "creates missing tags if some exist" do + Fabricate(:tag, name: 'totally') + expect { + @result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['totally', 'update'] }) + }.to change { Tag.count }.by(1) + expect(@result).to eq(true) + post.reload + expect(post.topic.tags.map(&:name).sort).to eq(['totally', 'update']) + end + + it "can remove all tags" do + topic.tags = [Fabricate(:tag, name: "super"), Fabricate(:tag, name: "stuff")] + result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: [] }) + expect(result).to eq(true) + post.reload + expect(post.topic.tags.size).to eq(0) + end + + it "can't add staff-only tags" do + SiteSetting.staff_tags = "important" + result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['important', 'stuff'] }) + expect(result).to eq(false) + expect(post.topic.errors.present?).to eq(true) + end + + it "staff can add staff-only tags" do + SiteSetting.staff_tags = "important" + result = subject.revise!(Fabricate(:admin), { raw: "lets totally update the body", tags: ['important', 'stuff'] }) + expect(result).to eq(true) + post.reload + expect(post.topic.tags.map(&:name).sort).to eq(['important', 'stuff']) + end + + context "with staff-only tags" do + before do + SiteSetting.staff_tags = "important" + topic = post.topic + topic.tags = [Fabricate(:tag, name: "super"), Fabricate(:tag, name: "important"), Fabricate(:tag, name: "stuff")] + end + + it "staff-only tags can't be removed" do + result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['stuff'] }) + expect(result).to eq(false) + expect(post.topic.errors.present?).to eq(true) + post.reload + expect(post.topic.tags.map(&:name).sort).to eq(['important', 'stuff', 'super']) + end + + it "can't remove all tags if some are staff-only" do + result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: [] }) + expect(result).to eq(false) + expect(post.topic.errors.present?).to eq(true) + post.reload + expect(post.topic.tags.map(&:name).sort).to eq(['important', 'stuff', 'super']) + end + + it "staff-only tags can be removed by staff" do + result = subject.revise!(Fabricate(:admin), { raw: "lets totally update the body", tags: ['stuff'] }) + expect(result).to eq(true) + post.reload + expect(post.topic.tags.map(&:name)).to eq(['stuff']) + end + + it "staff can remove all tags" do + result = subject.revise!(Fabricate(:admin), { raw: "lets totally update the body", tags: [] }) + expect(result).to eq(true) + post.reload + expect(post.topic.tags.size).to eq(0) + end + end + + end + + context "cannot create tags" do + before do + SiteSetting.min_trust_to_create_tag = 4 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + it "only uses existing tags" do + Fabricate(:tag, name: 'totally') + expect { + @result = subject.revise!(Fabricate(:user), { raw: "lets totally update the body", tags: ['totally', 'update'] }) + }.to_not change { Tag.count } + expect(@result).to eq(true) + post.reload + expect(post.topic.tags.map(&:name)).to eq(['totally']) + end + end + end + end end end diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index d66de21387c..430bf7bad81 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -417,12 +417,12 @@ HTML describe "tag and category links" do it "produces tag links" do - # TODO where is our tags table? - TopicCustomField.create!(topic_id: 1, name: DiscourseTagging::TAGS_FIELD_NAME, value: "known") - # TODO does it make sense to generate hashtags for tags that are missing in action? + Fabricate(:topic, {tags: [Fabricate(:tag, name: 'known')]}) expect(PrettyText.cook(" #unknown::tag #known::tag")).to match_html("

#unknown::tag #known

") end + # TODO does it make sense to generate hashtags for tags that are missing in action? + end end diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 8d14de9411d..128c7764629 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -553,6 +553,16 @@ describe Search do expect(Search.execute('testing again #category-24:sub-category').posts.length).to eq(1) expect(Search.execute('testing again #sub-category').posts.length).to eq(0) end + + it "can find with tag" do + topic1 = Fabricate(:topic, title: 'Could not, would not, on a boat') + topic1.tags = [Fabricate(:tag, name: 'eggs'), Fabricate(:tag, name: 'ham')] + post1 = Fabricate(:post, topic: topic1) + post2 = Fabricate(:post, topic: topic1, raw: "It probably doesn't help that they're green...") + + expect(Search.execute('green tags:eggs').posts.map(&:id)).to eq([post2.id]) + expect(Search.execute('green tags:plants').posts.size).to eq(0) + end end it 'can parse complex strings using ts_query helper' do diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index fcb0bb1f8a6..663fdef532d 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -112,8 +112,47 @@ describe TopicQuery do end end + end + context 'tag filter' do + let(:tag) { Fabricate(:tag) } + let(:other_tag) { Fabricate(:tag) } + it "returns topics with the tag when filtered to it" do + tagged_topic1 = Fabricate(:topic, {tags: [tag]}) + tagged_topic2 = Fabricate(:topic, {tags: [other_tag]}) + tagged_topic3 = Fabricate(:topic, {tags: [tag, other_tag]}) + no_tags_topic = Fabricate(:topic) + + expect(TopicQuery.new(moderator, tags: [tag.name]).list_latest.topics.map(&:id).sort).to eq([tagged_topic1.id, tagged_topic3.id].sort) + expect(TopicQuery.new(moderator, tags: [tag.id]).list_latest.topics.map(&:id).sort).to eq([tagged_topic1.id, tagged_topic3.id].sort) + + two_tag_topic = TopicQuery.new(moderator, tags: [tag.name]).list_latest.topics.find { |t| t.id == tagged_topic3.id } + expect(two_tag_topic.tags.size).to eq(2) + + # topics with ANY of the given tags: + expect(TopicQuery.new(moderator, tags: [tag.name, other_tag.name]).list_latest.topics.map(&:id).sort).to eq([tagged_topic1.id, tagged_topic2.id, tagged_topic3.id].sort) + expect(TopicQuery.new(moderator, tags: [tag.id, other_tag.id]).list_latest.topics.map(&:id).sort).to eq([tagged_topic1.id, tagged_topic2.id, tagged_topic3.id].sort) + + # TODO: topics with ALL of the given tags: + # expect(TopicQuery.new(moderator, tags: [tag.name, other_tag.name]).list_latest.topics.map(&:id)).to eq([tagged_topic3.id].sort) + # expect(TopicQuery.new(moderator, tags: [tag.id, other_tag.id]).list_latest.topics.map(&:id)).to eq([tagged_topic3.id].sort) + end + + context "and categories too" do + let(:category1) { Fabricate(:category) } + let(:category2) { Fabricate(:category) } + + it "returns topics in the given category with the given tag" do + tagged_topic1 = Fabricate(:topic, {category: category1, tags: [tag]}) + tagged_topic2 = Fabricate(:topic, {category: category2, tags: [tag]}) + tagged_topic3 = Fabricate(:topic, {category: category1, tags: [tag, other_tag]}) + no_tags_topic = Fabricate(:topic, {category: category1}) + + expect(TopicQuery.new(moderator, category: category1.id, tags: [tag.name]).list_latest.topics.map(&:id).sort).to eq([tagged_topic1.id, tagged_topic3.id].sort) + expect(TopicQuery.new(moderator, category: category2.id, tags: [other_tag.name]).list_latest.topics.size).to eq(0) + end + end end context 'muted categories' do diff --git a/spec/components/topics_bulk_action_spec.rb b/spec/components/topics_bulk_action_spec.rb index 0003ee2dc5c..6aa99fcd106 100644 --- a/spec/components/topics_bulk_action_spec.rb +++ b/spec/components/topics_bulk_action_spec.rb @@ -180,4 +180,56 @@ describe TopicsBulkAction do end end end + + describe "change_tags" do + let(:topic) { Fabricate(:topic) } + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + + before do + SiteSetting.tagging_enabled = true + SiteSetting.min_trust_level_to_tag_topics = 0 + topic.tags = [tag1, tag2] + end + + it "can change the tags, and can create new tags" do + SiteSetting.min_trust_to_create_tag = 0 + tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_tags', tags: ['newtag', tag1.name]) + topic_ids = tba.perform! + expect(topic_ids).to eq([topic.id]) + topic.reload + expect(topic.tags.map(&:name).sort).to eq(['newtag', tag1.name].sort) + end + + it "can change the tags but not create new ones" do + SiteSetting.min_trust_to_create_tag = 4 + tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_tags', tags: ['newtag', tag1.name]) + topic_ids = tba.perform! + expect(topic_ids).to eq([topic.id]) + topic.reload + expect(topic.tags.map(&:name)).to eq([tag1.name]) + end + + it "can remove all tags" do + tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_tags', tags: []) + topic_ids = tba.perform! + expect(topic_ids).to eq([topic.id]) + topic.reload + expect(topic.tags.size).to eq(0) + end + + context "when user can't edit topic" do + before do + Guardian.any_instance.expects(:can_edit?).returns(false) + end + + it "doesn't change the tags" do + tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_tags', tags: ['newtag', tag1.name]) + topic_ids = tba.perform! + expect(topic_ids).to eq([]) + topic.reload + expect(topic.tags.map(&:name)).to eq([tag1.name, tag2.name]) + end + end + end end diff --git a/spec/fabricators/tag_fabricator.rb b/spec/fabricators/tag_fabricator.rb new file mode 100644 index 00000000000..45940081a1c --- /dev/null +++ b/spec/fabricators/tag_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:tag) do + name { sequence(:name) { |i| "tag#{i}" } } +end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb new file mode 100644 index 00000000000..22cda4a4b99 --- /dev/null +++ b/spec/models/tag_spec.rb @@ -0,0 +1,32 @@ +require 'rails_helper' + +describe Tag do + describe '#tags_by_count_query' do + it "returns empty hash if nothing is tagged" do + expect(described_class.tags_by_count_query.count).to eq({}) + end + + context "with some tagged topics" do + before do + @topics = [] + @tags = [] + 3.times { @topics << Fabricate(:topic) } + 2.times { @tags << Fabricate(:tag) } + @topics[0].tags << @tags[0] + @topics[0].tags << @tags[1] + @topics[1].tags << @tags[0] + end + + it "returns tag names with topic counts in a hash" do + counts = described_class.tags_by_count_query.count + expect(counts[@tags[0].name]).to eq(2) + expect(counts[@tags[1].name]).to eq(1) + end + + it "can be used to filter before doing the count" do + counts = described_class.tags_by_count_query.where("topics.id = ?", @topics[1].id).count + expect(counts).to eq({@tags[0].name => 1}) + end + end + end +end diff --git a/spec/models/tag_user_spec.rb b/spec/models/tag_user_spec.rb new file mode 100644 index 00000000000..5b1753bbfbc --- /dev/null +++ b/spec/models/tag_user_spec.rb @@ -0,0 +1,87 @@ +# encoding: utf-8 + +require 'rails_helper' +require_dependency 'post_creator' + +describe TagUser do + + context "integration" do + before do + ActiveRecord::Base.observers.enable :all + SiteSetting.tagging_enabled = true + SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + let(:user) { Fabricate(:user) } + + let(:watched_tag) { Fabricate(:tag) } + let(:muted_tag) { Fabricate(:tag) } + let(:tracked_tag) { Fabricate(:tag) } + + context "with some tag notification settings" do + before do + TagUser.create!(user: user, tag: watched_tag, notification_level: TagUser.notification_levels[:watching]) + TagUser.create!(user: user, tag: muted_tag, notification_level: TagUser.notification_levels[:muted]) + TagUser.create!(user: user, tag: tracked_tag, notification_level: TagUser.notification_levels[:tracking]) + end + + it "sets notification levels correctly" do + watched_post = create_post(tags: [watched_tag.name]) + muted_post = create_post(tags: [muted_tag.name]) + tracked_post = create_post(tags: [tracked_tag.name]) + + expect(Notification.where(user_id: user.id, topic_id: watched_post.topic_id).count).to eq 1 + expect(Notification.where(user_id: user.id, topic_id: tracked_post.topic_id).count).to eq 0 + + tu = TopicUser.get(tracked_post.topic, user) + expect(tu.notification_level).to eq TopicUser.notification_levels[:tracking] + expect(tu.notifications_reason_id).to eq TopicUser.notification_reasons[:auto_track_tag] + end + + it "sets notification level to the highest one if there are multiple tags" do + post = create_post(tags: [muted_tag.name, tracked_tag.name, watched_tag.name]) + expect(Notification.where(user_id: user.id, topic_id: post.topic_id).count).to eq 1 + tu = TopicUser.get(post.topic, user) + expect(tu.notification_level).to eq TopicUser.notification_levels[:watching] + expect(tu.notifications_reason_id).to eq TopicUser.notification_reasons[:auto_watch_tag] + end + + it "can start watching after tag has been added" do + post = create_post + expect(TopicUser.get(post.topic, user)).to be_blank + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), [watched_tag.name]) + tu = TopicUser.get(post.topic, user) + expect(tu.notification_level).to eq(TopicUser.notification_levels[:watching]) + end + + it "can start watching after tag has changed" do + post = create_post(tags: [Fabricate(:tag).name]) + expect(TopicUser.get(post.topic, user)).to be_blank + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), [watched_tag.name]) + tu = TopicUser.get(post.topic, user) + expect(tu.notification_level).to eq(TopicUser.notification_levels[:watching]) + end + + it "can stop watching after tag has changed" do + post = create_post(tags: [watched_tag.name]) + expect(TopicUser.get(post.topic, user)).to be_present + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), [Fabricate(:tag).name]) + expect(TopicUser.get(post.topic, user)).to be_blank + end + + it "can stop watching after tags have been removed" do + post = create_post(tags: [muted_tag.name, tracked_tag.name, watched_tag.name]) + expect(TopicUser.get(post.topic, user)).to be_present + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), []) + expect(TopicUser.get(post.topic, user)).to be_blank + end + + it "is destroyed when a user is deleted" do + expect(TagUser.where(user_id: user.id).count).to eq(3) + user.destroy! + expect(TagUser.where(user_id: user.id).count).to eq(0) + end + end + end +end