From 4161ee210a086bad6217f160f3a896dc2593a921 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 8 Jul 2016 12:58:18 +1000 Subject: [PATCH] FEATURE: improved tag and category watching and tracking - present tags watched on the user prefs page - automatically watch or unwatch old topics based on watch status New watching and tracking logic takes care of handling old topics (either with or without read state) When you watch a topic you now watch historically Also removes confusing warnings from user. --- .../discourse/components/tag-chooser.js.es6 | 9 +- .../discourse/controllers/preferences.js.es6 | 24 --- .../javascripts/discourse/models/user.js.es6 | 11 +- .../discourse/templates/user/preferences.hbs | 21 +++ app/assets/stylesheets/common/base/user.scss | 9 + app/controllers/users_controller.rb | 4 - app/models/category_user.rb | 149 +++++++++++----- app/models/tag_user.rb | 167 ++++++++++++------ app/models/topic.rb | 15 +- app/models/topic_user.rb | 64 +++++-- app/serializers/user_serializer.rb | 14 ++ app/services/post_alerter.rb | 43 ++++- app/services/user_updater.rb | 10 ++ config/locales/client.en.yml | 17 +- lib/discourse_tagging.rb | 13 +- lib/topic_creator.rb | 8 - spec/models/category_user_spec.rb | 76 ++++++-- spec/models/tag_user_spec.rb | 131 +++++++++++--- spec/services/user_updater_spec.rb | 22 +++ 19 files changed, 583 insertions(+), 224 deletions(-) diff --git a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 index 4f33e3b6d..37d7f5fd2 100644 --- a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 +++ b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 @@ -36,8 +36,13 @@ export default Ember.TextField.extend({ const site = this.site, self = this, filterRegexp = new RegExp(this.site.tags_filter_regexp, "g"); + var limit = this.siteSettings.max_tags_per_topic; + if (this.get('allowCreate') !== false) { + this.set('allowCreate', site.get('can_create_tag')); + } + if (this.get('unlimitedTagCount')) { limit = null; } else if (this.get('limit')) { @@ -46,7 +51,7 @@ export default Ember.TextField.extend({ this.$().select2({ tags: true, - placeholder: I18n.t(this.get('placeholderKey') || 'tagging.choose_for_topic'), + placeholder: this.get('placeholder') === "" ? "" : I18n.t(this.get('placeholderKey') || 'tagging.choose_for_topic'), maximumInputLength: this.siteSettings.max_tag_length, maximumSelectionSize: limit, initSelection(element, callback) { @@ -73,7 +78,7 @@ export default Ember.TextField.extend({ term = term.replace(filterRegexp, '').trim(); // No empty terms, make sure the user has permission to create the tag - if (!term.length || !site.get('can_create_tag')) { return; } + if (!term.lenght || !this.get('allowCreate')) return; if ($(data).filter(function() { return this.text.localeCompare(term) === 0; diff --git a/app/assets/javascripts/discourse/controllers/preferences.js.es6 b/app/assets/javascripts/discourse/controllers/preferences.js.es6 index d37c428d6..dd9da3e4f 100644 --- a/app/assets/javascripts/discourse/controllers/preferences.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences.js.es6 @@ -2,7 +2,6 @@ import { setting } from 'discourse/lib/computed'; import CanCheckEmails from 'discourse/mixins/can-check-emails'; import { popupAjaxError } from 'discourse/lib/ajax-error'; import computed from "ember-addons/ember-computed-decorators"; -import { categoryBadgeHTML } from "discourse/helpers/category-link"; export default Ember.Controller.extend(CanCheckEmails, { @@ -136,24 +135,6 @@ export default Ember.Controller.extend(CanCheckEmails, { const model = this.get('model'); - - // watched status changes warn user - const changedWatch = model.changedCategoryNotifications("watched"); - - if (changedWatch.remove.length > 0 && !this.get("warnedRemoveWatch")) { - var categories = Discourse.Category.findByIds(changedWatch.remove).map((cat) => { - return categoryBadgeHTML(cat); - }).join(" "); - bootbox.confirm(I18n.t('user.warn_unwatch.message', {categories: categories}), - I18n.t('user.warn_unwatch.no_value', {count: changedWatch.remove.length}), I18n.t('user.warn_unwatch.yes_value'), - (yes)=>{ - this.set('unwatchCategoryTopics', yes ? changedWatch.remove : false); - this.send('save'); - }); - this.set("warnedRemoveWatch", true); - return; - } - const userFields = this.get('userFields'); // Update the user fields @@ -169,9 +150,6 @@ export default Ember.Controller.extend(CanCheckEmails, { // Cook the bio for preview model.set('name', this.get('newNameInput')); var options = {}; - if (this.get('warnedRemoveWatch') && this.get('unwatchCategoryTopics')) { - options["unwatchCategoryTopics"] = this.get("unwatchCategoryTopics"); - } return model.save(options).then(() => { if (Discourse.User.currentProp('id') === model.get('id')) { @@ -179,8 +157,6 @@ export default Ember.Controller.extend(CanCheckEmails, { } model.set('bio_cooked', Discourse.Markdown.cook(Discourse.Markdown.sanitize(model.get('bio_raw')))); this.set('saved', true); - this.set("unwatchTopics", false); - this.set('warnedRemoveWatch', false); }).catch(popupAjaxError); }, diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 7b5c9e479..5afa76eed 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -141,7 +141,7 @@ const User = RestModel.extend({ return Discourse.User.create(this.getProperties(Object.keys(this))); }, - save(options) { + save() { const data = this.getProperties( 'bio_raw', 'website', @@ -152,7 +152,10 @@ const User = RestModel.extend({ 'user_fields', 'muted_usernames', 'profile_background', - 'card_background' + 'card_background', + 'muted_tags', + 'tracked_tags', + 'watched_tags' ); [ 'email_always', @@ -192,10 +195,6 @@ const User = RestModel.extend({ data['edit_history_public'] = this.get('user_option.edit_history_public'); } - if (options && options.unwatchCategoryTopics) { - data.unwatch_category_topics = options.unwatchCategoryTopics; - } - // TODO: We can remove this when migrated fully to rest model. this.set('isSaving', true); return Discourse.ajax(`/users/${this.get('username_lower')}`, { diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index e33b03afe..9892dcdeb 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -274,6 +274,27 @@ + {{#if siteSettings.tagging_enabled}} +
+ +
+ + {{tag-chooser tags=model.watched_tags blacklist=selectedTags allowCreate=false placeholder=""}} +
+
{{i18n 'user.watched_tags_instructions'}}
+
+ + {{tag-chooser tags=model.tracked_tags blacklist=selectedTags allowCreate=false placeholder=""}} +
+
{{i18n 'user.tracked_tags_instructions'}}
+
+ + {{tag-chooser tags=model.muted_tags blacklist=selectedTags allowCreate=false placeholder=""}} +
+
{{i18n 'user.muted_tags_instructions'}}
+
+ {{/if}} +
diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index ee5a7537f..f24f54400 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -279,3 +279,12 @@ and (max-width : 600px) { width: 100%; } } + +.user-preferences .tags .select2-container-multi { + border: 1px solid dark-light-diff($primary, $secondary, 90%, -60%); + width: 540px; + border-radius: 0; + .select2-choices { + border: none; + } +} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8ebc13380..32785d480 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -89,10 +89,6 @@ class UsersController < ApplicationController user = fetch_user_from_params guardian.ensure_can_edit!(user) - if params[:unwatch_category_topics] - TopicUser.unwatch_categories!(user, params[:unwatch_category_topics]) - end - if params[:user_fields].present? params[:custom_fields] = {} unless params[:custom_fields].present? diff --git a/app/models/category_user.rb b/app/models/category_user.rb index 19d43b3d4..7591e8cd3 100644 --- a/app/models/category_user.rb +++ b/app/models/category_user.rb @@ -20,85 +20,141 @@ class CategoryUser < ActiveRecord::Base [notification_levels[:watching], notification_levels[:watching_first_post]] end - %w{watch track}.each do |s| - define_singleton_method("auto_#{s}_new_topic") do |topic, new_category=nil| - category_id = topic.category_id - - if new_category && topic.created_at > 5.days.ago - # we want to apply default of the new category - category_id = new_category.id - # remove defaults from previous category - remove_default_from_topic(topic.id, category_id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"]) - end - - apply_default_to_topic(topic.id, category_id, TopicUser.notification_levels[:"#{s}ing"], TopicUser.notification_reasons[:"auto_#{s}_category"]) - end - end - def self.batch_set(user, level, category_ids) records = CategoryUser.where(user: user, notification_level: notification_levels[level]) old_ids = records.pluck(:category_id) + changed = false category_ids = Category.where('id in (?)', category_ids).pluck(:id) remove = (old_ids - category_ids) if remove.present? records.where('category_id in (?)', remove).destroy_all + changed = true end (category_ids - old_ids).each do |id| CategoryUser.create!(user: user, category_id: id, notification_level: notification_levels[level]) + changed = true end + + if changed + auto_watch(user_id: user.id) + auto_track(user_id: user.id) + end + + changed end def self.set_notification_level_for_category(user, level, category_id) record = CategoryUser.where(user: user, category_id: category_id).first + return if record && record.notification_level = level + if record.present? record.notification_level = level record.save! else CategoryUser.create!(user: user, category_id: category_id, notification_level: level) end + + auto_watch(user_id: user.id) + auto_track(user_id: user.id) end - def self.apply_default_to_topic(topic_id, category_id, level, reason) - # Can not afford to slow down creation of topics when a pile of users are watching new topics, reverting to SQL for max perf here - sql = <<-SQL - INSERT INTO topic_users(user_id, topic_id, notification_level, notifications_reason_id) - SELECT user_id, :topic_id, :level, :reason - FROM category_users - WHERE notification_level = :level - AND category_id = :category_id - AND NOT EXISTS(SELECT 1 FROM topic_users WHERE topic_id = :topic_id AND user_id = category_users.user_id) - SQL + def self.auto_track(opts={}) - exec_sql(sql, - topic_id: topic_id, - category_id: category_id, - level: level, - reason: reason - ) + builder = SqlBuilder.new < 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 + def self.auto_watch(opts) + builder = SqlBuilder.new <= 0 && auto_track_after <= (attrs[:total_msecs_viewed].to_i || 0) - attrs[:notification_level] ||= notification_levels[:tracking] - end - - TopicUser.create(attrs.merge!(user_id: user_id, topic_id: topic_id, first_visited_at: now ,last_visited_at: now)) + create_missing_record(user_id, topic_id, attrs) else observe_after_save_callbacks_for topic_id, user_id end @@ -153,12 +145,64 @@ SQL # In case of a race condition to insert, do nothing end + def create_missing_record(user_id, topic_id, attrs) + now = DateTime.now + + unless attrs[:notification_level] + category_notification_level = CategoryUser.where(user_id: user_id) + .where("category_id IN (SELECT category_id FROM topics WHERE id = :id)", id: topic_id) + .where("notification_level IN (:levels)", levels: [CategoryUser.notification_levels[:watching], + CategoryUser.notification_levels[:tracking]]) + .order("notification_level DESC") + .limit(1) + .pluck(:notification_level) + .first + + tag_notification_level = TagUser.where(user_id: user_id) + .where("tag_id IN (SELECT tag_id FROM topic_tags WHERE topic_id = :id)", id: topic_id) + .where("notification_level IN (:levels)", levels: [CategoryUser.notification_levels[:watching], + CategoryUser.notification_levels[:tracking]]) + .order("notification_level DESC") + .limit(1) + .pluck(:notification_level) + .first + + if category_notification_level && !(tag_notification_level && (tag_notification_level > category_notification_level)) + attrs[:notification_level] = category_notification_level + attrs[:notifications_changed_at] = DateTime.now + attrs[:notifications_reason_id] = category_notification_level == CategoryUser.notification_levels[:watching] ? + TopicUser.notification_reasons[:auto_watch_category] : + TopicUser.notification_reasons[:auto_track_category] + + elsif tag_notification_level + attrs[:notification_level] = tag_notification_level + attrs[:notifications_changed_at] = DateTime.now + attrs[:notifications_reason_id] = tag_notification_level == TagUser.notification_levels[:watching] ? + TopicUser.notification_reasons[:auto_watch_tag] : + TopicUser.notification_reasons[:auto_track_tag] + end + + + end + + unless attrs[:notification_level] + auto_track_after = UserOption.where(user_id: user_id).pluck(:auto_track_topics_after_msecs).first + auto_track_after ||= SiteSetting.default_other_auto_track_topics_after_msecs + + if auto_track_after >= 0 && auto_track_after <= (attrs[:total_msecs_viewed].to_i || 0) + attrs[:notification_level] ||= notification_levels[:tracking] + end + end + + TopicUser.create(attrs.merge!(user_id: user_id, topic_id: topic_id, first_visited_at: now ,last_visited_at: now)) + end + def track_visit!(topic_id, user_id) now = DateTime.now rows = TopicUser.where(topic_id: topic_id, user_id: user_id).update_all(last_visited_at: now) if rows == 0 - TopicUser.create(topic_id: topic_id, user_id: user_id, last_visited_at: now, first_visited_at: now) + change(user_id, topic_id, last_visited_at: now, first_visited_at: now) else observe_after_save_callbacks_for(topic_id, user_id) end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index d06899f26..3b59d49dc 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -83,6 +83,9 @@ class UserSerializer < BasicUserSerializer private_attributes :locale, :muted_category_ids, + :watched_tags, + :tracked_tags, + :muted_tags, :tracked_category_ids, :watched_category_ids, :private_messages_stats, @@ -246,6 +249,17 @@ class UserSerializer < BasicUserSerializer ### ### PRIVATE ATTRIBUTES ### + def muted_tags + TagUser.lookup(object, :muted).joins(:tag).pluck('tags.name') + end + + def tracked_tags + TagUser.lookup(object, :tracking).joins(:tag).pluck('tags.name') + end + + def watched_tags + TagUser.lookup(object, :watching).joins(:tag).pluck('tags.name') + end def muted_category_ids CategoryUser.lookup(object, :muted).pluck(:category_id) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index bd194ea9b..6e480951c 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -452,14 +452,47 @@ class PostAlerter end def notify_post_users(post, notified) - notify = TopicUser.where(topic_id: post.topic_id) - .where(notification_level: TopicUser.notification_levels[:watching]) + return unless post.topic + + condition = <\#\?\&\s]/ - # class Engine < ::Rails::Engine - # engine_name "discourse_tagging" - # isolate_namespace DiscourseTagging - # end def self.tag_topic_by_names(topic, guardian, tag_names_arg) if SiteSetting.tagging_enabled @@ -43,13 +39,11 @@ module DiscourseTagging end end - auto_notify_for(tags, topic) - topic.tags = tags else - auto_notify_for([], topic) topic.tags = [] end + topic.tags_changed=true end true end @@ -146,11 +140,6 @@ module DiscourseTagging query 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 diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 1e683f5b4..da0f7dcc1 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -90,14 +90,6 @@ class TopicCreator topic.notifier.send(action, gu.user_id) end 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 def setup_topic_params diff --git a/spec/models/category_user_spec.rb b/spec/models/category_user_spec.rb index 9d7121c59..3386cc35c 100644 --- a/spec/models/category_user_spec.rb +++ b/spec/models/category_user_spec.rb @@ -5,6 +5,14 @@ require_dependency 'post_creator' describe CategoryUser do + def tracking + CategoryUser.notification_levels[:tracking] + end + + def regular + CategoryUser.notification_levels[:regular] + end + it 'allows batch set' do user = Fabricate(:user) category1 = Fabricate(:category) @@ -22,6 +30,21 @@ describe CategoryUser do expect(watching.count).to eq 1 end + it 'should correctly auto_track' do + tracking_user = Fabricate(:user) + user = Fabricate(:user) + topic = Fabricate(:post).topic + + TopicUser.change(user.id, topic.id, total_msecs_viewed: 10) + TopicUser.change(tracking_user.id, topic.id, total_msecs_viewed: 10) + + CategoryUser.create!(user: tracking_user, category: topic.category, notification_level: tracking) + CategoryUser.auto_track(user_id: tracking_user.id) + + expect(TopicUser.get(topic, tracking_user).notification_level).to eq(tracking) + expect(TopicUser.get(topic, user).notification_level).to eq(regular) + end + context 'integration' do before do @@ -35,6 +58,8 @@ describe CategoryUser do user = Fabricate(:user) + early_watched_post = create_post(category: watched_category) + CategoryUser.create!(user: user, category: watched_category, notification_level: CategoryUser.notification_levels[:watching]) CategoryUser.create!(user: user, category: muted_category, notification_level: CategoryUser.notification_levels[:muted]) CategoryUser.create!(user: user, category: tracked_category, notification_level: CategoryUser.notification_levels[:tracking]) @@ -43,28 +68,36 @@ describe CategoryUser do _muted_post = create_post(category: muted_category) tracked_post = create_post(category: tracked_category) + create_post(topic_id: early_watched_post.topic_id) + 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: early_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 + # we must create a record so tracked flicks over + TopicUser.change(user.id, tracked_post.topic_id, total_msecs_viewed: 10) 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_category] end - it "watches categories that have been changed" do + it "topics that move to a tracked category should auto track" do user = Fabricate(:user) - watched_category = Fabricate(:category) - CategoryUser.create!(user: user, category: watched_category, notification_level: CategoryUser.notification_levels[:watching]) - post = create_post - expect(TopicUser.get(post.topic, user)).to be_blank + first_post = create_post + tracked_category = first_post.topic.category - # Now, change the topic's category - post.topic.change_category_to_id(watched_category.id) - tu = TopicUser.get(post.topic, user) - expect(tu.notification_level).to eq TopicUser.notification_levels[:watching] + TopicUser.change(user.id, first_post.topic_id, total_msecs_viewed: 10) + tu = TopicUser.get(first_post.topic, user) + expect(tu.notification_level).to eq TopicUser.notification_levels[:regular] + + CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:tracking], tracked_category.id) + + tu = TopicUser.get(first_post.topic, user) + expect(tu.notification_level).to eq TopicUser.notification_levels[:tracking] end + it "unwatches categories that have been changed" do user = Fabricate(:user) watched_category = Fabricate(:category) @@ -72,12 +105,15 @@ describe CategoryUser do post = create_post(category: watched_category) tu = TopicUser.get(post.topic, user) + + # we start watching cause a notification is sent to the watching user + # this position sent is tracking in topic users expect(tu.notification_level).to eq TopicUser.notification_levels[:watching] # Now, change the topic's category unwatched_category = Fabricate(:category) post.topic.change_category_to_id(unwatched_category.id) - expect(TopicUser.get(post.topic, user)).to be_blank + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:tracking] end it "does not delete TopicUser record when topic category is changed, and new category has same notification level" do @@ -87,16 +123,26 @@ describe CategoryUser do user = Fabricate(:user) watched_category_1 = Fabricate(:category) watched_category_2 = Fabricate(:category) + category_3 = Fabricate(:category) + + post = create_post(category: watched_category_1) + CategoryUser.create!(user: user, category: watched_category_1, notification_level: CategoryUser.notification_levels[:watching]) CategoryUser.create!(user: user, category: watched_category_2, notification_level: CategoryUser.notification_levels[:watching]) - post = create_post(category: watched_category_1) - tu = TopicUser.get(post.topic, user) - expect(tu.notification_level).to eq TopicUser.notification_levels[:watching] + # we must have a topic user record otherwise it will be watched implicitly + TopicUser.change(user.id, post.topic_id, total_msecs_viewed: 10) + + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:watching] + + post.topic.change_category_to_id(category_3.id) + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:tracking] - # Now, change the topic's category post.topic.change_category_to_id(watched_category_2.id) - expect(TopicUser.get(post.topic, user)).to eq tu + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:watching] + + post.topic.change_category_to_id(watched_category_1.id) + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:watching] end it "deletes TopicUser record when topic category is changed, and new category has different notification level" do diff --git a/spec/models/tag_user_spec.rb b/spec/models/tag_user_spec.rb index 5b1753bbf..cb01f6985 100644 --- a/spec/models/tag_user_spec.rb +++ b/spec/models/tag_user_spec.rb @@ -4,13 +4,72 @@ require 'rails_helper' require_dependency 'post_creator' describe TagUser do + before do + SiteSetting.tagging_enabled = true + SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + def regular + TagUser.notification_levels[:regular] + end + + def tracking + TagUser.notification_levels[:tracking] + end + + def watching + TagUser.notification_levels[:watching] + end + + context "change" do + it "watches or tracks on change" do + user = Fabricate(:user) + tag = Fabricate(:tag) + post = create_post(tags: [tag.name]) + topic = post.topic + + TopicUser.change(user.id, topic.id, total_msecs_viewed: 1) + + TagUser.change(user.id, tag.id, tracking) + expect(TopicUser.get(topic, user).notification_level).to eq tracking + + TagUser.change(user.id, tag.id, watching) + expect(TopicUser.get(topic, user).notification_level).to eq watching + + TagUser.change(user.id, tag.id, regular) + expect(TopicUser.get(topic, user).notification_level).to eq tracking + end + end + + context "batch_set" do + it "watches and unwatches tags correctly" do + + user = Fabricate(:user) + tag = Fabricate(:tag) + post = create_post(tags: [tag.name]) + topic = post.topic + + # we need topic user record to ensure watch picks up other wise it is implicit + TopicUser.change(user.id, topic.id, total_msecs_viewed: 1) + + TagUser.batch_set(user, :tracking, [tag.name]) + + expect(TopicUser.get(topic, user).notification_level).to eq tracking + + TagUser.batch_set(user, :watching, [tag.name]) + + expect(TopicUser.get(topic, user).notification_level).to eq watching + + TagUser.batch_set(user, :watching, []) + + expect(TopicUser.get(topic, user).notification_level).to eq tracking + end + end 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) } @@ -20,28 +79,43 @@ describe TagUser do let(:tracked_tag) { Fabricate(:tag) } context "with some tag notification settings" do - before do + + let :watched_post do TagUser.create!(user: user, tag: watched_tag, notification_level: TagUser.notification_levels[:watching]) + create_post(tags: [watched_tag.name]) + end + + let :muted_post do TagUser.create!(user: user, tag: muted_tag, notification_level: TagUser.notification_levels[:muted]) + create_post(tags: [muted_tag.name]) + end + + let :tracked_post do TagUser.create!(user: user, tag: tracked_tag, notification_level: TagUser.notification_levels[:tracking]) + create_post(tags: [tracked_tag.name]) 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 + TopicUser.change(user.id, tracked_post.topic.id, total_msecs_viewed: 1) 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 + TagUser.create!(user: user, tag: tracked_tag, notification_level: TagUser.notification_levels[:tracking]) + TagUser.create!(user: user, tag: muted_tag, notification_level: TagUser.notification_levels[:muted]) + TagUser.create!(user: user, tag: watched_tag, notification_level: TagUser.notification_levels[:watching]) + 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 + + TopicUser.change(user.id, post.topic.id, total_msecs_viewed: 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] @@ -49,36 +123,43 @@ describe TagUser do 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 + # this is assuming post was already visited in the past, but now cause tag + # was added we should start watching it + TopicUser.change(user.id, post.topic.id, total_msecs_viewed: 1) + TagUser.create!(user: user, tag: watched_tag, notification_level: TagUser.notification_levels[:watching]) + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), [watched_tag.name]) + post.topic.save! + 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 + watched_tag2 = Fabricate(:tag) + TagUser.create!(user: user, tag: watched_tag, notification_level: TagUser.notification_levels[:watching]) + TagUser.create!(user: user, tag: watched_tag2, notification_level: TagUser.notification_levels[:watching]) + + post = create_post(tags: [watched_tag.name, watched_tag2.name]) + + TopicUser.change(user.id, post.topic_id, total_msecs_viewed: 1) + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:watching] + + + DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), [watched_tag.name]) + post.topic.save! + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:watching] + - 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 + post.topic.save! + expect(TopicUser.get(post.topic, user).notification_level).to eq TopicUser.notification_levels[:tracking] + end it "is destroyed when a user is deleted" do - expect(TagUser.where(user_id: user.id).count).to eq(3) + TagUser.create!(user: user, tag: tracked_tag, notification_level: TagUser.notification_levels[:tracking]) user.destroy! expect(TagUser.where(user_id: user.id).count).to eq(0) end diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index d06150902..8cfeb9ec8 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -39,6 +39,28 @@ describe UserUpdater do expect(user.reload.name).to eq 'Jim Tom' end + it 'can update categories and tags' do + category = Fabricate(:category) + tag = Fabricate(:tag) + + user = Fabricate(:user) + updater = UserUpdater.new(acting_user, user) + updater.update(watched_tags: [tag.name], muted_category_ids: [category.id]) + + expect(TagUser.where( + user_id: user.id, + tag_id: tag.id, + notification_level: TagUser.notification_levels[:watching] + ).count).to eq(1) + + expect(CategoryUser.where( + user_id: user.id, + category_id: category.id, + notification_level: CategoryUser.notification_levels[:muted] + ).count).to eq(1) + + end + it 'updates various fields' do user = Fabricate(:user) updater = UserUpdater.new(acting_user, user)