From f85f73be88399d4bd1efa8efa058ac56119bf3a8 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 18 Sep 2020 12:45:09 -0300 Subject: [PATCH] FEATURE: Review posts with media. (#10693) To check if a post contains any embedded media, we look if the "image_sizes" attribute is present in the new post manager arguments. We want to see one boxed links, but we only store the raw content of the post. To work around this, I extracted the onebox logic from the composer editor into a module. --- .../app/components/composer-editor.js | 91 ++++--------------- .../discourse/app/components/cook-text.js | 16 ++++ .../discourse/app/lib/load-oneboxes.js | 78 ++++++++++++++++ .../components/reviewable-queued-post.hbs | 9 +- config/locales/server.en.yml | 2 + config/site_settings.yml | 3 + lib/new_post_manager.rb | 5 + spec/components/new_post_manager_spec.rb | 36 ++++++++ 8 files changed, 167 insertions(+), 73 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/load-oneboxes.js diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 4288f9f2f08..95078306260 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -16,8 +16,6 @@ import { fetchUnseenHashtags, } from "discourse/lib/link-hashtags"; import Composer from "discourse/models/composer"; -import { load, LOADING_ONEBOX_CSS_CLASS } from "pretty-text/oneboxer"; -import { applyInlineOneboxes } from "pretty-text/inline-oneboxer"; import { ajax } from "discourse/lib/ajax"; import EmberObject from "@ember/object"; import { findRawTemplate } from "discourse-common/lib/raw-templates"; @@ -43,6 +41,7 @@ import { resolveAllShortUrls, } from "pretty-text/upload-short-url"; import { isTesting } from "discourse-common/config/environment"; +import { loadOneboxes } from "discourse/lib/load-oneboxes"; const REBUILD_SCROLL_MAP_EVENTS = ["composer:resized", "composer:typed-reply"]; @@ -531,36 +530,6 @@ export default Component.extend({ } }, - _loadInlineOneboxes(inline) { - applyInlineOneboxes(inline, ajax, { - categoryId: this.get("composer.category.id"), - topicId: this.get("composer.topic.id"), - }); - }, - - _loadOneboxes(oneboxes) { - const post = this.get("composer.post"); - let refresh = false; - - // If we are editing a post, we'll refresh its contents once. - if (post && !post.get("refreshedPost")) { - refresh = true; - post.set("refreshedPost", true); - } - - Object.values(oneboxes).forEach((onebox) => { - onebox.forEach(($onebox) => { - load({ - elem: $onebox, - refresh, - ajax, - categoryId: this.get("composer.category.id"), - topicId: this.get("composer.topic.id"), - }); - }); - }); - }, - _warnMentionedGroups($preview) { schedule("afterRender", () => { var found = this.warnedGroupMentions || []; @@ -934,50 +903,28 @@ export default Component.extend({ } // Paint oneboxes - debounce( - this, - () => { - const oneboxes = {}; - const inlineOneboxes = {}; + const paintFunc = () => { + const post = this.get("composer.post"); + let refresh = false; - // Oneboxes = `a.onebox` -> `a.onebox-loading` -> `aside.onebox` - // Inline Oneboxes = `a.inline-onebox-loading` -> `a.inline-onebox` + //If we are editing a post, we'll refresh its contents once. + if (post && !post.get("refreshedPost")) { + refresh = true; + } - let loadedOneboxes = $preview.find( - `aside.onebox, a.${LOADING_ONEBOX_CSS_CLASS}, a.inline-onebox` - ).length; + const paintedCount = loadOneboxes( + $preview[0], + ajax, + this.get("composer.topic.id"), + this.get("composer.category.id"), + this.siteSettings.max_oneboxes_per_post, + refresh + ); - $preview.find(`a.onebox, a.inline-onebox-loading`).each((_, link) => { - const $link = $(link); - const text = $link.text(); - const isInline = $link.attr("class") === "inline-onebox-loading"; - const m = isInline ? inlineOneboxes : oneboxes; + if (refresh && paintedCount > 0) post.set("refreshedPost", true); + }; - if (loadedOneboxes < this.siteSettings.max_oneboxes_per_post) { - if (m[text] === undefined) { - m[text] = []; - loadedOneboxes++; - } - m[text].push(link); - } else { - if (m[text] !== undefined) { - m[text].push(link); - } else if (isInline) { - $link.removeClass("inline-onebox-loading"); - } - } - }); - - if (Object.keys(oneboxes).length > 0) { - this._loadOneboxes(oneboxes); - } - - if (Object.keys(inlineOneboxes).length > 0) { - this._loadInlineOneboxes(inlineOneboxes); - } - }, - 450 - ); + debounce(this, paintFunc, 450); // Short upload urls need resolution resolveAllShortUrls(ajax, this.siteSettings, $preview[0]); diff --git a/app/assets/javascripts/discourse/app/components/cook-text.js b/app/assets/javascripts/discourse/app/components/cook-text.js index e5088b27f93..c6c4e14d2ce 100644 --- a/app/assets/javascripts/discourse/app/components/cook-text.js +++ b/app/assets/javascripts/discourse/app/components/cook-text.js @@ -2,6 +2,7 @@ import Component from "@ember/component"; import { afterRender } from "discourse-common/utils/decorators"; import { ajax } from "discourse/lib/ajax"; import { cookAsync } from "discourse/lib/text"; +import { loadOneboxes } from "discourse/lib/load-oneboxes"; import { resolveAllShortUrls } from "pretty-text/upload-short-url"; const CookText = Component.extend({ @@ -11,10 +12,25 @@ const CookText = Component.extend({ this._super(...arguments); cookAsync(this.rawText).then((cooked) => { this.set("cooked", cooked); + if (this.paintOneboxes) this._loadOneboxes(); this._resolveUrls(); }); }, + @afterRender + _loadOneboxes() { + const refresh = false; + + loadOneboxes( + this.element, + ajax, + this.topicId, + this.categoryId, + this.siteSettings.max_oneboxes_per_post, + refresh + ); + }, + @afterRender _resolveUrls() { resolveAllShortUrls(ajax, this.siteSettings, this.element, this.opts); diff --git a/app/assets/javascripts/discourse/app/lib/load-oneboxes.js b/app/assets/javascripts/discourse/app/lib/load-oneboxes.js new file mode 100644 index 00000000000..c07d6abb481 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/load-oneboxes.js @@ -0,0 +1,78 @@ +import { applyInlineOneboxes } from "pretty-text/inline-oneboxer"; +import { load, LOADING_ONEBOX_CSS_CLASS } from "pretty-text/oneboxer"; + +export function loadOneboxes( + container, + ajax, + topicId, + categoryId, + maxOneboxes, + refresh +) { + const oneboxes = {}; + const inlineOneboxes = {}; + + // Oneboxes = `a.onebox` -> `a.onebox-loading` -> `aside.onebox` + // Inline Oneboxes = `a.inline-onebox-loading` -> `a.inline-onebox` + + let loadedOneboxes = container.querySelectorAll( + `aside.onebox, a.${LOADING_ONEBOX_CSS_CLASS}, a.inline-onebox` + ).length; + + container + .querySelectorAll(`a.onebox, a.inline-onebox-loading`) + .forEach((link) => { + const text = link.textContent; + const isInline = link.getAttribute("class") === "inline-onebox-loading"; + const m = isInline ? inlineOneboxes : oneboxes; + + if (loadedOneboxes < maxOneboxes) { + if (m[text] === undefined) { + m[text] = []; + loadedOneboxes++; + } + m[text].push(link); + } else { + if (m[text] !== undefined) { + m[text].push(link); + } else if (isInline) { + link.classList.remove("inline-onebox-loading"); + } + } + }); + + let newBoxes = 0; + + if (Object.keys(oneboxes).length > 0) { + _loadOneboxes(oneboxes, ajax, newBoxes, topicId, categoryId, refresh); + } + + if (Object.keys(inlineOneboxes).length > 0) { + _loadInlineOneboxes(inlineOneboxes, ajax, topicId, categoryId); + } + + return newBoxes; +} + +function _loadInlineOneboxes(inline, ajax, topicId, categoryId) { + applyInlineOneboxes(inline, ajax, { + categoryId: topicId, + topicId: categoryId, + }); +} + +function _loadOneboxes(oneboxes, ajax, count, topicId, categoryId, refresh) { + Object.values(oneboxes).forEach((onebox) => { + onebox.forEach((o) => { + load({ + elem: o, + refresh, + ajax, + categoryId: categoryId, + topicId: topicId, + }); + + count++; + }); + }); +} diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-queued-post.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-queued-post.hbs index a5abc9d00b6..1d0ce502223 100644 --- a/app/assets/javascripts/discourse/app/templates/components/reviewable-queued-post.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-queued-post.hbs @@ -18,7 +18,14 @@
{{reviewable-post-header reviewable=reviewable createdBy=reviewable.created_by tagName=""}} - {{cook-text reviewable.payload.raw class="post-body" opts=(hash removeMissing=true)}} + {{cook-text + reviewable.payload.raw + class="post-body" + categoryId=reviewable.category_id + topicId=reviewable.topic_id + paintOneboxes=true + opts=(hash removeMissing=true) + }} {{yield}}
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0585f7a7060..cd7c5f6bab4 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2145,6 +2145,7 @@ en: new_user_notice_tl: "Minimum trust level required to see new user post notices." returning_user_notice_tl: "Minimum trust level required to see returning user post notices." returning_users_days: "How many days should pass before a user is considered to be returning." + review_media_unless_trust_level: "Staff will review posts of users with lower trust levels if it contains embedded media." enable_page_publishing: "Allow staff members to publish topics to new URLs with their own styling." show_published_pages_login_required: "Anonymous users can see published pages, even when login is required." @@ -4812,6 +4813,7 @@ en: email_auth_res_enqueue: "This email failed a DMARC check, it most likely isn't from whom it seems to be from. Check the raw email headers for more information." email_spam: "This email was flagged as spam by the header defined in `email_in_spam_header`." suspect_user: "This new user entered profile information without reading any topics or posts, which strongly suggests they may be a spammer. See `approve_suspect_users`." + contains_media: "This posts includes embedded media. See `review_media_unless_trust_level`." actions: agree: diff --git a/config/site_settings.yml b/config/site_settings.yml index 433f94a9aca..f0a258fc01a 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -960,6 +960,9 @@ posting: returning_users_days: default: 120 max: 36500 + review_media_unless_trust_level: + default: 0 + enum: "TrustLevelSetting" enable_page_publishing: default: false show_published_pages_login_required: diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 3c4614df142..008f1113b7d 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -108,6 +108,11 @@ class NewPostManager return :category if post_needs_approval_in_its_category?(manager) + return :contains_media if ( + manager.args[:image_sizes].present? && + user.trust_level < SiteSetting.review_media_unless_trust_level.to_i + ) + :skip end diff --git a/spec/components/new_post_manager_spec.rb b/spec/components/new_post_manager_spec.rb index 847db93db57..2eca5bfb2b8 100644 --- a/spec/components/new_post_manager_spec.rb +++ b/spec/components/new_post_manager_spec.rb @@ -239,6 +239,42 @@ describe NewPostManager do end end + context 'with media' do + let(:user) { manager.user } + let(:manager_opts) do + { + raw: 'this is new post content', topic_id: topic.id, first_post_checks: false, + image_sizes: { + "http://localhost:3000/uploads/default/original/1X/652fc9667040b1b89dc4d9b061a823ddb3c0cef0.jpeg" => { + "width" => "500", "height" => "500" + } + } + } + end + + before do + user.update!(trust_level: 0) + end + + it 'queues the post for review because if it contains embedded media.' do + SiteSetting.review_media_unless_trust_level = 1 + manager = NewPostManager.new(topic.user, manager_opts) + + result = NewPostManager.default_handler(manager) + + expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:contains_media) + end + + it 'does not enqueue the post if the poster is a trusted user' do + SiteSetting.review_media_unless_trust_level = 0 + manager = NewPostManager.new(topic.user, manager_opts) + + result = NewPostManager.default_handler(manager) + + expect(result).to be_nil + end + end end context "new topic handler" do