diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index f8d3585f3ed..50ccc67abf4 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -203,7 +203,7 @@ class TopicsController < ApplicationController @topic = Topic.where(id: params[:topic_id].to_i).first guardian.ensure_can_see!(@topic) - @topic.toggle_mute(current_user, v) + @topic.toggle_mute(current_user) render nothing: true end diff --git a/app/models/topic.rb b/app/models/topic.rb index 9563ddc2ec3..ef15c0316fa 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -112,9 +112,7 @@ class Topic < ActiveRecord::Base after_create do changed_to_category(category) - TopicUser.change(user_id, id, - notification_level: TopicUser.notification_levels[:watching], - notifications_reason_id: TopicUser.notification_reasons[:created_topic]) + notifier.created_topic! user_id if archetype == Archetype.private_message DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE) else @@ -162,10 +160,7 @@ class Topic < ActiveRecord::Base end def sanitize_title - if self.title.present? - self.title = sanitize(title, tags: [], attributes: []) - self.title.strip! - end + self.title = sanitize(title.to_s, tags: [], attributes: []).strip.presence end def new_version_required? @@ -641,11 +636,6 @@ class Topic < ActiveRecord::Base TopicUser.where('starred_at > ?', sinceDaysAgo.days.ago).group('date(starred_at)').order('date(starred_at)').count end - # Enable/disable the mute on the topic - def toggle_mute(user, muted) - TopicUser.change(user, self.id, notification_level: muted?(user) ? TopicUser.notification_levels[:regular] : TopicUser.notification_levels[:muted] ) - end - def slug unless slug = read_attribute(:slug) return '' unless title.present? @@ -661,17 +651,17 @@ class Topic < ActiveRecord::Base end def title=(t) - slug = "" - slug = (Slug.for(t).presence || "topic") if t.present? + slug = (Slug.for(t.to_s).presence || "topic") write_attribute(:slug, slug) write_attribute(:title,t) end + # NOTE: These are probably better off somewhere else. + # Having a model know about URLs seems a bit strange. def last_post_url "/t/#{slug}/#{id}/#{posts_count}" end - def self.url(id, slug, post_number=nil) url = "#{Discourse.base_url}/t/#{slug}/#{id}" url << "/#{post_number}" if post_number.to_i > 1 @@ -684,12 +674,6 @@ class Topic < ActiveRecord::Base url end - def muted?(user) - return false unless user && user.id - tu = topic_users.where(user_id: user.id).first - tu && tu.notification_level == TopicUser.notification_levels[:muted] - end - def clear_pin_for(user) return unless user.present? TopicUser.change(user.id, id, cleared_pinned_at: Time.now) @@ -703,21 +687,36 @@ class Topic < ActiveRecord::Base "#{Draft::EXISTING_TOPIC}#{id}" end + def notifier + @topic_notifier ||= TopicNotifier.new(self) + end + # notification stuff def notify_watch!(user) - TopicUser.change(user, id, notification_level: TopicUser.notification_levels[:watching]) + notifier.watch! user end def notify_tracking!(user) - TopicUser.change(user, id, notification_level: TopicUser.notification_levels[:tracking]) + notifier.tracking! user end def notify_regular!(user) - TopicUser.change(user, id, notification_level: TopicUser.notification_levels[:regular]) + notifier.regular! user end def notify_muted!(user) - TopicUser.change(user, id, notification_level: TopicUser.notification_levels[:muted]) + notifier.muted! user + end + + def muted?(user) + if user && user.id + notifier.muted?(user.id) + end + end + + # Enable/disable the mute on the topic + def toggle_mute(user_id) + notifier.toggle_mute user_id end def auto_close_days=(num_days) diff --git a/app/models/topic_notifier.rb b/app/models/topic_notifier.rb new file mode 100644 index 00000000000..5ac1de2ed7f --- /dev/null +++ b/app/models/topic_notifier.rb @@ -0,0 +1,42 @@ +class TopicNotifier + def initialize(topic) + @topic = topic + end + + { :watch! => :watching, + :tracking! => :tracking, + :regular! => :regular, + :muted! => :muted }.each_pair do |method_name, level| + + define_method method_name do |user_id| + change_level user_id, level + end + + end + + def created_topic!(user_id) + change_level user_id, :watching, :created_topic + end + + # Enable/disable the mute on the topic + def toggle_mute(user_id) + change_level user_id, (muted?(user_id) ? levels[:regular] : levels[:muted]) + end + + def muted?(user_id) + tu = @topic.topic_users.where(user_id: user_id).first + tu && tu.notification_level == levels[:muted] + end + + private + + def levels + @notification_levels ||= TopicUser.notification_levels + end + + def change_level(user_id, level, reason=nil) + attrs = {notification_level: levels[level]} + attrs.merge!(notifications_reason_id: TopicUser.notification_reasons[reason]) if reason + TopicUser.change(user_id, @topic.id, attrs) + end +end diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 01d9f531df0..ff790c9fc1c 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -282,12 +282,12 @@ describe TopicsController do end it "changes the user's starred flag when the parameter is present" do - Topic.any_instance.expects(:toggle_mute).with(@topic.user, true) + Topic.any_instance.expects(:toggle_mute).with(@topic.user) xhr :put, :mute, topic_id: @topic.id, starred: 'true' end it "removes the user's starred flag when the parameter is not true" do - Topic.any_instance.expects(:toggle_mute).with(@topic.user, false) + Topic.any_instance.expects(:toggle_mute).with(@topic.user) xhr :put, :unmute, topic_id: @topic.id, starred: 'false' end