From c458cebfc360b748fccc4f2ec7a0b3d94320ad78 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Thu, 15 Jul 2021 21:16:35 +0200 Subject: [PATCH 01/77] FIX: User's "Top badges" grid (#13752) Fixes a regression introduced in #13719 --- .../discourse/app/templates/user/summary.hbs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/app/templates/user/summary.hbs b/app/assets/javascripts/discourse/app/templates/user/summary.hbs index de025dbf16..1199c4bf82 100644 --- a/app/assets/javascripts/discourse/app/templates/user/summary.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/summary.hbs @@ -151,13 +151,21 @@ {{#if siteSettings.enable_badges}}

{{i18n "user.summary.top_badges"}}

- {{#each model.badges as |badge|}} - {{badge-card badge=badge count=badge.count username=user.username_lower}} + + {{#if model.badges}} +
+ {{#each model.badges as |badge|}} + {{badge-card badge=badge count=badge.count username=user.username_lower}} + {{/each}} +
{{else}}

{{i18n "user.summary.no_badges"}}

- {{/each}} + {{/if}} + {{#if moreBadges}} -

{{#link-to "user.badges" user class="more"}}{{i18n "user.summary.more_badges"}}{{/link-to}}

+ {{#link-to "user.badges" user class="more"}} + {{i18n "user.summary.more_badges"}} + {{/link-to}} {{/if}}
{{/if}} From 23773b3f19929819383af433bc1d261c8af8c863 Mon Sep 17 00:00:00 2001 From: Tobias Eigen Date: Thu, 15 Jul 2021 13:22:21 -0700 Subject: [PATCH 02/77] updated copy for reply_as_new_group_message (#13754) I changed from "Create a new private message with the same recipients" to "Create new message starting with same recipients". Wanting to remove "private message" because these messages are not private (admins can read them). Also want to communicate that this is a way to expand the conversation to new people without having to explicitly invite them to the current message, or give them access to the past discussions. ref: https://dev.discourse.org/t/bring-invite-to-message-in-line-with-new-invite-system/49578/12?u=tobiaseigen ref: https://meta.discourse.org/t/how-do-you-add-another-person-to-a-private-message-when-its-already-sent/43357/8?u=tobiaseigen --- config/locales/client.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6ffc48acc9..b08d134030 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2132,7 +2132,7 @@ en: confirm: You have a new topic draft saved, which will be overwritten if you create a linked topic. reply_as_new_group_message: label: Reply as new group message - desc: Create a new private message with the same recipients + desc: Create new message starting with same recipients reply_as_private_message: label: New message desc: Create a new personal message From 8bdec18d5840105717ae8948aaf2699d683d896e Mon Sep 17 00:00:00 2001 From: Tobias Eigen Date: Thu, 15 Jul 2021 13:22:57 -0700 Subject: [PATCH 03/77] changed private message -> personal message (#13753) We are trying to not use "private message" in the interface, in favor of "personal message", because of course admins can read all messages so they are not really private unless encrypted messaging is turned on.. --- config/locales/client.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b08d134030..0066ca2ee6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -337,7 +337,7 @@ en: remove: "Remove" remove_confirmation: "Are you sure you want to delete this draft?" new_topic: "New topic draft" - new_private_message: "New private message draft" + new_private_message: "New personal message draft" topic_reply: "Draft reply" abandon: confirm: "You have a draft in progress for this topic. What would you like to do with it?" From 7323c65d535dcff92290c83127b622c91e3029e9 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 16 Jul 2021 05:51:13 +0300 Subject: [PATCH 04/77] FIX: Debounce group name validation correctly (#13757) --- .../components/groups-form-profile-fields.js | 76 +++++++++---------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/groups-form-profile-fields.js b/app/assets/javascripts/discourse/app/components/groups-form-profile-fields.js index d2a439ed54..43fb8c4d70 100644 --- a/app/assets/javascripts/discourse/app/components/groups-form-profile-fields.js +++ b/app/assets/javascripts/discourse/app/components/groups-form-profile-fields.js @@ -57,52 +57,50 @@ export default Component.extend({ ); } - this.checkGroupName(); + this.checkGroupNameDebounced(); return this._failedInputValidation( I18n.t("admin.groups.new.name.checking") ); }, - checkGroupName() { - discourseDebounce( - this, - function () { - if (isEmpty(this.nameInput)) { - return; + checkGroupNameDebounced() { + discourseDebounce(this, this._checkGroupName, 500); + }, + + _checkGroupName() { + if (isEmpty(this.nameInput)) { + return; + } + + Group.checkName(this.nameInput) + .then((response) => { + const validationName = "uniqueNameValidation"; + + if (response.available) { + this.set( + validationName, + EmberObject.create({ + ok: true, + reason: I18n.t("admin.groups.new.name.available"), + }) + ); + + this.set("disableSave", false); + this.set("model.name", this.nameInput); + } else { + let reason; + + if (response.errors) { + reason = response.errors.join(" "); + } else { + reason = I18n.t("admin.groups.new.name.not_available"); + } + + this.set(validationName, this._failedInputValidation(reason)); } - - Group.checkName(this.nameInput) - .then((response) => { - const validationName = "uniqueNameValidation"; - - if (response.available) { - this.set( - validationName, - EmberObject.create({ - ok: true, - reason: I18n.t("admin.groups.new.name.available"), - }) - ); - - this.set("disableSave", false); - this.set("model.name", this.nameInput); - } else { - let reason; - - if (response.errors) { - reason = response.errors.join(" "); - } else { - reason = I18n.t("admin.groups.new.name.not_available"); - } - - this.set(validationName, this._failedInputValidation(reason)); - } - }) - .catch(popupAjaxError); - }, - 500 - ); + }) + .catch(popupAjaxError); }, _failedInputValidation(reason) { From 444e21b12d4a98768f839ecffd8d2e60ade44b64 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 16 Jul 2021 04:10:04 +0100 Subject: [PATCH 05/77] FEATURE: Add 'users.list' API scope (#13742) --- app/models/api_key_scope.rb | 1 + config/locales/client.en.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/app/models/api_key_scope.rb b/app/models/api_key_scope.rb index 2bf69db9f2..f4cb44e2f8 100644 --- a/app/models/api_key_scope.rb +++ b/app/models/api_key_scope.rb @@ -41,6 +41,7 @@ class ApiKeyScope < ActiveRecord::Base log_out: { actions: %w[admin/users#log_out] }, anonymize: { actions: %w[admin/users#anonymize] }, delete: { actions: %w[admin/users#destroy] }, + list: { actions: %w[admin/users#index] }, }, email: { receive_emails: { actions: %w[admin/email#handle_mail] } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0066ca2ee6..81484815c4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4023,6 +4023,7 @@ en: log_out: Log out all sessions for a user. anonymize: Anonymize user accounts. delete: Delete user accounts. + list: Get a list of users. email: receive_emails: Combine this scope with the mail-receiver to process incoming emails. From 1cadae38796cd221a40258f2e70a148f2f928d31 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Fri, 16 Jul 2021 07:13:00 +0400 Subject: [PATCH 06/77] FIX: simplify and improve choosing favorite badges (#13743) * No need to return anything except a status code from the server * Switch a badge state before sending a request and then switch it back in case of an error --- .../discourse/app/models/user-badge.js | 14 ++++++----- app/controllers/user_badges_controller.rb | 6 +---- spec/requests/user_badges_controller_spec.rb | 24 +++++-------------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/app/assets/javascripts/discourse/app/models/user-badge.js b/app/assets/javascripts/discourse/app/models/user-badge.js index 07467f3909..2bba997031 100644 --- a/app/assets/javascripts/discourse/app/models/user-badge.js +++ b/app/assets/javascripts/discourse/app/models/user-badge.js @@ -22,12 +22,14 @@ const UserBadge = EmberObject.extend({ }, favorite() { - return ajax(`/user_badges/${this.id}/toggle_favorite`, { type: "PUT" }) - .then((json) => { - this.set("is_favorite", json.user_badge.is_favorite); - return this; - }) - .catch(popupAjaxError); + this.toggleProperty("is_favorite"); + return ajax(`/user_badges/${this.id}/toggle_favorite`, { + type: "PUT", + }).catch((e) => { + // something went wrong, switch the UI back: + this.toggleProperty("is_favorite"); + popupAjaxError(e); + }); }, }); diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb index 60b1275461..abf2b9f054 100644 --- a/app/controllers/user_badges_controller.rb +++ b/app/controllers/user_badges_controller.rb @@ -109,14 +109,10 @@ class UserBadgesController < ApplicationController return render json: failed_json, status: 400 end - new_is_favorite_value = !user_badge.is_favorite UserBadge .where(user_id: user_badge.user_id, badge_id: user_badge.badge_id) - .update_all(is_favorite: new_is_favorite_value) + .update_all(is_favorite: !user_badge.is_favorite) UserBadge.update_featured_ranks!(user_badge.user_id) - - user_badge.is_favorite = new_is_favorite_value - render_serialized(user_badge, DetailedUserBadgeSerializer, root: :user_badge) end private diff --git a/spec/requests/user_badges_controller_spec.rb b/spec/requests/user_badges_controller_spec.rb index 4c17ac2763..4154f5ba98 100644 --- a/spec/requests/user_badges_controller_spec.rb +++ b/spec/requests/user_badges_controller_spec.rb @@ -288,17 +288,14 @@ describe UserBadgesController do SiteSetting.max_favorite_badges = 3 put "/user_badges/#{user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) + expect(response.status).to eq(204) end it "favorites a badge" do sign_in(user) put "/user_badges/#{user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["user_badge"]["is_favorite"]).to eq(true) - + expect(response.status).to eq(204) user_badge = UserBadge.find_by(user: user, badge: badge) expect(user_badge.is_favorite).to eq(true) end @@ -308,10 +305,7 @@ describe UserBadgesController do user_badge.toggle!(:is_favorite) put "/user_badges/#{user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["user_badge"]["is_favorite"]).to eq(false) - + expect(response.status).to eq(204) user_badge = UserBadge.find_by(user: user, badge: badge) expect(user_badge.is_favorite).to eq(false) end @@ -328,23 +322,17 @@ describe UserBadgesController do other_user_badge = UserBadge.create(badge: other_badge, user: user, granted_by: Discourse.system_user, granted_at: Time.now) put "/user_badges/#{user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["user_badge"]["is_favorite"]).to eq(false) + expect(response.status).to eq(204) expect(user_badge.reload.is_favorite).to eq(false) expect(user_badge2.reload.is_favorite).to eq(false) put "/user_badges/#{user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["user_badge"]["is_favorite"]).to eq(true) + expect(response.status).to eq(204) expect(user_badge.reload.is_favorite).to eq(true) expect(user_badge2.reload.is_favorite).to eq(true) put "/user_badges/#{other_user_badge.id}/toggle_favorite.json" - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["user_badge"]["is_favorite"]).to eq(true) + expect(response.status).to eq(204) expect(other_user_badge.reload.is_favorite).to eq(true) end end From 207c3085fc377e5f08c1002c9913eed3c4debdd6 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 15 Jul 2021 23:43:31 -0400 Subject: [PATCH 07/77] DEV: Refactor stylesheet live-reloading (#13755) We have had reports of tabs freezing in Firefox, and reporting an error in this line. I haven't been able to reproduce, but I suspect the `forEach` loop is at the heart of the issue, so I have replaced it with (hopefully) a safer call. * More refactoring * Do not reload stylesheets with unchanged filenames * Select last matching stylesheet --- .../app/initializers/live-development.js | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse/app/initializers/live-development.js b/app/assets/javascripts/discourse/app/initializers/live-development.js index bf92d0e2c2..7b0c207082 100644 --- a/app/assets/javascripts/discourse/app/initializers/live-development.js +++ b/app/assets/javascripts/discourse/app/initializers/live-development.js @@ -60,12 +60,22 @@ export default { // Refresh if necessary document.location.reload(true); } else if (me.new_href && me.target) { - const link_target = me.theme_id - ? `[data-target="${me.target}"][data-theme-id="${me.theme_id}"]` - : `[data-target="${me.target}"]`; - document.querySelectorAll(`link${link_target}`).forEach((link) => { - this.refreshCSS(link, me.new_href); - }); + const link_target = !!me.theme_id + ? `[data-target='${me.target}'][data-theme-id='${me.theme_id}']` + : `[data-target='${me.target}']`; + + const links = document.querySelectorAll(`link${link_target}`); + if (links.length > 0) { + const lastLink = links[links.length - 1]; + // this check is useful when message-bus has multiple file updates + // it avoids the browser doing a lot of work for nothing + // should the filenames be unchanged + if ( + lastLink.href.split("/").pop() !== me.new_href.split("/").pop() + ) { + this.refreshCSS(lastLink, me.new_href); + } + } } }); }, @@ -74,21 +84,14 @@ export default { }, refreshCSS(node, newHref) { - if (node.dataset.reloading) { - clearTimeout(node.dataset.timeout); - } - - node.dataset.reloading = true; - let reloaded = node.cloneNode(true); reloaded.href = newHref; node.insertAdjacentElement("afterend", reloaded); - let timeout = setTimeout(() => { - node.parentNode.removeChild(node); - reloaded.dataset.reloading = false; - }, 2000); - - node.dataset.timeout = timeout; + setTimeout(() => { + if (node && node.parentNode) { + node.parentNode.removeChild(node); + } + }, 500); }, }; From c4d7545f356582d280ffeba2637b15392ad21fa0 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Fri, 16 Jul 2021 11:56:51 +0400 Subject: [PATCH 08/77] FIX: when updating timestamps on topic set a correct bump date (#13746) There was a bug with changing timestamps using the topic wrench button. Under some circumstances, a topic was disappearing from the top of the latest tab after changing timestamps. Steps to reproduce: - Choose a topic on the latest tab (the topic should be created some time ago, but has recent posts) - Change topic timestamps (for example, move them one day forward): - Go back to the latest tab and see that topic has disappeared. This PR fixes this. We were setting topic.bumped_at to the timestamp user specified on the modal. This is incorrect. Instead, we should be setting topic.bumped_at to the created_at timestamp of the last regular (not a whisper and so on) post on the topic. --- app/services/topic_timestamp_changer.rb | 2 +- spec/services/topic_timestamp_changer_spec.rb | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/services/topic_timestamp_changer.rb b/app/services/topic_timestamp_changer.rb index d02cda99f4..a8d5059c4d 100644 --- a/app/services/topic_timestamp_changer.rb +++ b/app/services/topic_timestamp_changer.rb @@ -29,6 +29,7 @@ class TopicTimestampChanger end end + @topic.reset_bumped_at update_topic(last_posted_at) yield(@topic) if block_given? @@ -48,7 +49,6 @@ class TopicTimestampChanger @topic.update( created_at: @timestamp, updated_at: @timestamp, - bumped_at: @timestamp, last_posted_at: last_posted_at ) end diff --git a/spec/services/topic_timestamp_changer_spec.rb b/spec/services/topic_timestamp_changer_spec.rb index c94cecfb92..255530f68c 100644 --- a/spec/services/topic_timestamp_changer_spec.rb +++ b/spec/services/topic_timestamp_changer_spec.rb @@ -26,19 +26,20 @@ describe TopicTimestampChanger do TopicTimestampChanger.new(topic: topic, timestamp: new_timestamp.to_f).change! topic.reload + p1.reload + p2.reload + last_post_created_at = p2.created_at + expect(topic.created_at).to eq_time(new_timestamp) expect(topic.updated_at).to eq_time(new_timestamp) - expect(topic.bumped_at).to eq_time(new_timestamp) + expect(topic.bumped_at).to eq_time(last_post_created_at) + expect(topic.last_posted_at).to eq_time(last_post_created_at) - p1.reload expect(p1.created_at).to eq_time(new_timestamp) expect(p1.updated_at).to eq_time(new_timestamp) - p2.reload expect(p2.created_at).to eq_time(new_timestamp + 1.day) expect(p2.updated_at).to eq_time(new_timestamp + 1.day) - - expect(topic.last_posted_at).to eq_time(p2.reload.created_at) end describe 'when posts have timestamps in the future' do From 810892139b2d27fb87a823180bb092db6bdfad8d Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 16 Jul 2021 11:02:24 +0300 Subject: [PATCH 09/77] FIX: Ascending/descending sorting in the group membership requests page The `GroupsController#members` endpoint accepts a `desc` parameter to determine how members are sorted, but it's been deprecated in favor of a boolean `asc` parameter. However, in the frontend, specifically the group membership requests page was not updated entirely to use the `asc` param and it still passes a `desc` param when changing how group requests are sorted. This commit updates the `group-requests` Ember controller so it passes a boolean `asc` param and removes all references of `desc`. The controller view/template has already been updated to use `asc`: https://github.com/discourse/discourse/blob/207c3085fc377e5f08c1002c9913eed3c4debdd6/app/assets/javascripts/discourse/app/templates/group-requests.hbs#L15-L16 --- .../discourse/app/controllers/group-requests.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/group-requests.js b/app/assets/javascripts/discourse/app/controllers/group-requests.js index 73da360192..9cb4dcf3a6 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-requests.js +++ b/app/assets/javascripts/discourse/app/controllers/group-requests.js @@ -7,10 +7,10 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; export default Controller.extend({ application: controller(), - queryParams: ["order", "desc", "filter"], + queryParams: ["order", "asc", "filter"], order: "", - desc: null, + asc: null, filter: null, filterInput: null, @@ -27,7 +27,7 @@ export default Controller.extend({ ); }, - @observes("order", "desc", "filter") + @observes("order", "asc", "filter") _filtersChanged() { this.findRequesters(true); }, @@ -57,9 +57,9 @@ export default Controller.extend({ }); }, - @discourseComputed("order", "desc", "filter") - memberParams(order, desc, filter) { - return { order, desc, filter }; + @discourseComputed("order", "asc", "filter") + memberParams(order, asc, filter) { + return { order, asc, filter }; }, @discourseComputed("model.requesters.[]") From 361c8be547342d6c297eaea51b007594e41bbdf1 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 16 Jul 2021 10:58:01 -0400 Subject: [PATCH 10/77] PERF: Add scheduled job to delete old stylesheet cache rows (#13747) --- .../scheduled/clean_up_stylesheet_cache.rb | 11 ++++++++++ app/models/stylesheet_cache.rb | 5 +++++ spec/models/stylesheet_cache_spec.rb | 22 ++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 app/jobs/scheduled/clean_up_stylesheet_cache.rb diff --git a/app/jobs/scheduled/clean_up_stylesheet_cache.rb b/app/jobs/scheduled/clean_up_stylesheet_cache.rb new file mode 100644 index 0000000000..51913a4f4b --- /dev/null +++ b/app/jobs/scheduled/clean_up_stylesheet_cache.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + class CleanUpStylesheetCache < ::Jobs::Scheduled + every 1.week + + def execute(args) + StylesheetCache.clean_up + end + end +end diff --git a/app/models/stylesheet_cache.rb b/app/models/stylesheet_cache.rb index 03d921c076..819f9ab366 100644 --- a/app/models/stylesheet_cache.rb +++ b/app/models/stylesheet_cache.rb @@ -4,6 +4,7 @@ class StylesheetCache < ActiveRecord::Base self.table_name = 'stylesheet_cache' MAX_TO_KEEP = 50 + CLEANUP_AFTER_DAYS = 150 def self.add(target, digest, content, source_map, max_to_keep: nil) max_to_keep ||= MAX_TO_KEEP @@ -42,6 +43,10 @@ class StylesheetCache < ActiveRecord::Base end end + def self.clean_up + StylesheetCache.where('created_at < ?', CLEANUP_AFTER_DAYS.days.ago).delete_all + end + end # == Schema Information diff --git a/spec/models/stylesheet_cache_spec.rb b/spec/models/stylesheet_cache_spec.rb index c83f6c5786..d47e9150ab 100644 --- a/spec/models/stylesheet_cache_spec.rb +++ b/spec/models/stylesheet_cache_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' describe StylesheetCache do - describe "add" do + describe ".add" do it "correctly cycles once MAX_TO_KEEP is hit" do StylesheetCache.destroy_all @@ -37,6 +37,26 @@ describe StylesheetCache do expect(StylesheetCache.order(:id).pluck(:target)).to eq(["desktop", "desktop", "mobile", "mobile"]) end + end + describe ".clean_up" do + it "removes items older than threshold" do + StylesheetCache.destroy_all + + StylesheetCache.add("a", "b", "c", "map") + StylesheetCache.add("d", "e", "f", "map") + + above_threshold = StylesheetCache::CLEANUP_AFTER_DAYS - 1 + StylesheetCache.first.update!(created_at: above_threshold.days.ago) + + StylesheetCache.clean_up + expect(StylesheetCache.all.size).to eq(2) + + below_threshold = StylesheetCache::CLEANUP_AFTER_DAYS + 1 + StylesheetCache.first.update!(created_at: below_threshold.days.ago) + + StylesheetCache.clean_up + expect(StylesheetCache.all.size).to eq(1) + end end end From 438a7629561397830fa9622f517ecec46e003e82 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 16 Jul 2021 11:08:20 -0400 Subject: [PATCH 11/77] FEATURE: Add assistant to quick search widget (#13650) Replaces the autocomplete overlay for categories and usernames on the search input and adds suggestions as items in the search results instead. Also adds the same behaviour for @mentions as well as special `in: status: order:` keywords. See PR for more details. --- .../discourse/app/components/site-header.js | 1 - .../discourse/app/lib/plugin-api.js | 15 +- .../javascripts/discourse/app/lib/search.js | 61 ++----- .../discourse/app/widgets/header.js | 15 +- .../app/widgets/search-menu-controls.js | 12 +- .../app/widgets/search-menu-results.js | 166 ++++++++++++++++++ .../discourse/app/widgets/search-menu.js | 97 +++++++++- .../discourse/tests/acceptance/search-test.js | 55 ++++++ .../stylesheets/common/base/search-menu.scss | 17 ++ 9 files changed, 370 insertions(+), 69 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index d9895801fa..9ac49c9cfe 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -218,7 +218,6 @@ const SiteHeaderComponent = MountWidget.extend( this.dispatch("notifications:changed", "user-notifications"); this.dispatch("header:keyboard-trigger", "header"); - this.dispatch("search-autocomplete:after-complete", "search-term"); this.dispatch("user-menu:navigation", "user-menu"); this.appEvents.on("dom:clean", this, "_cleanDom"); diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 994d45dfbd..cbf157d6cb 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -73,9 +73,10 @@ import { replaceFormatter } from "discourse/lib/utilities"; import { replaceTagRenderer } from "discourse/lib/render-tag"; import { setNewCategoryDefaultColors } from "discourse/routes/new-category"; import { addSearchResultsCallback } from "discourse/lib/search"; +import { addInSearchShortcut } from "discourse/widgets/search-menu-results"; // If you add any methods to the API ensure you bump up this number -const PLUGIN_API_VERSION = "0.11.5"; +const PLUGIN_API_VERSION = "0.11.6"; class PluginApi { constructor(version, container) { @@ -1295,6 +1296,18 @@ class PluginApi { addSearchResultsCallback(callback) { addSearchResultsCallback(callback); } + + /** + * Add a in: shortcut to search menu panel. + * + * ``` + * addInSearchShortcut("in:assigned"); + * ``` + * + */ + addInSearchShortcut(value) { + addInSearchShortcut(value); + } } // from http://stackoverflow.com/questions/6832596/how-to-compare-software-version-number-using-js-only-number diff --git a/app/assets/javascripts/discourse/app/lib/search.js b/app/assets/javascripts/discourse/app/lib/search.js index b2de359e91..a53769b512 100644 --- a/app/assets/javascripts/discourse/app/lib/search.js +++ b/app/assets/javascripts/discourse/app/lib/search.js @@ -206,53 +206,30 @@ export function isValidSearchTerm(searchTerm, siteSettings) { } } -export function applySearchAutocomplete( - $input, - siteSettings, - appEvents, - options -) { - const afterComplete = function () { - if (appEvents) { - appEvents.trigger("search-autocomplete:after-complete"); - } - }; - +export function applySearchAutocomplete($input, siteSettings) { $input.autocomplete( - deepMerge( - { - template: findRawTemplate("category-tag-autocomplete"), - key: "#", - width: "100%", - treatAsTextarea: true, - autoSelectFirstSuggestion: false, - transformComplete(obj) { - return obj.text; - }, - dataSource(term) { - return searchCategoryTag(term, siteSettings); - }, - afterComplete, - }, - options - ) + deepMerge({ + template: findRawTemplate("category-tag-autocomplete"), + key: "#", + width: "100%", + treatAsTextarea: true, + autoSelectFirstSuggestion: false, + transformComplete: (obj) => obj.text, + dataSource: (term) => searchCategoryTag(term, siteSettings), + }) ); if (siteSettings.enable_mentions) { $input.autocomplete( - deepMerge( - { - template: findRawTemplate("user-selector-autocomplete"), - key: "@", - width: "100%", - treatAsTextarea: true, - autoSelectFirstSuggestion: false, - transformComplete: (v) => v.username || v.name, - dataSource: (term) => userSearch({ term, includeGroups: true }), - afterComplete, - }, - options - ) + deepMerge({ + template: findRawTemplate("user-selector-autocomplete"), + key: "@", + width: "100%", + treatAsTextarea: true, + autoSelectFirstSuggestion: false, + transformComplete: (v) => v.username || v.name, + dataSource: (term) => userSearch({ term, includeGroups: true }), + }) ); } } diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index 784d5265db..0c292dea95 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -2,7 +2,6 @@ import DiscourseURL, { userPath } from "discourse/lib/url"; import I18n from "I18n"; import { addExtraUserClasses } from "discourse/helpers/user-avatar"; import { ajax } from "discourse/lib/ajax"; -import { applySearchAutocomplete } from "discourse/lib/search"; import { avatarImg } from "discourse/widgets/post"; import { createWidget } from "discourse/widgets/widget"; import { get } from "@ember/object"; @@ -484,17 +483,9 @@ export default createWidget("header", { if (this.state.searchVisible) { schedule("afterRender", () => { - const $searchInput = $("#search-term"); - $searchInput.focus().select(); - - applySearchAutocomplete( - $searchInput, - this.siteSettings, - this.appEvents, - { - appendSelector: ".menu-panel", - } - ); + const searchInput = document.querySelector("#search-term"); + searchInput.focus(); + searchInput.select(); }); } }, diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu-controls.js b/app/assets/javascripts/discourse/app/widgets/search-menu-controls.js index dd2f485467..dc329df3e2 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu-controls.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu-controls.js @@ -13,10 +13,6 @@ createWidget("search-term", { return { afterAutocomplete: false }; }, - searchAutocompleteAfterComplete() { - this.state.afterAutocomplete = true; - }, - buildAttributes(attrs) { return { type: "text", @@ -28,12 +24,8 @@ createWidget("search-term", { }, keyUp(e) { - if (e.which === 13) { - if (this.state.afterAutocomplete) { - this.state.afterAutocomplete = false; - } else { - return this.sendWidgetAction("fullSearch"); - } + if (e.which === 13 && !this.state.afterAutocomplete) { + return this.sendWidgetAction("fullSearch"); } const val = this.attrs.value; diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js index 3a5885d947..5ef9d058c1 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js @@ -10,6 +10,33 @@ import highlightSearch from "discourse/lib/highlight-search"; import { iconNode } from "discourse-common/lib/icon-library"; import renderTag from "discourse/lib/render-tag"; +const inSearchShortcuts = [ + "in:title", + "in:personal", + "in:seen", + "in:likes", + "in:bookmarks", + "in:created", +]; +const statusSearchShortcuts = [ + "status:open", + "status:closed", + "status:public", + "status:noreplies", +]; +const orderSearchShortcuts = [ + "order:latest", + "order:views", + "order:likes", + "order:latest_topic", +]; + +export function addInSearchShortcut(value) { + if (inSearchShortcuts.indexOf(value) === -1) { + inSearchShortcuts.push(value); + } +} + class Highlighted extends RawHtml { constructor(html, term) { super({ html: `${html}` }); @@ -207,6 +234,14 @@ createWidget("search-menu-results", { tagName: "div.results", html(attrs) { + if (attrs.suggestionKeyword) { + return this.attach("search-menu-assistant", { + fullTerm: attrs.term, + suggestionKeyword: attrs.suggestionKeyword, + results: attrs.suggestionResults || [], + }); + } + if (attrs.invalidTerm) { return h("div.no-results", I18n.t("search.too_short")); } @@ -320,3 +355,134 @@ createWidget("search-menu-results", { return content; }, }); + +createWidget("search-menu-assistant", { + tagName: "ul.search-menu-assistant", + + html(attrs) { + if (this.siteSettings.tagging_enabled) { + addInSearchShortcut("in:tagged"); + } + + const content = []; + const { fullTerm, suggestionKeyword } = attrs; + const prefix = fullTerm.split(suggestionKeyword)[0].trim() || null; + + switch (suggestionKeyword) { + case "#": + attrs.results.map((category) => { + const slug = prefix + ? `${prefix} #${category.slug} ` + : `#${category.slug} `; + + content.push( + this.attach("search-menu-assistant-item", { + prefix: prefix, + category, + slug, + }) + ); + }); + break; + case "@": + attrs.results.map((user) => { + const slug = prefix + ? `${prefix} @${user.username} ` + : `@${user.username} `; + + content.push( + this.attach("search-menu-assistant-item", { + prefix: prefix, + user, + slug, + }) + ); + }); + break; + case "in:": + inSearchShortcuts.map((item) => { + const slug = prefix ? `${prefix} ${item} ` : item; + content.push(this.attach("search-menu-assistant-item", { slug })); + }); + break; + case "status:": + statusSearchShortcuts.map((item) => { + const slug = prefix ? `${prefix} ${item} ` : item; + content.push(this.attach("search-menu-assistant-item", { slug })); + }); + break; + case "order:": + orderSearchShortcuts.map((item) => { + const slug = prefix ? `${prefix} ${item} ` : item; + content.push(this.attach("search-menu-assistant-item", { slug })); + }); + break; + } + + return content; + }, +}); + +createWidget("search-menu-assistant-item", { + tagName: "li.search-menu-assistant-item", + + html(attrs) { + if (attrs.category) { + return h( + "a.widget-link.search-link", + { + attributes: { + href: attrs.category.url, + }, + }, + [ + h("span.search-item-prefix", attrs.prefix), + this.attach("category-link", { + category: attrs.category, + allowUncategorized: true, + }), + ] + ); + } else if (attrs.user) { + const userResult = [ + avatarImg("small", { + template: attrs.user.avatar_template, + username: attrs.user.username, + }), + h("span.username", formatUsername(attrs.user.username)), + ]; + + return h( + "a.widget-link.search-link", + { + attributes: { + href: "#", + }, + }, + [ + h("span.search-item-prefix", attrs.prefix), + h("span.search-item-user", userResult), + ] + ); + } else { + return h( + "a.widget-link.search-link", + { + attributes: { + href: "#", + }, + }, + h("span.search-item-slug", attrs.slug) + ); + } + }, + + click(e) { + const searchInput = document.querySelector("#search-term"); + searchInput.value = this.attrs.slug; + searchInput.focus(); + this.sendWidgetAction("triggerAutocomplete", this.attrs.slug); + e.preventDefault(); + return false; + }, +}); diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu.js b/app/assets/javascripts/discourse/app/widgets/search-menu.js index 246aeb4626..6514e9a1d4 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu.js @@ -1,4 +1,5 @@ import { isValidSearchTerm, searchForTerm } from "discourse/lib/search"; +import Category from "discourse/models/category"; import DiscourseURL from "discourse/lib/url"; import { createWidget } from "discourse/widgets/widget"; import discourseDebounce from "discourse-common/lib/debounce"; @@ -6,8 +7,15 @@ import { get } from "@ember/object"; import getURL from "discourse-common/lib/get-url"; import { h } from "virtual-dom"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import userSearch from "discourse/lib/user-search"; + +const CATEGORY_SLUG_REGEXP = /(\#[a-zA-Z0-9\-:]*)$/gi; +// The backend user search query returns zero results for a term-free search +// so the regexp below only matches @ followed by a valid character +const USERNAME_REGEXP = /(\@[a-zA-Z0-9\-\_]+)$/gi; const searchData = {}; +const suggestionTriggers = ["in:", "status:", "order:"]; export function initSearchData() { searchData.loading = false; @@ -17,6 +25,7 @@ export function initSearchData() { searchData.typeFilter = null; searchData.invalidTerm = false; searchData.topicId = null; + searchData.afterAutocomplete = false; } initSearchData(); @@ -39,6 +48,49 @@ const SearchHelper = { const { term, typeFilter, contextEnabled } = searchData; const searchContext = contextEnabled ? widget.searchContext() : null; const fullSearchUrl = widget.fullSearchUrl(); + const matchSuggestions = this.matchesSuggestions(); + + if (matchSuggestions) { + searchData.noResults = true; + searchData.results = []; + searchData.loading = false; + + if (typeof matchSuggestions === "string") { + searchData.suggestionKeyword = matchSuggestions; + widget.scheduleRerender(); + return; + } else { + if (matchSuggestions.type === "category") { + const categorySearchTerm = matchSuggestions.categoriesMatch[0].replace( + "#", + "" + ); + + searchData.suggestionResults = Category.search(categorySearchTerm); + searchData.suggestionKeyword = "#"; + widget.scheduleRerender(); + return; + } + if (matchSuggestions.type === "username") { + userSearch({ + term: matchSuggestions.usernamesMatch[0], + includeGroups: true, + }).then((result) => { + if (result?.users.length > 0) { + searchData.suggestionResults = result.users; + searchData.suggestionKeyword = "@"; + } else { + searchData.noResults = true; + searchData.suggestionKeyword = false; + } + widget.scheduleRerender(); + }); + return; + } + } + } + + searchData.suggestionKeyword = false; if (!isValidSearchTerm(term, widget.siteSettings)) { searchData.noResults = true; @@ -73,10 +125,38 @@ const SearchHelper = { .catch(popupAjaxError) .finally(() => { searchData.loading = false; + searchData.afterAutocomplete = false; widget.scheduleRerender(); }); } }, + + matchesSuggestions() { + if (searchData.term === undefined) { + return false; + } + + const simpleSuggestion = suggestionTriggers.find( + (mod) => searchData.term === mod || searchData.term.endsWith(` ${mod}`) + ); + + if (simpleSuggestion) { + return simpleSuggestion; + } + + const categoriesMatch = searchData.term.match(CATEGORY_SLUG_REGEXP); + + if (categoriesMatch) { + return { type: "category", categoriesMatch }; + } + + const usernamesMatch = searchData.term.match(USERNAME_REGEXP); + if (usernamesMatch) { + return { type: "username", usernamesMatch }; + } + + return false; + }, }; export default createWidget("search-menu", { @@ -132,10 +212,14 @@ export default createWidget("search-menu", { }, panelContents() { - const contextEnabled = searchData.contextEnabled; + const { contextEnabled, afterAutocomplete } = searchData; let searchInput = [ - this.attach("search-term", { value: searchData.term, contextEnabled }), + this.attach( + "search-term", + { value: searchData.term, contextEnabled }, + { state: { afterAutocomplete } } + ), ]; if (searchData.term && searchData.loading) { searchInput.push(h("div.searching", h("div.spinner"))); @@ -157,6 +241,8 @@ export default createWidget("search-menu", { results: searchData.results, invalidTerm: searchData.invalidTerm, searchContextEnabled: searchData.contextEnabled, + suggestionKeyword: searchData.suggestionKeyword, + suggestionResults: searchData.suggestionResults, }) ); } @@ -211,7 +297,7 @@ export default createWidget("search-menu", { return false; } - if (searchData.loading || searchData.noResults) { + if (searchData.loading) { return; } @@ -306,6 +392,11 @@ export default createWidget("search-menu", { this.triggerSearch(); }, + triggerAutocomplete(term) { + searchData.afterAutocomplete = true; + this.searchTermChanged(term); + }, + fullSearch() { if (!isValidSearchTerm(searchData.term, this.siteSettings)) { return; diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-test.js index c759a1aa3d..a61257e3c3 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/search-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/search-test.js @@ -2,6 +2,7 @@ import { acceptance, count, exists, + query, queryAll, } from "discourse/tests/helpers/qunit-helpers"; import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers"; @@ -247,3 +248,57 @@ acceptance("Search - with tagging enabled", function (needs) { assert.equal(tags, "dev slow"); }); }); + +acceptance("Search - assistant", function (needs) { + needs.user(); + + test("shows category shortcuts when typing #", async function (assert) { + await visit("/"); + + await click("#search-button"); + + await fillIn("#search-term", "#"); + await triggerKeyEvent("#search-term", "keyup", 51); + + const firstCategory = + ".search-menu .results ul.search-menu-assistant .search-link"; + assert.ok(exists(query(firstCategory))); + + const firstResultSlug = query( + `${firstCategory} .category-name` + ).innerText.trim(); + + await click(firstCategory); + assert.equal(query("#search-term").value, `#${firstResultSlug} `); + + await fillIn("#search-term", "sam #"); + await triggerKeyEvent("#search-term", "keyup", 51); + + assert.ok(exists(query(firstCategory))); + assert.equal( + query( + ".search-menu .results ul.search-menu-assistant .search-item-prefix" + ).innerText, + "sam" + ); + + await click(firstCategory); + assert.equal(query("#search-term").value, `sam #${firstResultSlug} `); + }); + + test("shows in: shortcuts", async function (assert) { + await visit("/"); + await click("#search-button"); + + const firstTarget = + ".search-menu .results ul.search-menu-assistant .search-link .search-item-slug"; + + await fillIn("#search-term", "in:"); + await triggerKeyEvent("#search-term", "keyup", 51); + assert.equal(query(firstTarget).innerText, "in:title"); + + await fillIn("#search-term", "sam in:"); + await triggerKeyEvent("#search-term", "keyup", 51); + assert.equal(query(firstTarget).innerText, "sam in:title"); + }); +}); diff --git a/app/assets/stylesheets/common/base/search-menu.scss b/app/assets/stylesheets/common/base/search-menu.scss index 883318421a..b6c6926f22 100644 --- a/app/assets/stylesheets/common/base/search-menu.scss +++ b/app/assets/stylesheets/common/base/search-menu.scss @@ -251,6 +251,23 @@ } } } + + .search-menu-assistant { + min-width: 100%; + margin-top: -1em; + + .search-menu-assistant-item { + > span { + vertical-align: baseline; + display: inline-block; + } + } + + .search-item-user .avatar, + .search-item-prefix { + margin-right: 0.5em; + } + } } .searching { From 50b3d7708e6af60e7d08fb147c0da67b1337131e Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 16 Jul 2021 15:16:06 +0100 Subject: [PATCH 12/77] DEV: Update discourse_connect_overrides_ setting copy --- config/locales/server.en.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3ac856726c..1ae9ed99e9 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1699,11 +1699,11 @@ en: auth_overrides_email: "Overrides local email with external site email on every login, and prevent local changes. Applies to all authentication providers. (WARNING: discrepancies can occur due to normalization of local emails)" auth_overrides_username: "Overrides local username with external site username on every login, and prevent local changes. Applies to all authentication providers. (WARNING: discrepancies can occur due to differences in username length/requirements)" auth_overrides_name: "Overrides local full name with external site full name on every login, and prevent local changes. Applies to all authentication providers." - discourse_connect_overrides_avatar: "Overrides user avatar with external site avatar from DiscourseConnect payload. If enabled, users will not be allowed to upload avatars on Discourse." - discourse_connect_overrides_location: "Overrides user location with external location from DiscourseConnect payload and prevent local changes." - discourse_connect_overrides_website: "Overrides user website with external location from DiscourseConnect payload and prevent local changes." - discourse_connect_overrides_profile_background: "Overrides user profile background with external site avatar from DiscourseConnect payload." - discourse_connect_overrides_card_background: "Overrides user card background with external site avatar from DiscourseConnect payload." + discourse_connect_overrides_avatar: "Overrides user avatar with value from DiscourseConnect payload. If enabled, users will not be allowed to upload avatars on Discourse." + discourse_connect_overrides_location: "Overrides user location with value from DiscourseConnect payload and prevent local changes." + discourse_connect_overrides_website: "Overrides user website with value from DiscourseConnect payload and prevent local changes." + discourse_connect_overrides_profile_background: "Overrides user profile background with value from DiscourseConnect payload." + discourse_connect_overrides_card_background: "Overrides user card background with value from DiscourseConnect payload." discourse_connect_not_approved_url: "Redirect unapproved DiscourseConnect accounts to this URL" discourse_connect_allows_all_return_paths: "Do not restrict the domain for return_paths provided by DiscourseConnect (by default return path must be on current site)" From 422fa1b1d864c9664c6400f327b139d2e628f30c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 16 Jul 2021 15:16:30 +0100 Subject: [PATCH 13/77] FIX: Use correct setting for DiscourseConnect card background overrides --- app/models/discourse_single_sign_on.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index 4abfc61a4a..7be3b50537 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -342,7 +342,7 @@ class DiscourseSingleSignOn < SingleSignOn if card_background_url.present? card_background_missing = user.user_profile.card_background_upload.blank? || Upload.get_from_url(user.user_profile.card_background_upload.url).blank? - if card_background_missing || SiteSetting.discourse_connect_overrides_profile_background + if card_background_missing || SiteSetting.discourse_connect_overrides_card_background card_background_changed = sso_record.external_card_background_url != card_background_url if card_background_changed || card_background_missing Jobs.enqueue(:download_profile_background_from_url, From 9b15affaae020f084271b4e74eaba5a02b53ed6c Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Fri, 16 Jul 2021 11:10:35 -0500 Subject: [PATCH 14/77] DEV: Plugin outlet in topic-status component (#13763) --- .../javascripts/discourse/app/components/topic-status.js | 3 +-- .../discourse/app/templates/components/topic-status.hbs | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/topic-status.js b/app/assets/javascripts/discourse/app/components/topic-status.js index a370849c8c..0cea69125b 100644 --- a/app/assets/javascripts/discourse/app/components/topic-status.js +++ b/app/assets/javascripts/discourse/app/components/topic-status.js @@ -11,9 +11,8 @@ export default Component.extend({ if (this.canAct && $(e.target).hasClass("d-icon-thumbtack")) { const topic = this.topic; topic.get("pinned") ? topic.clearPin() : topic.rePin(); + return false; } - - return false; }, @discourseComputed("disableActions") diff --git a/app/assets/javascripts/discourse/app/templates/components/topic-status.hbs b/app/assets/javascripts/discourse/app/templates/components/topic-status.hbs index 23761e150f..1852bcbf40 100644 --- a/app/assets/javascripts/discourse/app/templates/components/topic-status.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/topic-status.hbs @@ -30,3 +30,4 @@ {{~#if topicInvisible~}} {{invisibleIcon}} {{~/if~}} +{{plugin-outlet name="after-topic-status" tagName="" args=(hash topic=topic)}} From 079d2af55fbb45b34b1c8ed5b82061d525266b17 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Fri, 16 Jul 2021 19:57:12 +0300 Subject: [PATCH 15/77] FIX: Clear stale status of reloaded reviewables (#13750) * FIX: Clear stale status of reloaded reviewables Navigating away from and back to the reviewables reloaded Reviewable records, but did not clear the "stale" attribute. * FEATURE: Show user who last acted on reviewable When a user acts on a reviewable, all other clients are notified and a generic "reviewable was resolved by someone" notice was shown instead of the buttons. There is no need to keep secret the username of the acting user. --- .../discourse/app/components/reviewable-item.js | 6 +++--- .../javascripts/discourse/app/routes/review-index.js | 3 ++- .../app/templates/components/reviewable-item.hbs | 4 ++-- .../javascripts/discourse/tests/acceptance/review-test.js | 8 +++++++- app/jobs/regular/notify_reviewable.rb | 5 ++++- app/models/reviewable.rb | 3 ++- config/locales/client.en.yml | 2 +- 7 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/reviewable-item.js b/app/assets/javascripts/discourse/app/components/reviewable-item.js index 23071cac96..d7173eb3ec 100644 --- a/app/assets/javascripts/discourse/app/components/reviewable-item.js +++ b/app/assets/javascripts/discourse/app/components/reviewable-item.js @@ -29,14 +29,14 @@ export default Component.extend({ @discourseComputed( "reviewable.type", - "reviewable.stale", + "reviewable.last_performing_username", "siteSettings.blur_tl0_flagged_posts_media", "reviewable.target_created_by_trust_level" ) - customClasses(type, stale, blurEnabled, trustLevel) { + customClasses(type, lastPerformingUsername, blurEnabled, trustLevel) { let classes = type.dasherize(); - if (stale) { + if (lastPerformingUsername) { classes = `${classes} reviewable-stale`; } diff --git a/app/assets/javascripts/discourse/app/routes/review-index.js b/app/assets/javascripts/discourse/app/routes/review-index.js index 187c91c953..a19b0f9952 100644 --- a/app/assets/javascripts/discourse/app/routes/review-index.js +++ b/app/assets/javascripts/discourse/app/routes/review-index.js @@ -39,6 +39,8 @@ export default DiscourseRoute.extend({ sort_order: meta.sort_order, additionalFilters: meta.additional_filters || {}, }); + + controller.reviewables.setEach("last_performing_username", null); }, activate() { @@ -62,7 +64,6 @@ export default DiscourseRoute.extend({ const updates = data.updates[reviewable.id]; if (updates) { reviewable.setProperties(updates); - reviewable.set("stale", true); } }); } diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs index 9288db16d2..df7a072379 100644 --- a/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-item.hbs @@ -44,8 +44,8 @@ {{/if}}
- {{#if reviewable.stale}} -
{{i18n "review.stale_help"}}
+ {{#if reviewable.last_performing_username}} +
{{html-safe (i18n "review.stale_help" username=reviewable.last_performing_username)}}
{{else}} {{#if claimEnabled}}
diff --git a/app/assets/javascripts/discourse/tests/acceptance/review-test.js b/app/assets/javascripts/discourse/tests/acceptance/review-test.js index 522fe2c189..23c7b64c13 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/review-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/review-test.js @@ -197,7 +197,7 @@ acceptance("Review", function (needs) { publishToMessageBus("/reviewable_counts", { review_count: 1, updates: { - 1234: { status: 1 }, + 1234: { last_performing_username: "foo", status: 1 }, }, }); @@ -206,5 +206,11 @@ acceptance("Review", function (needs) { assert.ok(reviewable.className.includes("reviewable-stale")); assert.equal(count("[data-reviewable-id=1234] .status .approved"), 1); assert.equal(count(".stale-help"), 1); + assert.ok(query(".stale-help").innerText.includes("foo")); + + await visit("/"); + await visit("/review"); // reload review + + assert.equal(count(".stale-help"), 0); }); }); diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb index 3205129333..9c3e729fe2 100644 --- a/app/jobs/regular/notify_reviewable.rb +++ b/app/jobs/regular/notify_reviewable.rb @@ -19,7 +19,10 @@ class Jobs::NotifyReviewable < ::Jobs::Base if args[:updated_reviewable_ids].present? Reviewable.where(id: args[:updated_reviewable_ids]).each do |r| - payload = { status: r.status } + payload = { + last_performing_username: args[:performing_username], + status: r.status + } all_updates[:admins][r.id] = payload all_updates[:moderators][r.id] = payload if r.reviewable_by_moderator? diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index e8998a1d14..bff1c63064 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -371,7 +371,8 @@ class Reviewable < ActiveRecord::Base Jobs.enqueue( :notify_reviewable, reviewable_id: self.id, - updated_reviewable_ids: result.remove_reviewable_ids, + performing_username: performed_by.username, + updated_reviewable_ids: result.remove_reviewable_ids ) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 81484815c4..96af2e71dd 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -426,7 +426,7 @@ en: type_bonus: name: "type bonus" title: "Certain reviewable types can be assigned a bonus by staff to make them a higher priority." - stale_help: "This reviewable has been resolved by someone else." + stale_help: "This reviewable has been resolved by %{username}." claim_help: optional: "You can claim this item to prevent others from reviewing it." required: "You must claim items before you can review them." From 366238bb81145f846c2e913a33a526fadd417a04 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Fri, 16 Jul 2021 14:19:59 -0300 Subject: [PATCH 16/77] FIX: Disable the post submit button during image processing properly (#13765) There was a UI bug when submitting multiple files in the same batch. We would remove the disabled status of the submit button after the previous file was sucesfully uploaded and the next one was still mid optimization. Reported at https://meta.discourse.org/t/-/194841/15?u=falco --- .../discourse/app/components/composer-editor.js | 17 ++++++++++------- .../discourse/app/controllers/composer.js | 2 +- .../discourse/app/templates/composer.hbs | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index ae3addd81a..ce73e3b597 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -652,7 +652,6 @@ export default Component.extend({ this.setProperties({ uploadProgress: 0, isUploading: false, - isProcessingUpload: false, isCancellable: false, }); } @@ -683,6 +682,14 @@ export default Component.extend({ }); $element + .on("fileuploadprocessstart", () => { + this.setProperties({ + uploadProgress: 0, + isUploading: true, + isProcessingUpload: true, + isCancellable: false, + }); + }) .on("fileuploadprocess", (e, data) => { this.appEvents.trigger( "composer:insert-text", @@ -690,12 +697,6 @@ export default Component.extend({ filename: data.files[data.index].name, })}]()\n` ); - this.setProperties({ - uploadProgress: 0, - isUploading: true, - isProcessingUpload: true, - isCancellable: false, - }); }) .on("fileuploadprocessalways", (e, data) => { this.appEvents.trigger( @@ -705,6 +706,8 @@ export default Component.extend({ })}]()\n`, "" ); + }) + .on("fileuploadprocessstop", () => { this.setProperties({ uploadProgress: 0, isUploading: false, diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js index 9443a74907..e9fa6a2c80 100644 --- a/app/assets/javascripts/discourse/app/controllers/composer.js +++ b/app/assets/javascripts/discourse/app/controllers/composer.js @@ -663,7 +663,7 @@ export default Controller.extend({ }, }, - disableSubmit: or("model.loading", "isUploading"), + disableSubmit: or("model.loading", "isUploading", "isProcessingUpload"), save(force, options = {}) { if (this.disableSubmit) { diff --git a/app/assets/javascripts/discourse/app/templates/composer.hbs b/app/assets/javascripts/discourse/app/templates/composer.hbs index 6cca5bcf1e..b74974e784 100644 --- a/app/assets/javascripts/discourse/app/templates/composer.hbs +++ b/app/assets/javascripts/discourse/app/templates/composer.hbs @@ -167,7 +167,7 @@ {{/if}} {{/if}} - {{#if isUploading}} + {{#if (or isUploading isProcessingUpload)}}
{{#if isProcessingUpload}} {{loading-spinner size="small"}}{{i18n "upload_selector.processing"}} From 27b97e4f648da1587e97c5f661da145e12a27bea Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Fri, 16 Jul 2021 21:50:50 +0400 Subject: [PATCH 17/77] DEV: add pick-files-button component (#13764) * DEV: add pick-files-button component * Scope querySelector to the component, add removeEventListener, fix formatting --- .../app/components/pick-files-button.js | 106 ++++++++++++++++++ .../components/pick-files-button.hbs | 6 + .../components/pick-files-button-tests.js | 78 +++++++++++++ .../stylesheets/common/components/_index.scss | 1 + .../common/components/pick-files-button.scss | 5 + config/locales/client.en.yml | 3 + 6 files changed, 199 insertions(+) create mode 100644 app/assets/javascripts/discourse/app/components/pick-files-button.js create mode 100644 app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs create mode 100644 app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js create mode 100644 app/assets/stylesheets/common/components/pick-files-button.scss diff --git a/app/assets/javascripts/discourse/app/components/pick-files-button.js b/app/assets/javascripts/discourse/app/components/pick-files-button.js new file mode 100644 index 0000000000..b4752881d2 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/pick-files-button.js @@ -0,0 +1,106 @@ +import Component from "@ember/component"; +import { action } from "@ember/object"; +import { empty } from "@ember/object/computed"; +import { bind, default as computed } from "discourse-common/utils/decorators"; +import I18n from "I18n"; + +export default Component.extend({ + classNames: ["pick-files-button"], + acceptedFileTypes: null, + acceptAnyFile: empty("acceptedFileTypes"), + + didInsertElement() { + this._super(...arguments); + const fileInput = this.element.querySelector("input"); + this.set("fileInput", fileInput); + fileInput.addEventListener("change", this.onChange, false); + }, + + willDestroyElement() { + this._super(...arguments); + this.fileInput.removeEventListener("change", this.onChange); + }, + + @bind + onChange() { + const files = this.fileInput.files; + this._filesPicked(files); + }, + + @computed + acceptedFileTypesString() { + if (!this.acceptedFileTypes) { + return null; + } + + return this.acceptedFileTypes.join(","); + }, + + @computed + acceptedExtensions() { + if (!this.acceptedFileTypes) { + return null; + } + + return this.acceptedFileTypes + .filter((type) => type.startsWith(".")) + .map((type) => type.substring(1)); + }, + + @computed + acceptedMimeTypes() { + if (!this.acceptedFileTypes) { + return null; + } + + return this.acceptedFileTypes.filter((type) => !type.startsWith(".")); + }, + + @action + openSystemFilePicker() { + this.fileInput.click(); + }, + + _filesPicked(files) { + if (!files || !files.length) { + return; + } + + if (!this._haveAcceptedTypes(files)) { + const message = I18n.t("pick_files_button.unsupported_file_picked", { + types: this.acceptedFileTypesString, + }); + bootbox.alert(message); + return; + } + this.onFilesPicked(files); + }, + + _haveAcceptedTypes(files) { + for (const file of files) { + if ( + !(this._hasAcceptedExtension(file) && this._hasAcceptedMimeType(file)) + ) { + return false; + } + } + return true; + }, + + _hasAcceptedExtension(file) { + const extension = this._fileExtension(file.name); + return ( + !this.acceptedExtensions || this.acceptedExtensions.includes(extension) + ); + }, + + _hasAcceptedMimeType(file) { + return ( + !this.acceptedMimeTypes || this.acceptedMimeTypes.includes(file.type) + ); + }, + + _fileExtension(fileName) { + return fileName.split(".").pop(); + }, +}); diff --git a/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs b/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs new file mode 100644 index 0000000000..409a3becbd --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs @@ -0,0 +1,6 @@ +{{d-button action=(action "openSystemFilePicker") label=label icon=icon}} +{{#if acceptAnyFile}} + +{{else}} + +{{/if}} diff --git a/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js b/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js new file mode 100644 index 0000000000..4c82e9d510 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js @@ -0,0 +1,78 @@ +import componentTest, { + setupRenderingTest, +} from "discourse/tests/helpers/component-test"; +import { discourseModule } from "discourse/tests/helpers/qunit-helpers"; +import hbs from "htmlbars-inline-precompile"; +import { triggerEvent } from "@ember/test-helpers"; +import sinon from "sinon"; + +function createBlob(mimeType, extension) { + const blob = new Blob(["content"], { + type: mimeType, + }); + blob.name = `filename${extension}`; + return blob; +} + +discourseModule( + "Integration | Component | pick-files-button", + function (hooks) { + setupRenderingTest(hooks); + + componentTest( + "it shows alert if a file with an unsupported extension was chosen", + { + skip: true, + template: hbs` + {{pick-files-button + acceptedFileTypes=this.acceptedFileTypes + onFilesChosen=this.onFilesChosen}}`, + + beforeEach() { + const expectedExtension = ".json"; + this.set("acceptedFileTypes", [expectedExtension]); + this.set("onFilesChosen", () => {}); + }, + + async test(assert) { + sinon.stub(bootbox, "alert"); + + const wrongExtension = ".txt"; + const file = createBlob("text/json", wrongExtension); + + await triggerEvent("input#file-input", "change", { files: [file] }); + + assert.ok(bootbox.alert.calledOnce); + }, + } + ); + + componentTest( + "it shows alert if a file with an unsupported MIME type was chosen", + { + skip: true, + template: hbs` + {{pick-files-button + acceptedFileTypes=this.acceptedFileTypes + onFilesChosen=this.onFilesChosen}}`, + + beforeEach() { + const expectedMimeType = "text/json"; + this.set("acceptedFileTypes", [expectedMimeType]); + this.set("onFilesChosen", () => {}); + }, + + async test(assert) { + sinon.stub(bootbox, "alert"); + + const wrongMimeType = "text/plain"; + const file = createBlob(wrongMimeType, ".json"); + + await triggerEvent("input#file-input", "change", { files: [file] }); + + assert.ok(bootbox.alert.calledOnce); + }, + } + ); + } +); diff --git a/app/assets/stylesheets/common/components/_index.scss b/app/assets/stylesheets/common/components/_index.scss index 57eb631bd2..02745f3cf0 100644 --- a/app/assets/stylesheets/common/components/_index.scss +++ b/app/assets/stylesheets/common/components/_index.scss @@ -17,6 +17,7 @@ @import "ignored-user-list"; @import "keyboard_shortcuts"; @import "navs"; +@import "pick-files-button"; @import "relative-time-picker"; @import "share-and-invite-modal"; @import "svg"; diff --git a/app/assets/stylesheets/common/components/pick-files-button.scss b/app/assets/stylesheets/common/components/pick-files-button.scss new file mode 100644 index 0000000000..c9945168f0 --- /dev/null +++ b/app/assets/stylesheets/common/components/pick-files-button.scss @@ -0,0 +1,5 @@ +.pick-files-button { + input[type="file"] { + display: none; + } +} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 96af2e71dd..86ec2b065c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3784,6 +3784,9 @@ en: leader: "leader" detailed_name: "%{level}: %{name}" + pick_files_button: + unsupported_file_picked: "You have picked an unsupported file. Supported file types – %{types}." + # This section is exported to the javascript for i18n in the admin section admin_js: type_to_filter: "type to filter..." From 216dc99f18ff077072a3c99b64bd0862dedc1973 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Fri, 16 Jul 2021 15:13:16 -0300 Subject: [PATCH 18/77] FIX: Media optimization setting was misnamed (#13766) --- .../discourse/app/services/media-optimization-worker.js | 2 +- config/locales/server.en.yml | 2 +- config/site_settings.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js index f108061ad7..29f567404a 100644 --- a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js +++ b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js @@ -43,7 +43,7 @@ export default class MediaOptimizationWorkerService extends Service { if ( file.size < this.siteSettings - .composer_media_optimization_image_kilobytes_optimization_threshold + .composer_media_optimization_image_bytes_optimization_threshold ) { return data; } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1ae9ed99e9..3504f46d88 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1827,7 +1827,7 @@ en: strip_image_metadata: "Strip image metadata." composer_media_optimization_image_enabled: "Enables client-side media optimization of uploaded image files." - composer_media_optimization_image_kilobytes_optimization_threshold: "Minimum image file size to trigger client-side optimization" + composer_media_optimization_image_bytes_optimization_threshold: "Minimum image file size to trigger client-side optimization" composer_media_optimization_image_resize_dimensions_threshold: "Minimum image width to trigger client-side resize" composer_media_optimization_image_resize_width_target: "Images with widths larger than `composer_media_optimization_image_dimensions_resize_threshold` will be resized to this width. Must be >= than `composer_media_optimization_image_dimensions_resize_threshold`." composer_media_optimization_image_encode_quality: "JPEG encode quality used in the re-encode process." diff --git a/config/site_settings.yml b/config/site_settings.yml index c9b83f777c..f9c6c80ca1 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1415,7 +1415,7 @@ files: composer_media_optimization_image_enabled: default: false client: true - composer_media_optimization_image_kilobytes_optimization_threshold: + composer_media_optimization_image_bytes_optimization_threshold: default: 524288 client: true composer_media_optimization_image_resize_dimensions_threshold: From 2fc0a3fd93b6c2833e8ae6e68c4b25be744aab62 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Fri, 16 Jul 2021 15:23:04 -0300 Subject: [PATCH 19/77] FEATURE: Enable client-side image optimization by default (#13724) --- 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 f9c6c80ca1..950cff638c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1413,7 +1413,7 @@ files: default: 100000 hidden: true composer_media_optimization_image_enabled: - default: false + default: true client: true composer_media_optimization_image_bytes_optimization_threshold: default: 524288 From e12b00eab74674443d09ecf1292be8c9a921a618 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Fri, 16 Jul 2021 15:25:49 -0300 Subject: [PATCH 20/77] FEATURE: Stop checking referer for embeds (#13756) Flips content_security_policy_frame_ancestors default to enabled, and removes HTTP_REFERER checks on embed requests, as the new referer privacy options made the check fragile. --- app/controllers/embed_controller.rb | 24 ++++++------------------ app/models/embeddable_host.rb | 2 ++ config/site_settings.yml | 2 +- spec/components/topic_retriever_spec.rb | 15 +++++++++++++-- spec/requests/embed_controller_spec.rb | 12 ++---------- 5 files changed, 24 insertions(+), 31 deletions(-) diff --git a/app/controllers/embed_controller.rb b/app/controllers/embed_controller.rb index 2a063adaed..490ff8c698 100644 --- a/app/controllers/embed_controller.rb +++ b/app/controllers/embed_controller.rb @@ -5,14 +5,12 @@ class EmbedController < ApplicationController skip_before_action :check_xhr, :preload_json, :verify_authenticity_token - before_action :ensure_embeddable, except: [ :info, :topics ] before_action :prepare_embeddable, except: [ :info ] before_action :ensure_api_request, only: [ :info ] layout 'embed' rescue_from Discourse::InvalidAccess do - response.headers.delete('X-Frame-Options') if current_user.try(:admin?) @setup_url = "#{Discourse.base_url}/admin/customize/embedding" @show_reason = true @@ -24,7 +22,6 @@ class EmbedController < ApplicationController def topics discourse_expires_in 1.minute - response.headers.delete('X-Frame-Options') unless SiteSetting.embed_topics_list? render 'embed_topics_error', status: 400 return @@ -73,6 +70,11 @@ class EmbedController < ApplicationController def comments embed_url = params[:embed_url] embed_username = params[:discourse_username] + embed_topic_id = params[:topic_id]&.to_i + + unless embed_topic_id || EmbeddableHost.url_allowed?(embed_url) + raise Discourse::InvalidAccess.new('invalid embed host') + end topic_id = nil if embed_url.present? @@ -147,6 +149,7 @@ class EmbedController < ApplicationController private def prepare_embeddable + response.headers.delete('X-Frame-Options') @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? @@ -158,19 +161,4 @@ class EmbedController < ApplicationController def ensure_api_request raise Discourse::InvalidAccess.new('api key not set') if !is_api? end - - def ensure_embeddable - if !(Rails.env.development? && current_user&.admin?) - referer = request.referer - - unless referer && EmbeddableHost.url_allowed?(referer) - raise Discourse::InvalidAccess.new('invalid referer host') - end - end - - response.headers.delete('X-Frame-Options') - rescue URI::Error - raise Discourse::InvalidAccess.new('invalid referer host') - end - end diff --git a/app/models/embeddable_host.rb b/app/models/embeddable_host.rb index e76dbce643..212348bc3d 100644 --- a/app/models/embeddable_host.rb +++ b/app/models/embeddable_host.rb @@ -44,6 +44,8 @@ class EmbeddableHost < ActiveRecord::Base end def self.url_allowed?(url) + return false if url.nil? + # Work around IFRAME reload on WebKit where the referer will be set to the Forum URL return true if url&.starts_with?(Discourse.base_url) && EmbeddableHost.exists? diff --git a/config/site_settings.yml b/config/site_settings.yml index 950cff638c..e9855cdf53 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1640,7 +1640,7 @@ security: content_security_policy_collect_reports: default: false content_security_policy_frame_ancestors: - default: false + default: true content_security_policy_script_src: type: simple_list default: "" diff --git a/spec/components/topic_retriever_spec.rb b/spec/components/topic_retriever_spec.rb index e14379f5cc..c16be0c987 100644 --- a/spec/components/topic_retriever_spec.rb +++ b/spec/components/topic_retriever_spec.rb @@ -36,9 +36,9 @@ describe TopicRetriever do end end - context "when host is not invalid" do + context "when host is valid" do before do - topic_retriever.stubs(:invalid_url?).returns(false) + Fabricate(:embeddable_host, host: 'http://eviltrout.com/') end context "when topics have been retrieved recently" do @@ -64,6 +64,17 @@ describe TopicRetriever do end end + context "when host is invalid" do + before do + Fabricate(:embeddable_host, host: 'http://not-eviltrout.com/') + end + + it "does not perform_retrieve" do + topic_retriever.expects(:perform_retrieve).never + topic_retriever.retrieve + end + end + it "works with URLs with whitespaces" do expect { TopicRetriever.new(" https://example.com ").retrieve } .not_to raise_error diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb index ff89614634..44b21a9c63 100644 --- a/spec/requests/embed_controller_spec.rb +++ b/spec/requests/embed_controller_spec.rb @@ -150,9 +150,9 @@ describe EmbedController do Jobs.run_immediately! end - it "raises an error with no referer" do + it "doesn't raises an error with no referer" do get '/embed/comments', params: { embed_url: embed_url } - expect(response.body).to match(I18n.t('embed.error')) + expect(response.body).not_to match(I18n.t('embed.error')) end it "includes CSS from embedded_scss field" do @@ -266,14 +266,6 @@ describe EmbedController do expect(response.body).to match('class="example"') end - - it "doesn't work with a made up host" do - get '/embed/comments', - params: { embed_url: embed_url }, - headers: { 'REFERER' => "http://codinghorror.com/invalid-url" } - - expect(response.body).to match(I18n.t('embed.error')) - end end context "CSP frame-ancestors enabled" do From b0f06b8ed09e49a5121948f2d91c7d9ff5d5f773 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 16 Jul 2021 14:50:40 -0400 Subject: [PATCH 21/77] FIX: don't allow category and tag tracking settings on staged users (#13688) Configuring staged users to watch categories and tags is a way to sign them up to get many emails. These emails may be unwanted and get marked as spam, hurting the site's email deliverability. Users can opt-in to email notifications by logging on to their account and configuring their own preferences. If staff need to be able to configure these preferences on behalf of staged users, the "allow changing staged user tracking" site setting can be enabled. Default is to not allow it. Co-authored-by: Alan Guo Xiang Tan --- .../discourse/app/controllers/preferences.js | 1 + .../discourse/app/templates/preferences.hbs | 14 +- .../tests/acceptance/preferences-test.js | 33 +++++ .../discourse/tests/fixtures/user-fixtures.js | 125 ++++++++++++++++++ app/serializers/user_serializer.rb | 5 + app/serializers/web_hook_user_serializer.rb | 1 + app/services/user_updater.rb | 16 ++- config/locales/server.en.yml | 2 + config/site_settings.yml | 2 + lib/guardian/user_guardian.rb | 4 + .../components/guardian/user_guardian_spec.rb | 37 ++++++ spec/services/user_updater_spec.rb | 36 ++++- 12 files changed, 262 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/preferences.js b/app/assets/javascripts/discourse/app/controllers/preferences.js index 1c0ee0801d..fa4ba1ee7d 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences.js @@ -1,2 +1,3 @@ import Controller from "@ember/controller"; + export default Controller.extend({}); diff --git a/app/assets/javascripts/discourse/app/templates/preferences.hbs b/app/assets/javascripts/discourse/app/templates/preferences.hbs index 45f82f56c9..19f90739eb 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences.hbs @@ -25,17 +25,19 @@ {{i18n "user.preferences_nav.notifications"}} {{/link-to}} - + {{#if model.can_change_tracking_preferences}} + + {{/if}} - {{#if siteSettings.tagging_enabled}} + {{#if (and model.can_change_tracking_preferences siteSettings.tagging_enabled)}}
+ {{#if showFilter}} +
+ {{input + class="filter-input" + placeholder=(i18n "admin.customize.theme.filter_placeholder") + autocomplete="discourse" + value=(mut filterTerm) + }} + {{d-icon "search"}} +
+ {{/if}} {{#if hasThemes}} {{#if hasActiveThemes}} {{#each activeThemes as |theme|}} diff --git a/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js index 375340ae6e..cb820a2160 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js @@ -6,26 +6,37 @@ import componentTest, { import { count, discourseModule, + exists, + query, queryAll, } from "discourse/tests/helpers/qunit-helpers"; import hbs from "htmlbars-inline-precompile"; +import { click, fillIn } from "@ember/test-helpers"; + +function createThemes(itemsCount, customAttributesCallback) { + return [...Array(itemsCount)].map((_, i) => { + const attrs = { name: `Theme ${i + 1}` }; + if (customAttributesCallback) { + Object.assign(attrs, customAttributesCallback(i + 1)); + } + return Theme.create(attrs); + }); +} discourseModule("Integration | Component | themes-list", function (hooks) { setupRenderingTest(hooks); componentTest("current tab is themes", { template: hbs`{{themes-list themes=themes components=components currentTab=currentTab}}`, beforeEach() { - this.themes = [1, 2, 3, 4, 5].map((num) => - Theme.create({ name: `Theme ${num}` }) - ); - this.components = [1, 2, 3, 4, 5].map((num) => - Theme.create({ - name: `Child ${num}`, + this.themes = createThemes(5); + this.components = createThemes(5, (n) => { + return { + name: `Child ${n}`, component: true, - parentThemes: [this.themes[num - 1]], + parentThemes: [this.themes[n - 1]], parent_themes: [1, 2, 3, 4, 5], - }) - ); + }; + }); this.setProperties({ themes: this.themes, components: this.components, @@ -94,17 +105,15 @@ discourseModule("Integration | Component | themes-list", function (hooks) { componentTest("current tab is components", { template: hbs`{{themes-list themes=themes components=components currentTab=currentTab}}`, beforeEach() { - this.themes = [1, 2, 3, 4, 5].map((num) => - Theme.create({ name: `Theme ${num}` }) - ); - this.components = [1, 2, 3, 4, 5].map((num) => - Theme.create({ - name: `Child ${num}`, + this.themes = createThemes(5); + this.components = createThemes(5, (n) => { + return { + name: `Child ${n}`, component: true, - parentThemes: [this.themes[num - 1]], + parentThemes: [this.themes[n - 1]], parent_themes: [1, 2, 3, 4, 5], - }) - ); + }; + }); this.setProperties({ themes: this.themes, components: this.components, @@ -144,4 +153,139 @@ discourseModule("Integration | Component | themes-list", function (hooks) { ); }, }); + + componentTest( + "themes filter is not visible when there are less than 10 themes", + { + template: hbs`{{themes-list themes=themes components=[] currentTab=currentTab}}`, + + beforeEach() { + const themes = createThemes(9); + this.setProperties({ + themes, + currentTab: THEMES, + }); + }, + async test(assert) { + assert.ok( + !exists(".themes-list-filter"), + "filter input not shown when we have fewer than 10 themes" + ); + }, + } + ); + + componentTest( + "themes filter keeps themes whose names include the filter term", + { + template: hbs`{{themes-list themes=themes components=[] currentTab=currentTab}}`, + + beforeEach() { + const themes = ["osama", "OsAmaa", "osAMA 1234"] + .map((name) => Theme.create({ name: `Theme ${name}` })) + .concat(createThemes(7)); + this.setProperties({ + themes, + currentTab: THEMES, + }); + }, + async test(assert) { + assert.ok(exists(".themes-list-filter")); + await fillIn(".themes-list-filter .filter-input", " oSAma "); + assert.deepEqual( + Array.from(queryAll(".themes-list-item .name")).map((node) => + node.textContent.trim() + ), + ["Theme osama", "Theme OsAmaa", "Theme osAMA 1234"], + "only themes whose names include the filter term are shown" + ); + }, + } + ); + + componentTest( + "switching between themes and components tabs keeps the filter visible only if both tabs have at least 10 items", + { + template: hbs`{{themes-list themes=themes components=components currentTab=currentTab}}`, + + beforeEach() { + const themes = createThemes(10, (n) => { + return { name: `Theme ${n}${n}` }; + }); + const components = createThemes(5, (n) => { + return { + name: `Component ${n}${n}`, + component: true, + parent_themes: [1], + parentThemes: [1], + }; + }); + this.setProperties({ + themes, + components, + currentTab: THEMES, + }); + }, + async test(assert) { + await fillIn(".themes-list-filter .filter-input", "11"); + assert.equal( + query(".themes-list-container").textContent.trim(), + "Theme 11", + "only 1 theme is shown" + ); + await click(".themes-list-header .components-tab"); + assert.ok( + !exists(".themes-list-filter"), + "filter input/term do not persist when we switch to the other" + + " tab because it has fewer than 10 items" + ); + assert.deepEqual( + Array.from(queryAll(".themes-list-item .name")).map((node) => + node.textContent.trim() + ), + [ + "Component 11", + "Component 22", + "Component 33", + "Component 44", + "Component 55", + ], + "all components are shown" + ); + + this.set( + "components", + this.components.concat( + createThemes(5, (n) => { + n += 5; + return { + name: `Component ${n}${n}`, + component: true, + parent_themes: [1], + parentThemes: [1], + }; + }) + ) + ); + assert.ok( + exists(".themes-list-filter"), + "filter is now shown for the components tab" + ); + + await fillIn(".themes-list-filter .filter-input", "66"); + assert.equal( + query(".themes-list-container").textContent.trim(), + "Component 66", + "only 1 component is shown" + ); + + await click(".themes-list-header .themes-tab"); + assert.equal( + query(".themes-list-container").textContent.trim(), + "Theme 66", + "filter term persisted between tabs because both have more than 10 items" + ); + }, + } + ); }); diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index 045235d3c1..4eeb081142 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -295,6 +295,37 @@ width: 100%; } } + + .themes-list-filter { + display: flex; + align-items: center; + position: sticky; + top: 0; + background: var(--secondary); + z-index: z("base"); + height: 3em; + + .d-icon { + position: absolute; + padding-left: 0.5em; + } + + .filter-input { + width: 100%; + height: 100%; + margin: 0; + border: 0; + padding-left: 2em; + + &:focus { + outline: 0; + + ~ .d-icon { + color: var(--tertiary-hover); + } + } + } + } } .theme.settings { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4eda0fe814..0a5974c9fe 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4230,6 +4230,7 @@ en: theme: "Theme" component: "Component" components: "Components" + filter_placeholder: "type to filter…" theme_name: "Theme name" component_name: "Component name" themes_intro: "Select an existing theme or install a new one to get started" From 8de89895761876a2f8f174afb46b4e4dffc09ef2 Mon Sep 17 00:00:00 2001 From: Kris Date: Sun, 18 Jul 2021 21:34:44 -0400 Subject: [PATCH 28/77] UX: consistent share modal & popup, refactoring (#13759) --- .../discourse/app/components/share-popup.js | 7 +- .../app/templates/components/share-popup.hbs | 53 ++-- .../app/templates/components/share-source.hbs | 8 +- .../app/templates/modal/create-invite.hbs | 4 +- .../app/templates/modal/share-topic.hbs | 81 ++--- .../tests/acceptance/share-topic-test.js | 2 +- .../discourse/tests/acceptance/topic-test.js | 4 +- app/assets/stylesheets/common/base/modal.scss | 160 ---------- .../stylesheets/common/base/share_link.scss | 288 +++++++++++++----- 9 files changed, 305 insertions(+), 302 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/share-popup.js b/app/assets/javascripts/discourse/app/components/share-popup.js index a2d9decc13..85e815ce50 100644 --- a/app/assets/javascripts/discourse/app/components/share-popup.js +++ b/app/assets/javascripts/discourse/app/components/share-popup.js @@ -49,8 +49,11 @@ export default Component.extend({ if (this.element) { const linkInput = this.element.querySelector("#share-link input"); linkInput.value = this.link; - linkInput.setSelectionRange(0, this.link.length); - linkInput.focus(); + if (!this.site.mobileView) { + // if the input is auto-focused on mobile, iOS requires two taps of the copy button + linkInput.setSelectionRange(0, this.link.length); + linkInput.focus(); + } } }, 200); }, diff --git a/app/assets/javascripts/discourse/app/templates/components/share-popup.hbs b/app/assets/javascripts/discourse/app/templates/components/share-popup.hbs index 695be0aeb5..23e32ea420 100644 --- a/app/assets/javascripts/discourse/app/templates/components/share-popup.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/share-popup.hbs @@ -4,13 +4,21 @@ {{#if date}} {{displayDate}} {{/if}} + + {{d-button + action=(action "close") + class="btn btn-flat close" + icon="times" + aria-label="share.close" + title="share.close" + }}
-
- + -
+