FEATURE: remove support for 'suppress_from_latest' category setting. (#8308)

This commit is contained in:
Vinoth Kannan 2019-11-18 12:28:35 +05:30 committed by GitHub
parent 4e4844f4db
commit 3bb7ad4be1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 64 additions and 182 deletions

View File

@ -132,7 +132,6 @@ const Category = RestModel.extend({
allow_badges: this.allow_badges,
custom_fields: this.custom_fields,
topic_template: this.topic_template,
suppress_from_latest: this.suppress_from_latest,
all_topics_wiki: this.all_topics_wiki,
allowed_tags: this.allowed_tags,
allowed_tag_groups: this.allowed_tag_groups,

View File

@ -5,11 +5,9 @@ import {
default as discourseComputed,
on
} from "discourse-common/utils/decorators";
import { defaultHomepage } from "discourse/lib/utilities";
import PreloadStore from "preload-store";
import Category from "discourse/models/category";
import EmberObject from "@ember/object";
import Site from "discourse/models/site";
import User from "discourse/models/user";
function isNew(topic) {
@ -144,18 +142,6 @@ const TopicTrackingState = EmberObject.extend({
}
}
if (filter === defaultHomepage()) {
const suppressed_from_latest_category_ids = Site.currentProp(
"suppressed_from_latest_category_ids"
);
if (
suppressed_from_latest_category_ids &&
suppressed_from_latest_category_ids.includes(data.payload.category_id)
) {
return;
}
}
if (
["all", "latest", "new"].includes(filter) &&
data.message_type === "new_topic"

View File

@ -58,13 +58,6 @@
</section>
{{/if}}
<section class="field">
<label>
{{input type="checkbox" checked=category.suppress_from_latest}}
{{i18n "category.suppress_from_latest"}}
</label>
</section>
<section class="field">
<label>
{{input type="checkbox" checked=category.navigate_to_first_post_after_read}}

View File

@ -47,8 +47,7 @@ class CategoriesController < ApplicationController
style = SiteSetting.desktop_category_page_style
topic_options = {
per_page: SiteSetting.categories_topics,
no_definitions: true,
exclude_category_ids: Category.where(suppress_from_latest: true).pluck(:id)
no_definitions: true
}
if style == "categories_and_latest_topics".freeze
@ -238,8 +237,7 @@ class CategoriesController < ApplicationController
topic_options = {
per_page: SiteSetting.categories_topics,
no_definitions: true,
exclude_category_ids: Category.where(suppress_from_latest: true).pluck(:id)
no_definitions: true
}
result = CategoryAndTopicLists.new
@ -291,7 +289,6 @@ class CategoriesController < ApplicationController
:email_in,
:email_in_allow_strangers,
:mailinglist_mirror,
:suppress_from_latest,
:all_topics_wiki,
:parent_category_id,
:auto_close_hours,

View File

@ -52,15 +52,7 @@ class ListController < ApplicationController
list_opts = build_topic_list_options
list_opts.merge!(options) if options
user = list_target_user
if params[:category].blank?
if filter == :latest
list_opts[:no_definitions] = true
end
if [:latest, :categories].include?(filter) && list_opts[:exclude_category_ids].blank?
list_opts[:exclude_category_ids] = get_excluded_category_ids(list_opts[:category])
end
end
list_opts[:no_definitions] = true if params[:category].blank? && filter == :latest
list = TopicQuery.new(user, list_opts).public_send("list_#{filter}")
@ -246,10 +238,6 @@ class ListController < ApplicationController
top_options.merge!(options) if options
top_options[:per_page] = SiteSetting.topics_per_period_in_top_page
if "top".freeze == current_homepage && top_options[:exclude_category_ids].blank?
top_options[:exclude_category_ids] = get_excluded_category_ids(top_options[:category])
end
user = list_target_user
list = TopicQuery.new(user, top_options).list_top_for(period)
list.for_period = period
@ -393,12 +381,6 @@ class ListController < ApplicationController
public_send(method, opts.merge(page_params)).sub('.json?', '?')
end
def get_excluded_category_ids(current_category = nil)
exclude_category_ids = Category.where(suppress_from_latest: true)
exclude_category_ids = exclude_category_ids.where.not(id: current_category) if current_category
exclude_category_ids.pluck(:id)
end
def self.best_period_for(previous_visit_at, category_id = nil)
default_period = ((category_id && Category.where(id: category_id).pluck_first(:default_top_period)) ||
SiteSetting.top_page_default_timeframe).to_sym

View File

@ -392,7 +392,6 @@ class TagsController < ::ApplicationController
options = super.merge(
page: params[:page],
topic_ids: param_to_integer_list(:topic_ids),
exclude_category_ids: params[:exclude_category_ids],
category: @filter_on_category ? @filter_on_category.id : params[:category],
order: params[:order],
ascending: params[:ascending],

View File

@ -3,6 +3,7 @@
class Category < ActiveRecord::Base
self.ignored_columns = %w{
uploaded_meta_id
suppress_from_latest
}
include Searchable
@ -72,7 +73,6 @@ class Category < ActiveRecord::Base
after_save :clear_url_cache
after_save :index_search
after_save :update_reviewables
after_save :clear_featured_cache
after_destroy :reset_topic_ids_cache
after_destroy :publish_category_deletion
@ -656,10 +656,6 @@ class Category < ActiveRecord::Base
@@url_cache.clear
end
def clear_featured_cache
CategoryFeaturedTopic.clear_exclude_category_ids
end
def full_slug(separator = "-")
start_idx = "#{Discourse.base_uri}/c/".length
url[start_idx..-1].gsub("/", separator)

View File

@ -38,16 +38,6 @@ class CategoryFeaturedTopic < ActiveRecord::Base
end
end
@@exclude_category_ids = DistributedCache.new('excluded_category_ids_from_featured')
def self.cached_exclude_category_ids
@@exclude_category_ids['ids'] ||= Category.where(suppress_from_latest: true).pluck(:id)
end
def self.clear_exclude_category_ids
@@exclude_category_ids.clear
end
def self.clear_batch!
$redis.del(NEXT_CATEGORY_ID_KEY)
end
@ -59,8 +49,7 @@ class CategoryFeaturedTopic < ActiveRecord::Base
per_page: c.num_featured_topics,
except_topic_ids: [c.topic_id],
visible: true,
no_definitions: true,
exclude_category_ids: CategoryFeaturedTopic.cached_exclude_category_ids
no_definitions: true
}
# It may seem a bit odd that we are running 2 queries here, when admin

View File

@ -74,10 +74,6 @@ class Site
Group.visible_groups(@guardian.user, "name ASC", include_everyone: true)
end
def suppressed_from_latest_category_ids
categories.select { |c| c.suppress_from_latest == true }.map(&:id)
end
def archetypes
Archetype.list.reject { |t| t.id == Archetype.private_message }
end

View File

@ -11,7 +11,6 @@ class CategorySerializer < SiteCategorySerializer
:email_in,
:email_in_allow_strangers,
:mailinglist_mirror,
:suppress_from_latest,
:all_topics_wiki,
:can_delete,
:cannot_delete_reason,
@ -82,10 +81,6 @@ class CategorySerializer < SiteCategorySerializer
scope && scope.can_edit?(object)
end
def include_suppress_from_latest?
scope && scope.can_edit?(object)
end
def notification_level
user = scope && scope.user
object.notification_level ||

View File

@ -15,7 +15,6 @@ class SiteSerializer < ApplicationSerializer
:is_readonly,
:disabled_plugins,
:user_field_max_length,
:suppressed_from_latest_category_ids,
:post_action_types,
:topic_flag_types,
:can_create_tag,

View File

@ -2651,7 +2651,6 @@ en:
email_in_disabled: "Posting new topics via email is disabled in the Site Settings. To enable posting new topics via email, "
email_in_disabled_click: 'enable the "email in" setting.'
mailinglist_mirror: "Category mirrors a mailing list"
suppress_from_latest: "Suppress category from latest topics."
show_subcategory_list: "Show subcategory list above topics in this category."
num_featured_topics: "Number of topics shown on the categories page:"
subcategory_num_featured_topics: "Number of featured topics on parent category's page:"

View File

@ -0,0 +1,46 @@
# frozen_string_literal: true
class RemoveSuppressFromLatestFromCategory < ActiveRecord::Migration[6.0]
DROPPED_COLUMNS ||= {
categories: %i{suppress_from_latest}
}
def up
ids = DB.query_single("SELECT id::text FROM categories WHERE suppress_from_latest = TRUE")
if ids.present?
muted_ids = DB.query_single("SELECT value from site_settings WHERE name = 'default_categories_muted'").first
ids += muted_ids.split("|") if muted_ids.present?
ids.uniq!
# We shouldn't encourage to have more than 10 categories in `default_categories_muted` site setting.
if ids.count <= 10
DB.exec(<<~SQL, muted: CategoryUser.notification_levels[:muted])
INSERT INTO category_users (category_id, user_id, notification_level)
SELECT c.id category_id, u.id user_id, :muted
FROM users u
CROSS JOIN categories c
LEFT JOIN category_users cu
ON u.id = cu.user_id
AND c.id = cu.category_id
WHERE c.suppress_from_latest = TRUE
AND cu.notification_level IS NULL
SQL
DB.exec(<<~SQL, value: ids.join("|"))
UPDATE site_settings
SET value = :value
WHERE name = 'default_categories_muted'
SQL
end
end
DROPPED_COLUMNS.each do |table, columns|
Migration::ColumnDropper.execute_drop(table, columns)
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -6,7 +6,7 @@ module ImportExport
CATEGORY_ATTRS = [:id, :name, :color, :created_at, :user_id, :slug, :description, :text_color,
:auto_close_hours, :parent_category_id, :auto_close_based_on_last_post,
:topic_template, :suppress_from_latest, :all_topics_wiki, :permissions_params]
:topic_template, :all_topics_wiki, :permissions_params]
GROUP_ATTRS = [ :id, :name, :created_at, :mentionable_level, :messageable_level, :visibility_level,
:automatic_membership_email_domains, :automatic_membership_retroactive,

View File

@ -28,8 +28,7 @@ class TopicQuery
{
max_posts: zero_up_to_max_int,
min_posts: zero_up_to_max_int,
page: zero_up_to_max_int,
exclude_category_ids: array_int_or_int
page: zero_up_to_max_int
}
end
end
@ -49,7 +48,6 @@ class TopicQuery
before
bumped_before
topic_ids
exclude_category_ids
category
order
ascending
@ -756,10 +754,6 @@ class TopicQuery
result = apply_ordering(result, options)
result = result.listable_topics
if options[:exclude_category_ids] && options[:exclude_category_ids].is_a?(Array) && options[:exclude_category_ids].size > 0
result = result.where("categories.id NOT IN (?)", options[:exclude_category_ids].map(&:to_i)).references(:categories)
end
# Don't include the category topics if excluded
if options[:no_definitions]
result = result.where('COALESCE(categories.topic_id, 0) <> topics.id')

View File

@ -275,7 +275,6 @@ class ImportScripts::DiscuzX < ImportScripts::Base
description: row['description'],
position: row['position'].to_i + max_position,
color: color,
suppress_from_latest: (row['status'] == (0) || row['status'] == (3)),
post_create_action: lambda do |category|
if slug = @category_slug[row['id']]
category.update(slug: slug)
@ -295,6 +294,10 @@ class ImportScripts::DiscuzX < ImportScripts::Base
category.save!
end
end
if row['status'] == (0) || row['status'] == (3)
SiteSetting.default_categories_muted = [SiteSetting.default_categories_muted, category.id].reject(&:blank?).join("|")
end
category
end
}

View File

@ -4,12 +4,12 @@
{"id":42,"name":"custom_group_import","created_at":"2017-10-26T15:33:46.328Z","mentionable_level":0,"messageable_level":0,"visibility_level":0,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"incoming_email":null,"user_ids":[2]}
],
"categories":[
{"id":8,"name":"Custom Category","color":"0088CC","created_at":"2017-10-26T15:32:44.083Z","user_id":1,"slug":"custom-category","description":null,"text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":3,"auto_close_based_on_last_post":false,"topic_template":"","suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{"custom_group":1,"everyone":2}},
{"id":10,"name":"Site Feedback Import","color":"27AA5B","created_at":"2017-10-26T17:12:39.995Z","user_id":-1,"slug":"site-feedback-import","description":"Discussion about this site, its organization, how it works, and how we can improve it.","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{}},
{"id":11,"name":"Uncategorized Import","color":"0088CC","created_at":"2017-10-26T17:12:32.359Z","user_id":-1,"slug":"uncategorized-import","description":"","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{}},
{"id":12,"name":"Lounge Import","color":"A461EF","created_at":"2017-10-26T17:12:39.490Z","user_id":-1,"slug":"lounge-import","description":"A category exclusive to members with trust level 3 and higher.","text_color":"652D90","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{"trust_level_3":1}},
{"id":13,"name":"Staff Import","color":"E45735","created_at":"2017-10-26T17:12:42.806Z","user_id":2,"slug":"staff-import","description":"Private category for staff discussions. Topics are only visible to admins and moderators.","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{"custom_group_import":1,"staff":1}},
{"id":15,"name":"Custom Category Import","color":"0088CC","created_at":"2017-10-26T15:32:44.083Z","user_id":2,"slug":"custom-category-import","description":null,"text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":10,"auto_close_based_on_last_post":false,"topic_template":"","suppress_from_latest":false,"all_topics_wiki":false,"permissions_params":{"custom_group_import":1,"everyone":2}}
{"id":8,"name":"Custom Category","color":"0088CC","created_at":"2017-10-26T15:32:44.083Z","user_id":1,"slug":"custom-category","description":null,"text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":3,"auto_close_based_on_last_post":false,"topic_template":"","all_topics_wiki":false,"permissions_params":{"custom_group":1,"everyone":2}},
{"id":10,"name":"Site Feedback Import","color":"27AA5B","created_at":"2017-10-26T17:12:39.995Z","user_id":-1,"slug":"site-feedback-import","description":"Discussion about this site, its organization, how it works, and how we can improve it.","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"all_topics_wiki":false,"permissions_params":{}},
{"id":11,"name":"Uncategorized Import","color":"0088CC","created_at":"2017-10-26T17:12:32.359Z","user_id":-1,"slug":"uncategorized-import","description":"","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"all_topics_wiki":false,"permissions_params":{}},
{"id":12,"name":"Lounge Import","color":"A461EF","created_at":"2017-10-26T17:12:39.490Z","user_id":-1,"slug":"lounge-import","description":"A category exclusive to members with trust level 3 and higher.","text_color":"652D90","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"all_topics_wiki":false,"permissions_params":{"trust_level_3":1}},
{"id":13,"name":"Staff Import","color":"E45735","created_at":"2017-10-26T17:12:42.806Z","user_id":2,"slug":"staff-import","description":"Private category for staff discussions. Topics are only visible to admins and moderators.","text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":null,"auto_close_based_on_last_post":false,"topic_template":null,"all_topics_wiki":false,"permissions_params":{"custom_group_import":1,"staff":1}},
{"id":15,"name":"Custom Category Import","color":"0088CC","created_at":"2017-10-26T15:32:44.083Z","user_id":2,"slug":"custom-category-import","description":null,"text_color":"FFFFFF","auto_close_hours":null,"parent_category_id":10,"auto_close_based_on_last_post":false,"topic_template":"","all_topics_wiki":false,"permissions_params":{"custom_group_import":1,"everyone":2}}
],
"users":[
{"id":1,"email":"email@example.com","username":"example","name":"Example","created_at":"2017-10-07T15:01:24.597Z","trust_level":4,"active":true,"last_emailed_at":null},

View File

@ -12,10 +12,6 @@ describe CategoryFeaturedTopic do
fab!(:category) { Fabricate(:category) }
let!(:category_post) { PostCreator.create(user, raw: "I put this post in the category", title: "categorize THIS", category: category.id) }
before do
CategoryFeaturedTopic.clear_exclude_category_ids
end
it "works in batched mode" do
category2 = Fabricate(:category)
post2 = create_post(category: category2.id)
@ -57,20 +53,6 @@ describe CategoryFeaturedTopic do
expect(CategoryFeaturedTopic.count).to be(1)
end
it 'should not include topics from suppressed categories' do
CategoryFeaturedTopic.feature_topics_for(category)
expect(
CategoryFeaturedTopic.where(category_id: category.id).order('rank asc').pluck(:topic_id)
).to contain_exactly(category_post.topic.id)
category.update(suppress_from_latest: true)
CategoryFeaturedTopic.feature_topics_for(category)
expect(
CategoryFeaturedTopic.where(category_id: category.id).order('rank asc').pluck(:topic_id)
).to_not contain_exactly(category_post.topic.id)
end
it 'should feature stuff in the correct order' do
category = Fabricate(:category, num_featured_topics: 2)
_t5 = Fabricate(:topic, category_id: category.id, bumped_at: 12.minutes.ago)

View File

@ -8,18 +8,6 @@ describe CategoriesController do
context 'index' do
it 'suppresses categories correctly' do
post = create_post(title: 'super AMAZING AMAZING post')
get "/categories"
expect(response.body).to include('AMAZING AMAZING')
post.topic.category.update_columns(suppress_from_latest: true)
get "/categories"
expect(response.body).not_to include('AMAZING AMAZING')
end
it 'web crawler view has correct urls for subfolder install' do
set_subfolder "/forum"
get '/categories', headers: { 'HTTP_USER_AGENT' => 'Googlebot' }

View File

@ -21,12 +21,6 @@ RSpec.describe ListController do
get "/latest?max_posts=bob"
expect(response.status).to eq(400)
get "/latest?exclude_category_ids=bob"
expect(response.status).to eq(400)
get "/latest?exclude_category_ids[]=bob"
expect(response.status).to eq(400)
get "/latest?max_posts=1111111111111111111111111111111111111111"
expect(response.status).to eq(400)
@ -41,10 +35,7 @@ RSpec.describe ListController do
end
it "returns 200 for legit requests" do
get "/latest.json?exclude_category_ids%5B%5D=69&exclude_category_ids%5B%5D=70&no_definitions=true&no_subcategories=false&page=1&_=1534296100767"
expect(response.status).to eq(200)
get "/latest.json?exclude_category_ids=-1"
get "/latest.json?no_definitions=true&no_subcategories=false&page=1&_=1534296100767"
expect(response.status).to eq(200)
get "/latest.json?max_posts=12"
@ -108,33 +99,6 @@ RSpec.describe ListController do
end
end
describe 'suppress from latest' do
it 'supresses categories' do
topic
get "/latest.json"
data = JSON.parse(response.body)
expect(data["topic_list"]["topics"].length).to eq(1)
get "/categories_and_latest.json"
data = JSON.parse(response.body)
expect(data["topic_list"]["topics"].length).to eq(1)
topic.category.suppress_from_latest = true
topic.category.save
get "/latest.json"
data = JSON.parse(response.body)
expect(data["topic_list"]["topics"].length).to eq(0)
get "/categories_and_latest.json"
data = JSON.parse(response.body)
expect(data["topic_list"]["topics"].length).to eq(0)
end
end
describe 'titles for crawler layout' do
it 'has no title for the default URL' do
topic
@ -650,29 +614,4 @@ RSpec.describe ListController do
expect(ListController.best_periods_for(nil, :daily)).to eq([:daily, :all])
end
end
describe "categories suppression" do
let(:category_one) { Fabricate(:category_with_definition) }
let(:sub_category) { Fabricate(:category_with_definition, parent_category: category_one, suppress_from_latest: true) }
let!(:topic_in_sub_category) { Fabricate(:topic, category: sub_category) }
let(:category_two) { Fabricate(:category_with_definition, suppress_from_latest: true) }
let!(:topic_in_category_two) { Fabricate(:topic, category: category_two) }
it "suppresses categories from the latest list" do
get "/#{SiteSetting.homepage}.json"
expect(response.status).to eq(200)
topic_titles = JSON.parse(response.body)["topic_list"]["topics"].map { |t| t["title"] }
expect(topic_titles).not_to include(topic_in_sub_category.title, topic_in_category_two.title)
end
it "does not suppress" do
get "/#{SiteSetting.homepage}.json", params: { category: category_one.id }
expect(response.status).to eq(200)
topic_titles = JSON.parse(response.body)["topic_list"]["topics"].map { |t| t["title"] }
expect(topic_titles).to include(topic_in_sub_category.title)
end
end
end