mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 22:21:55 +08:00
Topic Featured Links: move data from custom fields to topics and categories tables. Invert behaviour of topic_featured_link_allowed checkbox. Fix a bug with invalid topic records due to changing that category checkbox.
This commit is contained in:
parent
a2096a01fb
commit
923cf73c6e
|
@ -100,7 +100,8 @@ const Category = RestModel.extend({
|
|||
allowed_tags: this.get('allowed_tags'),
|
||||
allowed_tag_groups: this.get('allowed_tag_groups'),
|
||||
sort_order: this.get('sort_order'),
|
||||
sort_ascending: this.get('sort_ascending')
|
||||
sort_ascending: this.get('sort_ascending'),
|
||||
topic_featured_link_allowed: this.get('topic_featured_link_allowed')
|
||||
},
|
||||
type: id ? 'PUT' : 'POST'
|
||||
});
|
||||
|
@ -169,18 +170,6 @@ const Category = RestModel.extend({
|
|||
@computed("id")
|
||||
isUncategorizedCategory(id) {
|
||||
return id === Discourse.Site.currentProp("uncategorized_category_id");
|
||||
},
|
||||
|
||||
@computed('custom_fields.topic_featured_link_allowed')
|
||||
topicFeaturedLinkAllowed: {
|
||||
get(allowed) {
|
||||
return allowed === "true";
|
||||
},
|
||||
set(value) {
|
||||
value = value ? "true" : "false";
|
||||
this.set("custom_fields.topic_featured_link_allowed", value);
|
||||
return value;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
@ -114,7 +114,8 @@ const DiscoveryCategoriesRoute = Discourse.Route.extend(OpenComposer, {
|
|||
const model = this.store.createRecord('category', {
|
||||
color: "AB9364", text_color: "FFFFFF", group_permissions: [{group_name: everyoneName, permission_type: 1}],
|
||||
available_groups: groups.map(g => g.name),
|
||||
allow_badges: true
|
||||
allow_badges: true,
|
||||
topic_featured_link_allowed: true
|
||||
});
|
||||
|
||||
showModal("edit-category", { model });
|
||||
|
|
|
@ -23,7 +23,7 @@
|
|||
<section class='field'>
|
||||
<div class="allowed-topic-featured-link-category">
|
||||
<label class="checkbox-label">
|
||||
{{input type="checkbox" checked=category.topicFeaturedLinkAllowed}}
|
||||
{{input type="checkbox" checked=category.topic_featured_link_allowed}}
|
||||
{{i18n 'category.topic_featured_link_allowed'}}
|
||||
</label>
|
||||
</div>
|
||||
|
|
|
@ -240,6 +240,7 @@ class CategoriesController < ApplicationController
|
|||
:topic_template,
|
||||
:sort_order,
|
||||
:sort_ascending,
|
||||
:topic_featured_link_allowed,
|
||||
:custom_fields => [params[:custom_fields].try(:keys)],
|
||||
:permissions => [*p.try(:keys)],
|
||||
:allowed_tags => [],
|
||||
|
|
28
app/jobs/onceoff/migrate_featured_links.rb
Normal file
28
app/jobs/onceoff/migrate_featured_links.rb
Normal file
|
@ -0,0 +1,28 @@
|
|||
module Jobs
|
||||
|
||||
class MigrateFeaturedLinks < Jobs::Onceoff
|
||||
|
||||
def execute_onceoff(args)
|
||||
TopicCustomField.where(name: "featured_link").find_each do |tcf|
|
||||
if tcf.value.present?
|
||||
Topic.where(id: tcf.topic_id).update_all(featured_link: tcf.value)
|
||||
end
|
||||
end
|
||||
|
||||
# Plugin behaviour: only categories explicitly allowed to have featured links can have them.
|
||||
# All others implicitly DO NOT allow them.
|
||||
# If no categories were explicitly allowed to have them, then all implicitly DID allow them.
|
||||
|
||||
allowed = CategoryCustomField.where(name: "topic_featured_link_allowed").where(value: "true").pluck(:category_id)
|
||||
|
||||
if !allowed.empty?
|
||||
# all others are not allowed
|
||||
Category.where.not(id: allowed).update_all(topic_featured_link_allowed: false)
|
||||
else
|
||||
not_allowed = CategoryCustomField.where(name: "topic_featured_link_allowed").where.not(value: "true").pluck(:category_id)
|
||||
Category.where(id: not_allowed).update_all(topic_featured_link_allowed: false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
end
|
|
@ -7,7 +7,6 @@ require_dependency 'text_cleaner'
|
|||
require_dependency 'archetype'
|
||||
require_dependency 'html_prettify'
|
||||
require_dependency 'discourse_tagging'
|
||||
require_dependency 'discourse_featured_link'
|
||||
|
||||
class Topic < ActiveRecord::Base
|
||||
include ActionView::Helpers::SanitizeHelper
|
||||
|
@ -76,11 +75,12 @@ class Topic < ActiveRecord::Base
|
|||
|
||||
validates :featured_link, allow_nil: true, format: URI::regexp(%w(http https))
|
||||
validate if: :featured_link do
|
||||
errors.add(:featured_link, :invalid_category) unless Guardian.new.can_edit_featured_link?(category_id)
|
||||
errors.add(:featured_link, :invalid_category) unless !featured_link_changed? || Guardian.new.can_edit_featured_link?(category_id)
|
||||
end
|
||||
|
||||
before_validation do
|
||||
self.title = TextCleaner.clean_title(TextSentinel.title_sentinel(title).text) if errors[:title].empty?
|
||||
self.featured_link.strip! if self.featured_link
|
||||
end
|
||||
|
||||
belongs_to :category
|
||||
|
@ -383,14 +383,6 @@ class Topic < ActiveRecord::Base
|
|||
featured_topic_ids ? topics.where("topics.id NOT IN (?)", featured_topic_ids) : topics
|
||||
end
|
||||
|
||||
def featured_link
|
||||
custom_fields[DiscourseFeaturedLink::CUSTOM_FIELD_NAME]
|
||||
end
|
||||
|
||||
def featured_link=(link)
|
||||
custom_fields[DiscourseFeaturedLink::CUSTOM_FIELD_NAME] = link.strip
|
||||
end
|
||||
|
||||
def meta_data=(data)
|
||||
custom_fields.replace(data)
|
||||
end
|
||||
|
|
|
@ -1,5 +1,4 @@
|
|||
require_dependency 'avatar_lookup'
|
||||
require_dependency 'discourse_featured_link'
|
||||
|
||||
class TopicList
|
||||
include ActiveModel::Serialization
|
||||
|
@ -28,7 +27,6 @@ class TopicList
|
|||
end
|
||||
|
||||
preloaded_custom_fields << DiscourseTagging::TAGS_FIELD_NAME if SiteSetting.tagging_enabled
|
||||
preloaded_custom_fields << DiscourseFeaturedLink::CUSTOM_FIELD_NAME if SiteSetting.topic_featured_link_enabled
|
||||
end
|
||||
|
||||
def tags
|
||||
|
|
|
@ -15,7 +15,8 @@ class CategorySerializer < BasicCategorySerializer
|
|||
:allow_badges,
|
||||
:custom_fields,
|
||||
:allowed_tags,
|
||||
:allowed_tag_groups
|
||||
:allowed_tag_groups,
|
||||
:topic_featured_link_allowed
|
||||
|
||||
def group_permissions
|
||||
@group_permissions ||= begin
|
||||
|
|
|
@ -1909,7 +1909,7 @@ en:
|
|||
tags_allowed_tag_groups: "Tag groups that can only be used in this category:"
|
||||
tags_placeholder: "(Optional) list of allowed tags"
|
||||
tag_groups_placeholder: "(Optional) list of allowed tag groups"
|
||||
topic_featured_link_allowed: "Restricts editing the topic featured link in this category. Require site setting topic_featured_link_enabled is checked."
|
||||
topic_featured_link_allowed: "Allow featured links in this category"
|
||||
delete: 'Delete Category'
|
||||
create: 'New Category'
|
||||
create_long: 'Create a new category'
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
class MigrateFeaturedLinkFields < ActiveRecord::Migration
|
||||
def change
|
||||
add_column :topics, :featured_link, :string
|
||||
add_column :categories, :topic_featured_link_allowed, :boolean, default: true
|
||||
end
|
||||
end
|
|
@ -1,27 +0,0 @@
|
|||
module DiscourseFeaturedLink
|
||||
CUSTOM_FIELD_NAME = 'featured_link'.freeze
|
||||
|
||||
AdminDashboardData::GLOBAL_REPORTS << CUSTOM_FIELD_NAME
|
||||
|
||||
Report.add_report(CUSTOM_FIELD_NAME) do |report|
|
||||
report.data = []
|
||||
link_topics = TopicCustomField.where(name: CUSTOM_FIELD_NAME)
|
||||
link_topics = link_topics.joins(:topic).where("topics.category_id = ?", report.category_id) if report.category_id
|
||||
link_topics.where("topic_custom_fields.created_at >= ?", report.start_date)
|
||||
.where("topic_custom_fields.created_at <= ?", report.end_date)
|
||||
.group("DATE(topic_custom_fields.created_at)")
|
||||
.order("DATE(topic_custom_fields.created_at)")
|
||||
.count
|
||||
.each { |date, count| report.data << { x: date, y: count } }
|
||||
report.total = link_topics.count
|
||||
report.prev30Days = link_topics.where("topic_custom_fields.created_at >= ?", report.start_date - 30.days)
|
||||
.where("topic_custom_fields.created_at <= ?", report.start_date)
|
||||
.count
|
||||
end
|
||||
|
||||
def self.cache_onebox_link(link)
|
||||
# If the link is pasted swiftly, onebox may not have time to cache it
|
||||
Oneboxer.onebox(link, invalidate_oneboxes: false)
|
||||
link
|
||||
end
|
||||
end
|
|
@ -70,7 +70,6 @@ module CategoryGuardian
|
|||
end
|
||||
|
||||
def topic_featured_link_allowed_category_ids
|
||||
@topic_featured_link_allowed_category_ids = CategoryCustomField.where(name: "topic_featured_link_allowed", value: "true")
|
||||
.pluck(:category_id)
|
||||
@topic_featured_link_allowed_category_ids = Category.where(topic_featured_link_allowed: true).pluck(:id)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -106,8 +106,8 @@ module TopicGuardian
|
|||
end
|
||||
|
||||
def can_edit_featured_link?(category_id)
|
||||
SiteSetting.topic_featured_link_enabled &&
|
||||
(topic_featured_link_allowed_category_ids.empty? || # no per category restrictions
|
||||
category_id && topic_featured_link_allowed_category_ids.include?(category_id.to_i)) # category restriction exists
|
||||
return false unless SiteSetting.topic_featured_link_enabled
|
||||
return !Category.where(topic_featured_link_allowed: false).exists? unless category_id # uncategorized
|
||||
Category.where(id: category_id, topic_featured_link_allowed: true).exists?
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5,7 +5,6 @@ require_dependency 'topic_creator'
|
|||
require_dependency 'post_jobs_enqueuer'
|
||||
require_dependency 'distributed_mutex'
|
||||
require_dependency 'has_errors'
|
||||
require_dependency 'discourse_featured_link'
|
||||
|
||||
class PostCreator
|
||||
include HasErrors
|
||||
|
|
|
@ -2285,20 +2285,28 @@ describe Guardian do
|
|||
before { SiteSetting.topic_featured_link_enabled = true }
|
||||
let(:guardian) { Guardian.new }
|
||||
|
||||
it 'returns true if no category restricts editing link' do
|
||||
expect(guardian.can_edit_featured_link?(nil)).to eq(true)
|
||||
expect(guardian.can_edit_featured_link?(5)).to eq(true)
|
||||
context "uncategorized" do
|
||||
let!(:link_category) { Fabricate(:link_category) }
|
||||
|
||||
it "allows featured links if no categories forbid it" do
|
||||
expect(guardian.can_edit_featured_link?(nil)).to eq(true)
|
||||
end
|
||||
|
||||
it "forbids featured links if a category forbids it" do
|
||||
Fabricate(:category, topic_featured_link_allowed: false)
|
||||
expect(guardian.can_edit_featured_link?(nil)).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when exist' do
|
||||
let!(:category) { Fabricate(:category) }
|
||||
let!(:category) { Fabricate(:category, topic_featured_link_allowed: false) }
|
||||
let!(:link_category) { Fabricate(:link_category) }
|
||||
|
||||
it 'returns true if the category is listed' do
|
||||
expect(guardian.can_edit_featured_link?(link_category.id)).to eq(true)
|
||||
end
|
||||
|
||||
it 'returns false if the category is not listed' do
|
||||
it 'returns false if the category does not allow it' do
|
||||
expect(guardian.can_edit_featured_link?(category.id)).to eq(false)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -27,5 +27,5 @@ Fabricator(:private_category, from: :category) do
|
|||
end
|
||||
|
||||
Fabricator(:link_category, from: :category) do
|
||||
before_validation { |category, transients| category.custom_fields['topic_featured_link_allowed'] = 'true' }
|
||||
before_validation { |category, transients| category.topic_featured_link_allowed = true }
|
||||
end
|
||||
|
|
|
@ -1755,7 +1755,7 @@ describe Topic do
|
|||
topic.featured_link = ' https://github.com/discourse/discourse'
|
||||
|
||||
expect(topic.save).to be_truthy
|
||||
expect(topic.custom_fields['featured_link']).to eq('https://github.com/discourse/discourse')
|
||||
expect(topic.featured_link).to eq('https://github.com/discourse/discourse')
|
||||
end
|
||||
|
||||
context 'when category restricts present' do
|
||||
|
@ -1766,13 +1766,24 @@ describe Topic do
|
|||
it 'can save the featured link if it belongs to that category' do
|
||||
link_topic.featured_link = 'https://github.com/discourse/discourse'
|
||||
expect(link_topic.save).to be_truthy
|
||||
expect(link_topic.custom_fields['featured_link']).to eq('https://github.com/discourse/discourse')
|
||||
expect(link_topic.featured_link).to eq('https://github.com/discourse/discourse')
|
||||
end
|
||||
|
||||
it 'can not save the featured link if it belongs to that category' do
|
||||
it 'can not save the featured link if category does not allow it' do
|
||||
topic.category = Fabricate(:category, topic_featured_link_allowed: false)
|
||||
topic.featured_link = 'https://github.com/discourse/discourse'
|
||||
expect(topic.save).to be_falsey
|
||||
end
|
||||
|
||||
it 'if category changes to disallow it, topic remains valid' do
|
||||
t = Fabricate(:topic, category: link_category, featured_link: "https://github.com/discourse/discourse")
|
||||
|
||||
link_category.topic_featured_link_allowed = false
|
||||
link_category.save!
|
||||
t.reload
|
||||
|
||||
expect(t.valid?).to eq(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue
Block a user