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.
This commit is contained in:
Sam
2016-07-08 12:58:18 +10:00
parent 58c2389a7b
commit 4161ee210a
19 changed files with 583 additions and 224 deletions

View File

@@ -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

View File

@@ -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

View File

@@ -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)