Suppressing articles negative-follow tags from appearing in the "relevant" feed (#19948)
* Try suppressing all negative-follow tags * userData isn't always available * Bolster test coverage for tag filter scenarios * Antitags for the non-basic 'strategy' * Rename antitags -> hidden_tags * Rename 'anti_tags' -> 'hidden_tags' as well * Use userData.followed_tags to derive hidden_tags
This commit is contained in:
parent
3324bf47f1
commit
d227e1d285
12 changed files with 208 additions and 7 deletions
|
|
@ -389,6 +389,21 @@ function paginate(tag, params, requiresApproval) {
|
|||
var homeEl = document.getElementById('index-container');
|
||||
if (homeEl.dataset.feed === 'base-feed') {
|
||||
searchHash.class_name = 'Article';
|
||||
const isHomePageFeed = window.location.pathname == '/';
|
||||
if (isHomePageFeed && userData()) {
|
||||
const hidden_tags = JSON.parse(userData().followed_tags).reduce(function (
|
||||
array,
|
||||
tag,
|
||||
) {
|
||||
if (tag.points < 0) {
|
||||
array.push(tag.name);
|
||||
}
|
||||
return array;
|
||||
},
|
||||
[]);
|
||||
|
||||
searchHash.hidden_tags = hidden_tags;
|
||||
}
|
||||
} else if (homeEl.dataset.feed === 'latest') {
|
||||
searchHash.class_name = 'Article';
|
||||
searchHash.sort_by = 'published_at';
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ class SearchController < ApplicationController
|
|||
:user_id,
|
||||
{
|
||||
tag_names: [],
|
||||
hidden_tags: [],
|
||||
published_at: [:gte]
|
||||
},
|
||||
].freeze
|
||||
|
|
@ -106,6 +107,7 @@ class SearchController < ApplicationController
|
|||
user_id: feed_params[:user_id],
|
||||
organization_id: feed_params[:organization_id],
|
||||
tags: feed_params[:tag_names],
|
||||
hidden_tags: feed_params[:hidden_tags],
|
||||
sort_by: params[:sort_by],
|
||||
sort_direction: params[:sort_direction],
|
||||
page: params[:page],
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ module Homepage
|
|||
user_id: nil,
|
||||
organization_id: nil,
|
||||
tags: [],
|
||||
hidden_tags: [],
|
||||
sort_by: nil,
|
||||
sort_direction: nil,
|
||||
page: 0,
|
||||
|
|
@ -45,6 +46,7 @@ module Homepage
|
|||
@user_id = user_id
|
||||
@organization_id = organization_id
|
||||
@tags = tags.presence || []
|
||||
@hidden_tags = hidden_tags.presence || []
|
||||
|
||||
@sort_by = sort_by
|
||||
@sort_direction = sort_direction || DEFAULT_SORT_DIRECTION
|
||||
|
|
@ -59,8 +61,8 @@ module Homepage
|
|||
|
||||
private
|
||||
|
||||
attr_reader :relation, :approved, :published_at, :user_id, :organization_id, :tags, :sort_by, :sort_direction,
|
||||
:page, :per_page
|
||||
attr_reader :relation, :approved, :published_at, :user_id, :organization_id, :tags, :hidden_tags,
|
||||
:sort_by, :sort_direction, :page, :per_page
|
||||
|
||||
def filter
|
||||
@relation = @relation.where(approved: approved) unless approved.nil?
|
||||
|
|
@ -68,6 +70,7 @@ module Homepage
|
|||
@relation = @relation.where(user_id: user_id) if user_id.present?
|
||||
@relation = @relation.where(organization_id: organization_id) if organization_id.present?
|
||||
@relation = @relation.cached_tagged_with_any(tags) if tags.any?
|
||||
@relation = @relation.not_cached_tagged_with_any(hidden_tags) if hidden_tags.any?
|
||||
@relation = @relation.includes(:distinct_reaction_categories)
|
||||
|
||||
relation
|
||||
|
|
|
|||
|
|
@ -19,6 +19,9 @@ module Articles
|
|||
return articles unless @user
|
||||
|
||||
articles = articles.where.not(user_id: UserBlock.cached_blocked_ids_for_blocker(@user.id))
|
||||
if (hidden_tags = @user.cached_antifollowed_tag_names).any?
|
||||
articles = articles.not_cached_tagged_with_any(hidden_tags)
|
||||
end
|
||||
articles.sort_by.with_index do |article, index|
|
||||
tag_score = score_followed_tags(article)
|
||||
user_score = score_followed_user(article)
|
||||
|
|
|
|||
|
|
@ -130,11 +130,17 @@ module Articles
|
|||
# This sub-query allows us to take the hard work of the hand-coded unsanitized sql and
|
||||
# create a sub-query that we can use to help ensure that we can use all of the ActiveRecord
|
||||
# goodness of scopes (e.g., limited_column_select) and eager includes.
|
||||
Article.joins(join_fragment)
|
||||
scope = Article.joins(join_fragment)
|
||||
.limited_column_select
|
||||
.includes(top_comments: :user)
|
||||
.includes(:distinct_reaction_categories)
|
||||
.order(config.order_by.to_sql)
|
||||
|
||||
if @user.present? && (hidden_tags = @user.cached_antifollowed_tag_names).any?
|
||||
scope = scope.not_cached_tagged_with_any(hidden_tags)
|
||||
end
|
||||
|
||||
scope
|
||||
end
|
||||
|
||||
alias more_comments_minimal_weight_randomized call
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ module Homepage
|
|||
user_id: nil,
|
||||
organization_id: nil,
|
||||
tags: [],
|
||||
hidden_tags: [],
|
||||
sort_by: nil,
|
||||
sort_direction: nil,
|
||||
page: 0,
|
||||
|
|
@ -25,6 +26,7 @@ module Homepage
|
|||
user_id: user_id,
|
||||
organization_id: organization_id,
|
||||
tags: tags,
|
||||
hidden_tags: hidden_tags,
|
||||
sort_by: sort_by,
|
||||
sort_direction: sort_direction,
|
||||
page: page,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,88 @@
|
|||
describe('Home Feed should not filter when logged-out', () => {
|
||||
beforeEach(() => {
|
||||
cy.testSetup();
|
||||
|
||||
// Explicitly set the viewport to make sure we're in the full desktop view for these tests
|
||||
cy.viewport('macbook-15');
|
||||
|
||||
cy.visit('/');
|
||||
});
|
||||
|
||||
it('**should** show tag1 tagged article', () => {
|
||||
cy.findByRole('heading', { name: 'Tag test article' }).should('exist');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Home Feed should filter tagged article when logged-in as user with hidden tags', () => {
|
||||
beforeEach(() => {
|
||||
cy.testSetup();
|
||||
cy.fixture('users/hiddenTagUser.json').as('user');
|
||||
|
||||
// Explicitly set the viewport to make sure we're in the full desktop view for these tests
|
||||
cy.viewport('macbook-15');
|
||||
|
||||
cy.get('@user').then((user) => {
|
||||
cy.loginAndVisit(user, '/');
|
||||
});
|
||||
});
|
||||
|
||||
it('should **not** show tag1 tagged article', () => {
|
||||
cy.findByRole('heading', { name: 'Tag test article' }).should('not.exist');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Home Feed should not filter when logged-in as other user', () => {
|
||||
beforeEach(() => {
|
||||
cy.testSetup();
|
||||
cy.fixture('users/adminUser.json').as('user');
|
||||
|
||||
// Explicitly set the viewport to make sure we're in the full desktop view for these tests
|
||||
cy.viewport('macbook-15');
|
||||
|
||||
cy.visit('/');
|
||||
});
|
||||
|
||||
it('**should** show tag1 tagged article', () => {
|
||||
cy.findByRole('heading', { name: 'Tag test article' }).should('exist');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Search should **not** filter tagged article even when logged-in as user with hidden tags', () => {
|
||||
beforeEach(() => {
|
||||
cy.testSetup();
|
||||
cy.fixture('users/hiddenTagUser.json').as('user');
|
||||
|
||||
// Explicitly set the viewport to make sure we're in the full desktop view for these tests
|
||||
cy.viewport('macbook-15');
|
||||
|
||||
cy.get('@user').then((user) => {
|
||||
cy.loginAndVisit(user, '/');
|
||||
cy.findByRole('textbox', { name: /search/i }).type('Tag test');
|
||||
cy.findByRole('button', { name: /search/i }).click();
|
||||
|
||||
cy.url().should('include', '/search?q=Tag%20test');
|
||||
});
|
||||
});
|
||||
|
||||
it('**should** show tag1 tagged article', () => {
|
||||
cy.findByRole('heading', { name: 'Tag test article' }).should('exist');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Tag view should **not** filter tagged article even when logged-in as user with hidden tags', () => {
|
||||
beforeEach(() => {
|
||||
cy.testSetup();
|
||||
cy.fixture('users/hiddenTagUser.json').as('user');
|
||||
|
||||
// Explicitly set the viewport to make sure we're in the full desktop view for these tests
|
||||
cy.viewport('macbook-15');
|
||||
|
||||
cy.get('@user').then((user) => {
|
||||
cy.loginAndVisit(user, '/t/tag1');
|
||||
});
|
||||
});
|
||||
|
||||
it('**should** show tag1 tagged article', () => {
|
||||
cy.findByRole('heading', { name: 'Tag test article' }).should('exist');
|
||||
});
|
||||
});
|
||||
5
cypress/fixtures/users/antitagUser.json
Normal file
5
cypress/fixtures/users/antitagUser.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"username": "not-a-fan",
|
||||
"email": "not-a-fan@forem.local",
|
||||
"password": "password"
|
||||
}
|
||||
5
cypress/fixtures/users/hiddenTagUser.json
Normal file
5
cypress/fixtures/users/hiddenTagUser.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"username": "not-a-fan",
|
||||
"email": "not-a-fan@forem.local",
|
||||
"password": "password"
|
||||
}
|
||||
|
|
@ -130,7 +130,7 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
|
|||
article2.tag_list.add(:ruby)
|
||||
article2.save
|
||||
|
||||
expect(described_class.call(tags: %i[beginners ruby]).ids).to match_array([article1.id, article2.id])
|
||||
expect(described_class.call(tags: %i[beginners ruby]).ids).to contain_exactly(article1.id, article2.id)
|
||||
end
|
||||
|
||||
it "does not return results for partial matches", :aggregate_failures do
|
||||
|
|
@ -143,6 +143,36 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
|
|||
end
|
||||
end
|
||||
|
||||
describe "hidden_tags" do
|
||||
let!(:article1) { create(:article, tags: "twice, first") }
|
||||
let!(:article2) { create(:article, tags: "second, twice") }
|
||||
let!(:article3) { create(:article) }
|
||||
|
||||
it "removes articles matching any hidden_tags" do
|
||||
results = described_class.call(hidden_tags: ["twice"])
|
||||
expect(results).to contain_exactly(article3)
|
||||
|
||||
results = described_class.call(hidden_tags: ["first"])
|
||||
expect(results).not_to include(article1)
|
||||
expect(results).to include(article2)
|
||||
|
||||
results = described_class.call(hidden_tags: ["second"])
|
||||
expect(results).to include(article1)
|
||||
expect(results).not_to include(article2)
|
||||
end
|
||||
|
||||
it "behaves expectedly when hidden_tags is any variety of blank" do
|
||||
results = described_class.call(hidden_tags: [])
|
||||
expect(results).to contain_exactly(article1, article2, article3)
|
||||
|
||||
results = described_class.call(hidden_tags: nil)
|
||||
expect(results).to contain_exactly(article1, article2, article3)
|
||||
|
||||
results = described_class.call(hidden_tags: "")
|
||||
expect(results).to contain_exactly(article1, article2, article3)
|
||||
end
|
||||
end
|
||||
|
||||
describe "pagination" do
|
||||
it "paginates by default" do
|
||||
stub_const("Homepage::ArticlesQuery::DEFAULT_PER_PAGE", 1)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Articles::Feeds::Basic, type: :service do
|
||||
let(:user) { create(:user) }
|
||||
let(:second_user) { create(:user) }
|
||||
let(:unique_tag_name) { "foo" }
|
||||
let!(:article) { create(:article, hotness_score: 10) }
|
||||
|
|
@ -12,8 +11,10 @@ RSpec.describe Articles::Feeds::Basic, type: :service do
|
|||
let!(:low_scoring_article) { create(:article, score: -1000) }
|
||||
let!(:month_old_story) { create(:article, :past, past_published_at: 1.month.ago) } # rubocop:disable RSpec/LetSetup
|
||||
|
||||
let(:feed) { described_class.new(user: user, number_of_articles: 100, page: 1) }
|
||||
|
||||
context "without a user" do
|
||||
let(:feed) { described_class.new(user: nil, number_of_articles: 100, page: 1) }
|
||||
let(:user) { nil }
|
||||
|
||||
it "returns articles with score above 0 in order of hotness score" do
|
||||
result = feed.feed
|
||||
|
|
@ -25,7 +26,7 @@ RSpec.describe Articles::Feeds::Basic, type: :service do
|
|||
end
|
||||
|
||||
context "with a user" do
|
||||
let(:feed) { described_class.new(user: user, number_of_articles: 100, page: 1) }
|
||||
let(:user) { create(:user) }
|
||||
|
||||
it "returns articles with score above 0 sorted by user preference scores" do
|
||||
user.follow(old_story.user)
|
||||
|
|
@ -44,5 +45,23 @@ RSpec.describe Articles::Feeds::Basic, type: :service do
|
|||
result = feed.feed
|
||||
expect(result).not_to include(hot_story)
|
||||
end
|
||||
|
||||
context "when user has hidden tags" do
|
||||
let!(:hidden) { create(:article, tags: "notme") }
|
||||
let!(:visible) { create(:article, tags: "surewhynot") }
|
||||
|
||||
before do
|
||||
antitag = ActsAsTaggableOn::Tag.find_by(name: "notme") || create(:tag, name: "notme")
|
||||
user
|
||||
.follows_by_type("ActsAsTaggableOn::Tag")
|
||||
.create! followable: antitag, explicit_points: -5.0
|
||||
end
|
||||
|
||||
it "does not return articles with tags the user has hidden" do
|
||||
result = feed.feed
|
||||
expect(result).not_to include(hidden)
|
||||
expect(result).to include(visible)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -967,6 +967,29 @@ Settings::General.sidebar_tags = %i[tag1]
|
|||
|
||||
##############################################################################
|
||||
|
||||
seeder.create_if_doesnt_exist(User, "email", "not-a-fan@forem.local") do
|
||||
antitagger = User.create!(
|
||||
name: "Doesnt Like Tag1",
|
||||
email: "not-a-fan@forem.local",
|
||||
username: "not-a-fan",
|
||||
profile_image: Rails.root.join("app/assets/images/#{rand(1..40)}.png").open,
|
||||
confirmed_at: Time.current,
|
||||
registered_at: Time.current,
|
||||
password: "password",
|
||||
password_confirmation: "password",
|
||||
saw_onboarding: true,
|
||||
checked_code_of_conduct: true,
|
||||
checked_terms_and_conditions: true,
|
||||
)
|
||||
|
||||
antitag1 = ActsAsTaggableOn::Tag.find_by(name: "tag1") || create(:tag, name: "tag1")
|
||||
antitagger
|
||||
.follows_by_type("ActsAsTaggableOn::Tag")
|
||||
.create! followable: antitag1, explicit_points: -5.0
|
||||
end
|
||||
|
||||
##############################################################################
|
||||
|
||||
seeder.create_if_doesnt_exist(Article, "title", "Tag test article") do
|
||||
markdown = <<~MARKDOWN
|
||||
---
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue