From 3d52f690b3e4241d0a46f1fb1941c5da3ca27c8d Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Mon, 28 Jan 2019 22:51:06 +0530 Subject: [PATCH 01/44] UX: Add post action text in non-JS topic view --- app/views/topics/show.html.erb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb index 138646bd43..456135d668 100644 --- a/app/views/topics/show.html.erb +++ b/app/views/topics/show.html.erb @@ -47,10 +47,16 @@
- <%= "(#{u.name})" if (SiteSetting.display_name_on_posts && SiteSetting.enable_names? && !u.name.blank?) %> - + <%= "(#{u.name})" if (SiteSetting.display_name_on_posts && SiteSetting.enable_names? && !u.name.blank?) %> + <% + who_username = post.custom_fields["action_code_who"] || "" + if post.action_code + %> + <%= t("js.action_codes.#{post.action_code}", when: "", who: who_username).html_safe %> + <% end %> + #<%= post.post_number %>
From 51fdf7a11d61d8913e4c6d38687c858e174ea9f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 28 Jan 2019 18:40:52 +0100 Subject: [PATCH 02/44] FIX: don't duplicate attachments --- lib/email/receiver.rb | 1 + spec/components/email/receiver_spec.rb | 29 +++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 7a5bc22efe..736626194d 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -939,6 +939,7 @@ module Email attachments << part if part.attachment? && is_whitelisted_attachment?(part) end + attachments.uniq! attachments end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index d70631665e..75ac46d6db 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -524,47 +524,52 @@ describe Email::Receiver do it "supports attachments" do SiteSetting.authorized_extensions = "txt|jpg" expect { process(:attached_txt_file) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to match(/]*>text\.txt<\/a>/) - expect(topic.posts.last.uploads.length).to eq 1 + post = topic.posts.last + expect(post.raw).to match(/\APlease find some text file attached\.\s+text\.txt<\/a> \(20 Bytes\)\z/) + expect(post.uploads.size).to eq 1 expect { process(:apple_mail_attachment) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to match /\s+Picture above\.\z/) + expect(post.uploads.size).to eq 1 end it "supports eml attachments" do SiteSetting.authorized_extensions = "eml" expect { process(:attached_eml_file) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to match(/]*>sample\.eml<\/a>/) - expect(topic.posts.last.uploads.length).to eq 1 + post = topic.posts.last + expect(post.raw).to match(/\APlease find the eml file attached\.\s+sample\.eml<\/a> \(193 Bytes\)\z/) + expect(post.uploads.size).to eq 1 end context "when attachment is rejected" do it "sends out the warning email" do expect { process(:attached_txt_file) }.to change { EmailLog.count }.by(1) expect(EmailLog.last.email_type).to eq("email_reject_attachment") - expect(topic.posts.last.uploads.length).to eq 0 + expect(topic.posts.last.uploads.size).to eq 0 end it "doesn't send out the warning email if sender is staged user" do user.update_columns(staged: true) expect { process(:attached_txt_file) }.not_to change { EmailLog.count } - expect(topic.posts.last.uploads.length).to eq 0 + expect(topic.posts.last.uploads.size).to eq 0 end it "creates the post with attachment missing message" do missing_attachment_regex = Regexp.escape(I18n.t('emails.incoming.missing_attachment', filename: "text.txt")) expect { process(:attached_txt_file) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to match(/#{missing_attachment_regex}/) - expect(topic.posts.last.uploads.length).to eq 0 + post = topic.posts.last + expect(post.raw).to match(/#{missing_attachment_regex}/) + expect(post.uploads.size).to eq 0 end end it "supports emails with just an attachment" do SiteSetting.authorized_extensions = "pdf" expect { process(:attached_pdf_file) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to match(/]*>discourse\.pdf<\/a>/) - expect(topic.posts.last.uploads.length).to eq 1 + post = topic.posts.last + expect(post.raw).to match(/\A\s+discourse\.pdf<\/a> \(64 KB\)\z/) + expect(post.uploads.size).to eq 1 end it "supports liking via email" do From 62a0f88e97007d2999c34ccbc5336d5c3651f7d8 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Tue, 29 Jan 2019 02:26:55 +0530 Subject: [PATCH 03/44] FIX: More link in topic page search shouldn't navigate to full page result --- app/assets/javascripts/discourse/lib/search.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 index 8fab105cfe..a3f47f7a12 100644 --- a/app/assets/javascripts/discourse/lib/search.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -80,7 +80,7 @@ export function translateResults(results, opts) { more: groupedSearchResult[`more_${name}`] }; - if (result.more && name === "posts" && opts.fullSearchUrl) { + if (result.more && componentName === "topic" && opts.fullSearchUrl) { result.more = false; result.moreUrl = opts.fullSearchUrl; } From f7deb52c90095eefe1822e4ce15d15ae8a8de2cb Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 28 Jan 2019 17:15:36 -0500 Subject: [PATCH 04/44] FIX: Class name for external-link on customize page --- .../javascripts/admin/templates/customize-themes-index.hbs | 2 +- app/assets/stylesheets/common/admin/customize.scss | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/templates/customize-themes-index.hbs b/app/assets/javascripts/admin/templates/customize-themes-index.hbs index 39bdfc7c85..565d0024db 100644 --- a/app/assets/javascripts/admin/templates/customize-themes-index.hbs +++ b/app/assets/javascripts/admin/templates/customize-themes-index.hbs @@ -4,7 +4,7 @@

{{I18n "admin.customize.theme.themes_intro"}}

{{#each externalResources as |resource|}} - + {{d-icon resource.icon}} {{I18n resource.key}} diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index a552dd0293..3da2b22e0c 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -208,7 +208,6 @@ } } .external-link { - display: inline-block; display: block; margin-bottom: 5px; } From 530eeba855414d098ef61b84c8d770076e73ef3f Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 28 Jan 2019 22:30:01 -0500 Subject: [PATCH 05/44] Add test for user drafts stream Followup to 84a3da4b18d6dd5c2a0940fe04b339ebc1605123 --- .../acceptance/user-drafts-stream-test.js.es6 | 12 ++++++++++++ test/javascripts/fixtures/drafts.js.es6 | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 test/javascripts/acceptance/user-drafts-stream-test.js.es6 diff --git a/test/javascripts/acceptance/user-drafts-stream-test.js.es6 b/test/javascripts/acceptance/user-drafts-stream-test.js.es6 new file mode 100644 index 0000000000..06f7d9f4d2 --- /dev/null +++ b/test/javascripts/acceptance/user-drafts-stream-test.js.es6 @@ -0,0 +1,12 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("User Drafts", { loggedIn: true }); + +QUnit.test("Stream", async assert => { + await visit("/u/eviltrout/activity/drafts"); + assert.ok(find(".user-stream-item").length === 3, "has drafts"); + + await click(".user-stream-item:last-child .remove-draft"); + assert.ok(find(".user-stream-item").length === 2, "draft removed, list length diminished by one"); + +}); diff --git a/test/javascripts/fixtures/drafts.js.es6 b/test/javascripts/fixtures/drafts.js.es6 index 8aaa88289c..f5f31958f6 100644 --- a/test/javascripts/fixtures/drafts.js.es6 +++ b/test/javascripts/fixtures/drafts.js.es6 @@ -43,7 +43,7 @@ export default { excerpt: "here goes a reply to a PM.", created_at: "2018-07-20T16:58:47.433Z", draft_key: "topic_93", - sequence: 1, + sequence: 0, draft_username: "eviltrout", avatar_template: "/user_avatar/localhost/eviltrout/{size}/2_1.png", data: From ee437ef53bf8b4ccec098817281045d406d57a3f Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 28 Jan 2019 22:47:11 -0500 Subject: [PATCH 06/44] Fix prettier offence --- test/javascripts/acceptance/user-drafts-stream-test.js.es6 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/javascripts/acceptance/user-drafts-stream-test.js.es6 b/test/javascripts/acceptance/user-drafts-stream-test.js.es6 index 06f7d9f4d2..f3fbcdf0ea 100644 --- a/test/javascripts/acceptance/user-drafts-stream-test.js.es6 +++ b/test/javascripts/acceptance/user-drafts-stream-test.js.es6 @@ -7,6 +7,8 @@ QUnit.test("Stream", async assert => { assert.ok(find(".user-stream-item").length === 3, "has drafts"); await click(".user-stream-item:last-child .remove-draft"); - assert.ok(find(".user-stream-item").length === 2, "draft removed, list length diminished by one"); - + assert.ok( + find(".user-stream-item").length === 2, + "draft removed, list length diminished by one" + ); }); From 82b7795f368e59817ca1b78f002dffdcbb1235f2 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 29 Jan 2019 11:49:42 +0530 Subject: [PATCH 07/44] Revert "don't run specs on code-review for now" This reverts commit cb493b66e0e333b5d08d526f53ebcc3811447709. --- lib/tasks/plugin.rake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/tasks/plugin.rake b/lib/tasks/plugin.rake index 600fac6e94..c77525038f 100644 --- a/lib/tasks/plugin.rake +++ b/lib/tasks/plugin.rake @@ -7,8 +7,7 @@ task 'plugin:install_all_official' do 'discourse-nginx-performance-report', 'lazyYT', 'poll', - 'discourse-calendar', - 'discourse-code-review' + 'discourse-calendar' ]) map = { From 32f9fd1e511e5a43cba107aacded613b2675e5d4 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 29 Jan 2019 15:05:29 +0200 Subject: [PATCH 08/44] FIX: add autocomplete=off to composer textarea Somehow a plugin or some new Chrome bug is causing its heuristic to detect our textarea for the composer as a target for address autocomplete This is likely a chrome bug but this change is very safe regardless. --- .../javascripts/discourse/templates/components/d-editor.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/templates/components/d-editor.hbs b/app/assets/javascripts/discourse/templates/components/d-editor.hbs index 4391669a79..41694e0b9d 100644 --- a/app/assets/javascripts/discourse/templates/components/d-editor.hbs +++ b/app/assets/javascripts/discourse/templates/components/d-editor.hbs @@ -39,7 +39,7 @@
{{conditional-loading-spinner condition=loading}} - {{textarea tabindex=tabindex value=value class="d-editor-input" placeholder=placeholderTranslated disabled=disabled}} + {{textarea autocomplete="off" tabindex=tabindex value=value class="d-editor-input" placeholder=placeholderTranslated disabled=disabled}} {{popup-input-tip validation=validation}} {{plugin-outlet name="after-d-editor" tagName="" args=outletArgs}} From fb7f21fbe90a00af7d4533da8546aa0ebf1649de Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 29 Jan 2019 10:24:32 -0500 Subject: [PATCH 09/44] Clearer syntax in migration Following up on 1c1fd205 --- ...0110142917_enable_content_security_policy_for_new_sites.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb b/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb index bf7af9faec..abe55353a0 100644 --- a/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb +++ b/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb @@ -18,8 +18,8 @@ class EnableContentSecurityPolicyForNewSites < ActiveRecord::Migration[5.2] end def instance_is_new? - post = DB.query_single("SELECT created_at FROM posts ORDER BY created_at ASC LIMIT 1") - return post.blank? || (post.present? && post.first > 1.week.ago) + dates = DB.query_single("SELECT created_at FROM posts ORDER BY created_at ASC LIMIT 1") + dates.empty? || dates.first > 1.week.ago end end From 0d0303e7eae193cff439933bd24328b4d4927d43 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 29 Jan 2019 16:54:04 +0100 Subject: [PATCH 10/44] FIX: more resilient lookup in plugin-api (#6961) Ember3 is more picky about having a container being destroyed and it's easier to cause exceptions, especially in tests. This fix has been initially created for an exception occuring in tests when running discourse-code-review and discourse-polls tests at the same time. `getCurrentUser` method body was failing as the container was destroyed. Original stacktrace: ``` "Error: Assertion Failed: expected container not to be destroyed at new EmberError (ember:2929:31) at assert (ember:1793:23) at Container.lookup (ember:17736:64) at PluginApi.getCurrentUser (discourse/lib/plugin-api:56:31) at allowUser (javascripts/discourse/initializers/init-code-review:38:29) at eval (javascripts/discourse/initializers/init-code-review:78:11) at eval (select-kit/mixins/plugin-api:86:19) at Array.forEach () at eval (select-kit/mixins/plugin-api:85:44) at Array.forEach ()" ``` --- .../discourse/lib/plugin-api.js.es6 | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index 34a607d93e..a44548c175 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -55,7 +55,19 @@ class PluginApi { * If the user is not logged in, it will be `null`. **/ getCurrentUser() { - return this.container.lookup("current-user:main"); + return this._lookupContainer("current-user:main"); + } + + _lookupContainer(path) { + if ( + !this.container || + this.container.isDestroying || + this.container.isDestroyed + ) { + return; + } + + return this.container.lookup(path); } _resolveClass(resolverName, opts) { @@ -222,7 +234,7 @@ class PluginApi { * ``` **/ addPosterIcon(cb) { - const site = this.container.lookup("site:main"); + const site = this._lookupContainer("site:main"); const loc = site && site.mobileView ? "before" : "after"; decorateWidget(`poster-name:${loc}`, dec => { @@ -424,8 +436,8 @@ class PluginApi { ``` **/ onAppEvent(name, fn) { - let appEvents = this.container.lookup("app-events:main"); - appEvents.on(name, fn); + const appEvents = this._lookupContainer("app-events:main"); + appEvents && appEvents.on(name, fn); } /** @@ -562,7 +574,8 @@ class PluginApi { * will issue a request to `/mice.json` **/ addStorePluralization(thing, plural) { - this.container.lookup("service:store").addPluralization(thing, plural); + const store = this._lookupContainer("service:store"); + store && store.addPluralization(thing, plural); } /** From 6f656f6e7d2af94fae9e6b10b54b9285fb6844b1 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 29 Jan 2019 16:47:25 -0500 Subject: [PATCH 11/44] FIX: Better error handling if a file cannot be sent If for some reason `Discourse.store.path_for` returns `nil`, the forum would throw an error rather than returning 404. Why would it be `nil`? One cause could be changing the type of file store and having the `url` field no longer be relative. --- app/controllers/uploads_controller.rb | 6 +++++- spec/requests/uploads_controller_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 64d08d4e1a..481d5d9c29 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -75,7 +75,11 @@ class UploadsController < ApplicationController } opts[:disposition] = "inline" if params[:inline] opts[:disposition] ||= "attachment" unless FileHelper.is_supported_image?(upload.original_filename) - send_file(Discourse.store.path_for(upload), opts) + + file_path = Discourse.store.path_for(upload) + return render_404 unless file_path + + send_file(file_path, opts) else render_404 end diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index db79aabdad..9128d8515e 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -226,6 +226,14 @@ describe UploadsController do expect(response.status).to eq(404) end + it "returns 404 when the path is nil" do + upload = upload_file("logo.png") + upload.update_column(:url, "invalid-url") + + get "/uploads/#{site}/#{upload.sha1}.#{upload.extension}" + expect(response.status).to eq(404) + end + it 'uses send_file' do upload = upload_file("logo.png") get "/uploads/#{site}/#{upload.sha1}.#{upload.extension}" From fc5b2de85d86da2bd9fc90eecbff82d7c20b8400 Mon Sep 17 00:00:00 2001 From: Jeff Wong Date: Fri, 25 Jan 2019 09:43:27 -0800 Subject: [PATCH 12/44] FEATURE: add topic list before status plugin outlet for mobile --- .../discourse/templates/mobile/list/topic-list-item.raw.hbs | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/discourse/templates/mobile/list/topic-list-item.raw.hbs b/app/assets/javascripts/discourse/templates/mobile/list/topic-list-item.raw.hbs index 2363ec4371..7333de7d55 100644 --- a/app/assets/javascripts/discourse/templates/mobile/list/topic-list-item.raw.hbs +++ b/app/assets/javascripts/discourse/templates/mobile/list/topic-list-item.raw.hbs @@ -8,6 +8,7 @@
{{/unless~}}