From d17425cbb843eef005dd868471ca2e8adfc5f96e Mon Sep 17 00:00:00 2001 From: Rimian Perkins Date: Mon, 9 Sep 2019 11:27:41 +1000 Subject: [PATCH 001/126] FIX: Migrate post_edit_time_limit to tl2_post_edit_time_limit (#8082) If a user amended edit_time_limit keep the behavior as is, instead of introducing a potentially tighter time for tl2 edit time than what they had set in the past. --- ...0908234054_migrate_post_edit_time_limit.rb | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 db/migrate/20190908234054_migrate_post_edit_time_limit.rb diff --git a/db/migrate/20190908234054_migrate_post_edit_time_limit.rb b/db/migrate/20190908234054_migrate_post_edit_time_limit.rb new file mode 100644 index 0000000000..cd84b391f9 --- /dev/null +++ b/db/migrate/20190908234054_migrate_post_edit_time_limit.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class MigratePostEditTimeLimit < ActiveRecord::Migration[5.2] + def up + execute <<~SQL + INSERT INTO site_settings ( + name, + value, + data_type, + created_at, + updated_at + ) + SELECT + 'tl2_post_edit_time_limit', + value, + data_type, + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + FROM site_settings + WHERE + name = 'post_edit_time_limit' + ON CONFLICT + DO NOTHING + SQL + end +end From 7d5f3c1338f04ad3e4f6b51196231645464366f4 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Sun, 8 Sep 2019 22:29:15 -0300 Subject: [PATCH 002/126] UX/PERF: Update readers count when a post from another user is read. Don't fetch the post data again just to update the count. (#8078) --- .../discourse/controllers/topic.js.es6 | 15 +++++---------- .../discourse/models/post-stream.js.es6 | 13 +++++++++++++ app/controllers/post_readers_controller.rb | 2 +- app/models/post.rb | 5 +++++ app/models/topic_tracking_state.rb | 7 ++++--- app/serializers/post_serializer.rb | 7 ------- spec/requests/post_readers_controller_spec.rb | 9 --------- 7 files changed, 28 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 7a7996e627..fbfdc0f486 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -1352,17 +1352,12 @@ export default Ember.Controller.extend(bufferedProperty("model"), { }) .then(() => refresh({ id: data.id, refreshLikes: true })); break; - case "read": + case "read": { postStream - .triggerChangedPost(data.id, data.updated_at, { - preserveCooked: true - }) - .then(() => - refresh({ - id: data.id, - refreshReaders: topic.show_read_indicator - }) - ); + .triggerReadPost(data.id, data.readers_count) + .then(() => refresh({ id: data.id, refreshLikes: true })); + break; + } case "revised": case "rebaked": { postStream diff --git a/app/assets/javascripts/discourse/models/post-stream.js.es6 b/app/assets/javascripts/discourse/models/post-stream.js.es6 index 51e536965e..346226b9eb 100644 --- a/app/assets/javascripts/discourse/models/post-stream.js.es6 +++ b/app/assets/javascripts/discourse/models/post-stream.js.es6 @@ -716,6 +716,19 @@ export default RestModel.extend({ return resolved; }, + triggerReadPost(postId, readersCount) { + const resolved = Ember.RSVP.Promise.resolve(); + resolved.then(() => { + const post = this.findLoadedPost(postId); + if (post && readersCount > post.readers_count) { + post.set("readers_count", readersCount); + this.storePost(post); + } + }); + + return resolved; + }, + postForPostNumber(postNumber) { if (!this.hasPosts) { return; diff --git a/app/controllers/post_readers_controller.rb b/app/controllers/post_readers_controller.rb index a5fd76fafc..392f575c9b 100644 --- a/app/controllers/post_readers_controller.rb +++ b/app/controllers/post_readers_controller.rb @@ -12,7 +12,7 @@ class PostReadersController < ApplicationController .joins(:topic_users) .where.not(topic_users: { last_read_post_number: nil }) .where('topic_users.topic_id = ? AND topic_users.last_read_post_number >= ?', post.topic_id, post.post_number) - .where.not(id: [current_user.id, post.user_id]) + .where.not(id: post.user_id) readers = readers.map do |r| { diff --git a/app/models/post.rb b/app/models/post.rb index 56d96b0c33..c22d5aae6c 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -170,6 +170,11 @@ class Post < ActiveRecord::Base end end + def readers_count + read_count = reads - 1 # Excludes poster + read_count < 0 ? 0 : read_count + end + def publish_change_to_clients!(type, opts = {}) # special failsafe for posts missing topics consistency checks should fix, # but message is safe to skip diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 3ed182ba21..cec4fa99e1 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -357,7 +357,7 @@ SQL if topic&.private_message? groups = read_allowed_groups_of(topic) post = Post.find_by(topic_id: topic.id, post_number: last_read_post_number) - trigger_post_read_count_update(post, groups) + trigger_post_read_count_update(post, groups, last_read_post_number, user_id) update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, false) end end @@ -389,8 +389,9 @@ SQL MessageBus.publish("/private-messages/unread-indicator/#{topic.id}", message, user_ids: groups_to_update.flat_map(&:members)) end - def self.trigger_post_read_count_update(post, groups) + def self.trigger_post_read_count_update(post, groups, last_read_post_number, user_id) return if groups.empty? - post.publish_change_to_clients!(:read) + opts = { readers_count: post.readers_count, reader_id: user_id } + post.publish_change_to_clients!(:read, opts) end end diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index d0bd771a2a..0e2a7b3072 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -459,13 +459,6 @@ class PostSerializer < BasicPostSerializer can_review_topic? end - def readers_count - read_count = object.reads - 1 # Exclude logged user - read_count -= 1 unless yours - - read_count < 0 ? 0 : read_count - end - private def can_review_topic? diff --git a/spec/requests/post_readers_controller_spec.rb b/spec/requests/post_readers_controller_spec.rb index d5d60c8ed2..267dfea110 100644 --- a/spec/requests/post_readers_controller_spec.rb +++ b/spec/requests/post_readers_controller_spec.rb @@ -57,15 +57,6 @@ describe PostReadersController do expect(readers).to be_empty end - it "doesn't include current_user in the readers list" do - TopicUser.create!(user: admin, topic: @group_message, last_read_post_number: 3) - - get '/post_readers.json', params: { id: @post.id } - reader = JSON.parse(response.body)['post_readers'].detect { |r| r['username'] == admin.username } - - expect(reader).to be_nil - end - it "doesn't include users without reading progress on first post" do @post.update(post_number: 1) TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: nil) From 1d73754e8442bad3eea0173023230cc913668bfc Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 9 Sep 2019 17:38:37 +1000 Subject: [PATCH 003/126] FIX: Modify frozen String and profile_db_generator uses category id (#8080) --- script/profile_db_generator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/profile_db_generator.rb b/script/profile_db_generator.rb index 0c9d64a45d..d356a0b690 100644 --- a/script/profile_db_generator.rb +++ b/script/profile_db_generator.rb @@ -35,7 +35,7 @@ def sentence gabbler.learn(story) end - sentence = "" + sentence = +"" until sentence.length > 800 do sentence << @gabbler.sentence sentence << "\n" @@ -104,7 +104,7 @@ puts puts "Creating 100 topics" topic_ids = 100.times.map do - post = PostCreator.create(users.sample, raw: sentence, title: sentence[0..50].strip, category: categories.sample.name, skip_validations: true) + post = PostCreator.create(users.sample, raw: sentence, title: sentence[0..50].strip, category: categories.sample.id, skip_validations: true) putc "." post.topic_id From 530356f465c9411f55e3282f16fff4bab52bfaff Mon Sep 17 00:00:00 2001 From: Nick Quaranto Date: Mon, 9 Sep 2019 03:41:37 -0400 Subject: [PATCH 004/126] Use Discourse.getURL for /clicks/track so clicks can be tracked on relative URLs (#8079) --- app/assets/javascripts/discourse/lib/ajax.js.es6 | 2 +- app/assets/javascripts/discourse/lib/click-track.js.es6 | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/ajax.js.es6 b/app/assets/javascripts/discourse/lib/ajax.js.es6 index 1cec5148e7..cafa6e6f70 100644 --- a/app/assets/javascripts/discourse/lib/ajax.js.es6 +++ b/app/assets/javascripts/discourse/lib/ajax.js.es6 @@ -159,7 +159,7 @@ export function ajax() { if ( args.type && args.type.toUpperCase() !== "GET" && - url !== "/clicks/track" && + url !== Discourse.getURL("/clicks/track") && !Discourse.Session.currentProp("csrfToken") ) { promise = new Ember.RSVP.Promise((resolve, reject) => { diff --git a/app/assets/javascripts/discourse/lib/click-track.js.es6 b/app/assets/javascripts/discourse/lib/click-track.js.es6 index 438915c13f..42a65623a6 100644 --- a/app/assets/javascripts/discourse/lib/click-track.js.es6 +++ b/app/assets/javascripts/discourse/lib/click-track.js.es6 @@ -102,9 +102,9 @@ export default { data.append("url", href); data.append("post_id", postId); data.append("topic_id", topicId); - navigator.sendBeacon("/clicks/track", data); + navigator.sendBeacon(Discourse.getURL("/clicks/track"), data); } else { - trackPromise = ajax("/clicks/track", { + trackPromise = ajax(Discourse.getURL("/clicks/track"), { type: "POST", data: { url: href, From 27f7bd427365307ddaca511632a6e21821f7ddc6 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 9 Sep 2019 20:47:29 +0800 Subject: [PATCH 005/126] UX: adjusts RTL composer presence avatar alignment context: https://meta.discourse.org/t/user-is-typing-misaligned-and-mixed-with-reply-choices-with-rtl/127963 This commit adds a temporary fix for presence avatar overlap in the composer for RTL. More context on TMP here: https://meta.discourse.org/t/moving-the-voting-box-to-the-right-side-of-the-screen/126752 --- .../assets/stylesheets/presence.scss | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/plugins/discourse-presence/assets/stylesheets/presence.scss b/plugins/discourse-presence/assets/stylesheets/presence.scss index fdd8de25be..f3b9261a0a 100644 --- a/plugins/discourse-presence/assets/stylesheets/presence.scss +++ b/plugins/discourse-presence/assets/stylesheets/presence.scss @@ -74,3 +74,23 @@ } } } + +// TMP: RTL overrides +.rtl { + span.presence-text { + margin-left: 2px; + margin-right: 5px; + } + + .composer-fields .presence-users { + right: unset; + left: 95px; + } + + &.mobile-view { + .composer-fields .presence-users { + right: unset; + left: 65px; + } + } +} From 9b10a78d8256146bea15ba87df7d596b03f7a8b5 Mon Sep 17 00:00:00 2001 From: Kyle Zhao Date: Mon, 9 Sep 2019 08:03:57 -0700 Subject: [PATCH 006/126] FEATURE: Quick access panels in user menu (#8073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Extract QuickAccessPanel from UserNotifications. * FEATURE: Quick access panels in user menu. This feature adds quick access panels for bookmarks and personal messages. It allows uses to browse recent items directly in the user menu, without being redirected to the full pages. * REFACTOR: Use QuickAccessItem for messages. Reusing `DefaultNotificationItem` feels nice but it actually requires a lot of extra work that is not needed for a quick access item. Also, `DefaultNotificationItem` shows an incorrect tooptip ("unread private message"), and it is not trivial to remove / override that. * Use a plain JS object instead. An Ember object was required when `DefaultNotificationItem` was used. * Prefix instead suffix `_` for private helpers. * Set to null instead of deleting object keys. JavaScript engines can optimize object property access based on the object’s shape. https://mathiasbynens.be/notes/shapes-ics * Change trivial try/catch to one-liners. * Return the promise in case needs to be waited on. * Refactor showAll to a link with href * Store `emptyStatePlaceholderItemText` in state. * Store items in Session singleton instead. We can drop `staleItems` (and `findStaleItems`) altogether. Because `(old) items === staleItems` when switching back to a quick access panel. * Add `limit` parameter to the `user_actions` API. * Explicitly import Session instead. --- .../discourse/components/site-header.js.es6 | 6 + .../widgets/quick-access-bookmarks.js.es6 | 51 ++++++ .../widgets/quick-access-item.js.es6 | 43 +++++ .../widgets/quick-access-messages.js.es6 | 50 ++++++ .../widgets/quick-access-notifications.js.es6 | 55 +++++++ .../widgets/quick-access-panel.js.es6 | 143 ++++++++++++++++ .../widgets/quick-access-profile.js.es6 | 91 +++++++++++ .../discourse/widgets/user-menu.js.es6 | 152 ++++++++++-------- .../widgets/user-notifications.js.es6 | 131 --------------- .../stylesheets/common/base/menu-panel.scss | 89 +++++++--- app/controllers/user_actions_controller.rb | 5 +- config/locales/client.en.yml | 2 +- lib/svg_sprite/svg_sprite.rb | 2 + .../fixtures/private_messages_fixtures.js.es6 | 79 +++++++++ .../javascripts/fixtures/user_fixtures.js.es6 | 2 +- .../helpers/create-pretender.js.es6 | 2 +- test/javascripts/helpers/create-store.js.es6 | 6 + .../javascripts/widgets/user-menu-test.js.es6 | 87 ++++++++-- 18 files changed, 765 insertions(+), 231 deletions(-) create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-notifications.js.es6 create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-panel.js.es6 create mode 100644 app/assets/javascripts/discourse/widgets/quick-access-profile.js.es6 delete mode 100644 app/assets/javascripts/discourse/widgets/user-notifications.js.es6 create mode 100644 test/javascripts/fixtures/private_messages_fixtures.js.es6 diff --git a/app/assets/javascripts/discourse/components/site-header.js.es6 b/app/assets/javascripts/discourse/components/site-header.js.es6 index 99c41d5350..41c71a48e5 100644 --- a/app/assets/javascripts/discourse/components/site-header.js.es6 +++ b/app/assets/javascripts/discourse/components/site-header.js.es6 @@ -362,6 +362,12 @@ export default SiteHeaderComponent; export function headerHeight() { const $header = $("header.d-header"); + + // Header may not exist in tests (e.g. in the user menu component test). + if ($header.length === 0) { + return 0; + } + const headerOffset = $header.offset(); const headerOffsetTop = headerOffset ? headerOffset.top : 0; return parseInt( diff --git a/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 new file mode 100644 index 0000000000..ca1642b897 --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 @@ -0,0 +1,51 @@ +import { h } from "virtual-dom"; +import QuickAccessPanel from "discourse/widgets/quick-access-panel"; +import UserAction from "discourse/models/user-action"; +import { ajax } from "discourse/lib/ajax"; +import { createWidgetFrom } from "discourse/widgets/widget"; +import { postUrl } from "discourse/lib/utilities"; + +const ICON = "bookmark"; + +createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", { + buildKey: () => "quick-access-bookmarks", + + hasMore() { + // Always show the button to the bookmarks page. + return true; + }, + + showAllHref() { + return `${this.attrs.path}/activity/bookmarks`; + }, + + emptyStatePlaceholderItem() { + return h("li.read", this.state.emptyStatePlaceholderItemText); + }, + + findNewItems() { + return ajax("/user_actions.json", { + cache: "false", + data: { + username: this.currentUser.username, + filter: UserAction.TYPES.bookmarks, + limit: this.estimateItemLimit(), + no_results_help_key: "user_activity.no_bookmarks" + } + }).then(({ user_actions, no_results_help }) => { + // The empty state help text for bookmarks page is localized on the + // server. + this.state.emptyStatePlaceholderItemText = no_results_help; + return user_actions; + }); + }, + + itemHtml(bookmark) { + return this.attach("quick-access-item", { + icon: ICON, + href: postUrl(bookmark.slug, bookmark.topic_id, bookmark.post_number), + content: bookmark.title, + username: bookmark.username + }); + } +}); diff --git a/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 new file mode 100644 index 0000000000..a869484cc7 --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 @@ -0,0 +1,43 @@ +import { h } from "virtual-dom"; +import RawHtml from "discourse/widgets/raw-html"; +import { createWidget } from "discourse/widgets/widget"; +import { emojiUnescape } from "discourse/lib/text"; +import { iconNode } from "discourse-common/lib/icon-library"; + +createWidget("quick-access-item", { + tagName: "li", + + buildClasses(attrs) { + const result = []; + if (attrs.className) { + result.push(attrs.className); + } + if (attrs.read === undefined || attrs.read) { + result.push("read"); + } + return result; + }, + + html({ icon, href, content }) { + return h("a", { attributes: { href } }, [ + iconNode(icon), + new RawHtml({ + html: `
${this._usernameHtml()}${emojiUnescape( + Handlebars.Utils.escapeExpression(content) + )}
` + }) + ]); + }, + + click(e) { + this.attrs.read = true; + if (this.attrs.action) { + e.preventDefault(); + return this.sendWidgetAction(this.attrs.action, this.attrs.actionParam); + } + }, + + _usernameHtml() { + return this.attrs.username ? `${this.attrs.username} ` : ""; + } +}); diff --git a/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 new file mode 100644 index 0000000000..9988e649d7 --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 @@ -0,0 +1,50 @@ +import QuickAccessPanel from "discourse/widgets/quick-access-panel"; +import { createWidgetFrom } from "discourse/widgets/widget"; +import { postUrl } from "discourse/lib/utilities"; + +const ICON = "notification.private_message"; + +function toItem(message) { + const lastReadPostNumber = message.last_read_post_number || 0; + const nextUnreadPostNumber = Math.min( + lastReadPostNumber + 1, + message.highest_post_number + ); + + return { + content: message.fancy_title, + href: postUrl(message.slug, message.id, nextUnreadPostNumber), + icon: ICON, + read: message.last_read_post_number >= message.highest_post_number, + username: message.last_poster_username + }; +} + +createWidgetFrom(QuickAccessPanel, "quick-access-messages", { + buildKey: () => "quick-access-messages", + emptyStatePlaceholderItemKey: "choose_topic.none_found", + + hasMore() { + // Always show the button to the messages page for composing, archiving, + // etc. + return true; + }, + + showAllHref() { + return `${this.attrs.path}/messages`; + }, + + findNewItems() { + return this.store + .findFiltered("topicList", { + filter: `topics/private-messages/${this.currentUser.username_lower}` + }) + .then(({ topic_list }) => { + return topic_list.topics.map(toItem).slice(0, this.estimateItemLimit()); + }); + }, + + itemHtml(message) { + return this.attach("quick-access-item", message); + } +}); diff --git a/app/assets/javascripts/discourse/widgets/quick-access-notifications.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-notifications.js.es6 new file mode 100644 index 0000000000..515e702ace --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-notifications.js.es6 @@ -0,0 +1,55 @@ +import { ajax } from "discourse/lib/ajax"; +import { createWidgetFrom } from "discourse/widgets/widget"; +import QuickAccessPanel from "discourse/widgets/quick-access-panel"; + +createWidgetFrom(QuickAccessPanel, "quick-access-notifications", { + buildKey: () => "quick-access-notifications", + emptyStatePlaceholderItemKey: "notifications.empty", + + markReadRequest() { + return ajax("/notifications/mark-read", { method: "PUT" }); + }, + + newItemsLoaded() { + if (!this.currentUser.enforcedSecondFactor) { + this.currentUser.set("unread_notifications", 0); + } + }, + + itemHtml(notification) { + const notificationName = this.site.notificationLookup[ + notification.notification_type + ]; + + return this.attach( + `${notificationName.dasherize()}-notification-item`, + notification, + {}, + { fallbackWidgetName: "default-notification-item" } + ); + }, + + findNewItems() { + return this._findStaleItemsInStore().refresh(); + }, + + showAllHref() { + return `${this.attrs.path}/notifications`; + }, + + hasUnread() { + return this.getItems().filterBy("read", false).length > 0; + }, + + _findStaleItemsInStore() { + return this.store.findStale( + "notification", + { + recent: true, + silent: this.currentUser.enforcedSecondFactor, + limit: this.estimateItemLimit() + }, + { cacheKey: "recent-notifications" } + ); + } +}); diff --git a/app/assets/javascripts/discourse/widgets/quick-access-panel.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-panel.js.es6 new file mode 100644 index 0000000000..e70985fde2 --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-panel.js.es6 @@ -0,0 +1,143 @@ +import Session from "discourse/models/session"; +import { createWidget } from "discourse/widgets/widget"; +import { h } from "virtual-dom"; +import { headerHeight } from "discourse/components/site-header"; + +const AVERAGE_ITEM_HEIGHT = 55; + +/** + * This tries to enforce a consistent flow of fetching, caching, refreshing, + * and rendering for "quick access items". + * + * There are parts to introducing a new quick access panel: + * 1. A user menu link that sends a `quickAccess` action, with a unique `type`. + * 2. A `quick-access-${type}` widget, extended from `quick-access-panel`. + */ +export default createWidget("quick-access-panel", { + tagName: "div.quick-access-panel", + emptyStatePlaceholderItemKey: "", + + buildKey: () => { + throw Error('Cannot attach abstract widget "quick-access-panel".'); + }, + + markReadRequest() { + return Ember.RSVP.Promise.resolve(); + }, + + hasUnread() { + return false; + }, + + showAllHref() { + return ""; + }, + + hasMore() { + return this.getItems().length >= this.estimateItemLimit(); + }, + + findNewItems() { + return Ember.RSVP.Promise.resolve([]); + }, + + newItemsLoaded() {}, + + itemHtml(item) {}, // eslint-disable-line no-unused-vars + + emptyStatePlaceholderItem() { + if (this.emptyStatePlaceholderItemKey) { + return h("li.read", I18n.t(this.emptyStatePlaceholderItemKey)); + } else { + return ""; + } + }, + + defaultState() { + return { items: [], loading: false, loaded: false }; + }, + + markRead() { + return this.markReadRequest().then(() => { + this.refreshNotifications(this.state); + }); + }, + + estimateItemLimit() { + // Estimate (poorly) the amount of notifications to return. + let limit = Math.round( + ($(window).height() - headerHeight()) / AVERAGE_ITEM_HEIGHT + ); + + // We REALLY don't want to be asking for negative counts of notifications + // less than 5 is also not that useful. + if (limit < 5) { + limit = 5; + } else if (limit > 40) { + limit = 40; + } + + return limit; + }, + + refreshNotifications(state) { + if (this.loading) { + return; + } + + if (this.getItems().length === 0) { + state.loading = true; + } + + this.findNewItems() + .then(newItems => this.setItems(newItems)) + .catch(() => this.setItems([])) + .finally(() => { + state.loading = false; + state.loaded = true; + this.newItemsLoaded(); + this.sendWidgetAction("itemsLoaded", { + hasUnread: this.hasUnread(), + markRead: () => this.markRead() + }); + this.scheduleRerender(); + }); + }, + + html(attrs, state) { + if (!state.loaded) { + this.refreshNotifications(state); + } + + if (state.loading) { + return [h("div.spinner-container", h("div.spinner"))]; + } + + const items = this.getItems().length + ? this.getItems().map(item => this.itemHtml(item)) + : [this.emptyStatePlaceholderItem()]; + + if (this.hasMore()) { + items.push( + h( + "li.read.last.show-all", + this.attach("link", { + title: "view_all", + icon: "chevron-down", + href: this.showAllHref() + }) + ) + ); + } + + return [h("ul", items)]; + }, + + getItems() { + return Session.currentProp(`${this.key}-items`) || []; + }, + + setItems(newItems) { + Session.currentProp(`${this.key}-items`, newItems); + } +}); diff --git a/app/assets/javascripts/discourse/widgets/quick-access-profile.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-profile.js.es6 new file mode 100644 index 0000000000..b74054e243 --- /dev/null +++ b/app/assets/javascripts/discourse/widgets/quick-access-profile.js.es6 @@ -0,0 +1,91 @@ +import QuickAccessPanel from "discourse/widgets/quick-access-panel"; +import { createWidgetFrom } from "discourse/widgets/widget"; + +createWidgetFrom(QuickAccessPanel, "quick-access-profile", { + buildKey: () => "quick-access-profile", + + hasMore() { + // Never show the button to the full profile page. + return false; + }, + + findNewItems() { + return Ember.RSVP.Promise.resolve(this._getItems()); + }, + + itemHtml(item) { + return this.attach("quick-access-item", item); + }, + + _getItems() { + const items = this._getDefaultItems(); + if (this._showToggleAnonymousButton()) { + items.push(this._toggleAnonymousButton()); + } + if (this.attrs.showLogoutButton) { + items.push(this._logOutButton()); + } + return items; + }, + + _getDefaultItems() { + return [ + { + icon: "user", + href: `${this.attrs.path}/summary`, + content: I18n.t("user.summary.title") + }, + { + icon: "stream", + href: `${this.attrs.path}/activity`, + content: I18n.t("user.activity_stream") + }, + { + icon: "envelope", + href: `${this.attrs.path}/messages`, + content: I18n.t("user.private_messages") + }, + { + icon: "cog", + href: `${this.attrs.path}/preferences`, + content: I18n.t("user.preferences") + } + ]; + }, + + _toggleAnonymousButton() { + if (this.currentUser.is_anonymous) { + return { + action: "toggleAnonymous", + className: "disable-anonymous", + content: I18n.t("switch_from_anon"), + icon: "ban" + }; + } else { + return { + action: "toggleAnonymous", + className: "enable-anonymous", + content: I18n.t("switch_to_anon"), + icon: "user-secret" + }; + } + }, + + _logOutButton() { + return { + action: "logout", + className: "logout", + content: I18n.t("user.log_out"), + icon: "sign-out-alt" + }; + }, + + _showToggleAnonymousButton() { + return ( + (this.siteSettings.allow_anonymous_posting && + this.currentUser.trust_level >= + this.siteSettings.anonymous_posting_min_trust_level) || + this.currentUser.is_anonymous + ); + } +}); diff --git a/app/assets/javascripts/discourse/widgets/user-menu.js.es6 b/app/assets/javascripts/discourse/widgets/user-menu.js.es6 index a25240db72..96e56fb7ac 100644 --- a/app/assets/javascripts/discourse/widgets/user-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/user-menu.js.es6 @@ -3,6 +3,17 @@ import { h } from "virtual-dom"; import { formatUsername } from "discourse/lib/utilities"; import hbs from "discourse/widgets/hbs-compiler"; +const UserMenuAction = { + QUICK_ACCESS: "quickAccess" +}; + +const QuickAccess = { + BOOKMARKS: "bookmarks", + MESSAGES: "messages", + NOTIFICATIONS: "notifications", + PROFILE: "profile" +}; + let extraGlyphs; export function addUserMenuGlyph(glyph) { @@ -15,6 +26,8 @@ createWidget("user-menu-links", { profileLink() { const link = { + action: UserMenuAction.QUICK_ACCESS, + actionParam: QuickAccess.PROFILE, route: "user", model: this.currentUser, className: "user-activity-link", @@ -30,8 +43,21 @@ createWidget("user-menu-links", { return link; }, + notificationsGlyph() { + return { + label: "user.notifications", + className: "user-notifications-link", + icon: "bell", + href: `${this.attrs.path}/notifications`, + action: UserMenuAction.QUICK_ACCESS, + actionParam: QuickAccess.NOTIFICATIONS + }; + }, + bookmarksGlyph() { return { + action: UserMenuAction.QUICK_ACCESS, + actionParam: QuickAccess.BOOKMARKS, label: "user.bookmarks", className: "user-bookmarks-link", icon: "bookmark", @@ -41,6 +67,8 @@ createWidget("user-menu-links", { messagesGlyph() { return { + action: UserMenuAction.QUICK_ACCESS, + actionParam: QuickAccess.MESSAGES, label: "user.private_messages", className: "user-pms-link", icon: "envelope", @@ -49,24 +77,20 @@ createWidget("user-menu-links", { }, linkHtml(link) { + if (this.isActive(link)) { + link = this.markAsActive(link); + } return this.attach("link", link); }, glyphHtml(glyph) { + if (this.isActive(glyph)) { + glyph = this.markAsActive(glyph); + } return this.attach("link", $.extend(glyph, { hideLabel: true })); }, - html(attrs) { - const { currentUser, siteSettings } = this; - - const isAnon = currentUser.is_anonymous; - const allowAnon = - (siteSettings.allow_anonymous_posting && - currentUser.trust_level >= - siteSettings.anonymous_posting_min_trust_level) || - isAnon; - - const path = attrs.path; + html() { const links = [this.profileLink()]; const glyphs = []; @@ -81,42 +105,39 @@ createWidget("user-menu-links", { }); } + glyphs.push(this.notificationsGlyph()); glyphs.push(this.bookmarksGlyph()); - if (siteSettings.enable_personal_messages) { + if (this.siteSettings.enable_personal_messages) { glyphs.push(this.messagesGlyph()); } - if (allowAnon) { - if (!isAnon) { - glyphs.push({ - action: "toggleAnonymous", - label: "switch_to_anon", - className: "enable-anonymous", - icon: "user-secret" - }); - } else { - glyphs.push({ - action: "toggleAnonymous", - label: "switch_from_anon", - className: "disable-anonymous", - icon: "ban" - }); - } - } - - // preferences always goes last - glyphs.push({ - label: "user.preferences", - className: "user-preferences-link", - icon: "cog", - href: `${path}/preferences` - }); - return h("ul.menu-links-row", [ links.map(l => h("li.user", this.linkHtml(l))), h("li.glyphs", glyphs.map(l => this.glyphHtml(l))) ]); + }, + + markAsActive(definition) { + // Clicking on an active quick access tab icon should redirect the user to + // the full page. + definition.action = null; + definition.actionParam = null; + + if (definition.className) { + definition.className += " active"; + } else { + definition.className = "active"; + } + + return definition; + }, + + isActive({ action, actionParam }) { + return ( + action === UserMenuAction.QUICK_ACCESS && + actionParam === this.attrs.currentQuickAccess + ); } }); @@ -148,6 +169,7 @@ export default createWidget("user-menu", { defaultState() { return { + currentQuickAccess: QuickAccess.NOTIFICATIONS, hasUnread: false, markUnread: null }; @@ -155,37 +177,18 @@ export default createWidget("user-menu", { panelContents() { const path = this.currentUser.get("path"); + const { currentQuickAccess } = this.state; - let result = [ - this.attach("user-menu-links", { path }), - this.attach("user-notifications", { path }) + const result = [ + this.attach("user-menu-links", { + path, + currentQuickAccess + }), + this.quickAccessPanel(path) ]; - if (this.settings.showLogoutButton || this.state.hasUnread) { - result.push(h("hr.bottom-area")); - } - - if (this.settings.showLogoutButton) { - result.push( - h("div.logout-link", [ - h( - "ul.menu-links", - h( - "li", - this.attach("link", { - action: "logout", - className: "logout", - icon: "sign-out-alt", - href: "", - label: "user.log_out" - }) - ) - ) - ]) - ); - } - if (this.state.hasUnread) { + result.push(h("hr.bottom-area")); result.push(this.attach("user-menu-dismiss-link")); } @@ -196,8 +199,8 @@ export default createWidget("user-menu", { return this.state.markRead(); }, - notificationsLoaded({ notifications, markRead }) { - this.state.hasUnread = notifications.filterBy("read", false).length > 0; + itemsLoaded({ hasUnread, markRead }) { + this.state.hasUnread = hasUnread; this.state.markRead = markRead; }, @@ -234,5 +237,20 @@ export default createWidget("user-menu", { } else { this.sendWidgetAction("toggleUserMenu"); } + }, + + quickAccess(type) { + if (this.state.currentQuickAccess !== type) { + this.state.currentQuickAccess = type; + } + }, + + quickAccessPanel(path) { + const { showLogoutButton } = this.settings; + // This deliberately does NOT fallback to a default quick access panel. + return this.attach(`quick-access-${this.state.currentQuickAccess}`, { + path, + showLogoutButton + }); } }); diff --git a/app/assets/javascripts/discourse/widgets/user-notifications.js.es6 b/app/assets/javascripts/discourse/widgets/user-notifications.js.es6 deleted file mode 100644 index fb19fb56a7..0000000000 --- a/app/assets/javascripts/discourse/widgets/user-notifications.js.es6 +++ /dev/null @@ -1,131 +0,0 @@ -import { createWidget } from "discourse/widgets/widget"; -import { headerHeight } from "discourse/components/site-header"; -import { h } from "virtual-dom"; -import DiscourseURL from "discourse/lib/url"; -import { ajax } from "discourse/lib/ajax"; - -export default createWidget("user-notifications", { - tagName: "div.notifications", - buildKey: () => "user-notifications", - - defaultState() { - return { notifications: [], loading: false, loaded: false }; - }, - - markRead() { - ajax("/notifications/mark-read", { method: "PUT" }).then(() => { - this.refreshNotifications(this.state); - }); - }, - - refreshNotifications(state) { - if (this.loading) { - return; - } - - // estimate (poorly) the amount of notifications to return - let limit = Math.round(($(window).height() - headerHeight()) / 55); - // we REALLY don't want to be asking for negative counts of notifications - // less than 5 is also not that useful - if (limit < 5) { - limit = 5; - } - if (limit > 40) { - limit = 40; - } - - const silent = this.currentUser.get("enforcedSecondFactor"); - const stale = this.store.findStale( - "notification", - { recent: true, silent, limit }, - { cacheKey: "recent-notifications" } - ); - - if (stale.hasResults) { - const results = stale.results; - let content = results.get("content"); - - // we have to truncate to limit, otherwise we will render too much - if (content && content.length > limit) { - content = content.splice(0, limit); - results.set("content", content); - results.set("totalRows", limit); - } - - state.notifications = results; - } else { - state.loading = true; - } - - stale - .refresh() - .then(notifications => { - if (!silent) { - this.currentUser.set("unread_notifications", 0); - } - state.notifications = notifications; - }) - .catch(() => { - state.notifications = []; - }) - .finally(() => { - state.loading = false; - state.loaded = true; - this.sendWidgetAction("notificationsLoaded", { - notifications: state.notifications, - markRead: () => this.markRead() - }); - this.scheduleRerender(); - }); - }, - - html(attrs, state) { - if (!state.loaded) { - this.refreshNotifications(state); - } - - const result = []; - if (state.loading) { - result.push(h("div.spinner-container", h("div.spinner"))); - } else if (state.notifications.length) { - const notificationItems = state.notifications.map(notificationAttrs => { - const notificationName = this.site.notificationLookup[ - notificationAttrs.notification_type - ]; - - return this.attach( - `${notificationName.dasherize()}-notification-item`, - notificationAttrs, - {}, - { fallbackWidgetName: "default-notification-item" } - ); - }); - - result.push(h("hr")); - - const items = [notificationItems]; - - if (notificationItems.length > 5) { - items.push( - h( - "li.read.last.heading.show-all", - this.attach("button", { - title: "notifications.more", - icon: "chevron-down", - action: "showAllNotifications", - className: "btn" - }) - ) - ); - } - - result.push(h("ul", items)); - } - - return result; - }, - - showAllNotifications() { - DiscourseURL.routeTo(`${this.attrs.path}/notifications`); - } -}); diff --git a/app/assets/stylesheets/common/base/menu-panel.scss b/app/assets/stylesheets/common/base/menu-panel.scss index b0b84006d3..44f1d6be7d 100644 --- a/app/assets/stylesheets/common/base/menu-panel.scss +++ b/app/assets/stylesheets/common/base/menu-panel.scss @@ -147,7 +147,7 @@ } .user-menu { - .notifications { + .quick-access-panel { width: 100%; display: table; @@ -187,6 +187,11 @@ padding: 0; > div { overflow: hidden; // clears the text from wrapping below icons + + // Truncate items with more than 2 lines. + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; } } @@ -223,9 +228,12 @@ border-width: 2px; margin: 0 auto; } - .show-all .btn { + .show-all a { width: 100%; - padding: 2px 0; + display: flex; + justify-content: center; + align-items: center; + min-height: 30px; color: dark-light-choose($primary-medium, $secondary-high); background: blend-primary-secondary(5%); &:hover { @@ -237,29 +245,24 @@ @include unselectable; } - .logout-link, .dismiss-link { display: inline-block; - } - .dismiss-link { float: right; } } -.notifications .logout { - padding: 0.25em; - &:hover { - background-color: $highlight-medium; - } -} - div.menu-links-header { width: 100%; display: table; border-collapse: separate; border-spacing: 0 0.5em; .menu-links-row { + border-bottom: 1px solid dark-light-choose($primary-low, $secondary-medium); display: flex; + + // Tabs should have "ears". + padding: 0 4px; + li { display: inline-flex; align-items: center; @@ -271,6 +274,42 @@ div.menu-links-header { flex-wrap: wrap; text-align: right; max-width: 65%; //IE11 + + a { + // Expand the click area a bit. + padding-left: 0.6em; + padding-right: 0.6em; + } + } + + a { + // This is to make sure active and inactive tab icons have the same + // size. `box-sizing` does not work and I have no idea why. + border: 1px solid transparent; + border-bottom: 0; + } + + a.active { + border: 1px solid dark-light-choose($primary-low, $secondary-medium); + border-bottom: 0; + position: relative; + + &:after { + display: block; + position: absolute; + top: 100%; + left: 0; + z-index: z("header") + 1; // Higher than .menu-panel + width: 100%; + height: 0; + content: ""; + border-top: 1px solid $secondary; + } + + &:focus, + &:hover { + background-color: inherit; + } } } } @@ -283,12 +322,24 @@ div.menu-links-header { padding: 0.3em 0.5em; } a.user-activity-link { - max-width: 150px; - display: block; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; + align-items: center; + display: flex; margin: -0.5em 0; + max-width: 130px; + + // `overflow: hidden` on `.user-activity-link` would hide the `::after` + // pseudo element (used to create the tab-looking effect). Sets `overflow: + // hidden` on the child username label instead. + overflow: visible; + + span.d-label { + display: block; + max-width: 130px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + @include breakpoint(mobile-medium) { max-width: 125px; } @@ -311,6 +362,6 @@ div.menu-links-header { } .d-icon-user { - margin-right: 0.2em; + margin-right: 0.475em; } } diff --git a/app/controllers/user_actions_controller.rb b/app/controllers/user_actions_controller.rb index 489dabd865..2c133273cf 100644 --- a/app/controllers/user_actions_controller.rb +++ b/app/controllers/user_actions_controller.rb @@ -4,19 +4,20 @@ class UserActionsController < ApplicationController def index params.require(:username) - params.permit(:filter, :offset, :acting_username) + params.permit(:filter, :offset, :acting_username, :limit) user = fetch_user_from_params(include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts)) raise Discourse::NotFound unless guardian.can_see_profile?(user) offset = [0, params[:offset].to_i].max action_types = (params[:filter] || "").split(",").map(&:to_i) + limit = params.fetch(:limit, 30).to_i opts = { user_id: user.id, user: user, offset: offset, - limit: 30, + limit: limit, action_types: action_types, guardian: guardian, ignore_private_messages: params[:filter] ? false : true, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 2b0eab6e19..bc0dcbb0e0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1726,7 +1726,6 @@ en: title: "notifications of @name mentions, replies to your posts and topics, messages, etc" none: "Unable to load notifications at this time." empty: "No notifications found." - more: "view older notifications" post_approved: "Your post was approved" reviewable_items: "items requiring review" mentioned: "{{username}} {{description}}" @@ -1892,6 +1891,7 @@ en: go_back: "go back" not_logged_in_user: "user page with summary of current activity and preferences" current_user: "go to your user page" + view_all: "view all" topics: new_messages_marker: "last visit" diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index 6bdc6d2653..3630efc0c5 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -26,6 +26,7 @@ module SvgSprite "ban", "bars", "bed", + "bell", "bell-slash", "bold", "book", @@ -167,6 +168,7 @@ module SvgSprite "signal", "step-backward", "step-forward", + "stream", "sync", "table", "tag", diff --git a/test/javascripts/fixtures/private_messages_fixtures.js.es6 b/test/javascripts/fixtures/private_messages_fixtures.js.es6 new file mode 100644 index 0000000000..e6e22c502a --- /dev/null +++ b/test/javascripts/fixtures/private_messages_fixtures.js.es6 @@ -0,0 +1,79 @@ +/*jshint maxlen:10000000 */ +export default { + "/topics/private-messages/eviltrout.json": { + users: [ + { + id: 19, + username: "eviltrout", + name: null, + avatar_template: "/letter_avatar_proxy/v4/letter/t/f9ae1b/{size}.png" + }, + { + id: 13, + username: "mixtape", + name: null, + avatar_template: "/letter_avatar_proxy/v4/letter/m/34f0e0/{size}.png" + } + ], + primary_groups: [], + topic_list: { + can_create_topic: true, + draft: null, + draft_key: "new_topic", + draft_sequence: 33, + per_page: 30, + topics: [ + { + id: 174, + title: "BUG: Can not render emoji properly :/", + fancy_title: "BUG: Can not render emoji properly :confused:", + slug: "bug-can-not-render-emoji-properly", + posts_count: 1, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2019-07-26T01:29:24.008Z", + last_posted_at: "2019-07-26T01:29:24.177Z", + bumped: true, + bumped_at: "2019-07-26T01:29:24.177Z", + unseen: false, + last_read_post_number: 2, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + liked: false, + views: 5, + like_count: 0, + has_summary: false, + archetype: "private_message", + last_poster_username: "mixtape", + category_id: null, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 13, + primary_group_id: null + } + ], + participants: [ + { + extras: "latest", + description: null, + user_id: 13, + primary_group_id: null + } + ] + } + ] + } + } +}; diff --git a/test/javascripts/fixtures/user_fixtures.js.es6 b/test/javascripts/fixtures/user_fixtures.js.es6 index e68bb101eb..c801b5d355 100644 --- a/test/javascripts/fixtures/user_fixtures.js.es6 +++ b/test/javascripts/fixtures/user_fixtures.js.es6 @@ -302,7 +302,7 @@ export default { acting_username: "Abhishek_Gupta", acting_name: "Abhishek Gupta", acting_user_id: 8021, - title: "How to check the user level via ajax?", + title: "How to check the user level via ajax? :/", deleted: false, hidden: false, moderator_action: false, diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index ea7a4cd71d..9d229a654c 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -139,7 +139,7 @@ export default function() { }); this.get("/topics/private-messages/eviltrout.json", () => { - return response({ topic_list: { topics: [] } }); + return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); }); this.get("/topics/feature_stats.json", () => { diff --git a/test/javascripts/helpers/create-store.js.es6 b/test/javascripts/helpers/create-store.js.es6 index dfb5a1b4e2..f0c55cd386 100644 --- a/test/javascripts/helpers/create-store.js.es6 +++ b/test/javascripts/helpers/create-store.js.es6 @@ -1,6 +1,7 @@ import Store from "discourse/models/store"; import RestAdapter from "discourse/adapters/rest"; import KeyValueStore from "discourse/lib/key-value-store"; +import TopicListAdapter from "discourse/adapters/topic-list"; import TopicTrackingState from "discourse/models/topic-tracking-state"; import { buildResolver } from "discourse-common/resolver"; @@ -16,6 +17,11 @@ export default function() { } return this._restAdapter; } + if (type === "adapter:topicList") { + this._topicListAdapter = + this._topicListAdapter || TopicListAdapter.create({ owner: this }); + return this._topicListAdapter; + } if (type === "key-value-store:main") { this._kvs = this._kvs || new KeyValueStore(); return this._kvs; diff --git a/test/javascripts/widgets/user-menu-test.js.es6 b/test/javascripts/widgets/user-menu-test.js.es6 index 66abb7cc87..35d73a8542 100644 --- a/test/javascripts/widgets/user-menu-test.js.es6 +++ b/test/javascripts/widgets/user-menu-test.js.es6 @@ -1,3 +1,4 @@ +import DiscourseURL from "discourse/lib/url"; import { moduleForWidget, widgetTest } from "helpers/widget-test"; moduleForWidget("user-menu"); @@ -8,9 +9,9 @@ widgetTest("basics", { test(assert) { assert.ok(find(".user-menu").length); assert.ok(find(".user-activity-link").length); + assert.ok(find(".user-notifications-link").length); assert.ok(find(".user-bookmarks-link").length); - assert.ok(find(".user-preferences-link").length); - assert.ok(find(".notifications").length); + assert.ok(find(".quick-access-panel").length); assert.ok(find(".dismiss-link").length); } }); @@ -18,8 +19,8 @@ widgetTest("basics", { widgetTest("notifications", { template: '{{mount-widget widget="user-menu"}}', - test(assert) { - const $links = find(".notifications li a"); + async test(assert) { + const $links = find(".quick-access-panel li a"); assert.equal($links.length, 5); assert.ok($links[0].href.includes("/t/a-slug/123")); @@ -62,6 +63,13 @@ widgetTest("notifications", { }) ) ); + + const routeToStub = sandbox.stub(DiscourseURL, "routeTo"); + await click(".user-notifications-link"); + assert.ok( + routeToStub.calledWith(find(".user-notifications-link")[0].href), + "a second click should redirect to the full notifications page" + ); } }); @@ -73,6 +81,7 @@ widgetTest("log out", { }, async test(assert) { + await click(".user-activity-link"); assert.ok(find(".logout").length); await click(".logout"); @@ -97,8 +106,63 @@ widgetTest("private messages - enabled", { this.siteSettings.enable_personal_messages = true; }, - test(assert) { - assert.ok(find(".user-pms-link").length); + async test(assert) { + const userPmsLink = find(".user-pms-link")[0]; + assert.ok(userPmsLink); + await click(".user-pms-link"); + + const message = find(".quick-access-panel li a")[0]; + assert.ok(message); + + assert.ok( + message.href.includes("/t/bug-can-not-render-emoji-properly/174/2"), + "should link to the next unread post" + ); + assert.ok( + message.innerHTML.includes("mixtape"), + "should include the last poster's username" + ); + assert.ok( + message.innerHTML.match(//), + "should correctly render emoji in message title" + ); + + const routeToStub = sandbox.stub(DiscourseURL, "routeTo"); + await click(".user-pms-link"); + assert.ok( + routeToStub.calledWith(userPmsLink.href), + "a second click should redirect to the full private messages page" + ); + } +}); + +widgetTest("bookmarks", { + template: '{{mount-widget widget="user-menu"}}', + + async test(assert) { + await click(".user-bookmarks-link"); + + const bookmark = find(".quick-access-panel li a")[0]; + assert.ok(bookmark); + + assert.ok( + bookmark.href.includes("/t/how-to-check-the-user-level-via-ajax/11993") + ); + assert.ok( + bookmark.innerHTML.includes("Abhishek_Gupta"), + "should include the last poster's username" + ); + assert.ok( + bookmark.innerHTML.match(//), + "should correctly render emoji in bookmark title" + ); + + const routeToStub = sandbox.stub(DiscourseURL, "routeTo"); + await click(".user-bookmarks-link"); + assert.ok( + routeToStub.calledWith(find(".user-bookmarks-link")[0].href), + "a second click should redirect to the full bookmarks page" + ); } }); @@ -115,7 +179,9 @@ widgetTest("anonymous", { }, async test(assert) { + await click(".user-activity-link"); assert.ok(find(".enable-anonymous").length); + await click(".enable-anonymous"); assert.ok(this.anonymous); } @@ -128,7 +194,8 @@ widgetTest("anonymous - disabled", { this.siteSettings.allow_anonymous_posting = false; }, - test(assert) { + async test(assert) { + await click(".user-activity-link"); assert.ok(!find(".enable-anonymous").length); } }); @@ -141,12 +208,14 @@ widgetTest("anonymous - switch back", { this.currentUser.setProperties({ is_anonymous: true }); this.siteSettings.allow_anonymous_posting = true; - this.on("toggleAnonymous", () => (this.anonymous = true)); + this.on("toggleAnonymous", () => (this.anonymous = false)); }, async test(assert) { + await click(".user-activity-link"); assert.ok(find(".disable-anonymous").length); + await click(".disable-anonymous"); - assert.ok(this.anonymous); + assert.notOk(this.anonymous); } }); From 972c1acef8d1eca679f7f50fc6affabe265222f2 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Mon, 9 Sep 2019 21:33:53 +0530 Subject: [PATCH 007/126] DEV: better class names for Flexbox --- app/assets/stylesheets/embed.scss | 6 +++--- app/views/embed/topics.html.erb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/embed.scss b/app/assets/stylesheets/embed.scss index 286570b081..c4c7046463 100644 --- a/app/assets/stylesheets/embed.scss +++ b/app/assets/stylesheets/embed.scss @@ -196,10 +196,10 @@ div.lightbox-wrapper { } } - .topic-details-flex { + .topic-column-wrapper { display: flex; - .topic-details-column-1 { + .topic-column.details-column { flex-direction: column; width: 80%; @@ -225,7 +225,7 @@ div.lightbox-wrapper { } } - .topic-details-column-2 { + .topic-column.featured-image-column { .topic-featured-image img { max-width: 200px; max-height: 100px; diff --git a/app/views/embed/topics.html.erb b/app/views/embed/topics.html.erb index 5a4660165a..2aaf326796 100644 --- a/app/views/embed/topics.html.erb +++ b/app/views/embed/topics.html.erb @@ -4,8 +4,8 @@
<%- if @template == "complete" %>
-
-
+
+
<%= t.title %>
@@ -38,7 +38,7 @@
-
+
<%- if t.image_url.present? %>
From 7453e19c68bc419072e264a416d5804920250507 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 9 Sep 2019 12:17:28 -0400 Subject: [PATCH 008/126] DEV: Update set-value dependency to 2.0.1 --- yarn.lock | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/yarn.lock b/yarn.lock index c91fa705d3..8f4e935f61 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1352,7 +1352,7 @@ is-number@^3.0.0: dependencies: kind-of "^3.0.2" -is-plain-object@^2.0.1, is-plain-object@^2.0.3, is-plain-object@^2.0.4: +is-plain-object@^2.0.3, is-plain-object@^2.0.4: version "2.0.4" resolved "https://registry.yarnpkg.com/is-plain-object/-/is-plain-object-2.0.4.tgz#2c163b3fafb1b606d9d17928f05c2a1c38e07677" integrity sha512-h5PpgXkWitc38BBMYawTYMWJHFZJVnBquFE57xFpjB8pJFiF6gZ+bU+WyI/yqXiFR5mdLsgYNaPe8uao6Uv9Og== @@ -1535,12 +1535,12 @@ linkify-it@^2.0.0: uc.micro "^1.0.1" "lodash-cli@https://github.com/lodash-archive/lodash-cli.git": - version "4.17.14" + version "4.17.15" resolved "https://github.com/lodash-archive/lodash-cli.git#91176e422d7c10634a59c7e367fe224795ef5a24" dependencies: closure-compiler "0.2.12" glob "7.1.1" - lodash "4.17.14" + lodash "4.17.15" semver "5.3.0" uglify-js "2.7.5" @@ -2188,16 +2188,6 @@ semver@^5.3.0, semver@^5.5.0: resolved "https://registry.yarnpkg.com/semver/-/semver-5.6.0.tgz#7e74256fbaa49c75aa7c7a205cc22799cac80004" integrity sha512-RS9R6R35NYgQn++fkDWaOmqGoj4Ek9gGs+DPxNUZKuwE183xjJroKvyo1IzVFeXvUrvmALy6FWD5xrdJT25gMg== -set-value@^0.4.3: - version "0.4.3" - resolved "https://registry.yarnpkg.com/set-value/-/set-value-0.4.3.tgz#7db08f9d3d22dc7f78e53af3c3bf4666ecdfccf1" - integrity sha1-fbCPnT0i3H945Trzw79GZuzfzPE= - dependencies: - extend-shallow "^2.0.1" - is-extendable "^0.1.1" - is-plain-object "^2.0.1" - to-object-path "^0.3.0" - set-value@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/set-value/-/set-value-2.0.0.tgz#71ae4a88f0feefbbf52d1ea604f3fb315ebb6274" @@ -2208,6 +2198,16 @@ set-value@^2.0.0: is-plain-object "^2.0.3" split-string "^3.0.1" +set-value@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/set-value/-/set-value-2.0.1.tgz#a18d40530e6f07de4228c7defe4227af8cad005b" + integrity sha512-JxHc1weCN68wRY0fhCoXpyK55m/XPHafOmK4UWD7m2CI14GMcFypt4w/0+NV5f/ZMby2F6S2wwA7fgynh9gWSw== + dependencies: + extend-shallow "^2.0.1" + is-extendable "^0.1.1" + is-plain-object "^2.0.3" + split-string "^3.0.1" + shebang-command@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-1.2.0.tgz#44aac65b695b03398968c39f363fee5deafdf1ea" @@ -2497,14 +2497,14 @@ ultron@~1.1.0: integrity sha512-UIEXBNeYmKptWH6z8ZnqTeS8fV74zG0/eRU9VGkpzz+LIJNs8W/zM/L+7ctCkRrgbNnnR0xxw4bKOr0cW0N0Og== union-value@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/union-value/-/union-value-1.0.0.tgz#5c71c34cb5bad5dcebe3ea0cd08207ba5aa1aea4" - integrity sha1-XHHDTLW61dzr4+oM0IIHulqhrqQ= + version "1.0.1" + resolved "https://registry.yarnpkg.com/union-value/-/union-value-1.0.1.tgz#0b6fe7b835aecda61c6ea4d4f02c14221e109847" + integrity sha512-tJfXmxMeWYnczCVs7XAEvIV7ieppALdyepWMkHkwciRpZraG/xwT+s2JN8+pr1+8jCRf80FFzvr+MpQeeoF4Xg== dependencies: arr-union "^3.1.0" get-value "^2.0.6" is-extendable "^0.1.1" - set-value "^0.4.3" + set-value "^2.0.1" unset-value@^1.0.0: version "1.0.0" From 30bba6252d179229230cafc16943ec82de258832 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 9 Sep 2019 15:01:48 -0400 Subject: [PATCH 009/126] Allow CORS combined with HTTP Auth --- config/initializers/008-rack-cors.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/008-rack-cors.rb b/config/initializers/008-rack-cors.rb index 8c6560d476..5f1c6f4068 100644 --- a/config/initializers/008-rack-cors.rb +++ b/config/initializers/008-rack-cors.rb @@ -39,7 +39,7 @@ class Discourse::Cors end headers['Access-Control-Allow-Origin'] = origin || cors_origins[0] - headers['Access-Control-Allow-Headers'] = 'Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Visible, User-Api-Key, User-Api-Client-Id' + headers['Access-Control-Allow-Headers'] = 'Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Visible, User-Api-Key, User-Api-Client-Id, Authorization' headers['Access-Control-Allow-Credentials'] = 'true' headers['Access-Control-Allow-Methods'] = 'POST, PUT, GET, OPTIONS, DELETE' end From ddd45d1419fa6d76287149c4c65fd889c29bb062 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 9 Sep 2019 15:07:40 -0400 Subject: [PATCH 010/126] FIX: Broken spec --- spec/components/hijack_spec.rb | 2 +- yarn.lock | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/components/hijack_spec.rb b/spec/components/hijack_spec.rb index f99d71f693..a0ef15e97f 100644 --- a/spec/components/hijack_spec.rb +++ b/spec/components/hijack_spec.rb @@ -109,7 +109,7 @@ describe Hijack do expected = { "Access-Control-Allow-Origin" => "www.rainbows.com", - "Access-Control-Allow-Headers" => "Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Visible, User-Api-Key, User-Api-Client-Id", + "Access-Control-Allow-Headers" => "Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Visible, User-Api-Key, User-Api-Client-Id, Authorization", "Access-Control-Allow-Credentials" => "true", "Access-Control-Allow-Methods" => "POST, PUT, GET, OPTIONS, DELETE" } diff --git a/yarn.lock b/yarn.lock index 8f4e935f61..48534aa809 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1544,7 +1544,12 @@ linkify-it@^2.0.0: semver "5.3.0" uglify-js "2.7.5" -lodash@4.17.14, lodash@^4.17.11, lodash@^4.17.4, lodash@^4.2.0, lodash@^4.3.0: +lodash@4.17.15: + version "4.17.15" + resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.15.tgz#b447f6670a0455bbfeedd11392eff330ea097548" + integrity sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A== + +lodash@^4.17.11, lodash@^4.17.4, lodash@^4.2.0, lodash@^4.3.0: version "4.17.14" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.14.tgz#9ce487ae66c96254fe20b599f21b6816028078ba" integrity sha512-mmKYbW3GLuJeX+iGP+Y7Gp1AiGHGbXHCOh/jZmrawMmsE7MS4znI3RL2FsjbqOyMayHInjOeykW7PEajUk1/xw== From 0d22beb81db354aeddddf94ea7e80addacd3abc4 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 10 Sep 2019 13:59:48 +0300 Subject: [PATCH 011/126] FIX: Improve Onebox detection (#8019) Follow-up to 7c83d2eeb261ac676a8320e6a704752c56fd242e. --- app/models/post_analyzer.rb | 9 ++++++--- lib/cooked_post_processor.rb | 2 +- .../discourse_narrative_bot/new_user_narrative_spec.rb | 1 + spec/models/post_analyzer_spec.rb | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/models/post_analyzer.rb b/app/models/post_analyzer.rb index a9540320f1..cfc64e5308 100644 --- a/app/models/post_analyzer.rb +++ b/app/models/post_analyzer.rb @@ -9,10 +9,11 @@ class PostAnalyzer @raw = raw @topic_id = topic_id @onebox_urls = [] + @found_oneboxes = false end def found_oneboxes? - @onebox_urls.present? + @found_oneboxes end def has_oneboxes? @@ -36,7 +37,9 @@ class PostAnalyzer result = Oneboxer.apply(cooked) do |url| @onebox_urls << url Oneboxer.invalidate(url) if opts[:invalidate_oneboxes] - Oneboxer.cached_onebox(url) + onebox = Oneboxer.cached_onebox(url) + @found_oneboxes = true if onebox.present? + onebox end cooked = result.to_html if result.changed? @@ -126,7 +129,7 @@ class PostAnalyzer # How many links are present in the post def link_count - raw_links.size + raw_links.size + @onebox_urls.size end def cooked_stripped diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 46050a69e4..f709b3d9cf 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -28,7 +28,7 @@ class CookedPostProcessor @cooking_options = @cooking_options.symbolize_keys @doc = Nokogiri::HTML::fragment(post.cook(post.raw, @cooking_options)) - @has_oneboxes = @doc.css("aside.onebox").count > 0 + @has_oneboxes = post.post_analyzer.found_oneboxes? @size_cache = {} @disable_loading_image = !!opts[:disable_loading_image] diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb index 89199058c7..1b35acd3d8 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb +++ b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb @@ -268,6 +268,7 @@ describe DiscourseNarrativeBot::NewUserNarrative do describe 'onebox tutorial' do before do + Oneboxer.stubs(:cached_onebox).with('https://en.wikipedia.org/wiki/ROT13').returns('oneboxed Wikipedia') narrative.set_data(user, state: :tutorial_onebox, topic_id: topic.id) end diff --git a/spec/models/post_analyzer_spec.rb b/spec/models/post_analyzer_spec.rb index ed3000b891..14afb3f80e 100644 --- a/spec/models/post_analyzer_spec.rb +++ b/spec/models/post_analyzer_spec.rb @@ -16,7 +16,7 @@ describe PostAnalyzer do before { Oneboxer.stubs(:onebox) } it 'fetches the cached onebox for any urls in the post' do - Oneboxer.expects(:cached_onebox).with url + Oneboxer.expects(:cached_onebox).with(url).returns('something') post_analyzer.cook(raw, options) expect(post_analyzer.found_oneboxes?).to be(true) end From cf23016360d12040b7679f95c95032aef4f4c99f Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 10 Sep 2019 11:11:15 -0400 Subject: [PATCH 012/126] FIX: Display emojis in search result blurbs --- app/assets/javascripts/discourse/lib/search.js.es6 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 index 28380ddc93..b8e7dd0546 100644 --- a/app/assets/javascripts/discourse/lib/search.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -4,6 +4,7 @@ import Category from "discourse/models/category"; import { search as searchCategoryTag } from "discourse/lib/category-tag-search"; import userSearch from "discourse/lib/user-search"; import { userPath } from "discourse/lib/url"; +import { emojiUnescape } from "discourse/lib/text"; import User from "discourse/models/user"; import Post from "discourse/models/post"; import Topic from "discourse/models/topic"; @@ -31,6 +32,7 @@ export function translateResults(results, opts) { } post = Post.create(post); post.set("topic", topicMap[post.topic_id]); + post.blurb = emojiUnescape(post.blurb); return post; }); From 1cebe7670a3c02087c333d3985708726d1d7433f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 10 Sep 2019 12:27:07 -0400 Subject: [PATCH 013/126] FEATURE: Allow embedding to ignore HTTP REFERER New site setting: `embed_any_origin` that will send postMessages to wildcard origins `*` instead of the referer. Most of the time you won't want to do this, so the setting is default to `false`. However, there are certain situations where you want to allow embedding to send post messages when there is no HTTP REFERER. For example, if you created a native mobile app and you wanted to embed a list of Discourse topics as HTML. In the code your HTML would be a static file/string, which would not be able to send a referer. In this case, the site setting will allow the embed to work. From a security standpoint we currently only use `postMessage` to send data about the size of the HTML document and scroll position, so it should be enable if required with minimal security ramifications. --- app/controllers/embed_controller.rb | 7 +++++-- app/views/layouts/embed.html.erb | 2 +- config/locales/server.en.yml | 3 ++- config/site_settings.yml | 1 + spec/requests/embed_controller_spec.rb | 19 ++++++++++++++++++- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/app/controllers/embed_controller.rb b/app/controllers/embed_controller.rb index c4bde730a5..5490fa59fa 100644 --- a/app/controllers/embed_controller.rb +++ b/app/controllers/embed_controller.rb @@ -8,7 +8,7 @@ class EmbedController < ApplicationController skip_before_action :check_xhr, :preload_json, :verify_authenticity_token before_action :ensure_embeddable, except: [ :info, :topics ] - before_action :get_embeddable_css_class, except: [ :info, :topics ] + before_action :prepare_embeddable, except: [ :info ] before_action :ensure_api_request, only: [ :info ] layout 'embed' @@ -123,10 +123,13 @@ class EmbedController < ApplicationController private - def get_embeddable_css_class + def prepare_embeddable @embeddable_css_class = "" embeddable_host = EmbeddableHost.record_for_url(request.referer) @embeddable_css_class = " class=\"#{embeddable_host.class_name}\"" if embeddable_host.present? && embeddable_host.class_name.present? + + @data_referer = request.referer + @data_referer = '*' if SiteSetting.embed_any_origin? && @data_referer.blank? end def ensure_api_request diff --git a/app/views/layouts/embed.html.erb b/app/views/layouts/embed.html.erb index d1cf2be33e..b7b714fbc5 100644 --- a/app/views/layouts/embed.html.erb +++ b/app/views/layouts/embed.html.erb @@ -13,7 +13,7 @@ <%= @topic_view.page_title %> - <%= SiteSetting.title %> <%- end %> - + <%= preload_script 'embed-application' %> <%= yield :head %> diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ff522b004e..9f3b705a25 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1959,6 +1959,7 @@ en: autohighlight_all_code: "Force apply code highlighting to all preformatted code blocks even when they didn't explicitly specify the language." highlighted_languages: "Included syntax highlighting rules. (Warning: including too many languages may impact performance) see: https://highlightjs.org/static/demo for a demo" + embed_any_origin: "Allow embeddable content regardless of origin. This is required for mobile apps with static HTML." embed_topics_list: "Support HTML embedding of topics lists" embed_truncate: "Truncate the embedded posts." embed_support_markdown: "Support Markdown formatting for embedded posts." @@ -4598,4 +4599,4 @@ en: html_missing_placeholder: "The html template must include %{placeholder}" discord: - not_in_allowed_guild: 'Authentication failed. You are not a member of a permitted Discord guild.' \ No newline at end of file + not_in_allowed_guild: 'Authentication failed. You are not a member of a permitted Discord guild.' diff --git a/config/site_settings.yml b/config/site_settings.yml index c1a6cbfd74..9a5327f980 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -859,6 +859,7 @@ posting: choices: - 4-spaces-indent - code-fences + embed_any_origin: false embed_topics_list: false embed_truncate: default: true diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb index 8d945dcb44..5f6abb61f4 100644 --- a/spec/requests/embed_controller_spec.rb +++ b/spec/requests/embed_controller_spec.rb @@ -88,12 +88,29 @@ describe EmbedController do it "returns a list of topics" do topic = Fabricate(:topic) - get '/embed/topics?discourse_embed_id=de-1234', headers: headers + get '/embed/topics?discourse_embed_id=de-1234', headers: { + 'REFERER' => 'https://example.com/evil-trout' + } expect(response.status).to eq(200) expect(response.headers['X-Frame-Options']).to eq("ALLOWALL") expect(response.body).to match("data-embed-id=\"de-1234\"") expect(response.body).to match("data-topic-id=\"#{topic.id}\"") + expect(response.body).to match("data-referer=\"https://example.com/evil-trout\"") end + + it "returns no referer if not supplied" do + get '/embed/topics?discourse_embed_id=de-1234' + expect(response.status).to eq(200) + expect(response.body).to match("data-referer=\"\"") + end + + it "returns * for the referer if `embed_any_origin` is set" do + SiteSetting.embed_any_origin = true + get '/embed/topics?discourse_embed_id=de-1234' + expect(response.status).to eq(200) + expect(response.body).to match("data-referer=\"\\*\"") + end + end end From a25869969ad9d4758a366931cdfc946a6a0436a3 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 10 Sep 2019 11:58:08 -0500 Subject: [PATCH 014/126] DEV: adds event hook when add/remove user to group (#8038) --- .../regular/automatic_group_membership.rb | 2 +- app/models/group.rb | 5 +++- spec/jobs/automatic_group_membership_spec.rb | 17 ++++++++++++- spec/models/group_spec.rb | 24 +++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/app/jobs/regular/automatic_group_membership.rb b/app/jobs/regular/automatic_group_membership.rb index 9335b6e34a..8695737ab2 100644 --- a/app/jobs/regular/automatic_group_membership.rb +++ b/app/jobs/regular/automatic_group_membership.rb @@ -22,7 +22,7 @@ module Jobs .where(staged: false) .find_each do |user| next unless user.email_confirmed? - group.add(user) + group.add(user, automatic: true) GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user) end diff --git a/app/models/group.rb b/app/models/group.rb index a04cdef63e..c3f632acd1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -613,7 +613,7 @@ class Group < ActiveRecord::Base PUBLISH_CATEGORIES_LIMIT = 10 - def add(user, notify: false) + def add(user, notify: false, automatic: false) self.users.push(user) unless self.users.include?(user) if notify @@ -635,12 +635,15 @@ class Group < ActiveRecord::Base Discourse.request_refresh!(user_ids: [user.id]) end + DiscourseEvent.trigger(:user_added_to_group, user, self, automatic: automatic) + self end def remove(user) self.group_users.where(user: user).each(&:destroy) user.update_columns(primary_group_id: nil) if user.primary_group_id == self.id + DiscourseEvent.trigger(:user_removed_from_group, user, self) end def add_owner(user) diff --git a/spec/jobs/automatic_group_membership_spec.rb b/spec/jobs/automatic_group_membership_spec.rb index cb1544dab7..697b684361 100644 --- a/spec/jobs/automatic_group_membership_spec.rb +++ b/spec/jobs/automatic_group_membership_spec.rb @@ -23,7 +23,22 @@ describe Jobs::AutomaticGroupMembership do group = Fabricate(:group, automatic_membership_email_domains: "wat.com", automatic_membership_retroactive: true) - Jobs::AutomaticGroupMembership.new.execute(group_id: group.id) + begin + automatic = nil + called = false + + DiscourseEvent.on(:user_added_to_group) do |_u, _g, options| + automatic = options[:automatic] + called = true + end + + Jobs::AutomaticGroupMembership.new.execute(group_id: group.id) + + expect(automatic).to eql(true) + expect(called).to eq(true) + ensure + DiscourseEvent.off(:user_added_to_group) + end group.reload expect(group.users.include?(user1)).to eq(false) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 4739357d19..d215a89458 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -806,6 +806,11 @@ describe Group do user.update(primary_group: group) expect { group.remove(user) }.to change { user.reload.primary_group }.from(group).to(nil) end + + it 'triggers a user_removed_from_group event' do + events = DiscourseEvent.track_events { group.remove(user) }.map { |e| e[:event_name] } + expect(events).to include(:user_removed_from_group) + end end describe '#add' do @@ -839,6 +844,25 @@ describe Group do expect(notification.user_id).to eq(user.id) end + it 'triggers a user_added_to_group event' do + begin + automatic = nil + called = false + + DiscourseEvent.on(:user_added_to_group) do |_u, _g, options| + automatic = options[:automatic] + called = true + end + + group.add(user) + + expect(automatic).to eql(false) + expect(called).to eq(true) + ensure + DiscourseEvent.off(:user_added_to_group) + end + end + context 'when adding a user into a public group' do fab!(:category) { Fabricate(:category) } From bd930a9c77919ced40d593063bf7672b0d1bb29e Mon Sep 17 00:00:00 2001 From: romanrizzi Date: Tue, 10 Sep 2019 14:21:34 -0300 Subject: [PATCH 015/126] FIX: Do not show staged users avatars when expanding the read count indicator --- app/controllers/post_readers_controller.rb | 1 + spec/requests/post_readers_controller_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/app/controllers/post_readers_controller.rb b/app/controllers/post_readers_controller.rb index 392f575c9b..e70d49b216 100644 --- a/app/controllers/post_readers_controller.rb +++ b/app/controllers/post_readers_controller.rb @@ -10,6 +10,7 @@ class PostReadersController < ApplicationController readers = User .joins(:topic_users) + .where(staged: false) .where.not(topic_users: { last_read_post_number: nil }) .where('topic_users.topic_id = ? AND topic_users.last_read_post_number >= ?', post.topic_id, post.post_number) .where.not(id: post.user_id) diff --git a/spec/requests/post_readers_controller_spec.rb b/spec/requests/post_readers_controller_spec.rb index 267dfea110..954e14b6f3 100644 --- a/spec/requests/post_readers_controller_spec.rb +++ b/spec/requests/post_readers_controller_spec.rb @@ -66,6 +66,16 @@ describe PostReadersController do expect(readers).to be_empty end + + it "doesn't include staged users" do + TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: 4) + reader.update(staged: true) + + get '/post_readers.json', params: { id: @post.id } + readers = JSON.parse(response.body)['post_readers'] + + expect(readers).to be_empty + end end def assert_reader_is_correctly_serialized(reader_data, reader, post) From 509e61e2722862eae22d51b1e97a4288816b2047 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 10 Sep 2019 14:59:51 -0400 Subject: [PATCH 016/126] UX: Show installed version with SHA instead of number of commits Removes the "+ 437" string at the end of "v2.4.0.beta2 + 437", and adds a link to the exact commit on github underneath. --- app/assets/javascripts/admin/templates/version-checks.hbs | 5 ++++- app/assets/stylesheets/common/admin/dashboard.scss | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/templates/version-checks.hbs b/app/assets/javascripts/admin/templates/version-checks.hbs index 111d863412..60dd8a814b 100644 --- a/app/assets/javascripts/admin/templates/version-checks.hbs +++ b/app/assets/javascripts/admin/templates/version-checks.hbs @@ -5,7 +5,10 @@

{{i18n 'admin.dashboard.installed_version'}}

-

{{dash-if-empty versionCheck.installed_describe}}

+

{{dash-if-empty versionCheck.installed_version}}

+
+ ({{versionCheck.shortSha}}) +
{{#if versionCheck.noCheckPerformed}} diff --git a/app/assets/stylesheets/common/admin/dashboard.scss b/app/assets/stylesheets/common/admin/dashboard.scss index 8d42b716bb..faec4aca61 100644 --- a/app/assets/stylesheets/common/admin/dashboard.scss +++ b/app/assets/stylesheets/common/admin/dashboard.scss @@ -626,10 +626,13 @@ flex: 1 0 auto; white-space: nowrap; } - h4 { + h4, .sha-link { font-size: $font-down-2; margin-bottom: 0; } + .sha-link { + font-weight: normal; + } } .version-status { display: flex; From 82398907aff21ae8b70351eb8bebd8be2d81c97c Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 10 Sep 2019 15:09:16 -0400 Subject: [PATCH 017/126] Prettier --- app/assets/stylesheets/common/admin/dashboard.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/common/admin/dashboard.scss b/app/assets/stylesheets/common/admin/dashboard.scss index faec4aca61..d92e0191b7 100644 --- a/app/assets/stylesheets/common/admin/dashboard.scss +++ b/app/assets/stylesheets/common/admin/dashboard.scss @@ -626,7 +626,8 @@ flex: 1 0 auto; white-space: nowrap; } - h4, .sha-link { + h4, + .sha-link { font-size: $font-down-2; margin-bottom: 0; } From f0586ece3205dc0a117501527f682f18e5203be0 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 10 Sep 2019 16:38:23 -0400 Subject: [PATCH 018/126] FIX: errors in qunit tests when version check info is missing --- app/assets/javascripts/admin/models/version-check.js.es6 | 8 +++++--- app/assets/javascripts/admin/templates/version-checks.hbs | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/models/version-check.js.es6 b/app/assets/javascripts/admin/models/version-check.js.es6 index ba0bdf018d..ce41d66713 100644 --- a/app/assets/javascripts/admin/models/version-check.js.es6 +++ b/app/assets/javascripts/admin/models/version-check.js.es6 @@ -19,16 +19,18 @@ const VersionCheck = Discourse.Model.extend({ @computed("git_branch", "installed_sha") gitLink(gitBranch, installedSHA) { - if (gitBranch) { + if (gitBranch && installedSHA) { return `https://github.com/discourse/discourse/compare/${installedSHA}...${gitBranch}`; - } else { + } else if (installedSHA) { return `https://github.com/discourse/discourse/tree/${installedSHA}`; } }, @computed("installed_sha") shortSha(installedSHA) { - return installedSHA.substr(0, 10); + if (installedSHA) { + return installedSHA.substr(0, 10); + } } }); diff --git a/app/assets/javascripts/admin/templates/version-checks.hbs b/app/assets/javascripts/admin/templates/version-checks.hbs index 60dd8a814b..ced3da8b79 100644 --- a/app/assets/javascripts/admin/templates/version-checks.hbs +++ b/app/assets/javascripts/admin/templates/version-checks.hbs @@ -6,9 +6,11 @@

{{i18n 'admin.dashboard.installed_version'}}

{{dash-if-empty versionCheck.installed_version}}

-
- ({{versionCheck.shortSha}}) -
+ {{#if versionCheck.gitLink}} +
+ ({{versionCheck.shortSha}}) +
+ {{/if}}
{{#if versionCheck.noCheckPerformed}} From 503a11cc88f63a555a1afd9e2cc5f02708467c4f Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 10 Sep 2019 21:50:48 -0400 Subject: [PATCH 019/126] FIX: inline_uploads and subfolder (#8076) * FIX: inline_uploads and subfolder * if subfolder, also look for images with a path containing cdn_url + relative_url_root * FIX: migrate_to_s3 task and subfolder --- app/services/inline_uploads.rb | 5 ++++- lib/tasks/uploads.rake | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index 039d5f3773..7ffa42943a 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -282,7 +282,10 @@ class InlineUploads /(#{base_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/, ] - regexps << /(#{cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ if cdn_url + if cdn_url + regexps << /(#{cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ unless GlobalSetting.relative_url_root.nil? + end if Discourse.store.external? if Rails.configuration.multisite diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index bf7b539b54..05b803026f 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -306,7 +306,7 @@ def migrate_to_s3 puts "*" * 30 + " DRY RUN " + "*" * 30 if dry_run puts "Migrating uploads to S3 for '#{db}'..." - if Upload.by_users.where("url NOT LIKE '//%' AND url NOT LIKE '/uploads/#{db}/original/_X/%'").exists? + if Upload.by_users.where("url NOT LIKE '//%' AND url NOT LIKE '#{GlobalSetting.relative_url_root}/uploads/#{db}/original/_X/%'").exists? puts <<~TEXT Some uploads were not migrated to the new scheme. Please run these commands in the rails console From 93b0ffc85c951a16165b677e738c92df5af6693d Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 11 Sep 2019 12:02:42 +1000 Subject: [PATCH 020/126] DEV: improve subfolder inline logic This follows up on #503a11cc88f with a spec --- app/services/inline_uploads.rb | 5 ++++- spec/services/inline_uploads_spec.rb | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index 7ffa42943a..d8ddc4eaf0 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -284,7 +284,10 @@ class InlineUploads if cdn_url regexps << /(#{cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ - regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ unless GlobalSetting.relative_url_root.nil? + if GlobalSetting.relative_url_root.present? + regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ + + end end if Discourse.store.external? diff --git a/spec/services/inline_uploads_spec.rb b/spec/services/inline_uploads_spec.rb index 41646c341e..b42f7c75f7 100644 --- a/spec/services/inline_uploads_spec.rb +++ b/spec/services/inline_uploads_spec.rb @@ -217,13 +217,16 @@ RSpec.describe InlineUploads do end it "should correct subfolder images" do - md = <<~MD + + #{Discourse.base_url}#{upload.url} MD expect(InlineUploads.process(md)).to eq(<<~MD) ![](#{upload.short_url}) + + ![](#{upload.short_url}) MD end end From 7252c14a1679be4d758f79ef73b8dc4563a7b470 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Wed, 11 Sep 2019 00:18:12 -0400 Subject: [PATCH 021/126] FIX: Switch to full screen external login for Safari This is a temporary solution, currently popup auth fails in Safari. --- app/assets/javascripts/discourse/controllers/login.js.es6 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 19da455a26..13c77deab7 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -217,7 +217,10 @@ export default Ember.Controller.extend(ModalFunctionality, { // On Mobile, Android or iOS always go with full screen if ( this.isMobileDevice || - (capabilities && (capabilities.isIOS || capabilities.isAndroid)) + (capabilities && + (capabilities.isIOS || + capabilities.isAndroid || + capabilities.isSafari)) ) { fullScreenLogin = true; } From fb8d1f35a40c83d7a290114bfa7efd7926b92090 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 11 Sep 2019 17:35:58 +1000 Subject: [PATCH 022/126] DEV: support USR2 to restart unicorn in dev mode In dev mode sending USR2 to the unicorn master supervisor process will restart unicorn. This allows a simple script like this to restart unicorn in dev: ``` #!/usr/bin/env bash kill -s USR2 `ps aux | grep ruby\ bin\/unicorn | grep -v grep | awk '{print $2}'` ``` --- bin/unicorn | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/bin/unicorn b/bin/unicorn index cff2163997..01b90020bc 100755 --- a/bin/unicorn +++ b/bin/unicorn @@ -8,11 +8,15 @@ ENV['BUNDLE_GEMFILE'] ||= File.expand_path("../../Gemfile", require 'rubygems' require 'bundler/setup' +dev_mode = false + # in development do some fussing around, to automate config if !ARGV.include?("-E") && !ARGV.include?("--env") && (ENV["RAILS_ENV"] == "development" || !ENV["RAILS_ENV"]) + dev_mode = true + ARGV.push("-N") if !ARGV.include?("-c") && !ARGV.include?("--config-file") ARGV.push("-c") @@ -46,4 +50,37 @@ if ARGV.include?("--help") exit end -load Gem.bin_path('unicorn', 'unicorn') +# this dev_mode hackery enables, the following script to be used to restart unicorn: +# +# #!/usr/bin/env bash +# kill -s USR2 `ps aux | grep ruby\ bin\/unicorn | grep -v grep | awk '{print $2}'` +# +# This is handy if you want to bind a key to restarting unicorn in dev + +if dev_mode + restart = true + while restart + restart = false + pid = fork do + load Gem.bin_path('unicorn', 'unicorn') + end + done = false + + Signal.trap('INT') do + # wait for parent to be done + end + + Signal.trap('USR2') do + Process.kill('QUIT', pid) + puts "RESTARTING UNICORN" + restart = true + end + + while !done + sleep 1 + done = Process.waitpid(pid, Process::WNOHANG) + end + end +else + load Gem.bin_path('unicorn', 'unicorn') +end From 7c084c7cab30c38f1a7604e279b6937d01abed9b Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 11 Sep 2019 17:43:36 +1000 Subject: [PATCH 023/126] DEV: allow USR2 to restart unicorn in dev Well all this does is amends a commit comment cause I was over eager and pushed previous commit. This is the comment I wanted.... This allows `pkill -USR2 -f 'ruby bin/unicorn'` to restart current unicorn It is handy if you want to bind a keyboard shortcut for unicorn restarts in dev. By doing so you can avoid finding the terminal, hitting ctrl-c and then hitting up, enter, heading back to browser. It saves time. Automate stuff, you will not regret it... --- bin/unicorn | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/unicorn b/bin/unicorn index 01b90020bc..865e8d87dc 100755 --- a/bin/unicorn +++ b/bin/unicorn @@ -50,10 +50,9 @@ if ARGV.include?("--help") exit end -# this dev_mode hackery enables, the following script to be used to restart unicorn: +# this dev_mode hackery enables, the following to be used to restart unicorn: # -# #!/usr/bin/env bash -# kill -s USR2 `ps aux | grep ruby\ bin\/unicorn | grep -v grep | awk '{print $2}'` +# pkill -USR2 -f 'ruby bin/unicorn' # # This is handy if you want to bind a key to restarting unicorn in dev From 5da7ffd46cc2cba8785fd4a4885915265feecc12 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 11 Sep 2019 18:43:02 +1000 Subject: [PATCH 024/126] FEATURE: update mini_scheduler to support history filtering New version of mini scheduler allows you to select the name of a schedule in the history page in `/sidekiq/scheduler/history`. This is handy for quickly looking up timing trends. --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 64fd90002c..868c67c591 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -187,7 +187,7 @@ GEM mini_portile2 (2.4.0) mini_racer (0.2.6) libv8 (>= 6.9.411) - mini_scheduler (0.12.1) + mini_scheduler (0.12.2) sidekiq mini_sql (0.2.2) mini_suffix (0.3.0) From 8b2c272f04632e048b4c572c0b498eb926c7c459 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 11 Sep 2019 13:00:53 +0300 Subject: [PATCH 025/126] UX: Use medium format for displaying time in post notices. (#8074) --- .../javascripts/discourse/lib/formatter.js.es6 | 16 +++++++++++++--- .../javascripts/discourse/widgets/post.js.es6 | 4 ++-- config/locales/client.en.yml | 6 ++++++ test/javascripts/widgets/post-test.js.es6 | 2 +- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/formatter.js.es6 b/app/assets/javascripts/discourse/lib/formatter.js.es6 index 2d5961a552..2f71c83658 100644 --- a/app/assets/javascripts/discourse/lib/formatter.js.es6 +++ b/app/assets/javascripts/discourse/lib/formatter.js.es6 @@ -262,7 +262,7 @@ function relativeAgeTinyShowsYear(relativeAgeString) { return relativeAgeString.match(/'[\d]{2}$/); } -function relativeAgeMediumSpan(distance, leaveAgo) { +export function relativeAgeMediumSpan(distance, leaveAgo) { let formatted; const distanceInMinutes = Math.round(distance / 60.0); @@ -283,14 +283,24 @@ function relativeAgeMediumSpan(distance, leaveAgo) { case distanceInMinutes >= 90 && distanceInMinutes <= 1409: formatted = t("x_hours", { count: Math.round(distanceInMinutes / 60.0) }); break; - case distanceInMinutes >= 1410 && distanceInMinutes <= 2159: + case distanceInMinutes >= 1410 && distanceInMinutes <= 2519: formatted = t("x_days", { count: 1 }); break; - case distanceInMinutes >= 2160: + case distanceInMinutes >= 2520 && distanceInMinutes <= 129599: formatted = t("x_days", { count: Math.round((distanceInMinutes - 720.0) / 1440.0) }); break; + case distanceInMinutes >= 129600 && distanceInMinutes <= 525599: + formatted = t("x_months", { + count: Math.round(distanceInMinutes / 43200.0) + }); + break; + default: + formatted = t("x_years", { + count: Math.round(distanceInMinutes / 525600.0) + }); + break; } return formatted || "—"; } diff --git a/app/assets/javascripts/discourse/widgets/post.js.es6 b/app/assets/javascripts/discourse/widgets/post.js.es6 index 4138fdbaa0..65f55a996b 100644 --- a/app/assets/javascripts/discourse/widgets/post.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post.js.es6 @@ -14,7 +14,7 @@ import { formatUsername } from "discourse/lib/utilities"; import hbs from "discourse/widgets/hbs-compiler"; -import { durationTiny } from "discourse/lib/formatter"; +import { relativeAgeMediumSpan } from "discourse/lib/formatter"; import { prioritizeNameInUx } from "discourse/lib/settings"; function transformWithCallbacks(post) { @@ -472,7 +472,7 @@ createWidget("post-notice", { "p", I18n.t("post.notice.returning_user", { user, - time: durationTiny(distance, { addAgo: true }) + time: relativeAgeMediumSpan(distance, true) }) ); } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index bc0dcbb0e0..eaece6536b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -124,6 +124,12 @@ en: x_days: one: "%{count} day ago" other: "%{count} days ago" + x_months: + one: "%{count} month ago" + other: "%{count} months ago" + x_years: + one: "%{count} year ago" + other: "%{count} years ago" later: x_days: one: "%{count} day later" diff --git a/test/javascripts/widgets/post-test.js.es6 b/test/javascripts/widgets/post-test.js.es6 index 3735d1036f..422e485a5d 100644 --- a/test/javascripts/widgets/post-test.js.es6 +++ b/test/javascripts/widgets/post-test.js.es6 @@ -893,7 +893,7 @@ widgetTest("post notice - with username", { .trim(), I18n.t("post.notice.returning_user", { user: "codinghorror", - time: "2d ago" + time: "2 days ago" }) ); } From 9ba4a67766d6934c1764cc828bc6f9bea1fe6156 Mon Sep 17 00:00:00 2001 From: Leo McArdle Date: Wed, 11 Sep 2019 15:24:22 +0100 Subject: [PATCH 026/126] pass model to above-user-preferences outlet (#8084) --- app/assets/javascripts/discourse/templates/preferences.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/templates/preferences.hbs b/app/assets/javascripts/discourse/templates/preferences.hbs index 3abd833348..854db06883 100644 --- a/app/assets/javascripts/discourse/templates/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/preferences.hbs @@ -54,7 +54,7 @@ {{/d-section}}
- {{plugin-outlet name="above-user-preferences"}} + {{plugin-outlet name="above-user-preferences" args=(hash model=model)}}
{{outlet}} From d93586c63f257b658fc72e4f9e4edbd1276a140f Mon Sep 17 00:00:00 2001 From: Rishabh Date: Wed, 11 Sep 2019 20:19:54 +0530 Subject: [PATCH 027/126] FEATURE: Whitelist the `allowfullscreen` iframe attribute (#8086) Adding this to core allows showing the fullscreen button on external iframe videos like Vimeo. https://meta.discourse.org/t/full-screen-videos-plugin/90869 will not be needed anymore. --- app/assets/javascripts/pretty-text/white-lister.js.es6 | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/pretty-text/white-lister.js.es6 b/app/assets/javascripts/pretty-text/white-lister.js.es6 index 40918c49c3..5ea109398b 100644 --- a/app/assets/javascripts/pretty-text/white-lister.js.es6 +++ b/app/assets/javascripts/pretty-text/white-lister.js.es6 @@ -167,6 +167,7 @@ const DEFAULT_LIST = [ "iframe[marginheight]", "iframe[marginwidth]", "iframe[width]", + "iframe[allowfullscreen]", "img[alt]", "img[height]", "img[title]", From f7ad87995de32177ad61fa61f018de334a62d8fa Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 11 Sep 2019 12:26:23 -0400 Subject: [PATCH 028/126] FIX: Missing translation --- config/locales/client.en.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index eaece6536b..50b1517c15 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -384,6 +384,9 @@ en: total: "Total" min_score_visibility: "Minimum Score for Visibility" score_to_hide: "Score to Hide Post" + take_action_bonus: + name: 'took action' + title: 'When a staff member chooses to take action the flag is given a bonus.' user_accuracy_bonus: name: "user accuracy" title: "Users whose flags have been historically agreed with are given a bonus." From 56c6e8e961ab59336953bb564debcad7288d4fc0 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Wed, 11 Sep 2019 18:43:02 +0200 Subject: [PATCH 029/126] FIX: POP3 doesn't work with TLS 1.3 --- app/jobs/scheduled/poll_mailbox.rb | 2 +- lib/tasks/emails.rake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 9bf10f93c0..80b858a11a 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -34,7 +34,7 @@ module Jobs if SiteSetting.pop3_polling_ssl if SiteSetting.pop3_polling_openssl_verify - pop3.enable_ssl + pop3.enable_ssl(max_version: OpenSSL::SSL::TLS1_2_VERSION) else pop3.enable_ssl(OpenSSL::SSL::VERIFY_NONE) end diff --git a/lib/tasks/emails.rake b/lib/tasks/emails.rake index 73be734363..9eb9250199 100644 --- a/lib/tasks/emails.rake +++ b/lib/tasks/emails.rake @@ -37,7 +37,7 @@ task "emails:import" => :environment do mails_left = 1 pop3 = Net::POP3.new(address, port) - pop3.enable_ssl if ssl + pop3.enable_ssl(max_version: OpenSSL::SSL::TLS1_2_VERSION) if ssl while mails_left > 0 pop3.start(username, password) do |pop| From 4195548a17135f8bdbee24a4c1157a8b080271d6 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 11 Sep 2019 23:23:11 +0530 Subject: [PATCH 030/126] Bump onebox version. - indicate and link to Flickr Album --- Gemfile | 2 +- Gemfile.lock | 4 ++-- app/assets/stylesheets/common/base/onebox.scss | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 1599efa815..1e9fe98481 100644 --- a/Gemfile +++ b/Gemfile @@ -46,7 +46,7 @@ gem 'redis-namespace' gem 'active_model_serializers', '~> 0.8.3' -gem 'onebox', '1.9.12' +gem 'onebox', '1.9.13' gem 'http_accept_language', '~>2.0.5', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 868c67c591..7fc71a5425 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -241,7 +241,7 @@ GEM omniauth-twitter (1.4.0) omniauth-oauth (~> 1.1) rack - onebox (1.9.12) + onebox (1.9.13) htmlentities (~> 4.3) moneta (~> 1.0) multi_json (~> 1.11) @@ -493,7 +493,7 @@ DEPENDENCIES omniauth-oauth2 omniauth-openid omniauth-twitter - onebox (= 1.9.12) + onebox (= 1.9.13) openid-redis-store parallel_tests pg diff --git a/app/assets/stylesheets/common/base/onebox.scss b/app/assets/stylesheets/common/base/onebox.scss index e1a6a3dfb7..1a1bd19f0d 100644 --- a/app/assets/stylesheets/common/base/onebox.scss +++ b/app/assets/stylesheets/common/base/onebox.scss @@ -464,8 +464,8 @@ aside.onebox.twitterstatus .onebox-body { } } -// Onebox - Imgur - Album -.onebox.imgur-album { +// Onebox - Imgur/Flickr - Album +.onebox.imgur-album, .onebox.flickr-album { .outer-box { position: absolute; z-index: z("base"); From 59f7c926611a71a4ee4c6c7241edc10811fbae1e Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 11 Sep 2019 23:32:58 +0530 Subject: [PATCH 031/126] fix the build. --- app/assets/stylesheets/common/base/onebox.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/common/base/onebox.scss b/app/assets/stylesheets/common/base/onebox.scss index 1a1bd19f0d..b00f53fb83 100644 --- a/app/assets/stylesheets/common/base/onebox.scss +++ b/app/assets/stylesheets/common/base/onebox.scss @@ -465,7 +465,8 @@ aside.onebox.twitterstatus .onebox-body { } // Onebox - Imgur/Flickr - Album -.onebox.imgur-album, .onebox.flickr-album { +.onebox.imgur-album, +.onebox.flickr-album { .outer-box { position: absolute; z-index: z("base"); From 7331e0865ad72bbdd4d7d9d17c818a64acd41deb Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 11 Sep 2019 15:18:17 -0400 Subject: [PATCH 032/126] FIX: user directory should not include unapproved users When the "must approve users" setting is enabled, new users who were waiting to be approved could show up in the user directory. --- app/models/directory_item.rb | 3 ++- spec/models/directory_item_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/directory_item.rb b/app/models/directory_item.rb index 34b32ee159..89f8daf2d3 100644 --- a/app/models/directory_item.rb +++ b/app/models/directory_item.rb @@ -65,7 +65,8 @@ class DirectoryItem < ActiveRecord::Base 0 FROM users u LEFT JOIN directory_items di ON di.user_id = u.id AND di.period_type = :period_type - WHERE di.id IS NULL AND u.id > 0 AND u.silenced_till IS NULL and u.active + WHERE di.id IS NULL AND u.id > 0 AND u.silenced_till IS NULL AND u.active + #{SiteSetting.must_approve_users ? 'AND u.approved' : ''} ", period_type: period_types[period_type] # Calculate new values and update records diff --git a/spec/models/directory_item_spec.rb b/spec/models/directory_item_spec.rb index b2465e070d..54f8d72951 100644 --- a/spec/models/directory_item_spec.rb +++ b/spec/models/directory_item_spec.rb @@ -143,5 +143,17 @@ describe DirectoryItem do expect(monthly_directory_item.days_visited).to eq(3) end + context 'must approve users' do + before do + SiteSetting.must_approve_users = true + end + + it "doesn't include user who hasn't been approved" do + user = Fabricate(:user, approved: false) + DirectoryItem.refresh! + expect(DirectoryItem.where(user_id: user.id).count).to eq(0) + end + end + end end From eeb84806bcadd83b82d6295a5865fe473e1aca3f Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Thu, 12 Sep 2019 02:31:51 +0200 Subject: [PATCH 033/126] Fix frontend tests on Travis (#8089) * Remove obsolete travis cache option (apt) See: https://github.com/travis-ci/travis-ci/issues/5876 * Travis config value "addons.postresql" is a string * Update Travis from trusty to xenial Fixes the frontend build, which was failing because Chrome has dropped Trusty support in the latest version. See: https://travis-ci.community/t/travis-is-downloading-older-chrome-versions-in-place-of-chrome-stable/5040/4 --- .travis.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index c134e72cea..7119b6cd2e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,7 +21,7 @@ env: addons: chrome: stable - postgresql: 9.6 + postgresql: "9.6" apt: update: true packages: @@ -40,10 +40,9 @@ services: - redis-server sudo: required -dist: trusty +dist: xenial cache: - apt: true yarn: true directories: - vendor/bundle From 32b8a2ccffd50d78b8f7be6f0b524506cd2dbd7e Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 12 Sep 2019 10:41:50 +1000 Subject: [PATCH 034/126] DEV: Upgrade Discourse to Rails 6 (#8083) * Adjustments to pass specs on Rails 6.0.0 * Use classic autoloader instead of Zeitwerk * Update Rails 6.0.0 deprecated methods * Rails 6.0.0 not allowing column with integer name * Drop freedom_patches/rails6.rb * Default value for trigger_transactional_callbacks? is true * Bump rspec-rails version to 4.0.0.beta2 --- Gemfile | 16 +-- Gemfile.lock | 88 +++++++------- app/views/users/show.html.erb | 2 +- config/application.rb | 2 + ...20130430052751_add_topic_allowed_groups.rb | 4 +- ...20130501105651_fix_topic_allowed_groups.rb | 2 +- lib/freedom_patches/rails6.rb | 110 ------------------ lib/mini_sql_multisite_connection.rb | 3 + .../components/migration/safe_migrate_spec.rb | 4 +- spec/components/post_revisor_spec.rb | 2 +- spec/mailers/user_notifications_spec.rb | 2 +- spec/requests/badges_controller_spec.rb | 2 +- spec/requests/groups_controller_spec.rb | 4 +- spec/requests/list_controller_spec.rb | 8 +- spec/requests/metadata_controller_spec.rb | 8 +- spec/requests/static_controller_spec.rb | 6 +- spec/requests/topics_controller_spec.rb | 2 +- 17 files changed, 80 insertions(+), 185 deletions(-) delete mode 100644 lib/freedom_patches/rails6.rb diff --git a/Gemfile b/Gemfile index 1e9fe98481..98e165f3ac 100644 --- a/Gemfile +++ b/Gemfile @@ -16,13 +16,13 @@ if rails_master? else # until rubygems gives us optional dependencies we are stuck with this # bundle update actionmailer actionpack actionview activemodel activerecord activesupport railties - gem 'actionmailer', '5.2.3' - gem 'actionpack', '5.2.3' - gem 'actionview', '5.2.3' - gem 'activemodel', '5.2.3' - gem 'activerecord', '5.2.3' - gem 'activesupport', '5.2.3' - gem 'railties', '5.2.3' + gem 'actionmailer', '6.0.0' + gem 'actionpack', '6.0.0' + gem 'actionview', '6.0.0' + gem 'activemodel', '6.0.0' + gem 'activerecord', '6.0.0' + gem 'activesupport', '6.0.0' + gem 'railties', '6.0.0' gem 'sprockets-rails' end @@ -140,7 +140,7 @@ group :test, :development do gem 'mocha', require: false gem 'rb-fsevent', require: RUBY_PLATFORM =~ /darwin/i ? 'rb-fsevent' : false gem 'rb-inotify', '~> 0.9', require: RUBY_PLATFORM =~ /linux/i ? 'rb-inotify' : false - gem 'rspec-rails', require: false + gem 'rspec-rails', '4.0.0.beta2', require: false gem 'shoulda-matchers', '~> 3.1', '>= 3.1.3', require: false gem 'rspec-html-matchers' gem 'pry-nav' diff --git a/Gemfile.lock b/Gemfile.lock index 7fc71a5425..55e2d279a3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,47 +1,46 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (5.2.3) - actionpack (= 5.2.3) - actionview (= 5.2.3) - activejob (= 5.2.3) + actionmailer (6.0.0) + actionpack (= 6.0.0) + actionview (= 6.0.0) + activejob (= 6.0.0) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.2.3) - actionview (= 5.2.3) - activesupport (= 5.2.3) + actionpack (6.0.0) + actionview (= 6.0.0) + activesupport (= 6.0.0) rack (~> 2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.2.3) - activesupport (= 5.2.3) + rails-html-sanitizer (~> 1.0, >= 1.2.0) + actionview (6.0.0) + activesupport (= 6.0.0) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.3) + rails-html-sanitizer (~> 1.1, >= 1.2.0) active_model_serializers (0.8.4) activemodel (>= 3.0) - activejob (5.2.3) - activesupport (= 5.2.3) + activejob (6.0.0) + activesupport (= 6.0.0) globalid (>= 0.3.6) - activemodel (5.2.3) - activesupport (= 5.2.3) - activerecord (5.2.3) - activemodel (= 5.2.3) - activesupport (= 5.2.3) - arel (>= 9.0) - activesupport (5.2.3) + activemodel (6.0.0) + activesupport (= 6.0.0) + activerecord (6.0.0) + activemodel (= 6.0.0) + activesupport (= 6.0.0) + activesupport (6.0.0) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) + zeitwerk (~> 2.1, >= 2.1.8) addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) annotate (2.7.5) activerecord (>= 3.2, < 7.0) rake (>= 10.4, < 13.0) - arel (9.0.0) ast (2.4.0) aws-eventstream (1.0.3) aws-partitions (1.154.0) @@ -183,7 +182,7 @@ GEM rack (>= 1.1.3) metaclass (0.0.4) method_source (0.9.2) - mini_mime (1.0.1) + mini_mime (1.0.2) mini_portile2 (2.4.0) mini_racer (0.2.6) libv8 (>= 6.9.411) @@ -282,20 +281,20 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.0.4) + rails-html-sanitizer (1.2.0) loofah (~> 2.2, >= 2.2.2) rails_multisite (2.0.7) activerecord (> 4.2, < 7) railties (> 4.2, < 7) - railties (5.2.3) - actionpack (= 5.2.3) - activesupport (= 5.2.3) + railties (6.0.0) + actionpack (= 6.0.0) + activesupport (= 6.0.0) method_source rake (>= 0.8.7) - thor (>= 0.19.0, < 2.0) + thor (>= 0.20.3, < 2.0) rainbow (3.0.0) raindrops (0.19.0) - rake (12.3.2) + rake (12.3.3) rake-compiler (1.0.7) rake rb-fsevent (0.10.3) @@ -330,14 +329,14 @@ GEM rspec-mocks (3.8.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) - rspec-rails (3.8.2) - actionpack (>= 3.0) - activesupport (>= 3.0) - railties (>= 3.0) - rspec-core (~> 3.8.0) - rspec-expectations (~> 3.8.0) - rspec-mocks (~> 3.8.0) - rspec-support (~> 3.8.0) + rspec-rails (4.0.0.beta2) + actionpack (>= 4.2) + activesupport (>= 4.2) + railties (>= 4.2) + rspec-core (~> 3.8) + rspec-expectations (~> 3.8) + rspec-mocks (~> 3.8) + rspec-support (~> 3.8) rspec-support (3.8.0) rtlit (0.0.5) rubocop (0.69.0) @@ -417,18 +416,19 @@ GEM hkdf (~> 0.2) jwt (~> 2.0) yaml-lint (0.0.10) + zeitwerk (2.1.10) PLATFORMS ruby DEPENDENCIES - actionmailer (= 5.2.3) - actionpack (= 5.2.3) - actionview (= 5.2.3) + actionmailer (= 6.0.0) + actionpack (= 6.0.0) + actionview (= 6.0.0) active_model_serializers (~> 0.8.3) - activemodel (= 5.2.3) - activerecord (= 5.2.3) - activesupport (= 5.2.3) + activemodel (= 6.0.0) + activerecord (= 6.0.0) + activesupport (= 6.0.0) annotate aws-sdk-s3 aws-sdk-sns @@ -504,7 +504,7 @@ DEPENDENCIES rack-mini-profiler rack-protection rails_multisite - railties (= 5.2.3) + railties (= 6.0.0) rake rb-fsevent rb-inotify (~> 0.9) @@ -517,7 +517,7 @@ DEPENDENCIES rqrcode rspec rspec-html-matchers - rspec-rails + rspec-rails (= 4.0.0.beta2) rtlit rubocop ruby-prof diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 55f9b38a17..007a600d54 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -1,5 +1,5 @@
- <%= @user.username %> + <%= @user.username %>

<%= @user.username %>

diff --git a/config/application.rb b/config/application.rb index 2d946b30ec..932aed2806 100644 --- a/config/application.rb +++ b/config/application.rb @@ -93,6 +93,8 @@ module Discourse # issue is image_optim crashes on missing dependencies config.assets.image_optim = false + config.autoloader = :classic + # Custom directories with classes and modules you want to be autoloadable. config.autoload_paths += Dir["#{config.root}/app/serializers"] config.autoload_paths += Dir["#{config.root}/lib/validators/"] diff --git a/db/migrate/20130430052751_add_topic_allowed_groups.rb b/db/migrate/20130430052751_add_topic_allowed_groups.rb index 27eb25054e..aae17685c6 100644 --- a/db/migrate/20130430052751_add_topic_allowed_groups.rb +++ b/db/migrate/20130430052751_add_topic_allowed_groups.rb @@ -4,8 +4,8 @@ class AddTopicAllowedGroups < ActiveRecord::Migration[4.2] def change create_table :topic_allowed_groups, force: true do |t| # oops - t.integer :group_id, :integer, null: false - t.integer :topic_id, :integer, null: false + t.integer :group_id, null: false + t.integer :topic_id, null: false end add_index :topic_allowed_groups, [:group_id, :topic_id], unique: true diff --git a/db/migrate/20130501105651_fix_topic_allowed_groups.rb b/db/migrate/20130501105651_fix_topic_allowed_groups.rb index cc0f49fbf0..bb19a27651 100644 --- a/db/migrate/20130501105651_fix_topic_allowed_groups.rb +++ b/db/migrate/20130501105651_fix_topic_allowed_groups.rb @@ -3,6 +3,6 @@ class FixTopicAllowedGroups < ActiveRecord::Migration[4.2] def change # big oops - remove_column :topic_allowed_groups, :integer + remove_column :topic_allowed_groups, :integer if column_exists?(:topic_allowed_groups, :integer) end end diff --git a/lib/freedom_patches/rails6.rb b/lib/freedom_patches/rails6.rb deleted file mode 100644 index f8c188ea93..0000000000 --- a/lib/freedom_patches/rails6.rb +++ /dev/null @@ -1,110 +0,0 @@ -# frozen_string_literal: true - -# this is a quick backport of a new method introduced in Rails 6 -# to be removed after we upgrade to Rails 6 -if ! defined? ActionView::Base.with_view_paths - class ActionView::Base - class << self - alias with_view_paths new - end - end -end - -# backport of https://github.com/rails/rails/commit/890485cfce4c361c03a41ec23b0ba187007818cc -if !defined? ActionDispatch::Http::ContentDisposition - module ActionDispatch - module Http - class ContentDisposition - def self.format(disposition:, filename:) - new(disposition: disposition, filename: filename).to_s - end - - attr_reader :disposition, :filename - - def initialize(disposition:, filename:) - @disposition = disposition - @filename = filename - end - - TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/ - - def ascii_filename - 'filename="' + percent_escape(I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"' - end - - RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/ - - def utf8_filename - "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR) - end - - def to_s - if filename - "#{disposition}; #{ascii_filename}; #{utf8_filename}" - else - "#{disposition}" - end - end - - private - def percent_escape(string, pattern) - string.gsub(pattern) do |char| - char.bytes.map { |byte| "%%%02X" % byte }.join - end - end - end - end - end - - module ActionController - module DataStreaming - private - def send_file_headers!(options) - type_provided = options.has_key?(:type) - - content_type = options.fetch(:type, DEFAULT_SEND_FILE_TYPE) - self.content_type = content_type - response.sending_file = true - - raise ArgumentError, ":type option required" if content_type.nil? - - if content_type.is_a?(Symbol) - extension = Mime[content_type] - raise ArgumentError, "Unknown MIME type #{options[:type]}" unless extension - self.content_type = extension - else - if !type_provided && options[:filename] - # If type wasn't provided, try guessing from file extension. - content_type = Mime::Type.lookup_by_extension(File.extname(options[:filename]).downcase.delete(".")) || content_type - end - self.content_type = content_type - end - - disposition = options.fetch(:disposition, DEFAULT_SEND_FILE_DISPOSITION) - if disposition - headers["Content-Disposition"] = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: options[:filename]) - end - - headers["Content-Transfer-Encoding"] = "binary" - - # Fix a problem with IE 6.0 on opening downloaded files: - # If Cache-Control: no-cache is set (which Rails does by default), - # IE removes the file it just downloaded from its cache immediately - # after it displays the "open/save" dialog, which means that if you - # hit "open" the file isn't there anymore when the application that - # is called for handling the download is run, so let's workaround that - response.cache_control[:public] ||= false - end - end - end - - module ActiveStorage - class Service - private - def content_disposition_with(type: "inline", filename:) - disposition = (type.to_s.presence_in(%w( attachment inline )) || "inline") - ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized) - end - end - end -end diff --git a/lib/mini_sql_multisite_connection.rb b/lib/mini_sql_multisite_connection.rb index ee1129c65f..c08d3813a9 100644 --- a/lib/mini_sql_multisite_connection.rb +++ b/lib/mini_sql_multisite_connection.rb @@ -36,6 +36,9 @@ class MiniSqlMultisiteConnection < MiniSql::Postgres::Connection def before_committed!(*); end def rolledback!(*); end + def trigger_transactional_callbacks? + true + end end # Allows running arbitrary code after the current transaction has been committed. diff --git a/spec/components/migration/safe_migrate_spec.rb b/spec/components/migration/safe_migrate_spec.rb index 5c04f5cec5..9bd0b690ae 100644 --- a/spec/components/migration/safe_migrate_spec.rb +++ b/spec/components/migration/safe_migrate_spec.rb @@ -14,8 +14,8 @@ describe Migration::SafeMigrate do end def migrate_up(path) - migrations = ActiveRecord::MigrationContext.new(path).migrations - ActiveRecord::Migrator.new(:up, migrations, migrations.first.version).run + migrations = ActiveRecord::MigrationContext.new(path, ActiveRecord::SchemaMigration).migrations + ActiveRecord::Migrator.new(:up, migrations, ActiveRecord::SchemaMigration, migrations.first.version).run end it "bans all table removal" do diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index ed4f2887e7..0f15c4b720 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -734,7 +734,7 @@ describe PostRevisor do let(:bumped_at) { 1.day.ago } before do - topic.update_attributes!(bumped_at: bumped_at) + topic.update!(bumped_at: bumped_at) create_hidden_tags(['important', 'secret']) topic = post.topic topic.tags = [Fabricate(:tag, name: "super"), Tag.where(name: "important").first, Fabricate(:tag, name: "stuff")] diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 420655b2ac..f0640a0335 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -344,7 +344,7 @@ describe UserNotifications do ] SiteSetting.post_excerpts_in_emails = true SiteSetting.post_excerpt_maxlength = paragraphs.first.length - response.update_attributes!(raw: paragraphs.join("\n\n")) + response.update!(raw: paragraphs.join("\n\n")) mail = UserNotifications.user_replied( user, post: response, diff --git a/spec/requests/badges_controller_spec.rb b/spec/requests/badges_controller_spec.rb index 8d04b98d80..16abe66686 100644 --- a/spec/requests/badges_controller_spec.rb +++ b/spec/requests/badges_controller_spec.rb @@ -39,7 +39,7 @@ describe BadgesController do it 'renders rss feed of a badge' do get "/badges/#{badge.id}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 124eaf5c7d..42d76b4d50 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -426,7 +426,7 @@ describe GroupsController do get "/groups/#{group.name}/posts.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end @@ -435,7 +435,7 @@ describe GroupsController do get "/groups/#{group.name}/mentions.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'fails when disabled' do diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index dd94355045..9727a06d4c 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ListController do it 'renders latest RSS' do get "/latest.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'renders links correctly with subfolder' do @@ -337,14 +337,14 @@ RSpec.describe ListController do it 'renders top RSS' do get "/top.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end TopTopic.periods.each do |period| it "renders #{period} top RSS" do get "/top/#{period}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end end end @@ -433,7 +433,7 @@ RSpec.describe ListController do it 'renders RSS' do get "/c/#{category.slug}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it "renders RSS in subfolder correctly" do diff --git a/spec/requests/metadata_controller_spec.rb b/spec/requests/metadata_controller_spec.rb index eaa8f05da2..041be7ebe8 100644 --- a/spec/requests/metadata_controller_spec.rb +++ b/spec/requests/metadata_controller_spec.rb @@ -20,7 +20,7 @@ RSpec.describe MetadataController do get "/manifest.webmanifest" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/manifest+json') + expect(response.media_type).to eq('application/manifest+json') manifest = JSON.parse(response.body) expect(manifest["name"]).to eq(title) @@ -98,7 +98,7 @@ RSpec.describe MetadataController do expect(response.body).to include("/search?q={searchTerms}") expect(response.body).to include('image/png') expect(response.body).to include(UrlHelper.absolute(upload.url)) - expect(response.content_type).to eq('application/xml') + expect(response.media_type).to eq('application/xml') end end @@ -121,7 +121,7 @@ RSpec.describe MetadataController do expect(response.status).to eq(200) expect(response.body).to include("hash_of_app_certificate") expect(response.body).to include("com.example.app") - expect(response.content_type).to eq('application/json') + expect(response.media_type).to eq('application/json') end end @@ -143,7 +143,7 @@ RSpec.describe MetadataController do expect(response.status).to eq(200) expect(response.body).to include("applinks") - expect(response.content_type).to eq('application/json') + expect(response.media_type).to eq('application/json') get "/apple-app-site-association.json" expect(response.status).to eq(404) diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 6fcb784ab7..dff4e02959 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -26,7 +26,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(SiteIconManager.favicon.filesize) end @@ -36,7 +36,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(upload.filesize) end end @@ -66,7 +66,7 @@ describe StaticController do get '/favicon/proxied' expect(response.status).to eq(200) - expect(response.content_type).to eq('image/png') + expect(response.media_type).to eq('image/png') expect(response.body.bytesize).to eq(upload.filesize) end end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 5ea6b9b6f0..804952ef25 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1998,7 +1998,7 @@ RSpec.describe TopicsController do it 'renders rss of the topic' do get "/t/foo/#{topic.id}.rss" expect(response.status).to eq(200) - expect(response.content_type).to eq('application/rss+xml') + expect(response.media_type).to eq('application/rss+xml') end it 'renders rss of the topic correctly with subfolder' do From f4f566a3010a7bf46af38eafb667e299112d7e7e Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 11 Sep 2019 22:22:00 -0400 Subject: [PATCH 035/126] Adding above-user-profile plugin outlet --- app/assets/javascripts/discourse/templates/user.hbs | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/discourse/templates/user.hbs b/app/assets/javascripts/discourse/templates/user.hbs index f8f8d31c60..8fbc7b440f 100644 --- a/app/assets/javascripts/discourse/templates/user.hbs +++ b/app/assets/javascripts/discourse/templates/user.hbs @@ -1,3 +1,4 @@ +{{plugin-outlet name="above-user-profile" tagName='' args=(hash model=model)}}
{{#d-section class="user-main"}} From 015051ecafe24975268a4022c40f17978f54890e Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 12 Sep 2019 17:34:04 +1000 Subject: [PATCH 036/126] PERF: avoid spinning a thread each time we close a connection This is a temporary workaround for the issue in https://github.com/rails/rails/pull/36949 Discussing a proper fix in Rails with the Rails team. Prior to this fix we were spinning up a thread every time we closed a connection to the db. --- lib/freedom_patches/rails6.rb | 50 +++++++++++++++++++++++++++++++ script/thread_detective.rb | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 lib/freedom_patches/rails6.rb create mode 100644 script/thread_detective.rb diff --git a/lib/freedom_patches/rails6.rb b/lib/freedom_patches/rails6.rb new file mode 100644 index 0000000000..06029e1deb --- /dev/null +++ b/lib/freedom_patches/rails6.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# see: https://github.com/rails/rails/pull/36949#issuecomment-530698779 +# +# Without this patch each time we close a DB connection we spin a thread + +class ::ActiveRecord::ConnectionAdapters::AbstractAdapter + class StaticThreadLocalVar + attr_reader :value + + def initialize(value) + @value = value + end + + def bind(value) + raise "attempting to change immutable local var" if value != @value + end + end + + # we have no choice but to perform an aggressive patch here + # if we simply hook the method we will still call a finalizer + # on Concurrent::ThreadLocalVar + + def initialize(connection, logger = nil, config = {}) # :nodoc: + super() + + @connection = connection + @owner = nil + @instrumenter = ActiveSupport::Notifications.instrumenter + @logger = logger + @config = config + @pool = ActiveRecord::ConnectionAdapters::NullPool.new + @idle_since = Concurrent.monotonic_time + @visitor = arel_visitor + @statements = build_statement_pool + @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new + + if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statement_status = Concurrent::ThreadLocalVar.new(true) + @visitor.extend(DetermineIfPreparableVisitor) + else + @prepared_statement_status = StaticThreadLocalVar.new(false) + end + + @advisory_locks_enabled = self.class.type_cast_config_to_boolean( + config.fetch(:advisory_locks, true) + ) + end + +end diff --git a/script/thread_detective.rb b/script/thread_detective.rb new file mode 100644 index 0000000000..c654158ede --- /dev/null +++ b/script/thread_detective.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +class Thread + attr_accessor :origin +end + +class ThreadDetective + def self.test_thread + Thread.new { sleep 1 } + end + def self.start(max_threads) + @thread ||= Thread.new do + self.new.monitor(max_threads) + end + + @trace = TracePoint.new(:thread_begin) do |tp| + Thread.current.origin = Thread.current.inspect + end + @trace.enable + end + + def self.stop + @thread&.kill + @thread = nil + @trace&.disable + @trace.stop + end + + def monitor(max_threads) + STDERR.puts "Monitoring threads in #{Process.pid}" + + while true + threads = Thread.list + + if threads.length > max_threads + str = +("-" * 60) + str << "#{threads.length} found in Process #{Process.pid}!\n" + + threads.each do |thread| + str << "\n" + if thread.origin + str << thread.origin + else + str << thread.inspect + end + str << "\n" + end + str << ("-" * 60) + + STDERR.puts str + end + sleep 1 + end + end + +end From 082f59842dd2dc20fbe8e8958428b5714174ad3d Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 12 Sep 2019 17:57:57 +1000 Subject: [PATCH 037/126] DEV: bind for thread local vars should yield block followup on #015051ec without this improvement monkey patch does not work. bind should yield the block it is passed for it to work. --- lib/freedom_patches/rails6.rb | 85 +++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/lib/freedom_patches/rails6.rb b/lib/freedom_patches/rails6.rb index 06029e1deb..cf2f96de05 100644 --- a/lib/freedom_patches/rails6.rb +++ b/lib/freedom_patches/rails6.rb @@ -4,47 +4,54 @@ # # Without this patch each time we close a DB connection we spin a thread -class ::ActiveRecord::ConnectionAdapters::AbstractAdapter - class StaticThreadLocalVar - attr_reader :value +module ::ActiveRecord + module ConnectionAdapters + class AbstractAdapter + class StaticThreadLocalVar + attr_reader :value - def initialize(value) - @value = value - end + def initialize(value) + @value = value + end - def bind(value) - raise "attempting to change immutable local var" if value != @value + def bind(value) + raise "attempting to change immutable local var" if value != @value + if block_given? + yield + end + end + end + + # we have no choice but to perform an aggressive patch here + # if we simply hook the method we will still call a finalizer + # on Concurrent::ThreadLocalVar + + def initialize(connection, logger = nil, config = {}) # :nodoc: + super() + + @connection = connection + @owner = nil + @instrumenter = ActiveSupport::Notifications.instrumenter + @logger = logger + @config = config + @pool = ActiveRecord::ConnectionAdapters::NullPool.new + @idle_since = Concurrent.monotonic_time + @visitor = arel_visitor + @statements = build_statement_pool + @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new + + if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statement_status = Concurrent::ThreadLocalVar.new(true) + @visitor.extend(DetermineIfPreparableVisitor) + else + #@prepared_statement_status = Concurrent::ThreadLocalVar.new(false) + @prepared_statement_status = StaticThreadLocalVar.new(false) + end + + @advisory_locks_enabled = self.class.type_cast_config_to_boolean( + config.fetch(:advisory_locks, true) + ) + end end end - - # we have no choice but to perform an aggressive patch here - # if we simply hook the method we will still call a finalizer - # on Concurrent::ThreadLocalVar - - def initialize(connection, logger = nil, config = {}) # :nodoc: - super() - - @connection = connection - @owner = nil - @instrumenter = ActiveSupport::Notifications.instrumenter - @logger = logger - @config = config - @pool = ActiveRecord::ConnectionAdapters::NullPool.new - @idle_since = Concurrent.monotonic_time - @visitor = arel_visitor - @statements = build_statement_pool - @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new - - if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) - @prepared_statement_status = Concurrent::ThreadLocalVar.new(true) - @visitor.extend(DetermineIfPreparableVisitor) - else - @prepared_statement_status = StaticThreadLocalVar.new(false) - end - - @advisory_locks_enabled = self.class.type_cast_config_to_boolean( - config.fetch(:advisory_locks, true) - ) - end - end From 73172f00d3e164a01b4d878494a2aaf0c0ac0749 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 12 Sep 2019 12:11:12 +0100 Subject: [PATCH 038/126] FIX: Clear authentication data from session after create account (#8040) --- app/services/user_authenticator.rb | 15 ++++++++------- spec/services/user_authenticator_spec.rb | 10 ++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb index c2361f6a5d..d988300ceb 100644 --- a/app/services/user_authenticator.rb +++ b/app/services/user_authenticator.rb @@ -4,7 +4,8 @@ class UserAuthenticator def initialize(user, session, authenticator_finder = Users::OmniauthCallbacksController) @user = user - @session = session[:authentication] + @session = session + @auth_session = session[:authentication] @authenticator_finder = authenticator_finder end @@ -15,7 +16,7 @@ class UserAuthenticator @user.password_required! end - @user.skip_email_validation = true if @session && @session[:skip_email_validation].present? + @user.skip_email_validation = true if @auth_session && @auth_session[:skip_email_validation].present? end def has_authenticator? @@ -24,18 +25,18 @@ class UserAuthenticator def finish if authenticator - authenticator.after_create_account(@user, @session) + authenticator.after_create_account(@user, @auth_session) confirm_email end - @session = nil + @session[:authentication] = @auth_session = nil if @auth_session end def email_valid? - @session && @session[:email_valid] + @auth_session && @auth_session[:email_valid] end def authenticated? - @session && @session[:email]&.downcase == @user.email.downcase && @session[:email_valid].to_s == "true" + @auth_session && @auth_session[:email]&.downcase == @user.email.downcase && @auth_session[:email_valid].to_s == "true" end private @@ -54,7 +55,7 @@ class UserAuthenticator end def authenticator_name - @session && @session[:authenticator_name] + @auth_session && @auth_session[:authenticator_name] end end diff --git a/spec/services/user_authenticator_spec.rb b/spec/services/user_authenticator_spec.rb index b54f282f15..7ee4543d14 100644 --- a/spec/services/user_authenticator_spec.rb +++ b/spec/services/user_authenticator_spec.rb @@ -57,5 +57,15 @@ describe UserAuthenticator do expect(user.email_confirmed?).to be_falsey expect(group.usernames).not_to include(user.username) end + + it "clears the authentication info from the session" do + user = Fabricate(:user, email: "user53@discourse.org") + session = { authentication: github_auth(true) } + + UserAuthenticator.new(user, session).finish + expect(user.email_confirmed?).to be_truthy + + expect(session[:authentication]).to eq(nil) + end end end From 1dcdcb5c31e18c44ca7beeb223ea3bbce5f545f8 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Thu, 12 Sep 2019 15:17:34 +0200 Subject: [PATCH 039/126] FIX: Cast all numerical values in reports (#8087) * FIX: Cast all numerical values in reports The backend can return some numerical values in report as strings. That results in unexpected order of values when sorting report tables. * Create `toNumber()` helper The `typeof` and `parseFloat` seem to be the fastest path: https://jsperf.com/number-vs-typeof-vs-parsefloat#results --- app/assets/javascripts/admin/models/report.js.es6 | 14 +++++++++----- .../javascripts/discourse/lib/utilities.js.es6 | 6 +++++- test/javascripts/fixtures/reports_bulk.js.es6 | 4 ++-- test/javascripts/models/report-test.js.es6 | 4 ++-- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/admin/models/report.js.es6 b/app/assets/javascripts/admin/models/report.js.es6 index 831fe8e765..c7967cc6d5 100644 --- a/app/assets/javascripts/admin/models/report.js.es6 +++ b/app/assets/javascripts/admin/models/report.js.es6 @@ -1,7 +1,11 @@ import { escapeExpression } from "discourse/lib/utilities"; import { ajax } from "discourse/lib/ajax"; import round from "discourse/lib/round"; -import { fillMissingDates, formatUsername } from "discourse/lib/utilities"; +import { + fillMissingDates, + formatUsername, + toNumber +} from "discourse/lib/utilities"; import computed from "ember-addons/ember-computed-decorators"; import { number, durationTiny } from "discourse/lib/formatter"; import { renderAvatar } from "discourse/helpers/user-avatar"; @@ -374,14 +378,14 @@ const Report = Discourse.Model.extend({ _secondsLabel(value) { return { - value, + value: toNumber(value), formatedValue: durationTiny(value) }; }, _percentLabel(value) { return { - value, + value: toNumber(value), formatedValue: value ? `${value}%` : "—" }; }, @@ -394,14 +398,14 @@ const Report = Discourse.Model.extend({ const formatedValue = () => (formatNumbers ? number(value) : value); return { - value, + value: toNumber(value), formatedValue: value ? formatedValue() : "—" }; }, _bytesLabel(value) { return { - value, + value: toNumber(value), formatedValue: I18n.toHumanSize(value) }; }, diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index 3f2be55872..5bc038e227 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -599,8 +599,12 @@ export function clipboardData(e, canUpload) { return { clipboard, types, canUpload, canPasteHtml }; } +export function toNumber(input) { + return typeof input === "number" ? input : parseFloat(input); +} + export function isNumeric(input) { - return !isNaN(parseFloat(input)) && isFinite(input); + return !isNaN(toNumber(input)) && isFinite(input); } export function fillMissingDates(data, startDate, endDate) { diff --git a/test/javascripts/fixtures/reports_bulk.js.es6 b/test/javascripts/fixtures/reports_bulk.js.es6 index f993978f8d..c315989170 100644 --- a/test/javascripts/fixtures/reports_bulk.js.es6 +++ b/test/javascripts/fixtures/reports_bulk.js.es6 @@ -7,7 +7,7 @@ let signups = { yaxis: "Number of signups", description: "New account registrations for this period", data: [ - { x: "2018-06-16", y: 12 }, + { x: "2018-06-16", y: "12" }, { x: "2018-06-17", y: 16 }, { x: "2018-06-18", y: 42 }, { x: "2018-06-19", y: 38 }, @@ -18,7 +18,7 @@ let signups = { { x: "2018-06-24", y: 17 }, { x: "2018-06-25", y: 27 }, { x: "2018-06-26", y: 32 }, - { x: "2018-06-27", y: 7 } + { x: "2018-06-27", y: "7" } ], start_date: "2018-06-16T00:00:00Z", end_date: "2018-07-16T23:59:59Z", diff --git a/test/javascripts/models/report-test.js.es6 b/test/javascripts/models/report-test.js.es6 index 3ba6a5198b..1fa622a1d3 100644 --- a/test/javascripts/models/report-test.js.es6 +++ b/test/javascripts/models/report-test.js.es6 @@ -398,7 +398,7 @@ QUnit.test("computed labels", assert => { username: "joffrey", user_id: 1, user_avatar: "/", - flag_count: 1876, + flag_count: "1876", time_read: 287362, note: "This is a long note", topic_id: 2, @@ -470,7 +470,7 @@ QUnit.test("computed labels", assert => { assert.equal(flagCountLabel.type, "number"); let computedFlagCountLabel = flagCountLabel.compute(row); assert.equal(computedFlagCountLabel.formatedValue, "1.9k"); - assert.equal(computedFlagCountLabel.value, 1876); + assert.strictEqual(computedFlagCountLabel.value, 1876); computedFlagCountLabel = flagCountLabel.compute(row, { formatNumbers: false }); From d7cfb06fa44a446d59578611c574a0b99b540513 Mon Sep 17 00:00:00 2001 From: Dannii Willis Date: Fri, 13 Sep 2019 00:32:44 +1000 Subject: [PATCH 040/126] Fix: let mailgun_api_key also support their "HTTP webhook signing key" (#8091) --- config/site_settings.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/site_settings.yml b/config/site_settings.yml index 9a5327f980..231e25d170 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1012,7 +1012,7 @@ email: default: "" mailgun_api_key: default: "" - regex: '^(key-\h{32}|\h{32}-\h{8}-\h{8})$' + regex: '^((key-)?\h{32}|\h{32}-\h{8}-\h{8})$' secret: true bounce_score_threshold: client: true From e4b813df4c315b03cafba114c2c475cbb8fda5f6 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 12 Sep 2019 10:38:53 -0400 Subject: [PATCH 041/126] FIX: Change admin dashboard sort caret icon color on hover --- app/assets/stylesheets/common/admin/admin_report_table.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/common/admin/admin_report_table.scss b/app/assets/stylesheets/common/admin/admin_report_table.scss index 7fc1661fb2..e63330355a 100644 --- a/app/assets/stylesheets/common/admin/admin_report_table.scss +++ b/app/assets/stylesheets/common/admin/admin_report_table.scss @@ -53,6 +53,9 @@ &:hover { color: $primary-medium; background: $primary-low; + .d-icon { + color: $primary-medium; + } } } } From 67a98946b8b19091533b86b5f7d6256da5ce06f6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 12 Sep 2019 15:55:45 +0100 Subject: [PATCH 042/126] FIX: Do not log 'pull_hotlinked_images' edits in the staff action log --- app/jobs/regular/pull_hotlinked_images.rb | 2 +- lib/post_revisor.rb | 3 ++- spec/components/post_revisor_spec.rb | 13 +++++++++++++ spec/jobs/pull_hotlinked_images_spec.rb | 3 ++- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 46457fe8fc..c741b659d4 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -148,7 +148,7 @@ module Jobs if start_raw == post.raw && raw != post.raw changes = { raw: raw, edit_reason: I18n.t("upload.edit_reason") } - post.revise(Discourse.system_user, changes, bypass_bump: true) + post.revise(Discourse.system_user, changes, bypass_bump: true, skip_staff_log: true) elsif has_downloaded_image || has_new_large_image || has_new_broken_image post.trigger_post_process(bypass_bump: true) end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 4b16797626..02b88d1173 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -116,6 +116,7 @@ class PostRevisor # - bypass_bump: do not bump the topic, even if last post # - skip_validations: ask ActiveRecord to skip validations # - skip_revision: do not create a new PostRevision record + # - skip_staff_log: skip creating an entry in the staff action log def revise!(editor, fields, opts = {}) @editor = editor @fields = fields.with_indifferent_access @@ -183,7 +184,7 @@ class PostRevisor end # We log staff edits to posts - if @editor.staff? && @editor.id != @post.user.id && @fields.has_key?('raw') + if @editor.staff? && @editor.id != @post.user.id && @fields.has_key?('raw') && !@opts[:skip_staff_log] StaffActionLogger.new(@editor).log_post_edit( @post, old_raw: old_raw diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 0f15c4b720..a47596fe38 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -494,6 +494,19 @@ describe PostRevisor do expect(log.details).to eq("Hello world\n\n---\n\nlets totally update the body") end + it "doesn't log an edit when skip_staff_log is true" do + subject.revise!( + moderator, + { raw: "lets totally update the body" }, + skip_staff_log: true + ) + log = UserHistory.where( + acting_user_id: moderator.id, + action: UserHistory.actions[:post_edit] + ).first + expect(log).to be_blank + end + it "doesn't log an edit when a staff member edits their own post" do revisor = PostRevisor.new( Fabricate(:post, user: moderator) diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index cf3b2b4c6e..a96e476ce7 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -51,7 +51,8 @@ describe Jobs::PullHotlinkedImages do expect do Jobs::PullHotlinkedImages.new.execute(post_id: post.id) - end.to change { Upload.count }.by(1) + end.to change { Upload.count }.by(1) & + change { UserHistory.count }.by(0) # Should not add to the staff log expect(post.reload.raw).to eq("![](#{Upload.last.short_url})") end From 7129637279ea1de43712801294df13853d688bb7 Mon Sep 17 00:00:00 2001 From: hawm Date: Thu, 12 Sep 2019 23:19:43 +0800 Subject: [PATCH 043/126] FEATURE: Make share button support custom javascript (#8090) * FEATURE: Make share button support custom javascript * clean code * formatting * formatting --- .../javascripts/discourse/lib/sharing.js.es6 | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/sharing.js.es6 b/app/assets/javascripts/discourse/lib/sharing.js.es6 index 4e57b9dfa5..2a3b4d191b 100644 --- a/app/assets/javascripts/discourse/lib/sharing.js.es6 +++ b/app/assets/javascripts/discourse/lib/sharing.js.es6 @@ -19,6 +19,11 @@ return "http://twitter.com/intent/tweet?url=" + encodeURIComponent(link) + "&text=" + encodeURIComponent(title); }, + // If provided, handle by custom javascript rather than default url open + clickHandler: function(link, title){ + alert("Hello!") + } + // If true, opens in a popup of `popupHeight` size. If false it's opened in a new tab shouldOpenInPopup: true, popupHeight: 265 @@ -48,23 +53,27 @@ export default { }, shareSource(source, data) { - const url = source.generateUrl(data.url, data.title); - const options = { - menubar: "no", - toolbar: "no", - resizable: "yes", - scrollbars: "yes", - width: 600, - height: source.popupHeight || 315 - }; - const stringOptions = Object.keys(options) - .map(k => `${k}=${options[k]}`) - .join(","); - - if (source.shouldOpenInPopup) { - window.open(url, "", stringOptions); + if (source.clickHandler) { + source.clickHandler(data.url, data.title); } else { - window.open(url, "_blank"); + const url = source.generateUrl(data.url, data.title); + const options = { + menubar: "no", + toolbar: "no", + resizable: "yes", + scrollbars: "yes", + width: 600, + height: source.popupHeight || 315 + }; + const stringOptions = Object.keys(options) + .map(k => `${k}=${options[k]}`) + .join(","); + + if (source.shouldOpenInPopup) { + window.open(url, "", stringOptions); + } else { + window.open(url, "_blank"); + } } }, From 568232052ec2a127991907a21776d0223d3c9420 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Thu, 12 Sep 2019 13:21:33 -0300 Subject: [PATCH 044/126] DEV: Remove FlagQuery class and old code (#8064) --- .../scheduled/pending_reviewables_reminder.rb | 2 - lib/flag_query.rb | 11 -- lib/plugin/instance.rb | 6 - lib/reviewable/conversation.rb | 12 +- lib/tasks/posts.rake | 20 --- spec/components/flag_query_spec.rb | 150 ------------------ 6 files changed, 11 insertions(+), 190 deletions(-) delete mode 100644 spec/components/flag_query_spec.rb diff --git a/app/jobs/scheduled/pending_reviewables_reminder.rb b/app/jobs/scheduled/pending_reviewables_reminder.rb index a0b4c6597c..92642b0327 100644 --- a/app/jobs/scheduled/pending_reviewables_reminder.rb +++ b/app/jobs/scheduled/pending_reviewables_reminder.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_dependency 'flag_query' - module Jobs class PendingReviewablesReminder < Jobs::Scheduled diff --git a/lib/flag_query.rb b/lib/flag_query.rb index 148477286a..c03763d74b 100644 --- a/lib/flag_query.rb +++ b/lib/flag_query.rb @@ -220,15 +220,4 @@ module FlagQuery users: User.where(id: user_ids) } end - - def self.excerpt(cooked) - excerpt = Post.excerpt(cooked, 200, keep_emoji_images: true) - # remove the first link if it's the first node - fragment = Nokogiri::HTML.fragment(excerpt) - if fragment.children.first == fragment.css("a:first").first && fragment.children.first - fragment.children.first.remove - end - fragment.to_html.strip - end - end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 7fe0198ada..96bdcc3e33 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -129,12 +129,6 @@ class Plugin::Instance end end - def whitelist_flag_post_custom_field(field) - reloadable_patch do |plugin| - ::FlagQuery.register_plugin_post_custom_field(field, plugin) # plugin.enabled? is checked at runtime - end - end - def whitelist_staff_user_custom_field(field) reloadable_patch do |plugin| ::User.register_plugin_staff_custom_field(field, plugin) # plugin.enabled? is checked at runtime diff --git a/lib/reviewable/conversation.rb b/lib/reviewable/conversation.rb index 4fc44c0adf..1b867888f3 100644 --- a/lib/reviewable/conversation.rb +++ b/lib/reviewable/conversation.rb @@ -11,7 +11,17 @@ class Reviewable < ActiveRecord::Base def initialize(post) @user = post.user @id = post.id - @excerpt = FlagQuery.excerpt(post.cooked) + @excerpt = self.class.excerpt(post.cooked) + end + + def self.excerpt(cooked) + excerpt = ::Post.excerpt(cooked, 200, keep_emoji_images: true) + # remove the first link if it's the first node + fragment = Nokogiri::HTML.fragment(excerpt) + if fragment.children.first == fragment.css("a:first").first && fragment.children.first + fragment.children.first.remove + end + fragment.to_html.strip end end diff --git a/lib/tasks/posts.rake b/lib/tasks/posts.rake index 5bcdf2983e..fec5a4f007 100644 --- a/lib/tasks/posts.rake +++ b/lib/tasks/posts.rake @@ -277,26 +277,6 @@ task 'posts:delete_all_likes' => :environment do puts "", "#{likes_deleted} likes deleted!", "" end -desc 'Defer all flags' -task 'posts:defer_all_flags' => :environment do - - active_flags = FlagQuery.flagged_post_actions('active') - - flags_deferred = 0 - total = active_flags.count - - active_flags.each do |post_action| - begin - PostAction.defer_flags!(Post.find(post_action.post_id), Discourse.system_user) - print_status(flags_deferred += 1, total) - rescue - # skip - end - end - - puts "", "#{flags_deferred} flags deferred!", "" -end - desc 'Refreshes each post that was received via email' task 'posts:refresh_emails', [:topic_id] => [:environment] do |_, args| posts = Post.where.not(raw_email: nil).where(via_email: true) diff --git a/spec/components/flag_query_spec.rb b/spec/components/flag_query_spec.rb deleted file mode 100644 index 6e37008265..0000000000 --- a/spec/components/flag_query_spec.rb +++ /dev/null @@ -1,150 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' -require_dependency 'flag_query' - -describe FlagQuery do - fab!(:admin) { Fabricate(:admin) } - fab!(:moderator) { Fabricate(:moderator) } - fab!(:codinghorror) { Fabricate(:coding_horror) } - - describe "flagged_topics" do - it "respects `reviewable_default_visibility`" do - Reviewable.set_priorities(medium: 10.0) - - post = create_post - - SiteSetting.reviewable_default_visibility = 'low' - PostActionCreator.spam(moderator, post) - - result = FlagQuery.flagged_topics - expect(result[:flagged_topics]).to be_present - ft = result[:flagged_topics].first - expect(ft.topic).to eq(post.topic) - expect(ft.flag_counts).to eq(PostActionType.types[:spam] => 1) - - SiteSetting.reviewable_default_visibility = 'medium' - - result = FlagQuery.flagged_topics - expect(result[:flagged_topics]).to be_blank - - PostActionCreator.create(admin, post, :inappropriate) - result = FlagQuery.flagged_topics - expect(result[:flagged_topics]).to be_present - ft = result[:flagged_topics].first - expect(ft.topic).to eq(post.topic) - expect(ft.flag_counts).to eq( - PostActionType.types[:spam] => 1, - PostActionType.types[:inappropriate] => 1 - ) - end - end - - describe "flagged_post_actions" do - - it "returns the proper count" do - post = create_post - PostActionCreator.spam(moderator, post) - expect(FlagQuery.flagged_post_actions(topic_id: post.topic_id).count).to eq(1) - expect(FlagQuery.flagged_post_actions(topic_id: post.topic_id, filter: 'old').count).to eq(0) - end - end - - describe "flagged_posts_report" do - it "does not return flags on system posts" do - post = create_post(user: Discourse.system_user) - PostActionCreator.create(codinghorror, post, :spam) - posts, topics, users = FlagQuery.flagged_posts_report(admin) - - expect(posts).to be_blank - expect(topics).to be_blank - expect(users).to be_blank - end - - it "operates correctly" do - post = create_post - post2 = create_post - - user2 = Fabricate(:user) - user3 = Fabricate(:user) - - PostActionCreator.spam(codinghorror, post) - PostActionCreator.create(user2, post, :spam) - result = PostActionCreator.new( - user3, - post, - PostActionType.types[:notify_moderators], - message: "this is a :one::zero:" - ).perform - mod_message = result.post_action - - PostActionCreator.spam(codinghorror, post2) - PostActionCreator.spam(user2, post2) - - posts, topics, users, all_actions = FlagQuery.flagged_posts_report(admin) - - expect(posts.count).to eq(2) - first = posts.first - - expect(users.count).to eq(5) - expect(first[:post_action_ids].count).to eq(2) - - expect(topics.count).to eq(2) - - second = posts[1] - expect(second[:post_action_ids].count).to eq(3) - - action = all_actions.find { |a| a[:id] == second[:post_action_ids][0] } - expect(action[:permalink]).to eq(mod_message.related_post.topic.relative_url) - expect(action[:conversation][:response][:excerpt]).to match(" Date: Thu, 12 Sep 2019 22:25:15 +0530 Subject: [PATCH 045/126] FIX: make markdown regexp patterns case insensitive. --- app/services/inline_uploads.rb | 6 +++--- spec/services/inline_uploads_spec.rb | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index d8ddc4eaf0..35e1f0b4fd 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -182,7 +182,7 @@ class InlineUploads end def self.match_bbcode_img(markdown, external_src: false) - markdown.scan(/(\[img\]\s*([^\[\]\s]+)\s*\[\/img\])/) do |match| + markdown.scan(/(\[img\]\s*([^\[\]\s]+)\s*\[\/img\])/i) do |match| if (matched_uploads(match[1]).present? && block_given?) || external_src yield(match[0], match[1], +"![](#{PLACEHOLDER})", $~.offset(0)[0]) end @@ -203,7 +203,7 @@ class InlineUploads end def self.match_anchor(markdown, external_href: false) - markdown.scan(/(()([^<\a>]*?)<\/a>)/) do |match| + markdown.scan(/(()([^<\a>]*?)<\/a>)/i) do |match| node = Nokogiri::HTML::fragment(match[0]).children[0] href = node.attributes["href"]&.value @@ -219,7 +219,7 @@ class InlineUploads end def self.match_img(markdown, external_src: false) - markdown.scan(/(([ ]*)<(?!img)[^<>]+\/?>)?([\r\n]*)(([ ]*)\n]+)>([ ]*))([\r\n]*)/) do |match| + markdown.scan(/(([ ]*)<(?!img)[^<>]+\/?>)?([\r\n]*)(([ ]*)\n]+)>([ ]*))([\r\n]*)/i) do |match| node = Nokogiri::HTML::fragment(match[3].strip).children[0] src = node.attributes["src"]&.value diff --git a/spec/services/inline_uploads_spec.rb b/spec/services/inline_uploads_spec.rb index b42f7c75f7..0bd1e6cb58 100644 --- a/spec/services/inline_uploads_spec.rb +++ b/spec/services/inline_uploads_spec.rb @@ -206,6 +206,20 @@ RSpec.describe InlineUploads do MD end + it "should correct html and markdown uppercase references" do + md = <<~MD + [IMG]#{upload.url}[/IMG] + + Text + MD + + expect(InlineUploads.process(md)).to eq(<<~MD) + ![](#{upload.short_url}) + ![](#{upload2.short_url}) + [Text|attachment](#{upload3.short_url}) + MD + end + context "subfolder" do before do global_setting :relative_url_root, "/community" From 95a9a544c9a3d9a39a16bf41e6eb2e23d64edb4c Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 12 Sep 2019 13:12:06 -0400 Subject: [PATCH 046/126] DEV: Allow custom hosts in development mode Rails 6 seems to introduce a whitelist of allowed hosts. I personally use `dev.local` for development and this no longer works. This introduces a new ENV variable, `DISCOURSE_DEV_HOST`. If present, it will whitelist that host for development mode. --- config/environments/development.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/environments/development.rb b/config/environments/development.rb index eb62346400..306bd99f25 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -41,6 +41,7 @@ Discourse::Application.configure do BetterErrors::Middleware.allow_ip! ENV['TRUSTED_IP'] if ENV['TRUSTED_IP'] config.load_mini_profiler = true + config.hosts << ENV['DISCOURSE_DEV_HOST'] if ENV['DISCOURSE_DEV_HOST'] require 'middleware/turbo_dev' config.middleware.insert 0, Middleware::TurboDev From 8f601d5025e176cc38d0d7db05b704d2c0048763 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 12 Sep 2019 14:14:32 -0400 Subject: [PATCH 047/126] UI: Add viewport meta tag to embedded topics layout --- app/views/layouts/embed.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/embed.html.erb b/app/views/layouts/embed.html.erb index b7b714fbc5..42b70d08fe 100644 --- a/app/views/layouts/embed.html.erb +++ b/app/views/layouts/embed.html.erb @@ -2,7 +2,7 @@ > - + <%= discourse_stylesheet_link_tag 'embed', theme_ids: nil %> <%- unless customization_disabled? %> <%= discourse_stylesheet_link_tag :embedded_theme %> From aa511c5b59839b79a884e57364d17b5e4398046f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 12 Sep 2019 21:25:14 +0200 Subject: [PATCH 048/126] FIX: support in code blocks when inlining uploads Simpler code is better :) Also added moar specs to ensure tag inside code blocks are properly ignored. --- app/services/inline_uploads.rb | 38 ++++-------------------- spec/jobs/pull_hotlinked_images_spec.rb | 2 +- spec/services/inline_uploads_spec.rb | 39 +++++++++++++------------ 3 files changed, 28 insertions(+), 51 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index 35e1f0b4fd..f16f4e5110 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -34,7 +34,7 @@ class InlineUploads if (actual_link = (node.attributes["href"]&.value || node.attributes["src"]&.value)) link_occurences << { link: actual_link, is_valid: true } elsif node.name != "p" - link_occurences << { link: actual_link, is_valid: false } + link_occurences << { link: seen_link, is_valid: false } end end end @@ -219,8 +219,8 @@ class InlineUploads end def self.match_img(markdown, external_src: false) - markdown.scan(/(([ ]*)<(?!img)[^<>]+\/?>)?([\r\n]*)(([ ]*)\n]+)>([ ]*))([\r\n]*)/i) do |match| - node = Nokogiri::HTML::fragment(match[3].strip).children[0] + markdown.scan(/(<(?!img)[^<>]+\/?>)?(\s*)(\n]+>)/i) do |match| + node = Nokogiri::HTML::fragment(match[2].strip).children[0] src = node.attributes["src"]&.value if src && (matched_uploads(src).present? || external_src) @@ -229,36 +229,10 @@ class InlineUploads height = node.attributes["height"]&.value.to_i title = node.attributes["title"]&.value text = "#{text}|#{width}x#{height}" if width > 0 && height > 0 - after_html_tag = match[0].present? + spaces_before = match[1].present? ? match[1][/ +$/].size : 0 + replacement = +"#{" " * spaces_before}![#{text}](#{PLACEHOLDER}#{title.present? ? " \"#{title}\"" : ""})" - spaces_before = - if after_html_tag && !match[0].end_with?("/>") - (match[4].length > 0 ? match[4] : " ") - else - "" - end - - replacement = +"#{spaces_before}![#{text}](#{PLACEHOLDER}#{title.present? ? " \"#{title}\"" : ""})" - - if after_html_tag && (num_newlines = match[2].length) <= 1 - replacement.prepend("\n" * (num_newlines == 0 ? 2 : 1)) - end - - if after_html_tag && !match[0].end_with?("/>") && (num_newlines = match[7].length) <= 1 - replacement += ("\n" * (num_newlines == 0 ? 2 : 1)) - end - - match[3].strip! if !after_html_tag - - if (match[1].nil? || match[1].length < 4) - if (match[4].nil? || match[4].length < 4) - yield(match[3], src, replacement, $~.offset(0)[0]) if block_given? - else - yield(match[3], src, match[3].sub(src, PATH_PLACEHOLDER), $~.offset(0)[0]) if block_given? - end - else - yield(match[3], src, match[3].sub(src, PATH_PLACEHOLDER), $~.offset(0)[0]) if block_given? - end + yield(match[2], src, replacement, $~.offset(0)[0]) if block_given? end end end diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index a96e476ce7..dd194f3b89 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -77,7 +77,7 @@ describe Jobs::PullHotlinkedImages do expect(post.reload.raw).to eq(<<~RAW.chomp)

- somelink + ![somelink](#{upload.short_url}) RAW end diff --git a/spec/services/inline_uploads_spec.rb b/spec/services/inline_uploads_spec.rb index 0bd1e6cb58..ea55a82352 100644 --- a/spec/services/inline_uploads_spec.rb +++ b/spec/services/inline_uploads_spec.rb @@ -329,6 +329,14 @@ RSpec.describe InlineUploads do + + `image inside code quotes` + + ``` + image inside code fences + ``` + + image inside code block MD expect(InlineUploads.process(md)).to eq(<<~MD) @@ -346,6 +354,14 @@ RSpec.describe InlineUploads do ![|5x4](#{upload.short_url}) ![](#{upload.short_url}) + + `image inside code quotes` + + ``` + image inside code fences + ``` + + image inside code block MD end @@ -381,7 +397,7 @@ RSpec.describe InlineUploads do expect(InlineUploads.process(md)).to eq(<<~MD)

- test + ![test|500x500](#{upload2.short_url}) @@ -391,7 +407,7 @@ RSpec.describe InlineUploads do md = "

\r\n\r\n \"test\"\r\n" - expect(InlineUploads.process(md)).to eq("

\r\n\r\n \"test\"\r\n") + expect(InlineUploads.process(md)).to eq("

\r\n\r\n ![test|500x500](#{upload.short_url})\r\n") end it "should correctly update image sources within anchor or paragraph tags" do @@ -423,40 +439,27 @@ RSpec.describe InlineUploads do expect(InlineUploads.process(md)).to eq(<<~MD) - ![test|500x500](#{upload.short_url}) -

- ![test](#{upload2.short_url}) -

- + ![test|500x500](#{upload3.short_url}) - ![test|500x500](#{upload3.short_url}) - - - - - - ![test|500x500](#{upload.short_url}) - - + ![test|500x500](#{upload.short_url}) - ![test|500x500](#{upload.short_url}) + ![test|500x500](#{upload.short_url})

Test ![test|500x500](#{upload2.short_url})

- ![test|500x500](#{upload2.short_url}) MD end From 025d4ee91f4727540c749e2162680e1042c34376 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Fri, 13 Sep 2019 09:17:54 +1000 Subject: [PATCH 049/126] FIX: Rails 6 multisite migrations and plugin migrations Prior to this change plugin migrations were not working and multisite migrations not working. Rails internals changed so we need to account for it. Specifically semantics of `db:migrate` in rails changed so it is sort of a "multisite:migrate". --- lib/plugin/instance.rb | 2 +- lib/tasks/db.rake | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 96bdcc3e33..4a3211d028 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -516,7 +516,7 @@ class Plugin::Instance Rake.add_rakelib(File.dirname(path) + "/lib/tasks") # Automatically include migrations - migration_paths = Rails.configuration.paths["db/migrate"] + migration_paths = ActiveRecord::Migrator.migrations_paths migration_paths << File.dirname(path) + "/db/migrate" unless Discourse.skip_post_deployment_migrations? diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index afc470f4f4..03bbceed19 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -48,8 +48,16 @@ task 'db:drop' => [:load_config] do |_, args| end end +begin + Rake::Task["db:migrate"].clear +end + # we need to run seed_fu every time we run rake db:migrate task 'db:migrate' => ['environment', 'set_locale'] do |_, args| + ActiveRecord::Tasks::DatabaseTasks.migrate + + Rake::Task['db:_dump'].invoke + SeedFu.seed(DiscoursePluginRegistry.seed_paths) unless Discourse.skip_post_deployment_migrations? From 6bbda8eae9827ce3b3561e6234a197d2968b708d Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 12 Sep 2019 22:42:48 -0400 Subject: [PATCH 050/126] FIX: Do not show latest count in tabs on tag lists --- app/assets/javascripts/discourse/models/nav-item.js.es6 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/models/nav-item.js.es6 b/app/assets/javascripts/discourse/models/nav-item.js.es6 index 8ae207a37c..85379d4bfc 100644 --- a/app/assets/javascripts/discourse/models/nav-item.js.es6 +++ b/app/assets/javascripts/discourse/models/nav-item.js.es6 @@ -19,7 +19,10 @@ const NavItem = Discourse.Model.extend({ displayName(categoryName, name, count) { count = count || 0; - if (name === "latest" && !Discourse.Site.currentProp("mobileView")) { + if ( + name === "latest" && + (!Discourse.Site.currentProp("mobileView") || this.tagId !== undefined) + ) { count = 0; } From f0f03acb2ce56e4e153685b1498517736dd0a695 Mon Sep 17 00:00:00 2001 From: Kyle Zhao Date: Fri, 13 Sep 2019 07:04:14 -0700 Subject: [PATCH 051/126] FIX: Do not escape `fancy_title` again. (#8095) `fancy_title` is already escaped by Rails. Escaping it again would print the HTML entity as-is, e.g. `"` instead of `"`. This fixes the issue by introducing a new `escapedContent` attribute on the `QuickAccessItem` widget. --- .../widgets/quick-access-item.js.es6 | 27 +++++++++++++--- .../widgets/quick-access-messages.js.es6 | 2 +- .../widgets/quick-access-item-test.js.es6 | 31 +++++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 test/javascripts/widgets/quick-access-item-test.js.es6 diff --git a/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 index a869484cc7..a37200e595 100644 --- a/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 +++ b/app/assets/javascripts/discourse/widgets/quick-access-item.js.es6 @@ -3,7 +3,22 @@ import RawHtml from "discourse/widgets/raw-html"; import { createWidget } from "discourse/widgets/widget"; import { emojiUnescape } from "discourse/lib/text"; import { iconNode } from "discourse-common/lib/icon-library"; +import { escapeExpression } from "discourse/lib/utilities"; +/** + * This helper widget tries to enforce a consistent look and behavior for any + * item under any quick access panels. + * + * It accepts the following attributes: + * action + * actionParam + * content + * escapedContent + * href + * icon + * read + * username + */ createWidget("quick-access-item", { tagName: "li", @@ -18,13 +33,11 @@ createWidget("quick-access-item", { return result; }, - html({ icon, href, content }) { + html({ icon, href }) { return h("a", { attributes: { href } }, [ iconNode(icon), new RawHtml({ - html: `
${this._usernameHtml()}${emojiUnescape( - Handlebars.Utils.escapeExpression(content) - )}
` + html: `
${this._usernameHtml()}${this._contentHtml()}
` }) ]); }, @@ -37,6 +50,12 @@ createWidget("quick-access-item", { } }, + _contentHtml() { + const content = + this.attrs.escapedContent || escapeExpression(this.attrs.content); + return emojiUnescape(content); + }, + _usernameHtml() { return this.attrs.username ? `${this.attrs.username} ` : ""; } diff --git a/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 index 9988e649d7..e8431bc3d2 100644 --- a/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 +++ b/app/assets/javascripts/discourse/widgets/quick-access-messages.js.es6 @@ -12,7 +12,7 @@ function toItem(message) { ); return { - content: message.fancy_title, + escapedContent: message.fancy_title, href: postUrl(message.slug, message.id, nextUnreadPostNumber), icon: ICON, read: message.last_read_post_number >= message.highest_post_number, diff --git a/test/javascripts/widgets/quick-access-item-test.js.es6 b/test/javascripts/widgets/quick-access-item-test.js.es6 new file mode 100644 index 0000000000..19da5eff69 --- /dev/null +++ b/test/javascripts/widgets/quick-access-item-test.js.es6 @@ -0,0 +1,31 @@ +import { moduleForWidget, widgetTest } from "helpers/widget-test"; + +moduleForWidget("quick-access-item"); + +const CONTENT_DIV_SELECTOR = "li > a > div"; + +widgetTest("content attribute is escaped", { + template: '{{mount-widget widget="quick-access-item" args=args}}', + + beforeEach() { + this.set("args", { content: "bold" }); + }, + + test(assert) { + const contentDiv = find(CONTENT_DIV_SELECTOR)[0]; + assert.equal(contentDiv.innerText, "bold"); + } +}); + +widgetTest("escapedContent attribute is not escaped", { + template: '{{mount-widget widget="quick-access-item" args=args}}', + + beforeEach() { + this.set("args", { escapedContent: ""quote"" }); + }, + + test(assert) { + const contentDiv = find(CONTENT_DIV_SELECTOR)[0]; + assert.equal(contentDiv.innerText, '"quote"'); + } +}); From 0cfe47471f3090c115c8cdd2ad592f343dd3b3b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 13 Sep 2019 18:21:19 +0200 Subject: [PATCH 052/126] FIX: add support for version query parameter in InlineUploads --- app/services/inline_uploads.rb | 52 ++++++++-------------------- spec/services/inline_uploads_spec.rb | 26 ++++++++++++++ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index f16f4e5110..66fead7921 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -6,7 +6,7 @@ class InlineUploads PLACEHOLDER = "__replace__" PATH_PLACEHOLDER = "__replace_path__" - UPLOAD_REGEXP_PATTERN = "/original/(\\dX/(?:[a-f0-9]/)*[a-f0-9]{40}[a-zA-Z0-9.]*)" + UPLOAD_REGEXP_PATTERN = "/original/(\\dX/(?:\\h/)*\\h{40}[a-zA-Z0-9.]*)(\\?v=\\d+)?" private_constant :UPLOAD_REGEXP_PATTERN def self.process(markdown, on_missing: nil) @@ -26,7 +26,6 @@ class InlineUploads # Do nothing elsif !(node.children.count == 1 && (node.children[0].name != "img" && node.children[0].children.blank?)) && !(node.name == "a" && node.children.count > 1 && !node_children_names(node).include?("img")) - next end @@ -64,7 +63,7 @@ class InlineUploads ] if Discourse.store.external? - regexps << /((https?:)?#{SiteSetting.Upload.s3_base_url}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /((?:https?:)?#{SiteSetting.Upload.s3_base_url}#{UPLOAD_REGEXP_PATTERN})/ regexps << /(#{SiteSetting.Upload.s3_cdn_url}#{UPLOAD_REGEXP_PATTERN})/ end @@ -72,8 +71,8 @@ class InlineUploads indexes = Set.new markdown.scan(/(\n{2,}|\A)#{regexp}$/) do |match| - if match[1].present? - extension = match[1].split(".")[-1].downcase + if match[1].present? && match[2].present? + extension = match[2].split(".")[-1].downcase index = $~.offset(2)[0] indexes << index if FileHelper.supported_images.include?(extension) @@ -87,36 +86,20 @@ class InlineUploads markdown.scan(/^#{regexp}(\s)/) do |match| if match[0].present? index = $~.offset(0)[0] - next if indexes.include?(index) - indexes << index - - raw_matches << [ - match[0], - match[0], - +"#{Discourse.base_url}#{PATH_PLACEHOLDER}", - $~.offset(0)[0] - ] + next if !indexes.add?(index) + raw_matches << [match[0], match[0], +"#{Discourse.base_url}#{PATH_PLACEHOLDER}", index] end end markdown.scan(/\[[^\[\]]*\]: #{regexp}/) do |match| - if match[0].present? - index = $~.offset(1)[0] - next if indexes.include?(index) - indexes << index - end + indexes.add($~.offset(1)[0]) if match[0].present? end markdown.scan(/(([\n\s\)\]\<])+)#{regexp}/) do |match| if matched_uploads(match[2]).present? - next if indexes.include?($~.offset(3)[0]) - - raw_matches << [ - match[2], - match[2], - +"#{Discourse.base_url}#{PATH_PLACEHOLDER}", - $~.offset(0)[0] - ] + next if !indexes.add?($~.offset(3)[0]) + index = $~.offset(0)[0] + raw_matches << [match[2], match[2], +"#{Discourse.base_url}#{PATH_PLACEHOLDER}", index] end end end @@ -160,7 +143,7 @@ class InlineUploads end end - markdown.scan(/(__([a-f0-9]{40})__)/) do |match| + markdown.scan(/(__(\h{40})__)/) do |match| upload = Upload.find_by(sha1: match[1]) markdown = markdown.sub(match[0], upload.short_path) end @@ -238,15 +221,8 @@ class InlineUploads end def self.matched_uploads(node) - matches = [] - - base_url = Discourse.base_url.sub(/https?:\/\//, "(https?://)") - - if GlobalSetting.cdn_url - cdn_url = GlobalSetting.cdn_url.sub(/https?:\/\//, "(https?://)") - end - db = RailsMultisite::ConnectionManagement.current_db + base_url = Discourse.base_url.sub(/https?:\/\//, "(https?://)") regexps = [ /(upload:\/\/([a-zA-Z0-9]+)[a-zA-Z0-9\.]*)/, @@ -256,11 +232,10 @@ class InlineUploads /(#{base_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/, ] - if cdn_url + if GlobalSetting.cdn_url && (cdn_url = GlobalSetting.cdn_url.sub(/https?:\/\//, "(https?://)")) regexps << /(#{cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ if GlobalSetting.relative_url_root.present? regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ - end end @@ -274,6 +249,7 @@ class InlineUploads end end + matches = [] node = node.to_s regexps.each do |regexp| diff --git a/spec/services/inline_uploads_spec.rb b/spec/services/inline_uploads_spec.rb index ea55a82352..61700733af 100644 --- a/spec/services/inline_uploads_spec.rb +++ b/spec/services/inline_uploads_spec.rb @@ -220,6 +220,32 @@ RSpec.describe InlineUploads do MD end + it "should correct image URLs with v parameters" do + md = <<~MD + + + + + + + #{Discourse.base_url}#{upload.url}?v=45 + + #{GlobalSetting.cdn_url}#{upload.url}?v=999 + MD + + expect(InlineUploads.process(md)).to eq(<<~MD) + ![](#{upload.short_url}) + + ![](#{upload.short_url}) + + ![](#{upload.short_url}) + + ![](#{upload.short_url}) + + ![](#{upload.short_url}) + MD + end + context "subfolder" do before do global_setting :relative_url_root, "/community" From b0211772cb38c114ba2eb0b572428c9093fa1b3d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 13 Sep 2019 17:22:43 +0100 Subject: [PATCH 053/126] FIX: Explicitly specify the format when loading `/associate/{{token}}` In IE11, the browser returns the cached HTML response, rather than the JSON formatted response. A better solution may be to add a `Vary: Accept` header to all of our HTML responses, but this commit should solve the immediate issue. --- .../javascripts/discourse/routes/associate-account.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/routes/associate-account.js.es6 b/app/assets/javascripts/discourse/routes/associate-account.js.es6 index dfbe2e52f7..d89654e078 100644 --- a/app/assets/javascripts/discourse/routes/associate-account.js.es6 +++ b/app/assets/javascripts/discourse/routes/associate-account.js.es6 @@ -7,7 +7,7 @@ export default Discourse.Route.extend({ const params = this.paramsFor("associate-account"); this.replaceWith(`preferences.account`, this.currentUser).then(() => Ember.run.next(() => - ajax(`/associate/${encodeURIComponent(params.token)}`) + ajax(`/associate/${encodeURIComponent(params.token)}.json`) .then(model => showModal("associate-account-confirm", { model })) .catch(popupAjaxError) ) From 98719bee10583fb602604a5bb119a28d69e82fcb Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 13 Sep 2019 18:01:16 +0100 Subject: [PATCH 054/126] FIX: Load raw hbs templates correctly from theme javascripts folder --- app/models/theme_field.rb | 2 +- spec/models/theme_field_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index a8b4ec52bf..e45bae20ac 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -138,7 +138,7 @@ class ThemeField < ActiveRecord::Base when "hbs" js_compiler.append_ember_template(filename.sub("discourse/templates/", ""), content) when "raw.hbs" - js_compiler.append_raw_template(filename, content) + js_compiler.append_raw_template(filename.sub("discourse/templates/", ""), content) else raise ThemeJavascriptCompiler::CompileError.new(I18n.t("themes.compile_error.unrecognized_extension", extension: extension)) end diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb index 87e182c7f6..5506305924 100644 --- a/spec/models/theme_field_spec.rb +++ b/spec/models/theme_field_spec.rb @@ -170,13 +170,13 @@ HTML expect(js_field.reload.value_baked).to eq(expected_js.strip) expect(hbs_field.reload.value_baked).to include('Ember.TEMPLATES["discovery"]') - expect(raw_hbs_field.reload.value_baked).to include('Discourse.RAW_TEMPLATES["discourse/templates/discovery"]') + expect(raw_hbs_field.reload.value_baked).to include('Discourse.RAW_TEMPLATES["discovery"]') expect(unknown_field.reload.value_baked).to eq("") expect(unknown_field.reload.error).to eq(I18n.t("themes.compile_error.unrecognized_extension", extension: "blah")) # All together expect(theme.javascript_cache.content).to include('Ember.TEMPLATES["discovery"]') - expect(theme.javascript_cache.content).to include('Discourse.RAW_TEMPLATES["discourse/templates/discovery"]') + expect(theme.javascript_cache.content).to include('Discourse.RAW_TEMPLATES["discovery"]') expect(theme.javascript_cache.content).to include('define("discourse/controllers/discovery"') expect(theme.javascript_cache.content).to include("var settings =") end From 1576b07a1019674cc0d9c88f72a0ab3e64af9911 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 13 Sep 2019 15:11:27 -0300 Subject: [PATCH 055/126] FIX: Improve protection against problematic usernames (#8097) --- app/controllers/users_controller.rb | 20 +++++++++++++++++++- config/site_settings.yml | 2 +- spec/requests/users_controller_spec.rb | 24 +++++++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index da400520d2..7a6225a15a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -141,6 +141,10 @@ class UsersController < ApplicationController def username params.require(:new_username) + if clashing_with_existing_route?(params[:new_username]) || User.reserved_username?(params[:new_username]) + return render_json_error(I18n.t("login.reserved_username")) + end + user = fetch_user_from_params guardian.ensure_can_edit_username!(user) @@ -359,7 +363,7 @@ class UsersController < ApplicationController return fail_with("login.email_too_long") end - if User.reserved_username?(params[:username]) + if clashing_with_existing_route?(params[:username]) || User.reserved_username?(params[:username]) return fail_with("login.reserved_username") end @@ -1355,4 +1359,18 @@ class UsersController < ApplicationController end end + def clashing_with_existing_route?(username) + normalized_username = User.normalize_username(username) + http_verbs = %w[GET POST PUT DELETE PATCH] + allowed_actions = %w[show update destroy] + + http_verbs.any? do |verb| + begin + path = Rails.application.routes.recognize_path("/u/#{normalized_username}", method: verb) + allowed_actions.exclude?(path[:action]) + rescue ActionController::RoutingError + false + end + end + end end diff --git a/config/site_settings.yml b/config/site_settings.yml index 231e25d170..3b090200a2 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -487,7 +487,7 @@ users: reserved_usernames: type: list list_type: compact - default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|hp|account-created|password-reset|admin-login|confirm-admin|account-created|activate-account|confirm-email-token|authorize-email" + default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|hp" min_password_length: client: true default: 10 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index c4b7d75ae9..ae5e2795bf 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1005,11 +1005,16 @@ describe UsersController do end context 'with a reserved username' do - let(:create_params) { { name: @user.name, username: 'Reserved', email: @user.email, password: "x" * 20 } } + let(:create_params) { { name: @user.name, username: 'Reserved', email: @user.email, password: 'strongpassword' } } before { SiteSetting.reserved_usernames = 'a|reserved|b' } include_examples 'failed signup' end + context 'with a username that matches a user route' do + let(:create_params) { { name: @user.name, username: 'account-created', email: @user.email, password: 'strongpassword' } } + include_examples 'failed signup' + end + context 'with a missing username' do let(:create_params) { { name: @user.name, email: @user.email, password: "x" * 20 } } @@ -1197,6 +1202,23 @@ describe UsersController do expect(user.reload.username).to eq(new_username) end + it 'raises an error when the username clashes with an existing user route' do + put "/u/#{user.username}/preferences/username.json", params: { new_username: 'account-created' } + + body = JSON.parse(response.body) + + expect(body['errors'].first).to include(I18n.t('login.reserved_username')) + end + + it 'raises an error when the username is in the reserved list' do + SiteSetting.reserved_usernames = 'reserved' + + put "/u/#{user.username}/preferences/username.json", params: { new_username: 'reserved' } + body = JSON.parse(response.body) + + expect(body['errors'].first).to include(I18n.t('login.reserved_username')) + end + it 'should fail if the user is old' do # Older than the change period and >1 post user.created_at = Time.now - (SiteSetting.username_change_period + 1).days From 08b7a3fdfee3bc843ee5802842111e52c82cb8c6 Mon Sep 17 00:00:00 2001 From: romanrizzi Date: Fri, 13 Sep 2019 17:14:31 -0300 Subject: [PATCH 056/126] Fix: Don't show non-members as readers when the post is a whisper --- app/controllers/post_readers_controller.rb | 25 +++++++++++++++---- spec/requests/post_readers_controller_spec.rb | 12 +++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/controllers/post_readers_controller.rb b/app/controllers/post_readers_controller.rb index e70d49b216..fdba0605a2 100644 --- a/app/controllers/post_readers_controller.rb +++ b/app/controllers/post_readers_controller.rb @@ -4,16 +4,20 @@ class PostReadersController < ApplicationController requires_login def index - post = Post.includes(topic: %i[allowed_groups]).find(params[:id]) - read_state = post.topic.allowed_groups.any? { |g| g.publish_read_state? && g.users.include?(current_user) } - raise Discourse::InvalidAccess unless read_state + post = Post.includes(topic: %i[topic_allowed_groups topic_allowed_users]).find(params[:id]) + ensure_can_see_readers!(post) readers = User - .joins(:topic_users) .where(staged: false) + .where.not(id: post.user_id) + .joins(:topic_users) .where.not(topic_users: { last_read_post_number: nil }) .where('topic_users.topic_id = ? AND topic_users.last_read_post_number >= ?', post.topic_id, post.post_number) - .where.not(id: post.user_id) + + if post.whisper? + non_group_members = post.topic.topic_allowed_users.map(&:user_id) + readers = readers.where.not(id: non_group_members) + end readers = readers.map do |r| { @@ -25,4 +29,15 @@ class PostReadersController < ApplicationController render_json_dump(post_readers: readers) end + + private + + def ensure_can_see_readers!(post) + show_readers = GroupUser + .where(user: current_user) + .joins(:group) + .where(groups: { id: post.topic.topic_allowed_groups.map(&:group_id), publish_read_state: true }).exists? + + raise Discourse::InvalidAccess unless show_readers + end end diff --git a/spec/requests/post_readers_controller_spec.rb b/spec/requests/post_readers_controller_spec.rb index 954e14b6f3..1c4fbcea4e 100644 --- a/spec/requests/post_readers_controller_spec.rb +++ b/spec/requests/post_readers_controller_spec.rb @@ -76,6 +76,18 @@ describe PostReadersController do expect(readers).to be_empty end + + it 'xxxxx' do + @post.update(post_type: Post.types[:whisper]) + non_member_reader = Fabricate(:user) + @group_message.allowed_users << non_member_reader + TopicUser.create!(user: non_member_reader, topic: @group_message, last_read_post_number: 4) + + get '/post_readers.json', params: { id: @post.id } + readers = JSON.parse(response.body)['post_readers'] + + expect(readers).to be_empty + end end def assert_reader_is_correctly_serialized(reader_data, reader, post) From 7fa2ae912cd5a892a6b256984d23199ff530076e Mon Sep 17 00:00:00 2001 From: romanrizzi Date: Fri, 13 Sep 2019 17:16:35 -0300 Subject: [PATCH 057/126] Add spec description --- spec/requests/post_readers_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/post_readers_controller_spec.rb b/spec/requests/post_readers_controller_spec.rb index 1c4fbcea4e..adb335e577 100644 --- a/spec/requests/post_readers_controller_spec.rb +++ b/spec/requests/post_readers_controller_spec.rb @@ -77,7 +77,7 @@ describe PostReadersController do expect(readers).to be_empty end - it 'xxxxx' do + it "doesn't include non-members when the post is a whisper" do @post.update(post_type: Post.types[:whisper]) non_member_reader = Fabricate(:user) @group_message.allowed_users << non_member_reader From e0a403edfccd3727e0d92aa3900e649e17488c7d Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Mon, 16 Sep 2019 17:38:13 +1000 Subject: [PATCH 058/126] PERF: ensure we warm up schema cache in the entire multisite This makes sure that all processes that fork off the master have a fully operation schema cache. In Rails 6, schema cache is now bolted to the connection pool. This change ensures the cache on all pools is fully populated prior to forking. The bolting of cache to connection pool does lead to some strange cases where a connection can "steal" the cache from another connection, which can cause stuff to possibly hang or deadlock. This change minimizes the risk of this happening cause it is already primed. We make a STRONG assumption that the schema is always the same on all sites when we spin up a multisite cluster. --- config/unicorn.conf.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/config/unicorn.conf.rb b/config/unicorn.conf.rb index 85b28005f5..9b140eaf39 100644 --- a/config/unicorn.conf.rb +++ b/config/unicorn.conf.rb @@ -61,6 +61,23 @@ before_fork do |server, worker| table.classify.constantize.first rescue nil end + # ensure we have a full schema cache in case we missed something above + ActiveRecord::Base.connection.data_sources.each do |table| + ActiveRecord::Base.connection.schema_cache.add(table) + end + + schema_cache = ActiveRecord::Base.connection.schema_cache + + # load up schema cache for all multisite assuming all dbs have + # an identical schema + RailsMultisite::ConnectionManagement.each_connection do + dup_cache = schema_cache.dup + # this line is not really needed, but just in case the + # underlying implementation changes lets give it a shot + dup_cache.connection = nil + ActiveRecord::Base.connection.schema_cache = dup_cache + end + # router warm up Rails.application.routes.recognize_path('abc') rescue nil From 39f7e98b60a317f60b059e79efad4a57410f6bca Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 16 Sep 2019 13:27:12 +0100 Subject: [PATCH 059/126] FIX: Ensure page is reloaded correctly when a hash is present (#8096) To demonstrate the issue: - Visit https://meta.discourse.org/#somethingHere while logged in - Click "log out" - You will be logged out, but the page will not be reloaded Setting `window.location.pathname = "/"` will not reload the page if there is a hash present. Using `window.location = "/"` gives us the desired behavior. --- app/assets/javascripts/admin/models/backup.js.es6 | 2 +- app/assets/javascripts/admin/routes/admin-backups.js.es6 | 2 +- app/assets/javascripts/discourse/controllers/login.js.es6 | 2 +- .../discourse/controllers/preferences/account.js.es6 | 4 ++-- app/assets/javascripts/discourse/lib/logout.js.es6 | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/models/backup.js.es6 b/app/assets/javascripts/admin/models/backup.js.es6 index 7dc945d1ae..7cd151378a 100644 --- a/app/assets/javascripts/admin/models/backup.js.es6 +++ b/app/assets/javascripts/admin/models/backup.js.es6 @@ -63,7 +63,7 @@ Backup.reopenClass({ bootbox.alert(result.message); } else { // redirect to homepage (session might be lost) - window.location.pathname = Discourse.getURL("/"); + window.location = Discourse.getURL("/"); } }); } diff --git a/app/assets/javascripts/admin/routes/admin-backups.js.es6 b/app/assets/javascripts/admin/routes/admin-backups.js.es6 index b06b3e8468..b70c978286 100644 --- a/app/assets/javascripts/admin/routes/admin-backups.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-backups.js.es6 @@ -36,7 +36,7 @@ export default Discourse.Route.extend({ ); if (log.operation === "restore") { // redirect to homepage when the restore is done (session might be lost) - window.location.pathname = Discourse.getURL("/"); + window.location = Discourse.getURL("/"); } } else { this.controllerFor("adminBackupsLogs") diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 13c77deab7..dfe7eaf68c 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -349,7 +349,7 @@ export default Ember.Controller.extend(ModalFunctionality, { $.removeCookie("destination_url"); window.location.href = destinationUrl; } else if (window.location.pathname === Discourse.getURL("/login")) { - window.location.pathname = Discourse.getURL("/"); + window.location = Discourse.getURL("/"); } else { window.location.reload(); } diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 index f5c1f6f5ad..8bf5d24d27 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 @@ -190,7 +190,7 @@ export default Ember.Controller.extend( () => { bootbox.alert( I18n.t("user.deleted_yourself"), - () => (window.location.pathname = Discourse.getURL("/")) + () => (window.location = Discourse.getURL("/")) ); }, () => { @@ -238,7 +238,7 @@ export default Ember.Controller.extend( if (!token) { const redirect = this.siteSettings.logout_redirect; if (Ember.isEmpty(redirect)) { - window.location.pathname = Discourse.getURL("/"); + window.location = Discourse.getURL("/"); } else { window.location.href = redirect; } diff --git a/app/assets/javascripts/discourse/lib/logout.js.es6 b/app/assets/javascripts/discourse/lib/logout.js.es6 index 888b07eca9..5e81d738e3 100644 --- a/app/assets/javascripts/discourse/lib/logout.js.es6 +++ b/app/assets/javascripts/discourse/lib/logout.js.es6 @@ -9,7 +9,7 @@ export default function logout(siteSettings, keyValueStore) { const redirect = siteSettings.logout_redirect; if (Ember.isEmpty(redirect)) { - window.location.pathname = Discourse.getURL("/"); + window.location = Discourse.getURL("/"); } else { window.location.href = redirect; } From 520d54d85feca20b815377756243d15b3d952359 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Mon, 16 Sep 2019 16:20:02 +0300 Subject: [PATCH 060/126] FIX: Open drafts for PMs from Activity > Drafts screen. Draft.get sometimes returns only the sequence number. --- .../discourse/components/user-stream.js.es6 | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/components/user-stream.js.es6 b/app/assets/javascripts/discourse/components/user-stream.js.es6 index c29b80b09f..7054358457 100644 --- a/app/assets/javascripts/discourse/components/user-stream.js.es6 +++ b/app/assets/javascripts/discourse/components/user-stream.js.es6 @@ -70,13 +70,16 @@ export default Ember.Component.extend(LoadMore, { } else { Draft.get(item.draft_key) .then(d => { - if (d.draft) { - composer.open({ - draft: d.draft, - draftKey: item.draft_key, - draftSequence: d.draft_sequence - }); + const draft = d.draft || item.data; + if (!draft) { + return; } + + composer.open({ + draft, + draftKey: item.draft_key, + draftSequence: d.draft_sequence + }); }) .catch(error => { popupAjaxError(error); From 6debd72c6ca5b88909f8e70ba99bce3c90727842 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 16 Sep 2019 11:25:55 -0400 Subject: [PATCH 061/126] DEV: Add qunit test for emojis in search blurbs Followup to cf230163 --- test/javascripts/lib/search-test.js.es6 | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/javascripts/lib/search-test.js.es6 diff --git a/test/javascripts/lib/search-test.js.es6 b/test/javascripts/lib/search-test.js.es6 new file mode 100644 index 0000000000..1cd16cc0ae --- /dev/null +++ b/test/javascripts/lib/search-test.js.es6 @@ -0,0 +1,35 @@ +import { translateResults } from "discourse/lib/search"; + +QUnit.module("lib:search"); + +QUnit.test("unescapesEmojisInBlurbs", assert => { + const source = { + posts: [ + { + id: 160, + username: "pmusaraj", + avatar_template: + "/user_avatar/localhost/pmusaraj/{size}/3_2.png", + created_at: "2019-07-22T03:47:04.864Z", + like_count: 1, + blurb: + ":thinking: This here is a test of emojis in search blurbs.", + post_number: 1, + topic_id: 41 + } + ], + topics: [], + users: [], + categories: [], + tags: [], + groups: [], + grouped_search_result: false + }; + + const results = translateResults(source); + const blurb = results.posts[0].get("blurb"); + + assert.ok(blurb.indexOf("thinking.png")); + assert.ok(blurb.indexOf(" Date: Mon, 16 Sep 2019 11:36:27 -0400 Subject: [PATCH 062/126] DEV: Fix Prettier --- test/javascripts/lib/search-test.js.es6 | 52 ++++++++++++------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/test/javascripts/lib/search-test.js.es6 b/test/javascripts/lib/search-test.js.es6 index 1cd16cc0ae..30918007db 100644 --- a/test/javascripts/lib/search-test.js.es6 +++ b/test/javascripts/lib/search-test.js.es6 @@ -3,33 +3,31 @@ import { translateResults } from "discourse/lib/search"; QUnit.module("lib:search"); QUnit.test("unescapesEmojisInBlurbs", assert => { - const source = { - posts: [ - { - id: 160, - username: "pmusaraj", - avatar_template: - "/user_avatar/localhost/pmusaraj/{size}/3_2.png", - created_at: "2019-07-22T03:47:04.864Z", - like_count: 1, - blurb: - ":thinking: This here is a test of emojis in search blurbs.", - post_number: 1, - topic_id: 41 - } - ], - topics: [], - users: [], - categories: [], - tags: [], - groups: [], - grouped_search_result: false - }; + const source = { + posts: [ + { + id: 160, + username: "pmusaraj", + avatar_template: "/user_avatar/localhost/pmusaraj/{size}/3_2.png", + created_at: "2019-07-22T03:47:04.864Z", + like_count: 1, + blurb: ":thinking: This here is a test of emojis in search blurbs.", + post_number: 1, + topic_id: 41 + } + ], + topics: [], + users: [], + categories: [], + tags: [], + groups: [], + grouped_search_result: false + }; - const results = translateResults(source); - const blurb = results.posts[0].get("blurb"); + const results = translateResults(source); + const blurb = results.posts[0].get("blurb"); - assert.ok(blurb.indexOf("thinking.png")); - assert.ok(blurb.indexOf(" Date: Mon, 16 Sep 2019 17:27:19 +0100 Subject: [PATCH 063/126] DEV: Live-reloading of core/plugin CSS alongside preview_theme_id In development, we track the last requested theme id, and use that to refresh the correct stylesheet targets. The after_action hook runs on every request, but the preview_theme_id parameter is only sent on the initial HTML request. This commit ensures we only fetch the development theme_id on HTML requests --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 069b3c097d..7940565ea5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -64,7 +64,7 @@ class ApplicationController < ActionController::Base after_action :remember_theme_id def remember_theme_id - if @theme_ids.present? + if @theme_ids.present? && request.format == "html" Stylesheet::Watcher.theme_id = @theme_ids.first if defined? Stylesheet::Watcher end end From fb200e30552316b991747839b24d260b68e8e9dc Mon Sep 17 00:00:00 2001 From: Kyle Zhao Date: Mon, 16 Sep 2019 10:52:49 -0700 Subject: [PATCH 064/126] FIX: Escape $ in translations before interpolating (#8100) The dollar sign (`$`) is a special replace pattern, and `$&` inserts the matched string. Thus dollars signs need to be escaped with the special pattern `$$`, which inserts a single `$`. --- app/assets/javascripts/locales/i18n.js | 10 +++++++++- test/javascripts/lib/i18n-test.js.es6 | 12 +++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/locales/i18n.js b/app/assets/javascripts/locales/i18n.js index cffc2e214a..68657e28b6 100644 --- a/app/assets/javascripts/locales/i18n.js +++ b/app/assets/javascripts/locales/i18n.js @@ -116,7 +116,15 @@ I18n.interpolate = function(message, options) { for (var i = 0; placeholder = matches[i]; i++) { name = placeholder.replace(this.PLACEHOLDER, "$1"); - value = options[name]; + if (typeof options[name] === "string") { + // The dollar sign (`$`) is a special replace pattern, and `$&` inserts + // the matched string. Thus dollars signs need to be escaped with the + // special pattern `$$`, which inserts a single `$`. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Specifying_a_string_as_a_parameter + value = options[name].replace(/\$/g, "$$$$"); + } else { + value = options[name]; + } if (!this.isValidNode(options, name)) { value = "[missing " + placeholder + " value]"; diff --git a/test/javascripts/lib/i18n-test.js.es6 b/test/javascripts/lib/i18n-test.js.es6 index f1c628ab25..9655f4420b 100644 --- a/test/javascripts/lib/i18n-test.js.es6 +++ b/test/javascripts/lib/i18n-test.js.es6 @@ -60,7 +60,8 @@ QUnit.module("lib:i18n", { days: { one: "%{count} day", other: "%{count} days" - } + }, + dollar_sign: "Hi {{description}}" } } }; @@ -223,3 +224,12 @@ QUnit.test("fallback", assert => { "falls back to English translations" ); }); + +QUnit.test("Dollar signs are properly escaped", assert => { + assert.equal( + I18n.t("dollar_sign", { + description: "$& $&" + }), + "Hi $& $&" + ); +}); From 445d305154ab9178f732a904b5d6c52880a8e56d Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 17 Sep 2019 13:38:01 +1000 Subject: [PATCH 065/126] DEV: initial migration can fail db:migrate can issue translations due to module loading localizing end user messages This allows db:migrate to work even when db is blank --- lib/freedom_patches/translate_accelerator.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index 89bba4fc92..b2a0f51f86 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -162,6 +162,12 @@ module I18n end by_site[locale].with_indifferent_access + rescue ActiveRecord::StatementInvalid => e + if PG::UndefinedTable === e.cause + {} + else + raise + end end def client_overrides_json(locale) From b282c893b20e5f8b5a6b5f31c96f436b7976603e Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 17 Sep 2019 16:01:39 +1000 Subject: [PATCH 066/126] DEV: support multiple hosts in dev This renames the DISCOURSE_ENV_HOST var @eviltrout introduced in 95a9a544 to DISCOURSE_ENV_HOSTS and allows for a comma delimited list of hosts This is useful for testing plugins and customized host names --- config/environments/development.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 306bd99f25..6c080046d7 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -41,7 +41,9 @@ Discourse::Application.configure do BetterErrors::Middleware.allow_ip! ENV['TRUSTED_IP'] if ENV['TRUSTED_IP'] config.load_mini_profiler = true - config.hosts << ENV['DISCOURSE_DEV_HOST'] if ENV['DISCOURSE_DEV_HOST'] + if hosts = ENV['DISCOURSE_DEV_HOSTS'] + config.hosts.concat(hosts.split(",")) + end require 'middleware/turbo_dev' config.middleware.insert 0, Middleware::TurboDev From 671ffc4e06c33db689f1657504079662e476f991 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Mon, 16 Sep 2019 16:22:48 +0530 Subject: [PATCH 067/126] FIX: do not allow posting of category topic template without any changes --- .../javascripts/discourse/models/composer.js.es6 | 11 +++++++++++ config/locales/client.en.yml | 1 + 2 files changed, 12 insertions(+) diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index 903d61b521..2df885db75 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -358,6 +358,17 @@ const Composer = RestModel.extend({ } } + if (topicFirstPost) { + // user should modify topic template + const category = this.category; + if (category && category.topic_template) { + if (this.reply.trim() === category.topic_template.trim()) { + bootbox.alert(I18n.t("composer.error.topic_template_not_modified")) + return true; + } + } + } + if (this.privateMessage) { // need at least one user when sending a PM return ( diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 50b1517c15..241b7247e8 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1626,6 +1626,7 @@ en: try_like: "Have you tried the {{heart}} button?" category_missing: "You must choose a category" tags_missing: "You must choose at least {{count}} tags" + topic_template_not_modified: "You have not modified the topic template. Please update the topic." save_edit: "Save Edit" overwrite_edit: "Overwrite Edit" From 4a11e7ee56b25a5531963b398726928d5b6f817d Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 17 Sep 2019 13:00:41 +0530 Subject: [PATCH 068/126] fix the build. --- app/assets/javascripts/discourse/models/composer.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index 2df885db75..ebe6d5aa6e 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -363,7 +363,7 @@ const Composer = RestModel.extend({ const category = this.category; if (category && category.topic_template) { if (this.reply.trim() === category.topic_template.trim()) { - bootbox.alert(I18n.t("composer.error.topic_template_not_modified")) + bootbox.alert(I18n.t("composer.error.topic_template_not_modified")); return true; } } From 081c36a4592c2d1285aa52fb671c20808fb929ee Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 16 Sep 2019 17:06:34 +0100 Subject: [PATCH 069/126] FIX: Do not include theme variables in plugin SCSS, and fix register_css --- app/models/theme_field.rb | 2 +- lib/plugin/instance.rb | 2 +- lib/stylesheet/compiler.rb | 4 +- lib/stylesheet/importer.rb | 133 +++++++++++--------- lib/stylesheet/manager.rb | 2 +- spec/components/stylesheet/compiler_spec.rb | 35 ++++++ 6 files changed, 112 insertions(+), 66 deletions(-) diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index e45bae20ac..8d8a0c6c99 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -343,7 +343,7 @@ class ThemeField < ActiveRecord::Base end def compile_scss - Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"theme_field\";", + Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"theme_field\";", "theme.scss", theme_field: self.value.dup, theme: self.theme diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 4a3211d028..28a7c34c36 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -266,7 +266,7 @@ class Plugin::Instance automatic_assets.each do |path, contents| write_asset(path, contents) paths << path - assets << [path] + assets << [path, nil, directory_name] end delete_extra_automatic_assets(paths) diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index 1dea378083..f1519aca5d 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -12,7 +12,9 @@ module Stylesheet if Importer.special_imports[asset.to_s] filename = "theme_#{options[:theme_id]}.scss" - file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"#{asset}\";" + file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\";" + file += " @import \"theme_variables\";" if Importer::THEME_TARGETS.include?(asset.to_s) + file += " @import \"#{asset}\";" else filename = "#{asset}.scss" path = "#{ASSET_ROOT}/#{filename}" diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index d06b4dbf0d..5cf12e026d 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -7,6 +7,8 @@ module Stylesheet class Importer < SassC::Importer include GlobalPath + THEME_TARGETS = %w{embedded_theme mobile_theme desktop_theme} + def self.special_imports @special_imports ||= {} end @@ -15,87 +17,94 @@ module Stylesheet special_imports[name] = blk end - register_import "theme_field" do - Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field) - end + # Contained in function so that it can be called repeatedly from test mode + def self.register_imports! + @special_imports = {} - Discourse.plugins.each do |plugin| - plugin_directory_name = plugin.directory_name + register_import "theme_field" do + Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field) + end - ["", "mobile", "desktop"].each do |type| - asset_name = type.present? ? "#{plugin_directory_name}_#{type}" : plugin_directory_name - stylesheets = type.present? ? DiscoursePluginRegistry.send("#{type}_stylesheets") : DiscoursePluginRegistry.stylesheets + Discourse.plugins.each do |plugin| + plugin_directory_name = plugin.directory_name - if stylesheets[plugin_directory_name].present? - register_import asset_name do - import_files(stylesheets[plugin_directory_name]) + ["", "mobile", "desktop"].each do |type| + asset_name = type.present? ? "#{plugin_directory_name}_#{type}" : plugin_directory_name + stylesheets = type.present? ? DiscoursePluginRegistry.send("#{type}_stylesheets") : DiscoursePluginRegistry.stylesheets + + if stylesheets[plugin_directory_name].present? + register_import asset_name do + import_files(stylesheets[plugin_directory_name]) + end end end end - end - register_import "plugins_variables" do - import_files(DiscoursePluginRegistry.sass_variables) - end - - register_import "theme_colors" do - contents = +"" - colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors - colors.each do |n, hex| - contents << "$#{n}: ##{hex} !default;\n" + register_import "plugins_variables" do + import_files(DiscoursePluginRegistry.sass_variables) end - Import.new("theme_colors.scss", source: contents) - end - - register_import "theme_variables" do - contents = +"" - - theme&.all_theme_variables&.each do |field| - if field.type_id == ThemeField.types[:theme_upload_var] - if upload = field.upload - url = upload_cdn_path(upload.url) - contents << "$#{field.name}: unquote(\"#{url}\");\n" - end - else - contents << to_scss_variable(field.name, field.value) + register_import "theme_colors" do + contents = +"" + colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors + colors.each do |n, hex| + contents << "$#{n}: ##{hex} !default;\n" end + + Import.new("theme_colors.scss", source: contents) end - theme&.included_settings&.each do |name, value| - next if name == "theme_uploads" - contents << to_scss_variable(name, value) + register_import "theme_variables" do + contents = +"" + + theme&.all_theme_variables&.each do |field| + if field.type_id == ThemeField.types[:theme_upload_var] + if upload = field.upload + url = upload_cdn_path(upload.url) + contents << "$#{field.name}: unquote(\"#{url}\");\n" + end + else + contents << to_scss_variable(field.name, field.value) + end + end + + theme&.included_settings&.each do |name, value| + next if name == "theme_uploads" + contents << to_scss_variable(name, value) + end + + Import.new("theme_variable.scss", source: contents) end - Import.new("theme_variable.scss", source: contents) - end + register_import "category_backgrounds" do + contents = +"" + Category.where('uploaded_background_id IS NOT NULL').each do |c| + contents << category_css(c) if c.uploaded_background&.url.present? + end - register_import "category_backgrounds" do - contents = +"" - Category.where('uploaded_background_id IS NOT NULL').each do |c| - contents << category_css(c) if c.uploaded_background&.url.present? + Import.new("category_background.scss", source: contents) end - Import.new("category_background.scss", source: contents) + register_import "embedded_theme" do + next unless @theme_id + + theme_import(:common, :embedded_scss) + end + + register_import "mobile_theme" do + next unless @theme_id + + theme_import(:mobile, :scss) + end + + register_import "desktop_theme" do + next unless @theme_id + + theme_import(:desktop, :scss) + end end - register_import "embedded_theme" do - next unless @theme_id - - theme_import(:common, :embedded_scss) - end - - register_import "mobile_theme" do - next unless @theme_id - - theme_import(:mobile, :scss) - end - - register_import "desktop_theme" do - next unless @theme_id - - theme_import(:desktop, :scss) - end + register_imports! def initialize(options) @theme = options[:theme] diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 2d9a2754eb..266c7f2717 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -168,7 +168,7 @@ class Stylesheet::Manager source_map_file: source_map_filename ) rescue SassC::SyntaxError => e - if %w{embedded_theme mobile_theme desktop_theme}.include?(@target.to_s) + if Importer::THEME_TARGETS.include?(@target.to_s) # no special errors for theme, handled in theme editor ["", nil] else diff --git a/spec/components/stylesheet/compiler_spec.rb b/spec/components/stylesheet/compiler_spec.rb index a5f2a95f0d..7443fbf626 100644 --- a/spec/components/stylesheet/compiler_spec.rb +++ b/spec/components/stylesheet/compiler_spec.rb @@ -15,6 +15,41 @@ describe Stylesheet::Compiler do end end + context "with a theme" do + let!(:theme) { Fabricate(:theme) } + let!(:upload) { Fabricate(:upload) } + let!(:upload_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "primary", upload: upload, value: "", type_id: ThemeField.types[:theme_upload_var]) } + let!(:stylesheet_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "scss", value: "body { background: $primary }", type_id: ThemeField.types[:scss]) } + before { stylesheet_theme_field.save! } + + it "theme stylesheet should be able to access theme asset variables" do + css, _map = Stylesheet::Compiler.compile_asset("desktop_theme", theme_id: theme.id) + expect(css).to include(upload.url) + end + + context "with a plugin" do + before do + plugin = Plugin::Instance.new + plugin.path = "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb" + plugin.register_css "body { background: $primary }" + Discourse.plugins << plugin + plugin.activate! + Stylesheet::Importer.register_imports! + end + + after do + Discourse.plugins.pop + Stylesheet::Importer.register_imports! + end + + it "does not include theme variables in plugins" do + css, _map = Stylesheet::Compiler.compile_asset("my_plugin", theme_id: theme.id) + expect(css).not_to include(upload.url) + expect(css).to include("background:") + end + end + end + it "supports asset-url" do css, _map = Stylesheet::Compiler.compile(".body{background-image: asset-url('/images/favicons/github.png');}", "test.scss") From 3da9b99dbf323ac6b98bafa9f35fb2a27dd82d63 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 16 Sep 2019 14:56:19 +0100 Subject: [PATCH 070/126] FIX: Live reload plugin stylesheets when the color scheme changes --- app/models/theme.rb | 1 + .../common/_discourse_stylesheet.html.erb | 2 +- lib/discourse.rb | 19 ++++++++++++------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index c4bda99cc7..613af2069d 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -264,6 +264,7 @@ class Theme < ActiveRecord::Base if with_scheme targets.prepend(:desktop, :mobile, :admin) + targets.append(*Discourse.find_plugin_css_assets(mobile_view: true, desktop_view: true)) Stylesheet::Manager.cache.clear if clear_manager_cache end diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb index 690f3a2711..b6f64114e7 100644 --- a/app/views/common/_discourse_stylesheet.html.erb +++ b/app/views/common/_discourse_stylesheet.html.erb @@ -12,6 +12,6 @@ <%= discourse_stylesheet_link_tag(mobile_view? ? :mobile_theme : :desktop_theme) %> <%- end %> -<%- Discourse.find_plugin_css_assets(include_official: allow_plugins?, include_unofficial: allow_third_party_plugins?, mobile_view: mobile_view?).each do |file| %> +<%- Discourse.find_plugin_css_assets(include_official: allow_plugins?, include_unofficial: allow_third_party_plugins?, mobile_view: mobile_view?, desktop_view: !mobile_view?).each do |file| %> <%= discourse_stylesheet_link_tag(file) %> <%- end %> diff --git a/lib/discourse.rb b/lib/discourse.rb index d44f028c71..ca603be076 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -217,14 +217,19 @@ module Discourse def self.find_plugin_css_assets(args) plugins = self.find_plugins(args) - assets = plugins.find_all do |plugin| - plugin.css_asset_exists? - end.map { |plugin| plugin.directory_name } + assets = [] - target = args[:mobile_view] ? :mobile : :desktop - assets += plugins.find_all do |plugin| - plugin.css_asset_exists?(target) - end.map { |plugin| "#{plugin.directory_name}_#{target}" } + targets = [nil] + targets << :mobile if args[:mobile_view] + targets << :desktop if args[:desktop_view] + + targets.each do |target| + assets += plugins.find_all do |plugin| + plugin.css_asset_exists?(target) + end.map do |plugin| + target.nil? ? plugin.directory_name : "#{plugin.directory_name}_#{target}" + end + end assets end From e74f8517282a97a792f0bcae1465ba2b5c7898b5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 16 Sep 2019 17:10:47 +0100 Subject: [PATCH 071/126] FIX: Live reload plugin stylesheets when editing in development --- lib/stylesheet/watcher.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/stylesheet/watcher.rb b/lib/stylesheet/watcher.rb index 1bfc7c2994..1fbe7f3aae 100644 --- a/lib/stylesheet/watcher.rb +++ b/lib/stylesheet/watcher.rb @@ -78,7 +78,9 @@ module Stylesheet Stylesheet::Manager.cache.clear - message = ["desktop", "mobile", "admin"].map do |name| + targets = ["desktop", "mobile", "admin"] + targets.append(*Discourse.find_plugin_css_assets(mobile_view: true, desktop_view: true)) + message = targets.map do |name| Stylesheet::Manager.stylesheet_data(name.to_sym, Stylesheet::Watcher.theme_id) end.flatten MessageBus.publish '/file-change', message From e1e8cac58f9dfa71e3badd8d69c51dcb89e82199 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 17 Sep 2019 10:19:59 +0100 Subject: [PATCH 072/126] FIX: Correct theme SCSS error handling --- lib/stylesheet/manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 266c7f2717..1b9b8757b3 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -168,7 +168,7 @@ class Stylesheet::Manager source_map_file: source_map_filename ) rescue SassC::SyntaxError => e - if Importer::THEME_TARGETS.include?(@target.to_s) + if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s) # no special errors for theme, handled in theme editor ["", nil] else From 5c897b6d0cb8e102a9a4b00d969e4526d9bdb5dc Mon Sep 17 00:00:00 2001 From: Joshua Rosenfeld Date: Tue, 17 Sep 2019 13:39:26 -0400 Subject: [PATCH 073/126] Copy: Update Dashboard Advice PM All admins receive the Dashboard Advice PM. If one admin takes action on the advice, future admins who follow the link in the PM will see no advice on the dashboard. This has caused some confusion, so we've updated the text to make this clearer. --- config/locales/server.en.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9f3b705a25..4dc5152f6e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -3058,6 +3058,8 @@ en: We have some new advice and recommendations for you based on your current site settings. [Visit your site dashboard](%{base_url}/admin) to see it. + + If nothing is visible on your dashboard, another staff member may have already acted on this advice. new_user_of_the_month: title: "You're a New User of the Month!" From 4cd620e36eaefa29035a859f4f74726a2d0e7a05 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 17 Sep 2019 13:37:22 -0400 Subject: [PATCH 074/126] Remove special cases for flagging Prior to the new review queue there were a couple special cases where posts would be auto hidden: * If a TL3 or above flagged a TL0 post as spam * If a TL4 or above flagged a non-staff, non-TL4 post as spam, inappropriate or off topic. These cases are now removed in favour of the scoring system. --- lib/post_action_creator.rb | 19 ++----------- spec/models/post_action_spec.rb | 50 --------------------------------- 2 files changed, 3 insertions(+), 66 deletions(-) diff --git a/lib/post_action_creator.rb b/lib/post_action_creator.rb index 120ee3a456..47538b1b45 100644 --- a/lib/post_action_creator.rb +++ b/lib/post_action_creator.rb @@ -156,22 +156,9 @@ private return if @post.hidden? return if !@created_by.staff? && @post.user&.staff? - if @post_action_name == :spam && - @created_by.has_trust_level?(TrustLevel[3]) && - @post.user&.trust_level == TrustLevel[0] - @post.hide!(@post_action_type_id, Post.hidden_reasons[:flagged_by_tl3_user]) - elsif PostActionType.auto_action_flag_types.include?(@post_action_name) - if @created_by.has_trust_level?(TrustLevel[4]) && - !@created_by.staff? && - @post.user&.trust_level != TrustLevel[4] - - @post.hide!(@post_action_type_id, Post.hidden_reasons[:flagged_by_tl4_user]) - else - score = ReviewableFlaggedPost.find_by(target: @post)&.score || 0 - if score >= Reviewable.score_required_to_hide_post - @post.hide!(@post_action_type_id) - end - end + score = ReviewableFlaggedPost.find_by(target: @post)&.score || 0 + if score >= Reviewable.score_required_to_hide_post + @post.hide!(@post_action_type_id) end end diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index ca27c8f856..cc2f449b1c 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -609,56 +609,6 @@ describe PostAction do expect(post.hidden).to eq(true) end - it "hide tl0 posts that are flagged as spam by a tl3 user" do - newuser = Fabricate(:newuser) - post = create_post(user: newuser) - - Discourse.stubs(:site_contact_user).returns(admin) - - PostActionCreator.spam(Fabricate(:leader), post) - - post.reload - - expect(post.hidden).to eq(true) - expect(post.hidden_at).to be_present - expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:flagged_by_tl3_user]) - end - - it "hide non-tl4 posts that are flagged by a tl4 user" do - SiteSetting.site_contact_username = admin.username - - tl4_user = Fabricate(:trust_level_4) - user = Fabricate(:leader) - post = create_post(user: user) - - PostActionCreator.spam(tl4_user, post) - - post.reload - - expect(post.hidden).to be_truthy - expect(post.hidden_at).to be_present - expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:flagged_by_tl4_user]) - - post = create_post(user: user) - PostActionCreator.spam(Fabricate(:leader), post) - post.reload - - expect(post.hidden).to be_falsey - - post = create_post(user: user) - PostActionCreator.spam(Fabricate(:moderator), post) - post.reload - - expect(post.hidden).to be_falsey - - user = Fabricate(:trust_level_4) - post = create_post(user: user) - PostActionCreator.spam(tl4_user, post) - post.reload - - expect(post.hidden).to be_falsey - end - it "can flag the topic instead of a post" do post1 = create_post create_post(topic: post1.topic) From c463a04c3f0f0d7bf5c8b3b3997b632bf381f59b Mon Sep 17 00:00:00 2001 From: Joshua Rosenfeld Date: Tue, 17 Sep 2019 14:41:52 -0400 Subject: [PATCH 075/126] Copy: Update too few topics/posts notice The current copy has caused some confusion that admins can only create 5 topics or 30 posts. Update copy to make it clearer this is a recommended minimum, not a limit. --- config/locales/client.en.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 241b7247e8..e33577afe8 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1325,9 +1325,9 @@ en: enabled: "This site is in read only mode. Please continue to browse, but replying, likes, and other actions are disabled for now." login_disabled: "Login is disabled while the site is in read only mode." logout_disabled: "Logout is disabled while the site is in read only mode." - too_few_topics_and_posts_notice: "Let's start the discussion! There are %{currentTopics} / %{requiredTopics} topics and %{currentPosts} / %{requiredPosts} posts – visitors need more to read and reply to. Only staff can see this message." - too_few_topics_notice: "Let's start the discussion! There are %{currentTopics} / %{requiredTopics} topics – visitors need more to read and reply to. Only staff can see this message." - too_few_posts_notice: "Let's start the discussion! There are %{currentPosts} / %{requiredPosts} posts – visitors need more to read and reply to. Only staff can see this message." + too_few_topics_and_posts_notice: "Let's start the discussion! There are %{currentTopics} topics and %{currentPosts} posts. Visitors need more to read and reply to – we recommend at least %{requiredTopics} topics and %{requiredPosts} posts. Only staff can see this message." + too_few_topics_notice: "Let's start the discussion! There are %{currentTopics} topics. Visitors need more to read and reply to – we recommend at least %{requiredTopics} topics. Only staff can see this message." + too_few_posts_notice: "Let's start the discussion! There are %{currentPosts} posts. Visitors need more to read and reply to – we recommend at least %{requiredPosts} posts. Only staff can see this message." logs_error_rate_notice: # keys ending with _MF use message format, see https://meta.discourse.org/t/message-format-support-for-localization/7035 for details reached_hour_MF: "{relativeAge} – {rate, plural, one {# error/hour} other {# errors/hour}} reached site setting limit of {limit, plural, one {# error/hour} other {# errors/hour}}." From 5bf3a00328c63755ddc8482a6c88ca9088f97881 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 17 Sep 2019 14:47:56 -0400 Subject: [PATCH 076/126] FIX: Ignored flags should not count in your accuracy score --- app/models/reviewable_score.rb | 11 +++-------- .../reviewable_score_explanation_serializer.rb | 6 +----- spec/models/reviewable_score_spec.rb | 5 +++-- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/app/models/reviewable_score.rb b/app/models/reviewable_score.rb index 14ba16ed48..823e8935ce 100644 --- a/app/models/reviewable_score.rb +++ b/app/models/reviewable_score.rb @@ -59,19 +59,14 @@ class ReviewableScore < ActiveRecord::Base user_stat = user&.user_stat return 0.0 if user_stat.blank? - calc_user_accuracy_bonus( - user_stat.flags_agreed, - user_stat.flags_disagreed, - user_stat.flags_ignored - ) + calc_user_accuracy_bonus(user_stat.flags_agreed, user_stat.flags_disagreed) end - def self.calc_user_accuracy_bonus(agreed, disagreed, ignored) + def self.calc_user_accuracy_bonus(agreed, disagreed) agreed ||= 0 disagreed ||= 0 - ignored ||= 0 - total = (agreed + disagreed + ignored).to_f + total = (agreed + disagreed).to_f return 0.0 if total <= 5 (agreed / total) * 5.0 diff --git a/app/serializers/reviewable_score_explanation_serializer.rb b/app/serializers/reviewable_score_explanation_serializer.rb index caff0cee26..73dccc01bf 100644 --- a/app/serializers/reviewable_score_explanation_serializer.rb +++ b/app/serializers/reviewable_score_explanation_serializer.rb @@ -14,11 +14,7 @@ class ReviewableScoreExplanationSerializer < ApplicationSerializer ) def user_accuracy_bonus - ReviewableScore.calc_user_accuracy_bonus( - object.flags_agreed, - object.flags_disagreed, - object.flags_ignored - ) + ReviewableScore.calc_user_accuracy_bonus(object.flags_agreed, object.flags_disagreed) end end diff --git a/spec/models/reviewable_score_spec.rb b/spec/models/reviewable_score_spec.rb index 6a1c409c1f..abf717e08f 100644 --- a/spec/models/reviewable_score_spec.rb +++ b/spec/models/reviewable_score_spec.rb @@ -107,10 +107,11 @@ RSpec.describe ReviewableScore, type: :model do user_stat.flags_disagreed = 2 expect(ReviewableScore.user_accuracy_bonus(user).floor(2)).to eq(3.33) + # Ignored flags don't count user_stat.flags_agreed = 121 user_stat.flags_disagreed = 44 user_stat.flags_ignored = 4 - expect(ReviewableScore.user_accuracy_bonus(user).floor(2)).to eq(3.57) + expect(ReviewableScore.user_accuracy_bonus(user).floor(2)).to eq(3.66) end end @@ -137,7 +138,7 @@ RSpec.describe ReviewableScore, type: :model do user_stat.flags_agreed = 12 user_stat.flags_disagreed = 2 user_stat.flags_ignored = 2 - expect(ReviewableScore.user_flag_score(user)).to eq(6.75) + expect(ReviewableScore.user_flag_score(user).floor(2)).to eq(7.28) end end end From 215f6d97e5e3fc7e4451b0cc20ec73631e28243e Mon Sep 17 00:00:00 2001 From: Jay Pfaffman Date: Tue, 17 Sep 2019 12:45:40 -0700 Subject: [PATCH 077/126] Make INSTALL-cloud agree with discourse-setup (#8102) `discourse-setup` will install Docker. Several people lately have had trouble with a `|` getting turned into a `>` when trying to `curl` the site into `sh`. Also, the recommended `git clone` command will create `/var/discourse`, so don't tell people to make that directory, as that too can be confusing. --- docs/INSTALL-cloud.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/docs/INSTALL-cloud.md b/docs/INSTALL-cloud.md index f7fb134a4d..ede6f1f015 100644 --- a/docs/INSTALL-cloud.md +++ b/docs/INSTALL-cloud.md @@ -26,15 +26,13 @@ Enter the root password from the email DigitalOcean sent you when the server was -### Install Docker / Git +### Install Docker / Git (Optional) - wget -qO- https://get.docker.com/ | sh - -This command installs the latest versions of Docker and Git on your server. Alternately, you can manually [install Git](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git) and the [Docker package for your OS](https://docs.docker.com/installation/). +If you have reason to install your own version of Docker, you may do so. If docker is not installed, `discourse-setup` will automatically install it from get.docker.com. ### Install Discourse -Create a `/var/discourse` folder, clone the [Official Discourse Docker Image][dd] into it: +Clone the [Official Discourse Docker Image][dd] into `/var/discourse`. sudo -s git clone https://github.com/discourse/discourse_docker.git /var/discourse From c3bbf643b16ee81fadac738edef4698d9743267c Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 17 Sep 2019 15:58:30 -0400 Subject: [PATCH 078/126] FIX: Put back the TL3 -> TL0 spam thing We talked about it and decided it's still relevant in the score world. --- lib/post_action_creator.rb | 9 +++++++++ spec/models/post_action_spec.rb | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/post_action_creator.rb b/lib/post_action_creator.rb index 47538b1b45..215345c425 100644 --- a/lib/post_action_creator.rb +++ b/lib/post_action_creator.rb @@ -156,6 +156,15 @@ private return if @post.hidden? return if !@created_by.staff? && @post.user&.staff? + # Special case: If you have TL3 and the user is TL0, and the flag is spam, + # hide it immediately. + if @post_action_name == :spam && + @created_by.has_trust_level?(TrustLevel[3]) && + @post.user&.trust_level == TrustLevel[0] + @post.hide!(@post_action_type_id, Post.hidden_reasons[:flagged_by_tl3_user]) + return + end + score = ReviewableFlaggedPost.find_by(target: @post)&.score || 0 if score >= Reviewable.score_required_to_hide_post @post.hide!(@post_action_type_id) diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index cc2f449b1c..80ff626e6c 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -608,6 +608,20 @@ describe PostAction do post.reload expect(post.hidden).to eq(true) end + it "hide tl0 posts that are flagged as spam by a tl3 user" do + newuser = Fabricate(:newuser) + post = create_post(user: newuser) + + Discourse.stubs(:site_contact_user).returns(admin) + + PostActionCreator.spam(Fabricate(:leader), post) + + post.reload + + expect(post.hidden).to eq(true) + expect(post.hidden_at).to be_present + expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:flagged_by_tl3_user]) + end it "can flag the topic instead of a post" do post1 = create_post From 3debdc8131bc2e44fca12be63bedb9b8dcac2905 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 17 Sep 2019 16:12:50 -0400 Subject: [PATCH 079/126] SECURITY: XSS when oneboxing user profile location field The XSS here is only possible if CSP is disabled. Low impact since CSP is enabled by default in SiteSettings. --- lib/oneboxer.rb | 2 +- spec/components/oneboxer_spec.rb | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index 94e9c40215..a4c9163804 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -245,7 +245,7 @@ module Oneboxer avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"), name: name, bio: user.user_profile.bio_excerpt(230), - location: user.user_profile.location, + location: Onebox::Helpers.sanitize(user.user_profile.location), joined: I18n.t('joined'), created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')), website: user.user_profile.website, diff --git a/spec/components/oneboxer_spec.rb b/spec/components/oneboxer_spec.rb index 5ed10343ce..659a975bcd 100644 --- a/spec/components/oneboxer_spec.rb +++ b/spec/components/oneboxer_spec.rb @@ -113,6 +113,25 @@ describe Oneboxer do expect(preview("#{path}.mov")).to include("