From ed627c823346f067f55a488b6f6a44cabea6c41c Mon Sep 17 00:00:00 2001 From: David McClure Date: Sun, 26 Feb 2017 04:54:07 -0800 Subject: [PATCH 01/95] FEATURE: Socialcast Importer: Set category and tags based on group --- script/import_scripts/socialcast/README.md | 22 +++++++--- script/import_scripts/socialcast/import.rb | 1 - .../socialcast/socialcast_message.rb | 40 ++++++++++++++++++- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/script/import_scripts/socialcast/README.md b/script/import_scripts/socialcast/README.md index 84e5d66c89..403acc7147 100644 --- a/script/import_scripts/socialcast/README.md +++ b/script/import_scripts/socialcast/README.md @@ -10,12 +10,22 @@ password: 'my-socialcast-password' ``` Create the directory for the json files to export: `mkdir output` -Then run `ruby export.rb /path/to/config.yml` +Then run `bundle exec ruby export.rb /path/to/config.yml` -Create a category named "Socialcast Import" or all topics will be imported into -the "Uncategorized" category. +If desired, edit the `socialcast_message.rb` file to set the category +and tags for each topic based on the name of the Socialcast group it was +originally posted to. -Topics will be tagged with the names of the groups they were originally posted -in on Socialcast. +You must create categories with the same names first in your site. -To run the import, run `ruby import.rb` +All topics will get the `DEFAULT_TAG` at minimum. + +Topics posted to a group that matches any group name in the `TAGS_AND_CATEGORIES` +map will get the associated category and tags. + +Other topics will be tagged with the original groupname and placed in the +`DEFAULT_CATEGORY`. + +To run the import, run `bundle exec ruby import.rb` + +To run the import in a production, run `RAILS_ENV=production bundle exec ruby import.rb` diff --git a/script/import_scripts/socialcast/import.rb b/script/import_scripts/socialcast/import.rb index b28b1e5058..7aaee20e18 100644 --- a/script/import_scripts/socialcast/import.rb +++ b/script/import_scripts/socialcast/import.rb @@ -65,7 +65,6 @@ class ImportScripts::Socialcast < ImportScripts::Base post = Post.find(post_id) # already imported this topic else topic[:user_id] = user_id_from_imported_user_id(topic[:author_id]) || -1 - topic[:category] = 'Socialcast Import' post = create_post(topic, topic[:id]) diff --git a/script/import_scripts/socialcast/socialcast_message.rb b/script/import_scripts/socialcast/socialcast_message.rb index 115ab77b12..602252be94 100644 --- a/script/import_scripts/socialcast/socialcast_message.rb +++ b/script/import_scripts/socialcast/socialcast_message.rb @@ -5,6 +5,19 @@ require_relative 'create_title.rb' class SocialcastMessage + DEFAULT_CATEGORY = "Socialcast Import" + DEFAULT_TAG = "socialcast-import" + TAGS_AND_CATEGORIES = { + "somegroupname" => { + category: "Apple Stems", + tags: ["waxy", "tough"] + }, + "someothergroupname" => { + category: "Orange Peels", + tags: ["oily"] + } + } + def initialize message_json @parsed_json = JSON.parse message_json end @@ -16,7 +29,8 @@ class SocialcastMessage topic[:title] = title topic[:raw] = @parsed_json['body'] topic[:created_at] = Time.parse @parsed_json['created_at'] - topic[:tags] = [group] if group + topic[:tags] = tags + topic[:category] = category topic end @@ -24,8 +38,30 @@ class SocialcastMessage CreateTitle.from_body @parsed_json['body'] end + def tags + tags = [] + if group + if TAGS_AND_CATEGORIES[group] + tags = TAGS_AND_CATEGORIES[group][:tags] + else + tags << group + end + end + tags << DEFAULT_TAG + tags + end + + + def category + category = DEFAULT_CATEGORY + if group && TAGS_AND_CATEGORIES[group] + category = TAGS_AND_CATEGORIES[group][:category] + end + category + end + def group - @parsed_json['group']['groupname'] if @parsed_json['group'] + @parsed_json['group']['groupname'].downcase if @parsed_json['group'] && @parsed_json['group']['groupname'] end def url From 689dd16be026ab5c83b6930b66aa1f5219eee88f Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 8 Mar 2017 11:49:39 +0800 Subject: [PATCH 02/95] FIX: Allow user to remove bookmark from posts as long as bookmark is present. https://meta.discourse.org/t/bookmark-issue-when-access-to-topic-is-lost-pms/51993 --- app/controllers/posts_controller.rb | 12 +++++--- spec/controllers/posts_controller_spec.rb | 37 ++++++++++++++++++++--- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 52618c5caf..7baedf4089 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -377,17 +377,19 @@ class PostsController < ApplicationController end def bookmark - post = find_post_from_params - if params[:bookmarked] == "true" + post = find_post_from_params PostAction.act(current_user, post, PostActionType.types[:bookmark]) else + post_action = PostAction.find_by(post_id: params[:post_id], user_id: current_user.id) + post = post_action.post + raise Discourse::InvalidParameters unless post_action + PostAction.remove_act(current_user, post, PostActionType.types[:bookmark]) end - tu = TopicUser.get(post.topic, current_user) - - render_json_dump(topic_bookmarked: tu.try(:bookmarked)) + topic_user = TopicUser.get(post.topic, current_user) + render_json_dump(topic_bookmarked: topic_user.try(:bookmarked)) end def wiki diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index 8c8ba76a89..98b198550a 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -426,7 +426,8 @@ describe PostsController do include_examples 'action requires login', :put, :bookmark, post_id: 2 describe 'when logged in' do - let(:post) { Fabricate(:post, user: log_in) } + let(:user) { log_in } + let(:post) { Fabricate(:post, user: user) } let(:private_message) { Fabricate(:private_message_post) } it "raises an error if the user doesn't have permission to see the post" do @@ -436,13 +437,39 @@ describe PostsController do end it 'creates a bookmark' do - PostAction.expects(:act).with(post.user, post, PostActionType.types[:bookmark]) xhr :put, :bookmark, post_id: post.id, bookmarked: 'true' + + post_action = PostAction.find_by(user:user, post: post) + + expect(post_action.post_action_type_id).to eq(PostActionType.types[:bookmark]) end - it 'removes a bookmark' do - PostAction.expects(:remove_act).with(post.user, post, PostActionType.types[:bookmark]) - xhr :put, :bookmark, post_id: post.id + context "removing a bookmark" do + let(:post_action) { PostAction.act(user, post, PostActionType.types[:bookmark]) } + let(:admin) { Fabricate(:admin) } + + it 'should be able to remove a bookmark' do + post_action + xhr :put, :bookmark, post_id: post.id + + expect(PostAction.find_by(id: post_action.id)).to eq(nil) + end + + describe "when user doesn't have permission to see bookmarked post" do + it "should still be able to remove a bookmark" do + post_action + post = post_action.post + topic = post.topic + topic.convert_to_private_message(admin) + topic.remove_allowed_user(admin, user.username) + + expect(Guardian.new(user).can_see_post?(post.reload)).to eq(false) + + xhr :put, :bookmark, post_id: post.id + + expect(PostAction.find_by(id: post_action.id)).to eq(nil) + end + end end end From 6a7773b681f43f6210cecba96fd0c1a0f940ed5b Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 8 Mar 2017 20:13:22 +0800 Subject: [PATCH 03/95] FIX: Category autocomplete breaks when search menu widget rerenders. https://github.com/discourse/discourse/pull/4717#issuecomment-284914585 --- .../javascripts/discourse/lib/autocomplete.js.es6 | 6 +++++- app/assets/javascripts/discourse/lib/search.js.es6 | 10 +++++----- app/assets/javascripts/discourse/widgets/header.js.es6 | 5 ++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/autocomplete.js.es6 b/app/assets/javascripts/discourse/lib/autocomplete.js.es6 index bfaa65d49a..4b49fdc014 100644 --- a/app/assets/javascripts/discourse/lib/autocomplete.js.es6 +++ b/app/assets/javascripts/discourse/lib/autocomplete.js.es6 @@ -261,7 +261,11 @@ export default function(options) { left: "-1000px" }); - me.parent().append(div); + if (options.appendSelector) { + me.parents(options.appendSelector).append(div); + } else { + me.parent().append(div); + } if (!isInput && !options.treatAsTextarea) { vOffset = div.height(); diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 index 7831868230..0824f8edca 100644 --- a/app/assets/javascripts/discourse/lib/search.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -130,14 +130,14 @@ export function isValidSearchTerm(searchTerm) { } }; -export function applySearchAutocomplete($input, siteSettings, appEvents) { +export function applySearchAutocomplete($input, siteSettings, appEvents, options) { const afterComplete = function() { if (appEvents) { appEvents.trigger("search-autocomplete:after-complete"); } }; - $input.autocomplete({ + $input.autocomplete(_.merge({ template: findRawTemplate('category-tag-autocomplete'), key: '#', width: '100%', @@ -153,9 +153,9 @@ export function applySearchAutocomplete($input, siteSettings, appEvents) { return searchCategoryTag(term, siteSettings); }, afterComplete - }); + }, options)); - $input.autocomplete({ + $input.autocomplete(_.merge({ template: findRawTemplate('user-selector-autocomplete'), key: "@", width: '100%', @@ -163,5 +163,5 @@ export function applySearchAutocomplete($input, siteSettings, appEvents) { transformComplete: v => v.username || v.name, dataSource: term => userSearch({ term, includeGroups: true }), afterComplete - }); + }, options)); }; diff --git a/app/assets/javascripts/discourse/widgets/header.js.es6 b/app/assets/javascripts/discourse/widgets/header.js.es6 index c74dac720f..1f66b5eec6 100644 --- a/app/assets/javascripts/discourse/widgets/header.js.es6 +++ b/app/assets/javascripts/discourse/widgets/header.js.es6 @@ -262,7 +262,10 @@ export default createWidget('header', { Ember.run.schedule('afterRender', () => { const $searchInput = $('#search-term'); $searchInput.focus().select(); - applySearchAutocomplete($searchInput, this.siteSettings, this.appEvents); + + applySearchAutocomplete($searchInput, this.siteSettings, this.appEvents, { + appendSelector: '.menu-panel' + }); }); } }, From c6239513063c184ec2d45c07d1b4d47d1208c5a4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 8 Mar 2017 22:46:23 +0800 Subject: [PATCH 04/95] FEATURE: Search can be scoped to posts that the current user has seen/unseen. https://meta.discourse.org/t/advanced-search-posts-that-i-have-seen/57966 --- lib/search.rb | 28 ++++++++++++++++++++-- spec/components/search_spec.rb | 44 ++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index ef30676570..4242ab78c6 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -315,6 +315,29 @@ class Search end end + advanced_filter(/in:seen/) do |posts| + if @guardian.user + posts + .joins("INNER JOIN post_timings ON + post_timings.topic_id = posts.topic_id + AND post_timings.post_number = posts.post_number + AND post_timings.user_id = #{Post.sanitize(@guardian.user.id)} + ") + end + end + + advanced_filter(/in:unseen/) do |posts| + if @guardian.user + posts + .joins("LEFT JOIN post_timings ON + post_timings.topic_id = posts.topic_id + AND post_timings.post_number = posts.post_number + AND post_timings.user_id = #{Post.sanitize(@guardian.user.id)} + ") + .where("post_timings.user_id IS NULL") + end + end + advanced_filter(/category:(.+)/) do |posts,match| exact = false @@ -729,10 +752,10 @@ class Search if @order == :likes # likes are a pain to aggregate so skip posts_query(@limit, private_messages: opts[:private_messages]) - .select('topics.id', "post_number") + .select('topics.id', "posts.post_number") else posts_query(@limit, aggregate_search: true, private_messages: opts[:private_messages]) - .select('topics.id', "#{min_or_max}(post_number) post_number") + .select('topics.id', "#{min_or_max}(posts.post_number) post_number") .group('topics.id') end @@ -761,6 +784,7 @@ class Search post_sql = aggregate_post_sql(opts) added = 0 + aggregate_posts(post_sql[:default]).each do |p| @results.add(p) added += 1 diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 5f1c210337..4be4c155e7 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -462,9 +462,49 @@ describe Search do it 'supports wiki' do topic = Fabricate(:topic) - Fabricate(:post, raw: 'this is a test 248', wiki: true, topic: topic) + topic_2 = Fabricate(:topic) + post = Fabricate(:post, raw: 'this is a test 248', wiki: true, topic: topic) + Fabricate(:post, raw: 'this is a test 248', wiki: false, topic: topic_2) - expect(Search.execute('test 248 in:wiki').posts.length).to eq(1) + expect(Search.execute('test 248').posts.length).to eq(2) + expect(Search.execute('test 248 in:wiki').posts.first).to eq(post) + end + + it 'supports searching for posts that the user has seen/unseen' do + topic = Fabricate(:topic) + topic_2 = Fabricate(:topic) + post = Fabricate(:post, raw: 'logan is longan', topic: topic) + post_2 = Fabricate(:post, raw: 'longan is logan', topic: topic_2) + + [post.user, topic.user].each do |user| + PostTiming.create!( + post_number: post.post_number, + topic: topic, + user: user, + msecs: 1 + ) + end + + expect(post.seen?(post.user)).to eq(true) + + expect(Search.execute('longan').posts.sort).to eq([post, post_2]) + + expect(Search.execute('longan in:seen', guardian: Guardian.new(post.user)).posts) + .to eq([post]) + + expect(Search.execute('longan in:seen').posts.sort).to eq([post, post_2]) + + expect(Search.execute('longan in:seen', guardian: Guardian.new(post_2.user)).posts) + .to eq([]) + + expect(Search.execute('longan', guardian: Guardian.new(post_2.user)).posts.sort) + .to eq([post, post_2]) + + expect(Search.execute('longan in:unseen', guardian: Guardian.new(post_2.user)).posts.sort) + .to eq([post, post_2]) + + expect(Search.execute('longan in:unseen', guardian: Guardian.new(post.user)).posts) + .to eq([post_2]) end it 'supports before and after, in:first, user:, @username' do From 23b06d28952f957108eea6ac231a99cf1744a94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 8 Mar 2017 19:19:11 +0100 Subject: [PATCH 05/95] FIX: should not try to send digest to users who reached the bounce threshold --- app/jobs/regular/user_email.rb | 34 ++++++--------------- app/jobs/scheduled/enqueue_digest_emails.rb | 12 ++++---- spec/jobs/enqueue_digest_emails_spec.rb | 9 ++++++ 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 500f8cddb4..6f316d530e 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -67,16 +67,13 @@ module Jobs signup_after_approval } - def message_for_email(user, post, type, notification, - notification_type=nil, notification_data_hash=nil, - email_token=nil, to_address=nil) - + def message_for_email(user, post, type, notification, notification_type=nil, notification_data_hash=nil, email_token=nil, to_address=nil) set_skip_context(type, user.id, to_address || user.email, post.try(:id)) return skip_message(I18n.t("email_log.anonymous_user")) if user.anonymous? return skip_message(I18n.t("email_log.suspended_not_pm")) if user.suspended? && type.to_s != "user_private_message" - return if user.staged && type == :digest + return if user.staged && type.to_s == "digest" seen_recently = (user.last_seen_at.present? && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago) seen_recently = false if user.user_option.email_always || user.staged @@ -87,9 +84,7 @@ module Jobs return skip_message(I18n.t('email_log.seen_recently')) if seen_recently && !user.suspended? end - if post - email_args[:post] = post - end + email_args[:post] = post if post if notification || notification_type email_args[:notification_type] ||= notification_type || notification.try(:notification_type) @@ -118,18 +113,13 @@ module Jobs end skip_reason = skip_email_for_post(post, user) - return skip_message(skip_reason) if skip_reason + return skip_message(skip_reason) if skip_reason.present? # Make sure that mailer exists raise Discourse::InvalidParameters.new("type=#{type}") unless UserNotifications.respond_to?(type) - if email_token.present? - email_args[:email_token] = email_token - end - - if type.to_s == "notify_old_email" - email_args[:new_email] = user.email - end + email_args[:email_token] = email_token if email_token.present? + email_args[:new_email] = user.email if type.to_s == "notify_old_email" if EmailLog.reached_max_emails?(user) return skip_message(I18n.t('email_log.exceeded_emails_limit')) @@ -144,9 +134,7 @@ module Jobs end # Update the to address if we have a custom one - if message && to_address.present? - message.to = to_address - end + message.to = to_address if message && to_address.present? [message, nil] end @@ -179,12 +167,10 @@ module Jobs return I18n.t('email_log.topic_nil') if post.topic.blank? return I18n.t('email_log.post_user_deleted') if post.user.blank? return I18n.t('email_log.post_deleted') if post.user_deleted? - return I18n.t('email_log.user_suspended') if (user.suspended? && !post.user.try(:staff?)) + return I18n.t('email_log.user_suspended') if user.suspended? && !post.user&.staff? - if !user.user_option.email_always? && - PostTiming.where(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id).present? - return I18n.t('email_log.already_read') - end + already_read = !user.user_option.email_always? && PostTiming.exists?(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id) + return I18n.t('email_log.already_read') if already_read else false end diff --git a/app/jobs/scheduled/enqueue_digest_emails.rb b/app/jobs/scheduled/enqueue_digest_emails.rb index 8969aee37c..bab41fb142 100644 --- a/app/jobs/scheduled/enqueue_digest_emails.rb +++ b/app/jobs/scheduled/enqueue_digest_emails.rb @@ -15,18 +15,18 @@ module Jobs def target_user_ids # Users who want to receive digest email within their chosen digest email frequency query = User.real - .where(active: true, staged: false) - .joins(:user_option) .not_suspended - .where(user_options: {email_digests: true}) + .activated + .where(staged: false) + .joins(:user_option, :user_stat) + .where("user_options.email_digests") + .where("user_stats.bounce_score < #{SiteSetting.bounce_score_threshold}") .where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)") .where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)") .where("COALESCE(last_seen_at, '2010-01-01') >= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * #{SiteSetting.suppress_digest_email_after_days})") # If the site requires approval, make sure the user is approved - if SiteSetting.must_approve_users? - query = query.where("approved OR moderator OR admin") - end + query = query.where("approved OR moderator OR admin") if SiteSetting.must_approve_users? query.pluck(:id) end diff --git a/spec/jobs/enqueue_digest_emails_spec.rb b/spec/jobs/enqueue_digest_emails_spec.rb index f988629403..9d00e3dd4c 100644 --- a/spec/jobs/enqueue_digest_emails_spec.rb +++ b/spec/jobs/enqueue_digest_emails_spec.rb @@ -102,6 +102,15 @@ describe Jobs::EnqueueDigestEmails do end end + context 'too many bounces' do + let!(:bounce_user) { Fabricate(:active_user, last_seen_at: 6.month.ago) } + + it "doesn't return users with too many bounces" do + bounce_user.user_stat.update(bounce_score: SiteSetting.bounce_score_threshold + 1) + expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(bounce_user.id)).to eq(false) + end + end + end describe '#execute' do From dfd5b06c82ab30632ba6fdef224f8971eab0a44e Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Thu, 9 Mar 2017 01:08:17 +0530 Subject: [PATCH 06/95] FIX: custom CSS/HTML files were not getting downloaded on Chrome --- .../javascripts/admin/templates/customize-css-html-show.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/templates/customize-css-html-show.hbs b/app/assets/javascripts/admin/templates/customize-css-html-show.hbs index 024c670845..6a10c7ae0b 100644 --- a/app/assets/javascripts/admin/templates/customize-css-html-show.hbs +++ b/app/assets/javascripts/admin/templates/customize-css-html-show.hbs @@ -1,7 +1,7 @@
{{text-field class="style-name" value=model.name}} - {{fa-icon "download"}} {{i18n 'admin.export_json.button_text'}} + {{fa-icon "download"}} {{i18n 'admin.export_json.button_text'}}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b72ffd8108..ba5ec4747e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1316,6 +1316,8 @@ en: first: are the very first post pinned: are pinned unpinned: are not pinned + seen: I've read + unseen: I've not read wiki: are wiki statuses: label: Where topics diff --git a/test/javascripts/acceptance/search-full-test.js.es6 b/test/javascripts/acceptance/search-full-test.js.es6 index 439ed16130..886dd4b13d 100644 --- a/test/javascripts/acceptance/search-full-test.js.es6 +++ b/test/javascripts/acceptance/search-full-test.js.es6 @@ -233,15 +233,18 @@ test("update in:private filter through advanced search ui", assert => { }); }); -test("update in:wiki filter through advanced search ui", assert => { +test("update in:seen filter through advanced search ui", assert => { visit("/search"); fillIn('.search input.full-page-search', 'none'); click('.search-advanced-btn'); - click('.search-advanced-options .in-wiki'); + click('.search-advanced-options .in-seen'); andThen(() => { - assert.ok(exists('.search-advanced-options .in-wiki:checked'), 'has "are wiki" populated'); - assert.equal(find('.search input.full-page-search').val(), "none in:wiki", 'has updated search term to "none in:wiki"'); + assert.ok(exists('.search-advanced-options .in-seen:checked'), 'it should check the right checkbox'); + + assert.equal(find('.search input.full-page-search').val(), "none in:seen", + 'it should update the search term' + ); }); }); From c4e22a12f97656ed35d1145830a08f2b8ce596ad Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 9 Mar 2017 18:40:07 +0800 Subject: [PATCH 09/95] Bump Redis. --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index deec6ecc55..46d316496b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -276,7 +276,7 @@ GEM ffi (>= 1.0.6) msgpack (>= 0.4.3) trollop (>= 1.16.2) - redis (3.3.1) + redis (3.3.3) redis-namespace (1.5.2) redis (~> 3.0, >= 3.0.4) rest-client (1.8.0) @@ -477,4 +477,4 @@ DEPENDENCIES unicorn BUNDLED WITH - 1.14.3 + 1.14.4 From ab3faeb0f924822f66dd4aed5b558d16ab914a28 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 9 Mar 2017 16:44:50 -0500 Subject: [PATCH 10/95] PERF: user mini racer to uglify assets --- Gemfile.lock | 6 +++--- lib/tasks/assets.rake | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 46d316496b..34145fb1a5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,7 +138,7 @@ GEM json (1.8.6) jwt (1.5.2) kgio (2.10.0) - libv8 (5.3.332.38.3) + libv8 (5.3.332.38.5) listen (0.7.3) logster (1.2.7) loofah (2.0.3) @@ -153,7 +153,7 @@ GEM method_source (0.8.2) mime-types (2.99.2) mini_portile2 (2.1.0) - mini_racer (0.1.7) + mini_racer (0.1.9) libv8 (~> 5.3) minitest (5.9.1) mocha (1.1.0) @@ -477,4 +477,4 @@ DEPENDENCIES unicorn BUNDLED WITH - 1.14.4 + 1.14.6 diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index 53d3ea80cd..bc11b20d22 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -11,10 +11,14 @@ task 'assets:precompile:before' do puts "Purging temp files" `rm -fr #{Rails.root}/tmp/cache` - if Rails.configuration.assets.js_compressor == :uglifier && !`which uglifyjs`.empty? && !ENV['SKIP_NODE_UGLIFY'] + if Rails.configuration.assets.js_compressor == :uglifier && !`which uglifyjs`.empty? && ENV['FORCE_NODE_UGLIFY'] $node_uglify = true end + unless ENV['USE_SPROCKETS_UGLIFY'] + $bypass_sprockets_uglify = true + end + puts "Bundling assets" # in the past we applied a patch that removed asset postfixes, but it is terrible practice @@ -28,7 +32,7 @@ task 'assets:precompile:before' do load "#{Rails.root}/lib/global_path.rb" include GlobalPath - if $node_uglify + if $bypass_sprockets_uglify Rails.configuration.assets.js_compressor = nil Rails.configuration.assets.gzip = false end @@ -84,8 +88,10 @@ def compress_ruby(from,to) uglified, map = Uglifier.new(comments: :none, screw_ie8: true, - source_filename: File.basename(from), - output_filename: File.basename(to) + source_map: { + filename: File.basename(from), + output_filename: File.basename(to) + } ) .compile_with_map(data) dest = "#{assets_path}/#{to}" @@ -108,7 +114,7 @@ def brotli(path) end def compress(from,to) - if @has_uglifyjs ||= !`which uglifyjs`.empty? + if $node_uglify compress_node(from,to) else compress_ruby(from,to) @@ -129,7 +135,7 @@ task 'assets:precompile' => 'assets:precompile:before' do # Run after assets:precompile Rake::Task["assets:precompile:css"].invoke - if $node_uglify + if $bypass_sprockets_uglify puts "Compressing Javascript and Generating Source Maps" manifest = Sprockets::Manifest.new(assets_path) From b68d08404da135352f2c518bc24ba37c8d084a23 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 9 Mar 2017 17:05:55 -0500 Subject: [PATCH 11/95] shell to node to avoid high memory usage --- lib/tasks/assets.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index bc11b20d22..8c299d0cf6 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -11,7 +11,7 @@ task 'assets:precompile:before' do puts "Purging temp files" `rm -fr #{Rails.root}/tmp/cache` - if Rails.configuration.assets.js_compressor == :uglifier && !`which uglifyjs`.empty? && ENV['FORCE_NODE_UGLIFY'] + if Rails.configuration.assets.js_compressor == :uglifier && !`which uglifyjs`.empty? && !ENV['SKIP_NODE_UGLIFY'] $node_uglify = true end From 3032aa7db93e327ffad8c1e65b2707221c203936 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 9 Mar 2017 18:00:55 -0500 Subject: [PATCH 12/95] PERF: avoid looking globals from providers after first call --- app/models/global_setting.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index 85abcce23b..bf570ed72d 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -39,8 +39,21 @@ class GlobalSetting default_provider = FileProvider.from(File.expand_path('../../../config/discourse_defaults.conf', __FILE__)) default_provider.keys.concat(@provider.keys).uniq.each do |key| default = default_provider.lookup(key, nil) + + instance_variable_set("@#{key}_cache", nil) + define_singleton_method(key) do - provider.lookup(key, default) + val = instance_variable_get("@#{key}_cache") + unless val.nil? + val == :missing ? nil : val + else + val = provider.lookup(key, default) + if val.nil? + val = :missing + end + instance_variable_set("@#{key}_cache", val) + val == :missing ? nil : val + end end end end From eb6ef0311e63383434cc6d16937e7db8a68907a6 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 10 Mar 2017 15:33:31 +0800 Subject: [PATCH 13/95] Hide special users from about pages. --- app/models/about.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/about.rb b/app/models/about.rb index dd87169ece..a59a73a019 100644 --- a/app/models/about.rb +++ b/app/models/about.rb @@ -35,14 +35,12 @@ class About def moderators @moderators ||= User.where(moderator: true, admin: false) - .where.not(id: Discourse::SYSTEM_USER_ID) + .where("id > 0") .order(:username_lower) end def admins - @admins ||= User.where(admin: true) - .where.not(id: Discourse::SYSTEM_USER_ID) - .order(:username_lower) + @admins ||= User.where(admin: true).where("id > 0").order(:username_lower) end def stats From 0e41b1181acd965431384d29ed3d4e7f222c7827 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 10 Mar 2017 17:15:49 +0800 Subject: [PATCH 14/95] UX: Display button to add a group when no group has been selected. https://meta.discourse.org/t/new-button-on-groups/44546 --- .../admin-groups-type-index.js.es6 | 11 +++++ .../admin/templates/groups-type-index.hbs | 9 ++++ .../admin/templates/groups-type.hbs | 46 ++++++++++--------- .../stylesheets/common/admin/admin_base.scss | 4 ++ config/locales/client.en.yml | 2 + 5 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/admin-groups-type-index.js.es6 create mode 100644 app/assets/javascripts/admin/templates/groups-type-index.hbs diff --git a/app/assets/javascripts/admin/controllers/admin-groups-type-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-groups-type-index.js.es6 new file mode 100644 index 0000000000..ca80f093d1 --- /dev/null +++ b/app/assets/javascripts/admin/controllers/admin-groups-type-index.js.es6 @@ -0,0 +1,11 @@ +import computed from 'ember-addons/ember-computed-decorators'; + +export default Ember.Controller.extend({ + adminGroupsType: Ember.inject.controller(), + sortedGroups: Ember.computed.alias("adminGroupsType.sortedGroups"), + + @computed("sortedGroups") + messageKey(sortedGroups) { + return `admin.groups.${sortedGroups.length > 0 ? 'none_selected' : 'no_custom_groups'}`; + } +}); diff --git a/app/assets/javascripts/admin/templates/groups-type-index.hbs b/app/assets/javascripts/admin/templates/groups-type-index.hbs new file mode 100644 index 0000000000..88d870c1f6 --- /dev/null +++ b/app/assets/javascripts/admin/templates/groups-type-index.hbs @@ -0,0 +1,9 @@ +
+

{{i18n messageKey}}

+ +
+ {{#link-to 'adminGroup' 'new' class="btn"}} + {{fa-icon "plus"}} {{i18n 'admin.groups.new'}} + {{/link-to}} +
+
diff --git a/app/assets/javascripts/admin/templates/groups-type.hbs b/app/assets/javascripts/admin/templates/groups-type.hbs index 6c9986fcb3..b4d2a0acf3 100644 --- a/app/assets/javascripts/admin/templates/groups-type.hbs +++ b/app/assets/javascripts/admin/templates/groups-type.hbs @@ -1,29 +1,31 @@
-
-

{{i18n 'admin.groups.edit'}}

-
    - {{#each sortedGroups as |group|}} -
  • - {{#link-to "adminGroup" group.type group.name}}{{group.name}} - {{#if group.userCountDisplay}} - {{number group.userCountDisplay}} - {{/if}} + {{#if sortedGroups}} +
    +

    {{i18n 'admin.groups.edit'}}

    +
      + {{#each sortedGroups as |group|}} +
    • + {{#link-to "adminGroup" group.type group.name}}{{group.name}} + {{#if group.userCountDisplay}} + {{number group.userCountDisplay}} + {{/if}} + {{/link-to}} +
    • + {{/each}} +
    +
    + {{#if isAuto}} + {{d-button action="refreshAutoGroups" icon="refresh" label="admin.groups.refresh" disabled=refreshingAutoGroups}} + {{else}} + {{#link-to 'adminGroup' 'new' class="btn"}} + {{fa-icon "plus"}} {{i18n 'admin.groups.new'}} {{/link-to}} -
  • - {{/each}} -
-
- {{#if isAuto}} - {{d-button action="refreshAutoGroups" icon="refresh" label="admin.groups.refresh" disabled=refreshingAutoGroups}} - {{else}} - {{#link-to 'adminGroup' 'new' class="btn"}} - {{fa-icon "plus"}} {{i18n 'admin.groups.new'}} - {{/link-to}} - {{/if}} + {{/if}} +
-
+ {{/if}} -
+
{{outlet}}
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 13893d7cc2..1d55d6b236 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -696,6 +696,10 @@ section.details { width: 100%; border-color: dark-light-choose(scale-color($primary, $lightness: 75%), scale-color($secondary, $lightness: 25%)); } + + .content-list { + margin-right: 20px; + } } // Customise area diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ba5ec4747e..d9ff0b1b0a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2568,6 +2568,8 @@ en: add_owners: Add owners incoming_email: "Custom incoming email address" incoming_email_placeholder: "enter email address" + none_selected: "Select a group to get started" + no_custom_groups: "Create a new custom group" api: generate_master: "Generate Master API Key" From f7e7ca3937216014b4cb55c0734310f6fd12fa2f Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 10 Mar 2017 18:46:00 +0530 Subject: [PATCH 15/95] FEATURE: anonymized site statistics --- app/controllers/site_controller.rb | 7 ++++- config/locales/server.en.yml | 2 ++ config/routes.rb | 1 + config/site_settings.yml | 2 ++ spec/controllers/site_controller_spec.rb | 34 ++++++++++++++++++++++++ 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index dbbeab8e7b..f86e45b969 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -3,7 +3,7 @@ require_dependency 'site_serializer' class SiteController < ApplicationController layout false skip_before_filter :preload_json, :check_xhr - skip_before_filter :redirect_to_login_if_required, only: ['basic_info'] + skip_before_filter :redirect_to_login_if_required, only: ['basic_info', 'statistics'] def site render json: Site.json_for(guardian) @@ -42,4 +42,9 @@ class SiteController < ApplicationController # this info is always available cause it can be scraped from a 404 page render json: results end + + def statistics + return redirect_to path('/') unless SiteSetting.share_anonymized_statistics? + render json: About.fetch_cached_stats + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b2eb4e45da..f68d72b22e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1395,6 +1395,8 @@ en: native_app_install_banner: "Asks recurring visitors to install Discourse native app." + share_anonymized_statistics: "Share anonymized usage statistics." + max_prints_per_hour_per_user: "Maximum number of /print page impressions (set to 0 to disable)" full_name_required: "Full name is a required field of a user's profile." diff --git a/config/routes.rb b/config/routes.rb index fc53ffb18f..7c42b21eb7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,6 +54,7 @@ Discourse::Application.routes.draw do end get "site/basic-info" => 'site#basic_info' + get "site/statistics" => 'site#statistics' get "site_customizations/:key" => "site_customizations#show" diff --git a/config/site_settings.yml b/config/site_settings.yml index 1fbc412056..b82e2b8be8 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1308,6 +1308,8 @@ uncategorized: native_app_install_banner: false + share_anonymized_statistics: true + user_preferences: default_email_digest_frequency: diff --git a/spec/controllers/site_controller_spec.rb b/spec/controllers/site_controller_spec.rb index 5e328f5a3e..1dccba86ad 100644 --- a/spec/controllers/site_controller_spec.rb +++ b/spec/controllers/site_controller_spec.rb @@ -24,4 +24,38 @@ describe SiteController do expect(json["mobile_logo_url"]).to eq("https://a.a/a.png") end end + + describe '.statistics' do + + it 'is visible for sites requiring login' do + SiteSetting.login_required = true + SiteSetting.share_anonymized_statistics = true + + xhr :get, :statistics + json = JSON.parse(response.body) + + expect(response).to be_success + expect(json["topic_count"]).to be_present + expect(json["post_count"]).to be_present + expect(json["user_count"]).to be_present + expect(json["topics_7_days"]).to be_present + expect(json["topics_30_days"]).to be_present + expect(json["posts_7_days"]).to be_present + expect(json["posts_30_days"]).to be_present + expect(json["users_7_days"]).to be_present + expect(json["users_30_days"]).to be_present + expect(json["active_users_7_days"]).to be_present + expect(json["active_users_30_days"]).to be_present + expect(json["like_count"]).to be_present + expect(json["likes_7_days"]).to be_present + expect(json["likes_30_days"]).to be_present + end + + it 'is not visible if site setting share_anonymized_statistics is disabled' do + SiteSetting.share_anonymized_statistics = false + + xhr :get, :statistics + expect(response).to redirect_to '/' + end + end end From f9f38873a2112fd21516dbbb45ce75f21aefa71e Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 11:35:25 -0500 Subject: [PATCH 16/95] FEATURE: add support for SIGTSTP which stops sidekiqs Out of the box this signal "suspends" the process, but we already use usr1 and usr2 and this is for an edge case where the end user suspends it by typing "stop" --- config/unicorn.conf.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/unicorn.conf.rb b/config/unicorn.conf.rb index 74723ef125..5f51df510a 100644 --- a/config/unicorn.conf.rb +++ b/config/unicorn.conf.rb @@ -77,6 +77,11 @@ before_fork do |server, worker| Demon::Sidekiq.start(sidekiqs) + Signal.trap("SIGTSTP") do + STDERR.puts "#{Time.now}: Issuing stop to sidekiq" + Demon::Sidekiq.stop + end + class ::Unicorn::HttpServer alias :master_sleep_orig :master_sleep From 20ed11f9a56783bd88e169b8aa7a55cdc71fbc28 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 11:35:54 -0500 Subject: [PATCH 17/95] We must GC here otherwise we risk not freeing our v8 contexts --- lib/tasks/assets.rake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index 8c299d0cf6..2f2a46b409 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -98,6 +98,8 @@ def compress_ruby(from,to) File.write(dest, uglified << "\n//# sourceMappingURL=#{cdn_path "/assets/#{to}.map"}") File.write(dest + ".map", map) + + GC.start end def gzip(path) From 15adbdcdd58cd855d417a8a2375dd1c0781e12f4 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 10 Mar 2017 12:06:15 -0500 Subject: [PATCH 18/95] FEATURE: new template parameters for notification emails that can be used in links: topic_title_url_encoded and site_title_url_encoded --- app/mailers/user_notifications.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 5c4ef82c67..ad73c45a9e 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -427,6 +427,7 @@ class UserNotifications < ActionMailer::Base email_opts = { topic_title: title, + topic_title_url_encoded: URI.encode(title), message: message, url: post.url, post_id: post.id, @@ -447,6 +448,7 @@ class UserNotifications < ActionMailer::Base html_override: html, site_description: SiteSetting.site_description, site_title: SiteSetting.title, + site_title_url_encoded: URI.encode(SiteSetting.title), style: :notification, locale: locale } From 402ddb810cf69949a9bc9db00a340ec77e27141f Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 10 Mar 2017 14:07:41 -0500 Subject: [PATCH 19/95] FIX: email customizations now apply to both html and text parts --- app/mailers/user_notifications.rb | 73 ++++++++++++++----------- spec/mailers/user_notifications_spec.rb | 18 +++++- 2 files changed, 59 insertions(+), 32 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index ad73c45a9e..b8b72b19d9 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -357,6 +357,12 @@ class UserNotifications < ActionMailer::Base user = opts[:user] locale = user_locale(user) + template = "user_notifications.user_#{notification_type}" + if post.topic.private_message? + template << "_pm" + template << "_staged" if user.staged? + end + # category name category = Topic.find_by(id: post.topic_id).category if opts[:show_category_in_subject] && post.topic_id && category && !category.uncategorized? @@ -384,10 +390,7 @@ class UserNotifications < ActionMailer::Base end end - reached_limit = SiteSetting.max_emails_per_day_per_user > 0 - reached_limit &&= (EmailLog.where(user_id: user.id, skipped: false) - .where('created_at > ?', 1.day.ago) - .count) >= (SiteSetting.max_emails_per_day_per_user-1) + translation_override_exists = TranslationOverride.where(locale: SiteSetting.default_locale, translation_key: "#{template}.text_body_template").exists? if opts[:use_invite_template] if post.topic.private_message? @@ -397,37 +400,42 @@ class UserNotifications < ActionMailer::Base end topic_excerpt = post.excerpt.tr("\n", " ") if post.is_first_post? && post.excerpt message = I18n.t(invite_template, username: username, topic_title: title, topic_excerpt: topic_excerpt, site_title: SiteSetting.title, site_description: SiteSetting.site_description) - html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( - template: 'email/invite', - format: :html, - locals: { message: PrettyText.cook(message, sanitize: false).html_safe, - classes: RTL.new(user).css_class - } - ) - else - in_reply_to_post = post.reply_to_post if user.user_option.email_in_reply_to - html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( - template: 'email/notification', - format: :html, - locals: { context_posts: context_posts, - reached_limit: reached_limit, - post: post, - in_reply_to_post: in_reply_to_post, - classes: RTL.new(user).css_class - } - ) - message = email_post_markdown(post) + (reached_limit ? "\n\n#{I18n.t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user}" : ""); - end - template = "user_notifications.user_#{notification_type}" - if post.topic.private_message? - template << "_pm" - template << "_staged" if user.staged? + unless translation_override_exists + html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( + template: 'email/invite', + format: :html, + locals: { message: PrettyText.cook(message, sanitize: false).html_safe, + classes: RTL.new(user).css_class + } + ) + end + else + reached_limit = SiteSetting.max_emails_per_day_per_user > 0 + reached_limit &&= (EmailLog.where(user_id: user.id, skipped: false) + .where('created_at > ?', 1.day.ago) + .count) >= (SiteSetting.max_emails_per_day_per_user-1) + + in_reply_to_post = post.reply_to_post if user.user_option.email_in_reply_to + message = email_post_markdown(post) + (reached_limit ? "\n\n#{I18n.t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user}" : ""); + + unless translation_override_exists + html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( + template: 'email/notification', + format: :html, + locals: { context_posts: context_posts, + reached_limit: reached_limit, + post: post, + in_reply_to_post: in_reply_to_post, + classes: RTL.new(user).css_class + } + ) + end end email_opts = { topic_title: title, - topic_title_url_encoded: URI.encode(title), + topic_title_url_encoded: title ? URI.encode(title) : title, message: message, url: post.url, post_id: post.id, @@ -445,7 +453,6 @@ class UserNotifications < ActionMailer::Base private_reply: post.topic.private_message?, include_respond_instructions: !(user.suspended? || user.staged?), template: template, - html_override: html, site_description: SiteSetting.site_description, site_title: SiteSetting.title, site_title_url_encoded: URI.encode(SiteSetting.title), @@ -453,6 +460,10 @@ class UserNotifications < ActionMailer::Base locale: locale } + unless translation_override_exists + email_opts[:html_override] = html + end + # If we have a display name, change the from address email_opts[:from_alias] = from_alias if from_alias.present? diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index d24d76588f..3cd4ca74b4 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -471,7 +471,7 @@ describe UserNotifications do data: {original_username: username}.to_json ) end - describe '.user_mentioned' do + describe 'email building' do it "has a username" do expects_build_with(has_entry(:username, username)) end @@ -484,6 +484,10 @@ describe UserNotifications do expects_build_with(has_entry(:template, "user_notifications.#{mail_type}")) end + it "overrides the html part" do + expects_build_with(has_key(:html_override)) + end + it "has a message" do expects_build_with(has_key(:message)) end @@ -524,6 +528,18 @@ describe UserNotifications do User.any_instance.stubs(:suspended?).returns(true) expects_build_with(has_entry(:include_respond_instructions, false)) end + + context "when customized" do + let(:custom_body) { "You are now officially notified." } + + before do + TranslationOverride.upsert!("en", "user_notifications.user_#{notification_type}.text_body_template", custom_body) + end + + it "shouldn't use the default html_override" do + expects_build_with(Not(has_key(:html_override))) + end + end end end From 19ad1e2c2e9b395a4c66989056cf736cc6429ac9 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 14:57:43 -0500 Subject: [PATCH 20/95] less opacity for quote button --- app/assets/stylesheets/common/base/topic-post.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 4c82cb4f8e..da4b50c2e6 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -141,7 +141,7 @@ aside.quote { color: dark-light-choose($secondary, $primary); padding: 10px; z-index: 401; - opacity: 0.8; + opacity: 0.9; &:before { font-family: "FontAwesome"; From f13367cecda6ef329f42b3980dcd2178a43dbe1d Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 15:17:51 -0500 Subject: [PATCH 21/95] FIX: latest + category not respecting homepage category suppression --- app/controllers/categories_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index d11a8ad57b..221375fbbc 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -57,7 +57,8 @@ class CategoriesController < ApplicationController topic_options = { per_page: SiteSetting.categories_topics, - no_definitions: true + no_definitions: true, + exclude_category_ids: Category.where(suppress_from_homepage: true).pluck(:id) } result = CategoryAndTopicLists.new From 16593ae8bf905def7a88d8e4b8d42e7fc2ea504b Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 15:45:48 -0500 Subject: [PATCH 22/95] FEATURE: log reason staff auto blocks a user --- app/services/user_blocker.rb | 2 +- config/locales/server.en.yml | 2 ++ lib/new_post_manager.rb | 6 ++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/services/user_blocker.rb b/app/services/user_blocker.rb index 01b5896e88..fe0091425e 100644 --- a/app/services/user_blocker.rb +++ b/app/services/user_blocker.rb @@ -20,7 +20,7 @@ class UserBlocker message_type = @opts[:message] || :blocked_by_staff post = SystemMessage.create(@user, message_type) if post && @by_user - StaffActionLogger.new(@by_user).log_block_user(@user, {context: "#{message_type}: '#{post.topic&.title rescue ''}'"}) + StaffActionLogger.new(@by_user).log_block_user(@user, {context: "#{message_type}: '#{post.topic&.title rescue ''}' #{@opts[:reason]}"}) end end else diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f68d72b22e..f3042f594b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1640,6 +1640,8 @@ en: deactivated: "Was deactivated due to too many bounced emails to '%{email}'." deactivated_by_staff: "Deactivated by staff" activated_by_staff: "Activated by staff" + new_user_typed_too_fast: "New user typed too fast" + content_matches_auto_block_regex: "Content matches auto block regex" username: short: "must be at least %{min} characters" long: "must be no more than %{max} characters" diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 27aebae663..537c7faff0 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -104,8 +104,10 @@ class NewPostManager result = manager.enqueue('default') - if is_fast_typer?(manager) || matches_auto_block_regex?(manager) - UserBlocker.block(manager.user, Discourse.system_user, keep_posts: true) + if is_fast_typer?(manager) + UserBlocker.block(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.new_user_typed_too_fast")) + elsif matches_auto_block_regex?(manager) + UserBlocker.block(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.content_matches_auto_block_regex")) end result From 6ebddc42d1b195ff3c3205b67d73c5ad550e5362 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2017 15:58:47 -0500 Subject: [PATCH 23/95] FIX: include children categories when searching a category --- lib/search.rb | 3 ++- spec/components/search_spec.rb | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index 4242ab78c6..7c055e0a40 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -665,7 +665,8 @@ class Search end elsif @search_context.is_a?(Category) - posts = posts.where("topics.category_id = #{@search_context.id}") + category_ids = [@search_context.id] + Category.where(parent_category_id: @search_context.id).pluck(:id) + posts = posts.where("topics.category_id in (?)", category_ids) elsif @search_context.is_a?(Topic) posts = posts.where("topics.id = #{@search_context.id}") .order("posts.post_number") diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 4be4c155e7..0cf29c59aa 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -397,12 +397,17 @@ describe Search do topic = Fabricate(:topic, category: category) topic_no_cat = Fabricate(:topic) + # includes subcategory in search + subcategory = Fabricate(:category, parent_category_id: category.id) + sub_topic = Fabricate(:topic, category: subcategory) + post = Fabricate(:post, topic: topic, user: topic.user ) _another_post = Fabricate(:post, topic: topic_no_cat, user: topic.user ) + sub_post = Fabricate(:post, raw: 'I am saying hello from a subcategory', topic: sub_topic, user: topic.user ) search = Search.execute('hello', search_context: category) - expect(search.posts.length).to eq(1) - expect(search.posts.first.id).to eq(post.id) + expect(search.posts.map(&:id).sort).to eq([post.id,sub_post.id].sort) + expect(search.posts.length).to eq(2) end end From 4d4a1a1552444ff1d6cb47d019950ad01e72547f Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Sat, 11 Mar 2017 14:25:09 +0800 Subject: [PATCH 24/95] Add scope for human users. --- app/models/about.rb | 4 ++-- app/models/user.rb | 4 +++- spec/models/user_spec.rb | 8 ++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/models/about.rb b/app/models/about.rb index a59a73a019..0d670183ba 100644 --- a/app/models/about.rb +++ b/app/models/about.rb @@ -35,12 +35,12 @@ class About def moderators @moderators ||= User.where(moderator: true, admin: false) - .where("id > 0") + .human_users .order(:username_lower) end def admins - @admins ||= User.where(admin: true).where("id > 0").order(:username_lower) + @admins ||= User.where(admin: true).human_users.order(:username_lower) end def stats diff --git a/app/models/user.rb b/app/models/user.rb index f6c297da9b..de36555acd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -124,8 +124,10 @@ class User < ActiveRecord::Base # set to true to optimize creation and save for imports attr_accessor :import_mode + scope :human_users, -> { where('users.id > 0') } + # excluding fake users like the system user or anonymous users - scope :real, -> { where('users.id > 0').where('NOT EXISTS( + scope :real, -> { human_users.where('NOT EXISTS( SELECT 1 FROM user_custom_fields ucf WHERE diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f52089126b..dfee88c7fb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1485,4 +1485,12 @@ describe User do end + describe '.human_users' do + it 'should only return users with a positive primary key' do + Fabricate(:user, id: -2) + user = Fabricate(:user) + + expect(User.human_users).to eq([user]) + end + end end From 848120c0985b5a98a5f89908efefb7a90a915dbc Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Mon, 13 Mar 2017 14:24:10 +0530 Subject: [PATCH 25/95] FEATURE: RSS feed for top page period filters --- app/controllers/list_controller.rb | 18 ++++++++-- config/locales/server.en.yml | 6 ++++ config/routes.rb | 1 + spec/controllers/list_controller_spec.rb | 44 ++++++++++++++++++++++-- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 431b68f143..91eeeab5b2 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -46,6 +46,7 @@ class ListController < ApplicationController :parent_category_category_top, # top pages (ie. with a period) TopTopic.periods.map { |p| :"top_#{p}" }, + TopTopic.periods.map { |p| :"top_#{p}_feed" }, TopTopic.periods.map { |p| :"category_top_#{p}" }, TopTopic.periods.map { |p| :"category_none_top_#{p}" }, TopTopic.periods.map { |p| :"parent_category_category_top_#{p}" }, @@ -168,7 +169,7 @@ class ListController < ApplicationController @link = "#{Discourse.base_url}/top" @atom_link = "#{Discourse.base_url}/top.rss" @description = I18n.t("rss_description.top") - @topic_list = TopicQuery.new(nil).list_top_for("monthly") + @topic_list = TopicQuery.new(nil).list_top_for(SiteSetting.top_page_default_timeframe.to_sym) render 'list', formats: [:rss] end @@ -232,7 +233,7 @@ class ListController < ApplicationController list.for_period = period list.more_topics_url = construct_url_with(:next, top_options) list.prev_topics_url = construct_url_with(:prev, top_options) - @rss = "top" + @rss = "top_#{period}" if use_crawler_layout? @title = I18n.t("js.filters.top.#{period}.title") @@ -252,6 +253,19 @@ class ListController < ApplicationController define_method("parent_category_category_top_#{period}") do self.send("top_#{period}", category: @category.id) end + + # rss feed + define_method("top_#{period}_feed") do |options = nil| + discourse_expires_in 1.minute + + @description = I18n.t("rss_description.top_#{period}") + @title = "#{SiteSetting.title} - #{@description}" + @link = "#{Discourse.base_url}/top/#{period}" + @atom_link = "#{Discourse.base_url}/top/#{period}.rss" + @topic_list = TopicQuery.new(nil).list_top_for(period) + + render 'list', formats: [:rss] + end end protected diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f3042f594b..aa6423dcce 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -229,6 +229,12 @@ en: latest: "Latest topics" hot: "Hot topics" top: "Top topics" + top_all: "All time top topics" + top_yearly: "Yearly top topics" + top_quarterly: "Quarterly top topics" + top_monthly: "Monthly top topics" + top_weekly: "Weekly top topics" + top_daily: "Daily top topics" posts: "Latest posts" private_posts: "Latest private messages" group_posts: "Latest posts from %{group_name}" diff --git a/config/routes.rb b/config/routes.rb index 7c42b21eb7..b62487d368 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -503,6 +503,7 @@ Discourse::Application.routes.draw do get "category_hashtags/check" => "category_hashtags#check" TopTopic.periods.each do |period| + get "top/#{period}.rss" => "list#top_#{period}_feed", format: :rss get "top/#{period}" => "list#top_#{period}" get "c/:category/l/top/#{period}" => "list#category_top_#{period}", as: "category_top_#{period}" get "c/:category/none/l/top/#{period}" => "list#category_none_top_#{period}", as: "category_none_top_#{period}" diff --git a/spec/controllers/list_controller_spec.rb b/spec/controllers/list_controller_spec.rb index db81150915..5a4a94d800 100644 --- a/spec/controllers/list_controller_spec.rb +++ b/spec/controllers/list_controller_spec.rb @@ -49,13 +49,53 @@ describe ListController do end describe 'RSS feeds' do - - it 'renders RSS' do + it 'renders latest RSS' do get "latest_feed", format: :rss expect(response).to be_success expect(response.content_type).to eq('application/rss+xml') end + it 'renders top RSS' do + get "top_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders all time top RSS' do + get "top_all_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders yearly top RSS' do + get "top_yearly_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders quarterly top RSS' do + get "top_quarterly_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders monthly top RSS' do + get "top_monthly_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders weekly top RSS' do + get "top_weekly_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end + + it 'renders daily top RSS' do + get "top_daily_feed", format: :rss + expect(response).to be_success + expect(response.content_type).to eq('application/rss+xml') + end end context 'category' do From dd60cb82c38b1dbf02251e6d4301f661ecf87630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 13 Mar 2017 11:31:37 +0100 Subject: [PATCH 26/95] UX: add client-side rate limit on click counters --- .../javascripts/discourse/lib/click-track.js.es6 | 11 ++++++++--- test/javascripts/lib/click-track-test.js.es6 | 12 ++++++------ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/click-track.js.es6 b/app/assets/javascripts/discourse/lib/click-track.js.es6 index f5f9119be5..0c7fa82727 100644 --- a/app/assets/javascripts/discourse/lib/click-track.js.es6 +++ b/app/assets/javascripts/discourse/lib/click-track.js.es6 @@ -46,12 +46,17 @@ export default { // Update badge clicks unless it's our own if (!ownLink) { - var $badge = $('span.badge', $link); + const $badge = $('span.badge', $link); if ($badge.length === 1) { // don't update counts in category badge nor in oneboxes (except when we force it) if (isValidLink($link)) { - var html = $badge.html(); - if (/^\d+$/.test(html)) { $badge.html(parseInt(html, 10) + 1); } + const html = $badge.html(); + const d = new Date(); + const key = `${d.getFullYear()}-${d.getMonth()}-${d.getDay()}-${postId}-${href}`; + if (/^\d+$/.test(html) && !sessionStorage.getItem(key)) { + sessionStorage.setItem(key, true); + $badge.html(parseInt(html, 10) + 1); + } } } } diff --git a/test/javascripts/lib/click-track-test.js.es6 b/test/javascripts/lib/click-track-test.js.es6 index bebe653d6f..dfe4f83c3a 100644 --- a/test/javascripts/lib/click-track-test.js.es6 +++ b/test/javascripts/lib/click-track-test.js.es6 @@ -19,14 +19,14 @@ module("lib:click-track", { `
google.com - google.com - google.com1 - google.com1 + google.fr + google.de1 + google.es1 - google.com + google.com.br forum log.txt #hashtag From 7ebfa3c901b75aee2e294819bfc0aaddaeebcd84 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 13 Mar 2017 19:19:42 +0800 Subject: [PATCH 27/95] SECURITY: Only allow users to resend activation email with a valid session. * Improve error when an active user tries to request for an activation email. --- .../controllers/not-activated.js.es6 | 11 ++++++--- app/controllers/session_controller.rb | 4 ++++ app/controllers/users_controller.rb | 18 ++++++++++++--- config/locales/server.en.yml | 1 + spec/controllers/users_controller_spec.rb | 23 +++++++++++++++++++ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 index 1c38cc58e5..86afe2ddd7 100644 --- a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 +++ b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 @@ -1,4 +1,5 @@ import { ajax } from 'discourse/lib/ajax'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; import ModalFunctionality from 'discourse/mixins/modal-functionality'; export default Ember.Controller.extend(ModalFunctionality, { @@ -9,9 +10,13 @@ export default Ember.Controller.extend(ModalFunctionality, { }, actions: { - sendActivationEmail: function() { - ajax('/users/action/send_activation_email', {data: {username: this.get('username')}, type: 'POST'}); - this.set('emailSent', true); + sendActivationEmail() { + ajax('/users/action/send_activation_email', { + data: { username: this.get('username') }, + type: 'POST' + }).then(() => { + this.set('emailSent', true); + }).catch(popupAjaxError); } } diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 0a18b3c539..8d4b2250bf 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -6,6 +6,8 @@ class SessionController < ApplicationController skip_before_filter :redirect_to_login_if_required skip_before_filter :preload_json, :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider', 'destroy'] + ACTIVATE_USER_KEY = "activate_user" + def csrf render json: {csrf: form_authenticity_token } end @@ -276,6 +278,7 @@ class SessionController < ApplicationController end def not_activated(user) + session[ACTIVATE_USER_KEY] = user.username render json: { error: I18n.t("login.not_activated"), reason: 'not_activated', @@ -303,6 +306,7 @@ class SessionController < ApplicationController end def login(user) + session.delete(ACTIVATE_USER_KEY) log_on_user(user) if payload = session.delete(:sso_payload) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b06e6721ef..0e429e8c8c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -567,13 +567,25 @@ class UsersController < ApplicationController RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed! end + if (current_user && !current_user.staff?) || + (params[:username] != session[SessionController::ACTIVATE_USER_KEY]) + + raise Discourse::InvalidAccess + end + @user = User.find_by_username_or_email(params[:username].to_s) raise Discourse::NotFound unless @user - @email_token = @user.email_tokens.unconfirmed.active.first - enqueue_activation_email if @user - render nothing: true + session.delete(SessionController::ACTIVATE_USER_KEY) + + if @user.active + render_json_error(I18n.t('activation.activated'), status: 409) + elsif @user + @email_token = @user.email_tokens.unconfirmed.active.first + enqueue_activation_email + render nothing: true + end end def enqueue_activation_email diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index aa6423dcce..1619309dd9 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -614,6 +614,7 @@ en: welcome_to: "Welcome to %{site_name}!" approval_required: "A moderator must manually approve your new account before you can access this forum. You'll get an email when your account is approved!" missing_session: "We cannot detect if your account was created, please ensure you have cookies enabled." + activated: "Sorry, this account has already been activated." post_action_types: off_topic: title: 'Off-Topic' diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 746d26a071..4bb4c871bc 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1403,10 +1403,29 @@ describe UsersController do context 'for an existing user' do let(:user) { Fabricate(:user, active: false) } + context 'for an activated account' do + it 'fails' do + active_user = Fabricate(:user, active: true) + session[SessionController::ACTIVATE_USER_KEY] = active_user.username + xhr :post, :send_activation_email, username: active_user.username + + expect(response.status).to eq(409) + + expect(JSON.parse(response.body)['errors']).to include(I18n.t( + 'activation.activated' + )) + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) + end + end + context 'with a valid email_token' do it 'should send the activation email' do + session["activate_user"] = user.username Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) end end @@ -1418,13 +1437,17 @@ describe UsersController do it 'should generate a new token' do expect { + session["activate_user"] = user.username xhr :post, :send_activation_email, username: user.username }.to change{ user.email_tokens(true).count }.by(1) end it 'should send an email' do + session["activate_user"] = user.username Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username + + expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) end end end From 9364d8ce71c76814d0982769375be71545aa7710 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 13 Mar 2017 20:20:25 +0800 Subject: [PATCH 28/95] FIX: Store user's id instead for sending activation email. * Email and username are both allowed to be used for logging in. Therefore, it is easier to just store the user's id rather than to store the username and email in the session. --- app/controllers/session_controller.rb | 2 +- app/controllers/users_controller.rb | 14 +++++++------- spec/controllers/users_controller_spec.rb | 17 +++++++++++++---- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 8d4b2250bf..f06022cf98 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -278,7 +278,7 @@ class SessionController < ApplicationController end def not_activated(user) - session[ACTIVATE_USER_KEY] = user.username + session[ACTIVATE_USER_KEY] = user.id render json: { error: I18n.t("login.not_activated"), reason: 'not_activated', diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0e429e8c8c..a06c6c376b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -567,21 +567,21 @@ class UsersController < ApplicationController RateLimiter.new(nil, "activate-min-#{request.remote_ip}", 6, 1.minute).performed! end - if (current_user && !current_user.staff?) || - (params[:username] != session[SessionController::ACTIVATE_USER_KEY]) - - raise Discourse::InvalidAccess - end - @user = User.find_by_username_or_email(params[:username].to_s) raise Discourse::NotFound unless @user + if (current_user && !current_user.staff?) || + @user.id != session[SessionController::ACTIVATE_USER_KEY] + + raise Discourse::InvalidAccess + end + session.delete(SessionController::ACTIVATE_USER_KEY) if @user.active render_json_error(I18n.t('activation.activated'), status: 409) - elsif @user + else @user @email_token = @user.email_tokens.unconfirmed.active.first enqueue_activation_email render nothing: true diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 4bb4c871bc..e98612fe44 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1406,7 +1406,7 @@ describe UsersController do context 'for an activated account' do it 'fails' do active_user = Fabricate(:user, active: true) - session[SessionController::ACTIVATE_USER_KEY] = active_user.username + session[SessionController::ACTIVATE_USER_KEY] = active_user.id xhr :post, :send_activation_email, username: active_user.username expect(response.status).to eq(409) @@ -1419,9 +1419,18 @@ describe UsersController do end end + describe 'when user does not have a valid session' do + it 'should not be valid' do + user = Fabricate(:user) + xhr :post, :send_activation_email, username: user.username + + expect(response.status).to eq(403) + end + end + context 'with a valid email_token' do it 'should send the activation email' do - session["activate_user"] = user.username + session[SessionController::ACTIVATE_USER_KEY] = user.id Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username @@ -1437,13 +1446,13 @@ describe UsersController do it 'should generate a new token' do expect { - session["activate_user"] = user.username + session[SessionController::ACTIVATE_USER_KEY] = user.id xhr :post, :send_activation_email, username: user.username }.to change{ user.email_tokens(true).count }.by(1) end it 'should send an email' do - session["activate_user"] = user.username + session[SessionController::ACTIVATE_USER_KEY] = user.id Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) xhr :post, :send_activation_email, username: user.username From 82ca0e368e8758b463c0c589f649f9bb73fc2d79 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Mar 2017 10:02:20 -0400 Subject: [PATCH 29/95] FEATURE: stop escaping special chars in title prettify This feature is confusting and just leads to inconsistency --- lib/html_prettify.rb | 19 ------------------- spec/components/html_prettify_spec.rb | 2 ++ 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/lib/html_prettify.rb b/lib/html_prettify.rb index 6d0b3d065b..39a8ca7d30 100644 --- a/lib/html_prettify.rb +++ b/lib/html_prettify.rb @@ -120,9 +120,6 @@ class HtmlPrettify < String unless in_pre t.gsub!("'", "'") - - t = process_escapes t - t.gsub!(""", '"') if do_dashes @@ -176,22 +173,6 @@ class HtmlPrettify < String protected - # Return the string, with after processing the following backslash - # escape sequences. This is useful if you want to force a "dumb" quote - # or other character to appear. - # - # Escaped are: - # \\ \" \' \. \- \` - # - def process_escapes(str) - str = str.gsub('\\\\', '\') - str.gsub!('\"', '"') - str.gsub!("\\\'", ''') - str.gsub!('\.', '.') - str.gsub!('\-', '-') - str.gsub!('\`', '`') - str - end # The string, with each instance of "--" translated to an # em-dash HTML entity. diff --git a/spec/components/html_prettify_spec.rb b/spec/components/html_prettify_spec.rb index 1f1b6fad2c..189bfec8b3 100644 --- a/spec/components/html_prettify_spec.rb +++ b/spec/components/html_prettify_spec.rb @@ -24,6 +24,8 @@ describe HtmlPrettify do t 'src="test.png"> yay', "src=“test.png”> yay" + t '\\\\mnt\\c', "\\\\mnt\\c" + t ERB::Util.html_escape(' yay'), "<img src=“test.png”> yay" end From ef24fd54ba1d8ceba1915f04b1cd82047dcbc038 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Mar 2017 10:19:02 -0400 Subject: [PATCH 30/95] FEATUE: automatically validate token is stored in redis This ensures we have some handling for redis flushall We attempt to recover our in-memory session token once every 30 seconds Code is careful to only set the token if it is nil, to allow for manual cycling to remain safe if needed --- app/models/global_setting.rb | 14 ++++++++++++++ spec/models/global_setting_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index bf570ed72d..a3ad8e4bee 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -11,6 +11,8 @@ class GlobalSetting # for legacy reasons REDIS_SECRET_KEY = 'SECRET_TOKEN' + REDIS_VALIDATE_SECONDS = 30 + # In Rails secret_key_base is used to encrypt the cookie store # the cookie store contains session data # Discourse also uses this secret key to digest user auth tokens @@ -19,9 +21,21 @@ class GlobalSetting # - generate a token on the fly if needed and cache in redis # - enforce rules about token format falling back to redis if needed def self.safe_secret_key_base + + if @safe_secret_key_base && @token_in_redis && (@token_last_validated + REDIS_VALIDATE_SECONDS) < Time.now + token = $redis.without_namespace.get(REDIS_SECRET_KEY) + if token.nil? + $redis.without_namespace.set(REDIS_SECRET_KEY, @safe_secret_key_base) + end + end + @safe_secret_key_base ||= begin token = secret_key_base if token.blank? || token !~ VALID_SECRET_KEY + + @token_in_redis = true + @token_last_validated = Time.now + token = $redis.without_namespace.get(REDIS_SECRET_KEY) unless token && token =~ VALID_SECRET_KEY token = SecureRandom.hex(64) diff --git a/spec/models/global_setting_spec.rb b/spec/models/global_setting_spec.rb index 7be0db648f..d3371ce13f 100644 --- a/spec/models/global_setting_spec.rb +++ b/spec/models/global_setting_spec.rb @@ -2,6 +2,27 @@ require 'rails_helper' require 'tempfile' describe GlobalSetting do + + describe '.safe_secret_key_base' do + it 'sets redis token if it is somehow flushed after 30 seconds' do + token = GlobalSetting.safe_secret_key_base + $redis.without_namespace.del(GlobalSetting::REDIS_SECRET_KEY) + freeze_time 20.seconds.from_now + + GlobalSetting.safe_secret_key_base + new_token = $redis.without_namespace.get(GlobalSetting::REDIS_SECRET_KEY) + expect(new_token).to eq(nil) + + freeze_time 31.seconds.from_now + + GlobalSetting.safe_secret_key_base + + new_token = $redis.without_namespace.get(GlobalSetting::REDIS_SECRET_KEY) + expect(new_token).to eq(token) + + end + end + describe '.redis_config' do describe 'when slave config is not present' do it "should not set any connector" do From 1a745ca16aaeaa53743226e70ef59535badc35d6 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Mar 2017 10:22:23 -0400 Subject: [PATCH 31/95] else @user makes no sense :) --- app/controllers/users_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a06c6c376b..a643264bdc 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -581,7 +581,7 @@ class UsersController < ApplicationController if @user.active render_json_error(I18n.t('activation.activated'), status: 409) - else @user + else @email_token = @user.email_tokens.unconfirmed.active.first enqueue_activation_email render nothing: true From a690121805df163bb3c1c560515151bb6f3e173c Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Mar 2017 10:32:24 -0400 Subject: [PATCH 32/95] SECURITY: always allow staff to resend activation mails --- app/controllers/users_controller.rb | 2 +- spec/controllers/users_controller_spec.rb | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a643264bdc..82276f6af4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -571,7 +571,7 @@ class UsersController < ApplicationController raise Discourse::NotFound unless @user - if (current_user && !current_user.staff?) || + if !current_user&.staff? && @user.id != session[SessionController::ACTIVATE_USER_KEY] raise Discourse::InvalidAccess diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index e98612fe44..cc8d418516 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1423,9 +1423,15 @@ describe UsersController do it 'should not be valid' do user = Fabricate(:user) xhr :post, :send_activation_email, username: user.username - expect(response.status).to eq(403) end + + it 'should allow staff regardless' do + log_in :admin + user = Fabricate(:user, active: false) + xhr :post, :send_activation_email, username: user.username + expect(response.status).to eq(200) + end end context 'with a valid email_token' do From 64680286f4d5f9d57eb443cd72d1d3ca10d754e5 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 13 Mar 2017 10:47:43 -0400 Subject: [PATCH 33/95] correct logic, so revalidation is reset correct test so it can run at any point --- app/models/global_setting.rb | 1 + spec/models/global_setting_spec.rb | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index a3ad8e4bee..8ecfd75004 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -23,6 +23,7 @@ class GlobalSetting def self.safe_secret_key_base if @safe_secret_key_base && @token_in_redis && (@token_last_validated + REDIS_VALIDATE_SECONDS) < Time.now + @token_last_validated = Time.now token = $redis.without_namespace.get(REDIS_SECRET_KEY) if token.nil? $redis.without_namespace.set(REDIS_SECRET_KEY, @safe_secret_key_base) diff --git a/spec/models/global_setting_spec.rb b/spec/models/global_setting_spec.rb index d3371ce13f..bc1ea0303f 100644 --- a/spec/models/global_setting_spec.rb +++ b/spec/models/global_setting_spec.rb @@ -1,19 +1,31 @@ require 'rails_helper' require 'tempfile' +class GlobalSetting + def self.reset_secret_key_base! + @safe_secret_key_base = nil + end +end + describe GlobalSetting do describe '.safe_secret_key_base' do it 'sets redis token if it is somehow flushed after 30 seconds' do + + # we have to reset so we reset all times and test runs consistently + GlobalSetting.reset_secret_key_base! + + freeze_time Time.now + token = GlobalSetting.safe_secret_key_base $redis.without_namespace.del(GlobalSetting::REDIS_SECRET_KEY) - freeze_time 20.seconds.from_now + freeze_time Time.now + 20 GlobalSetting.safe_secret_key_base new_token = $redis.without_namespace.get(GlobalSetting::REDIS_SECRET_KEY) expect(new_token).to eq(nil) - freeze_time 31.seconds.from_now + freeze_time Time.now + 11 GlobalSetting.safe_secret_key_base From 15f7fff561f41a819a06921e0ecc74b2c61ff6ad Mon Sep 17 00:00:00 2001 From: cpradio Date: Mon, 13 Mar 2017 08:34:12 -0400 Subject: [PATCH 34/95] UX: Add subcategory class to hamburger menu items that are subcategories UX: Add data-category-url to make targetting a category li element in the hamburger menu easier --- .../discourse/widgets/hamburger-categories.js.es6 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/assets/javascripts/discourse/widgets/hamburger-categories.js.es6 b/app/assets/javascripts/discourse/widgets/hamburger-categories.js.es6 index 37bdae69fb..15e3dc9b3f 100644 --- a/app/assets/javascripts/discourse/widgets/hamburger-categories.js.es6 +++ b/app/assets/javascripts/discourse/widgets/hamburger-categories.js.es6 @@ -6,6 +6,12 @@ createWidget('hamburger-category', { tagName: 'li.category-link', html(c) { + if (c.parent_category_id) { + this.tagName += '.subcategory'; + } + + this.tagName += '.category-' + Discourse.Category.slugFor(c, '-'); + const results = [ this.attach('category-link', { category: c, allowUncategorized: true }) ]; const unreadTotal = parseInt(c.get('unreadTopics'), 10) + parseInt(c.get('newTopics'), 10); From 30d5d611587c87b676f0204b3866bc5334fe403e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 13 Mar 2017 16:11:49 +0100 Subject: [PATCH 35/95] use 'toLocaleDateString()' --- app/assets/javascripts/discourse/lib/click-track.js.es6 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/click-track.js.es6 b/app/assets/javascripts/discourse/lib/click-track.js.es6 index 0c7fa82727..ff9cb9a862 100644 --- a/app/assets/javascripts/discourse/lib/click-track.js.es6 +++ b/app/assets/javascripts/discourse/lib/click-track.js.es6 @@ -51,8 +51,7 @@ export default { // don't update counts in category badge nor in oneboxes (except when we force it) if (isValidLink($link)) { const html = $badge.html(); - const d = new Date(); - const key = `${d.getFullYear()}-${d.getMonth()}-${d.getDay()}-${postId}-${href}`; + const key = `${new Date().toLocaleDateString()}-${postId}-${href}`; if (/^\d+$/.test(html) && !sessionStorage.getItem(key)) { sessionStorage.setItem(key, true); $badge.html(parseInt(html, 10) + 1); From 60dc53153186a7c26491b0710f1f9ced8140ea38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 13 Mar 2017 16:31:41 +0100 Subject: [PATCH 36/95] bump onebox --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 34145fb1a5..02733f779f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -206,7 +206,7 @@ GEM omniauth-twitter (1.3.0) omniauth-oauth (~> 1.1) rack - onebox (1.8.2) + onebox (1.8.3) fast_blank (>= 1.0.0) htmlentities (~> 4.3.4) moneta (~> 0.8) From 6d7e968e304b0d53d4c8ef1cfada45ebddc391ff Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 8 Mar 2017 11:31:30 -0500 Subject: [PATCH 37/95] FEATURE: box-style rendering of sub-categories --- .../components/categories-boxes.js.es6 | 11 +++ .../components/edit-category-panel.js.es6 | 7 +- .../components/edit-category-settings.js.es6 | 28 +++++--- .../controllers/discovery/categories.js.es6 | 17 ++++- .../discourse/models/category.js.es6 | 3 +- .../templates/components/categories-boxes.hbs | 16 +++++ .../components/edit-category-settings.hbs | 71 +++++++++++-------- app/assets/stylesheets/common/base/modal.scss | 10 +++ .../stylesheets/desktop/category-list.scss | 57 +++++++++++++++ app/controllers/categories_controller.rb | 8 ++- app/models/category.rb | 4 ++ app/serializers/basic_category_serializer.rb | 3 +- config/locales/client.en.yml | 9 ++- ...dd_subcategory_list_style_to_categories.rb | 14 ++++ .../acceptance/category-edit-test.js.es6 | 25 +++++++ .../acceptance/topic-discovery-test.js.es6 | 7 ++ .../fixtures/discovery_fixtures.js.es6 | 1 + .../javascripts/fixtures/site-fixtures.js.es6 | 61 ++++++++++++---- 18 files changed, 286 insertions(+), 66 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/categories-boxes.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/categories-boxes.hbs create mode 100644 db/migrate/20170308201552_add_subcategory_list_style_to_categories.rb diff --git a/app/assets/javascripts/discourse/components/categories-boxes.js.es6 b/app/assets/javascripts/discourse/components/categories-boxes.js.es6 new file mode 100644 index 0000000000..c4603a8235 --- /dev/null +++ b/app/assets/javascripts/discourse/components/categories-boxes.js.es6 @@ -0,0 +1,11 @@ +import computed from 'ember-addons/ember-computed-decorators'; + +export default Ember.Component.extend({ + tagName: "section", + classNameBindings: [':category-boxes', 'anyLogos:with-logos:no-logos'], + + @computed('categories.[].uploaded_logo.url') + anyLogos() { + return this.get("categories").any((c) => { return !Ember.isEmpty(c.get('uploaded_logo.url')); }); + } +}); diff --git a/app/assets/javascripts/discourse/components/edit-category-panel.js.es6 b/app/assets/javascripts/discourse/components/edit-category-panel.js.es6 index 5cf7a0e1f8..470dc5237f 100644 --- a/app/assets/javascripts/discourse/components/edit-category-panel.js.es6 +++ b/app/assets/javascripts/discourse/components/edit-category-panel.js.es6 @@ -1,11 +1,10 @@ -const EditCategoryPanel = Ember.Component.extend({ - classNameBindings: [':modal-tab', 'activeTab::invisible'], -}); +const EditCategoryPanel = Ember.Component.extend({}); export default EditCategoryPanel; export function buildCategoryPanel(tab, extras) { return EditCategoryPanel.extend({ - activeTab: Ember.computed.equal('selectedTab', tab) + activeTab: Ember.computed.equal('selectedTab', tab), + classNameBindings: [':modal-tab', 'activeTab::invisible', `:edit-category-tab-${tab}`] }, extras || {}); } diff --git a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 index f1b089ac1a..4a384fe585 100644 --- a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 +++ b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 @@ -5,9 +5,27 @@ import computed from "ember-addons/ember-computed-decorators"; export default buildCategoryPanel('settings', { emailInEnabled: setting('email_in'), showPositionInput: setting('fixed_category_positions'), - + isParentCategory: Em.computed.empty('category.parent_category_id'), + showSubcategoryListStyle: Em.computed.and('category.show_subcategory_list', 'isParentCategory'), isDefaultSortOrder: Em.computed.empty('category.sort_order'), + @computed + availableSubcategoryListStyles() { + return [ + {name: I18n.t('category.subcategory_list_styles.rows'), value: 'rows'}, + {name: I18n.t('category.subcategory_list_styles.rows_with_featured_topics'), value: 'rows_with_featured_topics'}, + {name: I18n.t('category.subcategory_list_styles.boxes'), value: 'boxes'} + ]; + }, + + @computed + availableViews() { + return [ + {name: I18n.t('filters.latest.title'), value: 'latest'}, + {name: I18n.t('filters.top.title'), value: 'top'} + ]; + }, + @computed availableSorts() { return ['likes', 'op_likes', 'views', 'posts', 'activity', 'posters', 'category', 'created'] @@ -21,13 +39,5 @@ export default buildCategoryPanel('settings', { {name: I18n.t('category.sort_ascending'), value: 'true'}, {name: I18n.t('category.sort_descending'), value: 'false'} ]; - }, - - @computed - availableViews() { - return [ - {name: I18n.t('filters.latest.title'), value: 'latest'}, - {name: I18n.t('filters.top.title'), value: 'top'} - ]; } }); diff --git a/app/assets/javascripts/discourse/controllers/discovery/categories.js.es6 b/app/assets/javascripts/discourse/controllers/discovery/categories.js.es6 index 19d6ae6b80..c412ccebc4 100644 --- a/app/assets/javascripts/discourse/controllers/discovery/categories.js.es6 +++ b/app/assets/javascripts/discourse/controllers/discovery/categories.js.es6 @@ -19,7 +19,22 @@ export default DiscoveryController.extend({ @computed("model.parentCategory") categoryPageStyle(parentCategory) { - const style = this.siteSettings.desktop_category_page_style; + let style = this.siteSettings.desktop_category_page_style; + + if (parentCategory) { + switch(parentCategory.get('subcategory_list_style')) { + case 'rows': + style = "categories_only"; + break; + case 'rows_with_featured_topics': + style = "categories_with_featured_topics"; + break; + case 'boxes': + style = "categories_boxes"; + break; + } + } + const componentName = (parentCategory && style === "categories_and_latest_topics") ? "categories_only" : style; diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 1ac1a9ca0c..8550159ddd 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -105,7 +105,8 @@ const Category = RestModel.extend({ topic_featured_link_allowed: this.get('topic_featured_link_allowed'), show_subcategory_list: this.get('show_subcategory_list'), num_featured_topics: this.get('num_featured_topics'), - default_view: this.get('default_view') + default_view: this.get('default_view'), + subcategory_list_style: this.get('subcategory_list_style') }, type: id ? 'PUT' : 'POST' }); diff --git a/app/assets/javascripts/discourse/templates/components/categories-boxes.hbs b/app/assets/javascripts/discourse/templates/components/categories-boxes.hbs new file mode 100644 index 0000000000..0010294581 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/categories-boxes.hbs @@ -0,0 +1,16 @@ +{{#if categories}} + {{#each categories as |c|}} + + {{/each}} +{{/if}} diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs index d3e6a73d3a..e12846ea3c 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs @@ -19,14 +19,51 @@ -{{#unless category.parent_category_id}} -
+{{#if isParentCategory}} +
-{{/unless}} +{{/if}} + +{{#if showSubcategoryListStyle}} +
+ +
+{{/if}} + +
+ +
+ +
+ +
+ +
{{/if}} -
- -
- -
- -
- {{#if emailInEnabled}}