From bbaa3e9166648ca4e61b034b8df4b1a611608408 Mon Sep 17 00:00:00 2001 From: Kris Date: Thu, 31 Jan 2019 19:31:09 -0500 Subject: [PATCH 01/74] Header icon focus color fix --- app/assets/stylesheets/common/base/header.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/common/base/header.scss b/app/assets/stylesheets/common/base/header.scss index b31d29a4a3..5689ca6198 100644 --- a/app/assets/stylesheets/common/base/header.scss +++ b/app/assets/stylesheets/common/base/header.scss @@ -117,6 +117,9 @@ border-top: 1px solid transparent; border-left: 1px solid transparent; border-right: 1px solid transparent; + .d-icon { + color: $primary-medium; + } } &:active { color: $primary; From a84aaf197a4d2767b85e76cb2a9d06aaab944747 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 1 Feb 2019 13:26:08 +1100 Subject: [PATCH 02/74] DEV: correct heisentest testing for avatars If for some reason we created andupload with id 1 in the test then the test would fail. This can happen if this is the absolute first test to run on the db. Fix sets the upload to a legitimate which in turn means the last upload will not be upload id 1 and stops using id hard coding for the testing. --- spec/models/user_avatar_spec.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/spec/models/user_avatar_spec.rb b/spec/models/user_avatar_spec.rb index 7571377566..fd28ca4708 100644 --- a/spec/models/user_avatar_spec.rb +++ b/spec/models/user_avatar_spec.rb @@ -125,8 +125,10 @@ describe UserAvatar do end it 'can leave gravatar alone' do - user = Fabricate(:user, uploaded_avatar_id: 1) - user.user_avatar.update_columns(gravatar_upload_id: 1) + upload = Fabricate(:upload) + + user = Fabricate(:user, uploaded_avatar_id: upload.id) + user.user_avatar.update_columns(gravatar_upload_id: upload.id) stub_request(:get, "http://thisfakesomething.something.com/") .to_return(status: 200, body: file_from_fixtures("logo.png"), headers: {}) @@ -138,8 +140,12 @@ describe UserAvatar do end.to change { Upload.count }.by(1) user.reload - expect(user.uploaded_avatar_id).to eq(1) - expect(user.user_avatar.custom_upload_id).to eq(Upload.last.id) + expect(user.uploaded_avatar_id).to eq(upload.id) + + last_id = Upload.last.id + + expect(last_id).not_to eq(upload.id) + expect(user.user_avatar.custom_upload_id).to eq(last_id) end describe 'when avatar url returns an invalid status code' do From a1b4d9b06121d818e7b4175190b68734dcd69aaa Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 1 Feb 2019 12:30:48 +0800 Subject: [PATCH 03/74] DEV: Fix specs using deprecated site settings. --- spec/helpers/application_helper_spec.rb | 62 ++++++++++++++++++------- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index b769cadb3a..0ddf281380 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -233,28 +233,58 @@ describe ApplicationHelper do describe 'crawlable_meta_data' do context "opengraph image" do it 'returns the correct image' do - SiteSetting.default_opengraph_image_url = '/images/og-image.png' - SiteSetting.twitter_summary_large_image_url = '/images/twitter.png' - SiteSetting.large_icon_url = '/images/large_icon.png' - SiteSetting.apple_touch_icon_url = '/images/default-apple-touch-icon.png' - SiteSetting.logo_url = '/images/d-logo-sketch.png' + SiteSetting.opengraph_image = Fabricate(:upload, + url: '/images/og-image.png' + ) - expect(helper.crawlable_meta_data(image: "some-image.png")).to include("some-image.png") - expect(helper.crawlable_meta_data).to include("/images/og-image.png") + SiteSetting.twitter_summary_large_image = Fabricate(:upload, + url: '/images/twitter.png' + ) - SiteSetting.default_opengraph_image_url = '' - expect(helper.crawlable_meta_data).to include("/images/twitter.png") + SiteSetting.large_icon = Fabricate(:upload, + url: '/images/large_icon.png' + ) - SiteSetting.twitter_summary_large_image_url = '' - expect(helper.crawlable_meta_data).to include("/images/large_icon.png") + SiteSetting.apple_touch_icon = Fabricate(:upload, + url: '/images/default-apple-touch-icon.png' + ) - SiteSetting.large_icon_url = '' - expect(helper.crawlable_meta_data).to include("/images/default-apple-touch-icon.png") + SiteSetting.logo = Fabricate(:upload, url: '/images/d-logo-sketch.png') - SiteSetting.apple_touch_icon_url = '' - expect(helper.crawlable_meta_data).to include("/images/d-logo-sketch.png") + expect( + helper.crawlable_meta_data(image: "some-image.png") + ).to include("some-image.png") + + expect(helper.crawlable_meta_data).to include( + SiteSetting.opengraph_image_url + ) + + SiteSetting.opengraph_image = nil + + expect(helper.crawlable_meta_data).to include( + SiteSetting.site_twitter_summary_large_image_url + ) + + SiteSetting.twitter_summary_large_image = nil + + expect(helper.crawlable_meta_data).to include( + SiteSetting.site_large_icon_url + ) + + SiteSetting.large_icon = nil + + expect(helper.crawlable_meta_data).to include( + SiteSetting.site_apple_touch_icon_url + ) + + SiteSetting.apple_touch_icon = nil + SiteSetting.apple_touch_icon_url = nil + + expect(helper.crawlable_meta_data).to include(SiteSetting.site_logo_url) + + SiteSetting.logo = nil + SiteSetting.logo_url = nil - SiteSetting.logo_url = '' expect(helper.crawlable_meta_data).to_not include("/images") end end From b4f713ca523a3f684adf6e8b35b97138454dbf9e Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Fri, 1 Feb 2019 10:10:48 +0530 Subject: [PATCH 04/74] FEATURE: Use amazon s3 inventory to manage upload stats (#6867) --- app/jobs/regular/update_s3_inventory.rb | 17 ++ config/locales/server.en.yml | 2 + config/site_settings.yml | 2 + lib/backup_restore/s3_backup_store.rb | 4 +- lib/discourse.rb | 5 + lib/file_store/s3_store.rb | 14 +- lib/s3_helper.rb | 18 +- lib/s3_inventory.rb | 193 ++++++++++++++++++ lib/site_settings/validations.rb | 4 + .../components/migration/safe_migrate_spec.rb | 10 - spec/components/s3_inventory_spec.rb | 77 +++++++ spec/fixtures/csv/s3_inventory.csv | 3 + spec/jobs/update_s3_inventory_spec.rb | 50 +++++ spec/support/helpers.rb | 10 + 14 files changed, 388 insertions(+), 21 deletions(-) create mode 100644 app/jobs/regular/update_s3_inventory.rb create mode 100644 lib/s3_inventory.rb create mode 100644 spec/components/s3_inventory_spec.rb create mode 100644 spec/fixtures/csv/s3_inventory.csv create mode 100644 spec/jobs/update_s3_inventory_spec.rb diff --git a/app/jobs/regular/update_s3_inventory.rb b/app/jobs/regular/update_s3_inventory.rb new file mode 100644 index 0000000000..946ec47892 --- /dev/null +++ b/app/jobs/regular/update_s3_inventory.rb @@ -0,0 +1,17 @@ +require "s3_inventory" + +module Jobs + # if upload bucket changes or inventory bucket changes we want to update s3 bucket policy and inventory configuration + class UpdateS3Inventory < Jobs::Base + + def execute(args) + return unless SiteSetting.enable_s3_inventory? && SiteSetting.enable_s3_uploads? + + [:upload, :optimized].each do |type| + s3_inventory = S3Inventory.new(Discourse.store.s3_helper, type) + s3_inventory.update_bucket_policy if type == :upload + s3_inventory.update_bucket_inventory_configuration + end + end + end +end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ddea713c1c..2af15b9479 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -191,6 +191,7 @@ en: other: "You specified the invalid choices %{name}" default_categories_already_selected: "You cannot select a category used in another list." s3_upload_bucket_is_required: "You cannot enable uploads to S3 unless you've provided the 's3_upload_bucket'." + enable_s3_uploads_is_required: "You cannot enable inventory to S3 unless you've enabled the S3 uploads." s3_backup_requires_s3_settings: "You cannot use S3 as backup location unless you've provided the '%{setting_name}'." s3_bucket_reused: "You cannot use the same bucket for 's3_upload_bucket' and 's3_backup_bucket'. Choose a different bucket or use a different path for each bucket." conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to
https://meta.discourse.org/t/76575' @@ -1488,6 +1489,7 @@ en: s3_force_path_style: "Enforce path-style addressing for your custom endpoint. IMPORTANT: Required for using Minio uploads and backups." s3_configure_tombstone_policy: "Enable automatic deletion policy for tombstone uploads. IMPORTANT: If disabled, no space will be reclaimed after uploads are deleted." s3_disable_cleanup: "Disable the removal of backups from S3 when removed locally." + enable_s3_inventory: "Generate reports and verify uploads using Amazon S3 inventory. IMPORTANT: requires valid S3 credentials (both access key id & secret access key)." backup_time_of_day: "Time of day UTC when the backup should occur." backup_with_uploads: "Include uploads in scheduled backups. Disabling this will only backup the database." backup_location: "Location where backups are stored. IMPORTANT: S3 requires valid S3 credentials entered in Files settings." diff --git a/config/site_settings.yml b/config/site_settings.yml index a897c96854..17ae4a2618 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1050,6 +1050,8 @@ files: s3_configure_tombstone_policy: default: true shadowed_by_global: true + enable_s3_inventory: + default: false allow_profile_backgrounds: client: true default: true diff --git a/lib/backup_restore/s3_backup_store.rb b/lib/backup_restore/s3_backup_store.rb index 03edc014c3..fbbb18f1d7 100644 --- a/lib/backup_restore/s3_backup_store.rb +++ b/lib/backup_restore/s3_backup_store.rb @@ -32,9 +32,7 @@ module BackupRestore end def download_file(filename, destination_path, failure_message = nil) - unless @s3_helper.object(filename).download_file(destination_path) - raise failure_message&.to_s || "Failed to download file" - end + @s3_helper.download_file(filename, destination_path, failure_message) end def upload_file(filename, source_path, content_type) diff --git a/lib/discourse.rb b/lib/discourse.rb index c8d76f3e15..8f621b838c 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -463,6 +463,11 @@ module Discourse end end + DiscourseEvent.on(:site_setting_saved) do |site_setting| + name = site_setting.name.to_s + Jobs.enqueue(:update_s3_inventory) if name.include?("s3_inventory") || name == "s3_upload_bucket" + end + def self.current_user_provider @current_user_provider || Auth::DefaultCurrentUserProvider end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 119dd9c0a1..d4bb6647d2 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -124,8 +124,14 @@ module FileStore end def list_missing_uploads(skip_optimized: false) - list_missing(Upload, "original/") - list_missing(OptimizedImage, "optimized/") unless skip_optimized + if SiteSetting.enable_s3_inventory + require 's3_inventory' + S3Inventory.new(s3_helper, :upload).list_missing + S3Inventory.new(s3_helper, :optimized).list_missing unless skip_optimized + else + list_missing(Upload, "original/") + list_missing(OptimizedImage, "optimized/") unless skip_optimized + end end private @@ -140,7 +146,7 @@ module FileStore verified_ids = [] files.each do |f| - id = model.where("url LIKE '%#{f.key}'").pluck(:id).first if f.size > 0 + id = model.where("url LIKE '%#{f.key}' AND etag = '#{f.etag}'").pluck(:id).first verified_ids << id if id.present? marker = f.key end @@ -150,7 +156,7 @@ module FileStore files = @s3_helper.list(prefix, marker) end - missing_uploads = model.where("id NOT IN (SELECT val FROM verified_ids)") + missing_uploads = model.joins('LEFT JOIN verified_ids ON verified_ids.val = id').where("verified_ids.val IS NULL") missing_count = missing_uploads.count if missing_count > 0 diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index fbdde9674a..380ba86e5a 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -205,6 +205,20 @@ class S3Helper opts end + def download_file(filename, destination_path, failure_message = nil) + unless object(filename).download_file(destination_path) + raise failure_message&.to_s || "Failed to download file" + end + end + + def s3_client + @s3_client ||= Aws::S3::Client.new(@s3_options) + end + + def s3_inventory_path(path = 'inventory') + get_path_for_s3_upload(path) + end + private def default_s3_options @@ -228,10 +242,6 @@ class S3Helper File.join("uploads", RailsMultisite::ConnectionManagement.current_db, "/") end - def s3_client - @s3_client ||= Aws::S3::Client.new(@s3_options) - end - def s3_resource Aws::S3::Resource.new(client: s3_client) end diff --git a/lib/s3_inventory.rb b/lib/s3_inventory.rb new file mode 100644 index 0000000000..321182f3d6 --- /dev/null +++ b/lib/s3_inventory.rb @@ -0,0 +1,193 @@ +# frozen_string_literal: true + +require "aws-sdk-s3" +require "csv" + +class S3Inventory + + attr_reader :inventory_id, :csv_filename, :model + + CSV_KEY_INDEX ||= 1 + CSV_ETAG_INDEX ||= 2 + INVENTORY_PREFIX ||= "inventory" + + def initialize(s3_helper, type) + @s3_helper = s3_helper + + if type == :upload + @inventory_id = "original" + @model = Upload + elsif type == :optimized + @inventory_id = "optimized" + @model = OptimizedImage + end + end + + def file + @file ||= unsorted_files.sort_by { |file| -file.last_modified.to_i }.first + end + + def list_missing + if file.blank? + error("Failed to list inventory from S3") + return + end + + DistributedMutex.synchronize("s3_inventory_list_missing_#{inventory_id}") do + current_db = RailsMultisite::ConnectionManagement.current_db + timestamp = Time.now.strftime("%Y-%m-%d-%H%M%S") + @tmp_directory = File.join(Rails.root, "tmp", INVENTORY_PREFIX, current_db, timestamp) + @archive_filename = File.join(@tmp_directory, File.basename(file.key)) + @csv_filename = @archive_filename[0...-3] + + FileUtils.mkdir_p(@tmp_directory) + download_inventory_file_to_tmp_directory + decompress_inventory_file + + begin + table_name = "#{inventory_id}_inventory" + connection = ActiveRecord::Base.connection.raw_connection + connection.exec("CREATE TEMP TABLE #{table_name}(key text UNIQUE, etag text PRIMARY KEY)") + connection.copy_data("COPY #{table_name} FROM STDIN CSV") do + CSV.foreach(csv_filename, headers: false) do |row| + connection.put_copy_data("#{row[CSV_KEY_INDEX]},#{row[CSV_ETAG_INDEX]}\n") + end + end + + uploads = model.where("created_at < ?", file.last_modified) + missing_uploads = uploads.joins("LEFT JOIN #{table_name} ON #{table_name}.etag = #{model.table_name}.etag").where("#{table_name}.etag is NULL") + + if (missing_count = missing_uploads.count) > 0 + missing_uploads.select(:id, :url).find_each do |upload| + log upload.url + end + + log "#{missing_count} of #{uploads.count} #{model.name.underscore.pluralize} are missing" + end + ensure + connection.exec("DROP TABLE #{table_name}") unless connection.nil? + end + end + end + + def download_inventory_file_to_tmp_directory + log "Downloading inventory file to tmp directory..." + failure_message = "Failed to inventory file to tmp directory." + + @s3_helper.download_file(file.key, @archive_filename, failure_message) + end + + def decompress_inventory_file + log "Decompressing inventory file, this may take a while..." + + FileUtils.cd(@tmp_directory) do + Discourse::Utils.execute_command('gzip', '--decompress', @archive_filename, failure_message: "Failed to decompress inventory file.") + end + end + + def update_bucket_policy + @s3_helper.s3_client.put_bucket_policy( + bucket: bucket_name, + policy: { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "InventoryAndAnalyticsPolicy", + "Effect": "Allow", + "Principal": { "Service": "s3.amazonaws.com" }, + "Action": ["s3:PutObject"], + "Resource": ["arn:aws:s3:::#{inventory_root_path}/*"], + "Condition": { + "ArnLike": { + "aws:SourceArn": "arn:aws:s3:::#{bucket_name}" + }, + "StringEquals": { + "s3:x-amz-acl": "bucket-owner-full-control" + } + } + } + ] + }.to_json + ) + end + + def update_bucket_inventory_configuration + @s3_helper.s3_client.put_bucket_inventory_configuration( + bucket: bucket_name, + id: inventory_id, + inventory_configuration: inventory_configuration, + use_accelerate_endpoint: false + ) + end + + private + + def inventory_configuration + filter_prefix = inventory_id + destination_prefix = File.join(INVENTORY_PREFIX, inventory_id) + + if bucket_folder_path.present? + filter_prefix = File.join(bucket_folder_path, filter_prefix) + destination_prefix = File.join(bucket_folder_path, destination_prefix) + end + + { + destination: { + s3_bucket_destination: { + bucket: "arn:aws:s3:::#{bucket_name}", + prefix: destination_prefix, + format: "CSV" + } + }, + filter: { + prefix: filter_prefix + }, + is_enabled: SiteSetting.enable_s3_inventory, + id: inventory_id, + included_object_versions: "Current", + optional_fields: ["ETag"], + schedule: { + frequency: "Daily" + } + } + end + + def bucket_name + @s3_helper.s3_bucket_name + end + + def bucket_folder_path + @s3_helper.s3_bucket_folder_path + end + + def unsorted_files + objects = [] + + @s3_helper.list(File.join(inventory_path, "data")).each do |obj| + if obj.key.match?(/\.csv\.gz$/i) + objects << obj + end + end + + objects + rescue Aws::Errors::ServiceError => e + log("Failed to list inventory from S3", e) + end + + def inventory_path + File.join(inventory_root_path, inventory_id) + end + + def inventory_root_path + File.join(bucket_name, bucket_folder_path || "", INVENTORY_PREFIX) + end + + def log(message, ex = nil) + puts(message) + Rails.logger.error("#{ex}\n" + (ex.backtrace || []).join("\n")) if ex + end + + def error(message) + log(message, StandardError.new(message)) + end +end diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index 2d03fefc3c..eaab98f5f2 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -53,6 +53,10 @@ module SiteSettings::Validations validate_error :s3_upload_bucket_is_required if new_val == "t" && SiteSetting.s3_upload_bucket.blank? end + def validate_enable_s3_inventory(new_val) + validate_error :enable_s3_uploads_is_required if new_val == "t" && !SiteSetting.enable_s3_uploads? + end + def validate_backup_location(new_val) return unless new_val == BackupLocationSiteSetting::S3 validate_error(:s3_backup_requires_s3_settings, setting_name: "s3_backup_bucket") if SiteSetting.s3_backup_bucket.blank? diff --git a/spec/components/migration/safe_migrate_spec.rb b/spec/components/migration/safe_migrate_spec.rb index 62b94805cb..0255006adb 100644 --- a/spec/components/migration/safe_migrate_spec.rb +++ b/spec/components/migration/safe_migrate_spec.rb @@ -11,16 +11,6 @@ describe Migration::SafeMigrate do Migration::SafeMigrate::SafeMigration.enable_safe! end - def capture_stdout - old_stdout = $stdout - io = StringIO.new - $stdout = io - yield - io.string - ensure - $stdout = old_stdout - end - def migrate_up(path) migrations = ActiveRecord::MigrationContext.new(path).migrations ActiveRecord::Migrator.new(:up, migrations, migrations.first.version).run diff --git a/spec/components/s3_inventory_spec.rb b/spec/components/s3_inventory_spec.rb new file mode 100644 index 0000000000..5f147f370f --- /dev/null +++ b/spec/components/s3_inventory_spec.rb @@ -0,0 +1,77 @@ +require "rails_helper" +require "s3_helper" +require "s3_inventory" +require "file_store/s3_store" + +describe "S3Inventory" do + let(:client) { Aws::S3::Client.new(stub_responses: true) } + let(:helper) { S3Helper.new(SiteSetting.Upload.s3_upload_bucket.downcase, "", client: client) } + let(:store) { FileStore::S3Store.new(helper) } + let(:inventory) { S3Inventory.new(helper, :upload) } + let(:csv_filename) { "#{Rails.root}/spec/fixtures/csv/s3_inventory.csv" } + + before do + SiteSetting.enable_s3_uploads = true + SiteSetting.s3_access_key_id = "abc" + SiteSetting.s3_secret_access_key = "def" + SiteSetting.enable_s3_inventory = true + + client.stub_responses(:list_objects, + contents: [ + { + etag: "\"70ee1738b6b21e2c8a43f3a5ab0eee71\"", + key: "example1.csv.gz", + last_modified: Time.parse("2014-11-21T19:40:05.000Z"), + owner: { + display_name: "myname", + id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", + }, + size: 11, + storage_class: "STANDARD", + }, + { + etag: "\"9c8af9a76df052144598c115ef33e511\"", + key: "example2.csv.gz", + last_modified: Time.parse("2013-11-15T01:10:49.000Z"), + owner: { + display_name: "myname", + id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", + }, + size: 713193, + storage_class: "STANDARD", + } + ], + next_marker: "eyJNYXJrZXIiOiBudWxsLCAiYm90b190cnVuY2F0ZV9hbW91bnQiOiAyfQ==" + ) + end + + it "should return the latest inventory file name" do + expect(inventory.file.key).to eq("example1.csv.gz") + end + + it "should raise error if an inventory file is not found" do + client.stub_responses(:list_objects, contents: []) + output = capture_stdout { inventory.list_missing } + expect(output).to eq("Failed to list inventory from S3\n") + end + + it "should display missing uploads correctly" do + freeze_time + + CSV.foreach(csv_filename, headers: false) do |row| + Fabricate(:upload, etag: row[S3Inventory::CSV_ETAG_INDEX], created_at: 2.days.ago) + end + upload = Fabricate(:upload, etag: "ETag", created_at: 1.days.ago) + Fabricate(:upload, etag: "ETag2", created_at: Time.now) + + inventory.expects(:decompress_inventory_file) + inventory.expects(:csv_filename).returns(csv_filename) + inventory.file.expects(:last_modified).returns(Time.now) + + output = capture_stdout do + inventory.list_missing + end + + expect(output).to eq("Downloading inventory file to tmp directory...\n#{upload.url}\n1 of 4 uploads are missing\n") + end +end diff --git a/spec/fixtures/csv/s3_inventory.csv b/spec/fixtures/csv/s3_inventory.csv new file mode 100644 index 0000000000..04325fb7c3 --- /dev/null +++ b/spec/fixtures/csv/s3_inventory.csv @@ -0,0 +1,3 @@ +"abc","original/0184537a4f419224404d013414e913a4f56018f2.jpg","defcaac0b4aca535c284e95f30d608d0" +"abc","original/050afc0ab01debe8cf48fd2ce50fbbf5eb072815.jpg","0cdc623af39cde0adb382670a6dc702a" +"abc","original/0789fbf5490babc68326b9cec90eeb0d6590db05.png","25c02eaceef4cb779fc17030d33f7f06" diff --git a/spec/jobs/update_s3_inventory_spec.rb b/spec/jobs/update_s3_inventory_spec.rb new file mode 100644 index 0000000000..1bcb8985a7 --- /dev/null +++ b/spec/jobs/update_s3_inventory_spec.rb @@ -0,0 +1,50 @@ +require 'rails_helper' +require "file_store/s3_store" + +describe Jobs::UpdateS3Inventory do + before do + SiteSetting.enable_s3_uploads = true + SiteSetting.s3_access_key_id = "abc" + SiteSetting.s3_secret_access_key = "def" + SiteSetting.enable_s3_inventory = true + + store = FileStore::S3Store.new + @client = Aws::S3::Client.new(stub_responses: true) + store.s3_helper.stubs(:s3_client).returns(@client) + Discourse.stubs(:store).returns(store) + end + + it "updates the bucket policy and inventory configuration in S3" do + id = "original" + @client.expects(:put_bucket_policy).with( + bucket: "bucket", + policy: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"InventoryAndAnalyticsPolicy\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"s3.amazonaws.com\"},\"Action\":[\"s3:PutObject\"],\"Resource\":[\"arn:aws:s3:::bucket/inventory/*\"],\"Condition\":{\"ArnLike\":{\"aws:SourceArn\":\"arn:aws:s3:::bucket\"},\"StringEquals\":{\"s3:x-amz-acl\":\"bucket-owner-full-control\"}}}]}" + ) + @client.expects(:put_bucket_inventory_configuration) + @client.expects(:put_bucket_inventory_configuration).with( + bucket: "bucket", + id: id, + inventory_configuration: { + destination: { + s3_bucket_destination: { + bucket: "arn:aws:s3:::bucket", + prefix: "inventory/#{id}", + format: "CSV" + } + }, + filter: { + prefix: id + }, + is_enabled: true, + id: id, + included_object_versions: "Current", + optional_fields: ["ETag"], + schedule: { frequency: "Daily" } + }, + use_accelerate_endpoint: false + ) + + described_class.new.execute(nil) + end + +end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 8877d23a9f..455230e1b4 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -102,4 +102,14 @@ module Helpers tag_group.tags << (Tag.where(name: name).first || Fabricate(:tag, name: name)) end end + + def capture_stdout + old_stdout = $stdout + io = StringIO.new + $stdout = io + yield + io.string + ensure + $stdout = old_stdout + end end From 565b524b024e3b0a6759bec6de5e52aa932c46cd Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Fri, 1 Feb 2019 14:17:10 +0530 Subject: [PATCH 05/74] FIX: don't raise error if s3 set via global setting --- lib/site_settings/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index eaab98f5f2..327aa7b680 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -54,7 +54,7 @@ module SiteSettings::Validations end def validate_enable_s3_inventory(new_val) - validate_error :enable_s3_uploads_is_required if new_val == "t" && !SiteSetting.enable_s3_uploads? + validate_error :enable_s3_uploads_is_required if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads end def validate_backup_location(new_val) From 3119e88efef4f612778548db15af5ac969a4b72b Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Fri, 1 Feb 2019 14:33:06 +0530 Subject: [PATCH 06/74] Allow 'enable_s3_inventory' site setting to be shadowed. --- config/site_settings.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/site_settings.yml b/config/site_settings.yml index 17ae4a2618..5a1fcd1afb 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1052,6 +1052,7 @@ files: shadowed_by_global: true enable_s3_inventory: default: false + shadowed_by_global: true allow_profile_backgrounds: client: true default: true From dfd63b185f3b5279f49f979638dd17c30431669e Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 1 Feb 2019 10:40:41 +0000 Subject: [PATCH 07/74] DEV: Allow rake plugin:spec to traverse symlinks --- lib/tasks/plugin.rake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/tasks/plugin.rake b/lib/tasks/plugin.rake index c77525038f..13f18c120e 100644 --- a/lib/tasks/plugin.rake +++ b/lib/tasks/plugin.rake @@ -87,7 +87,8 @@ desc 'run plugin specs' task 'plugin:spec', :plugin do |t, args| args.with_defaults(plugin: "*") ruby = `which ruby`.strip - files = Dir.glob("./plugins/#{args[:plugin]}/spec/**/*_spec.rb") + # Traverse symlinks (https://stackoverflow.com/questions/357754/can-i-traverse-symlinked-directories-in-ruby-with-a-glob) + files = Dir.glob("./plugins/#{args[:plugin]}/spec/**{,/*/**}/*_spec.rb") if files.length > 0 sh "LOAD_PLUGINS=1 #{ruby} -S rspec #{files.join(' ')}" else From 925606f8b14d334e21c03e7205c1f790013ee6fd Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 1 Feb 2019 11:01:59 +0000 Subject: [PATCH 08/74] FIX: Login button icons should be white --- app/assets/stylesheets/common/components/buttons.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/common/components/buttons.scss b/app/assets/stylesheets/common/components/buttons.scss index 6b45685f60..3fe12873c3 100644 --- a/app/assets/stylesheets/common/components/buttons.scss +++ b/app/assets/stylesheets/common/components/buttons.scss @@ -176,6 +176,9 @@ // -------------------------------------------------- .btn-social { + .d-icon { + color: #fff; + } color: #fff; background: #666; &:hover { From 68173cd234ed1941b943bb74817b68ae7d92b6fc Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 1 Feb 2019 12:40:29 +0000 Subject: [PATCH 09/74] Revert "DEV: Allow rake plugin:spec to traverse symlinks" This reverts commit dfd63b185f3b5279f49f979638dd17c30431669e. https://meta.discourse.org/t/108110/11 --- 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 13f18c120e..c77525038f 100644 --- a/lib/tasks/plugin.rake +++ b/lib/tasks/plugin.rake @@ -87,8 +87,7 @@ desc 'run plugin specs' task 'plugin:spec', :plugin do |t, args| args.with_defaults(plugin: "*") ruby = `which ruby`.strip - # Traverse symlinks (https://stackoverflow.com/questions/357754/can-i-traverse-symlinked-directories-in-ruby-with-a-glob) - files = Dir.glob("./plugins/#{args[:plugin]}/spec/**{,/*/**}/*_spec.rb") + files = Dir.glob("./plugins/#{args[:plugin]}/spec/**/*_spec.rb") if files.length > 0 sh "LOAD_PLUGINS=1 #{ruby} -S rspec #{files.join(' ')}" else From 5c9426be481eab5dbdc0ae9fff03d32bf279adf4 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 1 Feb 2019 13:10:59 +0000 Subject: [PATCH 10/74] SECURITY: Escape HTML in dashboard report tables --- .../javascripts/admin/models/report.js.es6 | 4 ++-- test/javascripts/models/report-test.js.es6 | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/admin/models/report.js.es6 b/app/assets/javascripts/admin/models/report.js.es6 index 21caf4aec5..d453dd04cc 100644 --- a/app/assets/javascripts/admin/models/report.js.es6 +++ b/app/assets/javascripts/admin/models/report.js.es6 @@ -333,7 +333,7 @@ const Report = Discourse.Model.extend({ const formatedValue = () => { const topicId = row[properties.id]; const href = Discourse.getURL(`/t/-/${topicId}`); - return `${topicTitle}`; + return `${escapeExpression(topicTitle)}`; }; return { @@ -352,7 +352,7 @@ const Report = Discourse.Model.extend({ property: properties.title, value: postTitle, formatedValue: - postTitle && href ? `${postTitle}` : "—" + postTitle && href ? `${escapeExpression(postTitle)}` : "—" }; }, diff --git a/test/javascripts/models/report-test.js.es6 b/test/javascripts/models/report-test.js.es6 index 3f1dd7a40d..3ba6a5198b 100644 --- a/test/javascripts/models/report-test.js.es6 +++ b/test/javascripts/models/report-test.js.es6 @@ -402,9 +402,9 @@ QUnit.test("computed labels", assert => { time_read: 287362, note: "This is a long note", topic_id: 2, - topic_title: "Test topic", + topic_title: "Test topic ", post_number: 3, - post_raw: "This is the beginning of", + post_raw: "This is the beginning of ", filesize: 582641 } ]; @@ -502,9 +502,9 @@ QUnit.test("computed labels", assert => { const computedTopicLabel = topicLabel.compute(row); assert.equal( computedTopicLabel.formatedValue, - "Test topic" + "Test topic <html>" ); - assert.equal(computedTopicLabel.value, "Test topic"); + assert.equal(computedTopicLabel.value, "Test topic "); const postLabel = computedLabels[5]; assert.equal(postLabel.mainProperty, "post_raw"); @@ -514,9 +514,9 @@ QUnit.test("computed labels", assert => { const computedPostLabel = postLabel.compute(row); assert.equal( computedPostLabel.formatedValue, - "This is the beginning of" + "This is the beginning of <html>" ); - assert.equal(computedPostLabel.value, "This is the beginning of"); + assert.equal(computedPostLabel.value, "This is the beginning of "); const filesizeLabel = computedLabels[6]; assert.equal(filesizeLabel.mainProperty, "filesize"); @@ -533,11 +533,11 @@ QUnit.test("computed labels", assert => { const postLink = computedLabels[5].compute(row).formatedValue; assert.equal( postLink, - "This is the beginning of" + "This is the beginning of <html>" ); const topicLink = computedLabels[4].compute(row).formatedValue; - assert.equal(topicLink, "Test topic"); + assert.equal(topicLink, "Test topic <html>"); const userLink = computedLabels[0].compute(row).formatedValue; assert.equal( From eb617dea4bd31a838022340ae5df62fca7026d69 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 1 Feb 2019 13:22:31 +0000 Subject: [PATCH 11/74] DEV: Run prettier on `report.js.es6` The prettier version I was running was different to the CI pipeline --- app/assets/javascripts/admin/models/report.js.es6 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/models/report.js.es6 b/app/assets/javascripts/admin/models/report.js.es6 index d453dd04cc..53e9d90a20 100644 --- a/app/assets/javascripts/admin/models/report.js.es6 +++ b/app/assets/javascripts/admin/models/report.js.es6 @@ -352,7 +352,9 @@ const Report = Discourse.Model.extend({ property: properties.title, value: postTitle, formatedValue: - postTitle && href ? `${escapeExpression(postTitle)}` : "—" + postTitle && href + ? `${escapeExpression(postTitle)}` + : "—" }; }, From 37c28cf0b7973d817e73bf2f47bc41ebc34b1f22 Mon Sep 17 00:00:00 2001 From: Kris Date: Fri, 1 Feb 2019 09:51:12 -0500 Subject: [PATCH 12/74] UX: Header icon color fix --- .../common/foundation/variables.scss | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/common/foundation/variables.scss b/app/assets/stylesheets/common/foundation/variables.scss index 1bbd8c1aa2..fe706f9231 100644 --- a/app/assets/stylesheets/common/foundation/variables.scss +++ b/app/assets/stylesheets/common/foundation/variables.scss @@ -206,10 +206,25 @@ $primary-high: dark-light-diff($primary, $secondary, 30%, -25%); $primary-very-high: dark-light-diff($primary, $secondary, 15%, -10%); //header_primary -$header_primary-low: dark-light-diff($header_primary, $secondary, 90%, -78%); -$header_primary-low-mid: dark-light-diff($primary, $secondary, 70%, -45%); +$header_primary-low: dark-light-diff( + $header_primary, + $header_background, + 90%, + -78% +); +$header_primary-low-mid: dark-light-diff( + $header_primary, + $header_background, + 70%, + -45% +); -$header_primary-medium: dark-light-diff($header_primary, $secondary, 50%, -35%); +$header_primary-medium: dark-light-diff( + $header_primary, + $header_background, + 50%, + -35% +); $header_primary-high: dark-light-diff( $header_primary, $header_background, From e75b240390e1e2c51e636bcbd32478609b6a93ed Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 1 Feb 2019 11:44:37 -0500 Subject: [PATCH 13/74] FIX: Some brittle tests with hardcoded ids If we're going to use hardcoded ids, we should make sure the records are not saved, or that the ID will not come up during normal use. --- app/models/category.rb | 5 +++++ spec/models/report_spec.rb | 43 ++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 45aac78824..a99117812c 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -113,6 +113,9 @@ class Category < ActiveRecord::Base # we may consider wrapping this in another spot attr_accessor :displayable_topics, :permission, :subcategory_ids, :notification_level, :has_children + # Allows us to skip creating the category definition topic in tests. + attr_accessor :skip_category_definition + @topic_id_cache = DistributedCache.new('category_topic_ids') def self.topic_ids @@ -212,6 +215,8 @@ class Category < ActiveRecord::Base end def create_category_definition + return if skip_category_definition + t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id) t.skip_callbacks = true t.ignore_category_auto_close = true diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index b0ddc2971c..fa6395a48e 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe Report do + let(:c0) { Fabricate(:category, skip_category_definition: true) } # id: 3 + let(:c1) { Fabricate(:category, parent_category: c0, skip_category_definition: true) } # id: 2 + let(:c2) { Fabricate(:category, skip_category_definition: true) } # id: 4 + shared_examples 'no data' do context "with no data" do it 'returns an empty report' do @@ -16,13 +20,6 @@ describe Report do end shared_examples 'category filtering on subcategories' do - before do - c = Fabricate(:category, id: 3) - c.topic.destroy - c = Fabricate(:category, id: 2, parent_category_id: 3) - c.topic.destroy - # destroy the category description topics so the count is right, on filtered data - end it 'returns the filtered data' do expect(report.total).to eq(1) @@ -753,7 +750,7 @@ describe Report do before(:each) do user = Fabricate(:user) post0 = Fabricate(:post) - post1 = Fabricate(:post, topic: Fabricate(:topic, category_id: 2)) + post1 = Fabricate(:post, topic: Fabricate(:topic, category: c1)) post2 = Fabricate(:post) post3 = Fabricate(:post) PostAction.act(user, post0, PostActionType.types[:off_topic]) @@ -765,12 +762,12 @@ describe Report do end context "with category filtering" do - let(:report) { Report.find('flags', category_id: 2) } + let(:report) { Report.find('flags', category_id: c1.id) } include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('flags', category_id: 3) } + let(:report) { Report.find('flags', category_id: c0.id) } include_examples 'category filtering on subcategories' end @@ -788,18 +785,18 @@ describe Report do before(:each) do Fabricate(:topic) - Fabricate(:topic, category_id: 2) + Fabricate(:topic, category: c1) Fabricate(:topic) Fabricate(:topic, created_at: 45.days.ago) end context "with category filtering" do - let(:report) { Report.find('topics', category_id: 2) } + let(:report) { Report.find('topics', category_id: c1.id) } include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('topics', category_id: 3) } + let(:report) { Report.find('topics', category_id: c0.id) } include_examples 'category filtering on subcategories' end @@ -876,7 +873,7 @@ describe Report do before(:each) do topic = Fabricate(:topic) - topic_with_category_id = Fabricate(:topic, category_id: 2) + topic_with_category_id = Fabricate(:topic, category: c1) Fabricate(:post, topic: topic) Fabricate(:post, topic: topic_with_category_id) Fabricate(:post, topic: topic) @@ -884,12 +881,12 @@ describe Report do end context "with category filtering" do - let(:report) { Report.find('posts', category_id: 2) } + let(:report) { Report.find('posts', category_id: c1.id) } include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('posts', category_id: 3) } + let(:report) { Report.find('posts', category_id: c0.id) } include_examples 'category filtering on subcategories' end @@ -908,19 +905,19 @@ describe Report do include_examples 'with data x/y' before(:each) do - Fabricate(:topic, category_id: 2) + Fabricate(:topic, category: c1) Fabricate(:post, topic: Fabricate(:topic)) Fabricate(:topic) Fabricate(:topic, created_at: 45.days.ago) end context "with category filtering" do - let(:report) { Report.find('topics_with_no_response', category_id: 2) } + let(:report) { Report.find('topics_with_no_response', category_id: c1.id) } include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('topics_with_no_response', category_id: 3) } + let(:report) { Report.find('topics_with_no_response', category_id: c0.id) } include_examples 'category filtering on subcategories' end @@ -937,11 +934,11 @@ describe Report do include_examples 'with data x/y' before(:each) do - topic = Fabricate(:topic, category_id: 2) + topic = Fabricate(:topic, category: c1) post = Fabricate(:post, topic: topic) PostAction.act(Fabricate(:user), post, PostActionType.types[:like]) - topic = Fabricate(:topic, category_id: 4) + topic = Fabricate(:topic, category: c2) post = Fabricate(:post, topic: topic) PostAction.act(Fabricate(:user), post, PostActionType.types[:like]) PostAction.act(Fabricate(:user), post, PostActionType.types[:like]) @@ -951,12 +948,12 @@ describe Report do end context "with category filtering" do - let(:report) { Report.find('likes', category_id: 2) } + let(:report) { Report.find('likes', category_id: c1.id) } include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('likes', category_id: 3) } + let(:report) { Report.find('likes', category_id: c0.id) } include_examples 'category filtering on subcategories' end From 2a2be093cac56792f909347fc8900d574d5b2fa0 Mon Sep 17 00:00:00 2001 From: Kris Date: Fri, 1 Feb 2019 21:07:57 -0500 Subject: [PATCH 14/74] UX: checkboxes were too close to other inputs --- app/assets/stylesheets/desktop/discourse.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/desktop/discourse.scss b/app/assets/stylesheets/desktop/discourse.scss index b5c4dcb651..bdab807843 100644 --- a/app/assets/stylesheets/desktop/discourse.scss +++ b/app/assets/stylesheets/desktop/discourse.scss @@ -15,7 +15,10 @@ body.widget-dragging { .form-vertical { .control-group { - margin-bottom: 24px; + margin-bottom: 1.25em; + } + .controls:not(.controls-dropdown) + .controls { + margin-top: 0.5em; } } From 377f3efb60e2e3675ee8b8bcdf539296190a0593 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 15:05:54 +1100 Subject: [PATCH 15/74] DEV: remove foreman gem and unsupported Procfile Launching Discourse no longer should require foreman in dev. We can simply use `bin/unicorn` which automatically launches a sidekiq worker. The foreman gem depends on thor ~> 0.19.1 which is no longer supported in rails 6. So this pre-emptively prepares us for it. --- Gemfile | 1 - Gemfile.lock | 3 --- Procfile | 2 -- 3 files changed, 6 deletions(-) delete mode 100644 Procfile diff --git a/Gemfile b/Gemfile index 345d8e7d82..65e5c2294f 100644 --- a/Gemfile +++ b/Gemfile @@ -143,7 +143,6 @@ group :development do gem 'better_errors' gem 'binding_of_caller' gem 'annotate' - gem 'foreman', require: false end # this is an optional gem, it provides a high performance replacement diff --git a/Gemfile.lock b/Gemfile.lock index b59803ab6a..95beb142f0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -144,8 +144,6 @@ GEM fastimage (2.1.3) ffi (1.9.25) flamegraph (0.9.5) - foreman (0.85.0) - thor (~> 0.19.1) fspath (3.1.0) gc_tracer (1.5.1) git (1.5.0) @@ -478,7 +476,6 @@ DEPENDENCIES fast_xs fastimage flamegraph - foreman gc_tracer highline (~> 1.7.0) hiredis diff --git a/Procfile b/Procfile deleted file mode 100644 index 4d752a6c18..0000000000 --- a/Procfile +++ /dev/null @@ -1,2 +0,0 @@ -web: bundle exec rails server -p $PORT -worker: bundle exec sidekiq -e $RAILS_ENV \ No newline at end of file From ab52241d328c18062013cf0f5d1bdd48c0000755 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 15:09:15 +1100 Subject: [PATCH 16/74] DEV: upgrade thor gem 0.19.4 was no longer compatible with Rails 6, this prepares us for Rails 6 support. --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 95beb142f0..8d4db47fd1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -415,7 +415,7 @@ GEM stackprof (0.2.11) terminal-table (1.8.0) unicode-display_width (~> 1.1, >= 1.1.1) - thor (0.19.4) + thor (0.20.3) thread_safe (0.3.6) tilt (2.0.8) trollop (2.1.2) From 1816bdf46ea314ad3c34def3212412554ab7295b Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 16:10:13 +1100 Subject: [PATCH 17/74] DEV: upgrade mail gem from pre-release 2.7.1 version of the mail gem was released! We no longer need to depend on the pre-release. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 65e5c2294f..e23f400bcf 100644 --- a/Gemfile +++ b/Gemfile @@ -26,7 +26,7 @@ else gem 'seed-fu' end -gem 'mail', '2.7.1.rc1', require: false +gem 'mail', require: false gem 'mini_mime' gem 'mini_suffix' diff --git a/Gemfile.lock b/Gemfile.lock index 8d4db47fd1..f39f20d467 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -188,7 +188,7 @@ GEM crass (~> 1.0.2) nokogiri (>= 1.5.9) lru_redux (1.1.0) - mail (2.7.1.rc1) + mail (2.7.1) mini_mime (>= 0.1.1) maxminddb (0.1.21) memory_profiler (0.9.12) @@ -487,7 +487,7 @@ DEPENDENCIES logstash-logger logster lru_redux - mail (= 2.7.1.rc1) + mail maxminddb memory_profiler message_bus (= 2.2.0.pre.1) From 1d2c4b0eee37b8c1fe4ffbfc6c5d8209125f2d82 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 16:11:19 +1100 Subject: [PATCH 18/74] DEV: we are waiting on an annotate gem release Once version 2.7.5 is released per: https://github.com/ctran/annotate_models/pull/595 we can drop this conditional. --- Gemfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index e23f400bcf..44384f31f0 100644 --- a/Gemfile +++ b/Gemfile @@ -142,7 +142,13 @@ group :development do gem 'bullet', require: !!ENV['BULLET'] gem 'better_errors' gem 'binding_of_caller' - gem 'annotate' + + # waiting on 2.7.5 per: https://github.com/ctran/annotate_models/pull/595 + if rails_master? + gem 'annotate', git: 'https://github.com/ctran/annotate_models.git' + else + gem 'annotate' + end end # this is an optional gem, it provides a high performance replacement From 9f5bbd663d7e423aa7755457eb1b2a1dbaa724a6 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 17:46:38 +1100 Subject: [PATCH 19/74] DEV: update mini_racer and message_bus Two very low risk updates, message_bus has been released no need to depend on pre-release. mini_racer update is for a very minor change (shared isolates are not used in discourse so it is not a fix we technically need) --- Gemfile | 7 +------ Gemfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/Gemfile b/Gemfile index 44384f31f0..f5bdeb3c16 100644 --- a/Gemfile +++ b/Gemfile @@ -45,12 +45,7 @@ gem 'discourse-ember-source', '~> 3.5.1' gem 'ember-handlebars-template', '0.8.0' gem 'barber' -# message bus 2.2.0 should be very stable -# we trimmed some of the internal API surface down so we went with -# a pre release here to make we don't do a full release prior to -# baking here. Remove 2.2.0.pre no later than Jan 2019 and move back -# to the standard releases -gem 'message_bus', '2.2.0.pre.1' +gem 'message_bus' gem 'rails_multisite' diff --git a/Gemfile.lock b/Gemfile.lock index f39f20d467..fb04f09024 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -192,13 +192,13 @@ GEM mini_mime (>= 0.1.1) maxminddb (0.1.21) memory_profiler (0.9.12) - message_bus (2.2.0.pre.1) + message_bus (2.2.0) rack (>= 1.1.3) metaclass (0.0.4) method_source (0.8.2) mini_mime (1.0.1) mini_portile2 (2.3.0) - mini_racer (0.2.3) + mini_racer (0.2.4) libv8 (>= 6.3) mini_scheduler (0.9.1) sidekiq @@ -490,7 +490,7 @@ DEPENDENCIES mail maxminddb memory_profiler - message_bus (= 2.2.0.pre.1) + message_bus mini_mime mini_racer mini_scheduler From 2c53dde9181d42d9998ad225aba8d1b770c2d260 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 17:48:31 +1100 Subject: [PATCH 20/74] DEV: more low risk gem updates redis/sidekiq/unicorn/nokogiri and pg all are fairly safe to update --- Gemfile.lock | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index fb04f09024..463e917a5c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -197,7 +197,7 @@ GEM metaclass (0.0.4) method_source (0.8.2) mini_mime (1.0.1) - mini_portile2 (2.3.0) + mini_portile2 (2.4.0) mini_racer (0.2.4) libv8 (>= 6.3) mini_scheduler (0.9.1) @@ -217,8 +217,8 @@ GEM mustache (1.0.5) nap (1.1.0) no_proxy_fix (0.1.2) - nokogiri (1.8.5) - mini_portile2 (~> 2.3.0) + nokogiri (1.10.1) + mini_portile2 (~> 2.4.0) nokogumbo (1.5.0) nokogiri oauth (0.5.4) @@ -272,7 +272,7 @@ GEM parallel (1.12.1) parser (2.5.3.0) ast (~> 2.4.0) - pg (1.1.3) + pg (1.1.4) powerpack (0.1.2) progress (3.4.0) pry (0.10.4) @@ -292,7 +292,7 @@ GEM rack-openid (1.3.1) rack (>= 1.1.0) ruby-openid (>= 2.1.8) - rack-protection (2.0.3) + rack-protection (2.0.5) rack rack-test (1.1.0) rack (>= 1.0, < 3) @@ -323,7 +323,7 @@ GEM msgpack (>= 0.4.3) trollop (>= 1.16.2) rchardet (1.8.0) - redis (4.0.1) + redis (4.1.0) redis-namespace (1.6.0) redis (>= 3.0.4) request_store (1.4.1) @@ -398,9 +398,9 @@ GEM shoulda-context (1.2.2) shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (5.1.3) - concurrent-ruby (~> 1.0) - connection_pool (~> 2.2, >= 2.2.0) + sidekiq (5.2.5) + connection_pool (~> 2.2, >= 2.2.2) + rack (>= 1.5.0) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) slop (3.6.0) @@ -427,7 +427,7 @@ GEM unf_ext unf_ext (0.0.7.5) unicode-display_width (1.4.1) - unicorn (5.4.0) + unicorn (5.4.1) kgio (~> 2.6) raindrops (~> 0.7) uniform_notifier (1.11.0) From ab2361507714571ee525b24cd0c4549186852eac Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 17:52:10 +1100 Subject: [PATCH 21/74] DEV: update rubocop gem to latest not much to say here, the new version seems compatible with Discourse, no changes appear to be needed --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 463e917a5c..e4f44e233c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -161,7 +161,7 @@ GEM concurrent-ruby (~> 1.0) image_size (1.5.0) in_threads (1.5.0) - jaro_winkler (1.5.1) + jaro_winkler (1.5.2) jmespath (1.4.0) jquery-rails (4.3.3) rails-dom-testing (>= 1, < 3) @@ -269,8 +269,8 @@ GEM openid-redis-store (0.0.2) redis ruby-openid - parallel (1.12.1) - parser (2.5.3.0) + parallel (1.13.0) + parser (2.6.0.0) ast (~> 2.4.0) pg (1.1.4) powerpack (0.1.2) @@ -357,7 +357,7 @@ GEM rspec-support (~> 3.7.0) rspec-support (3.7.1) rtlit (0.0.5) - rubocop (0.61.1) + rubocop (0.63.1) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) From 2c57b65bfc3cc7c53b53d2345441940ea44fba92 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 17:54:39 +1100 Subject: [PATCH 22/74] DEV: update more gems including i18n This updates a few more lower risk gems, the main goal here is to have nothing outdated. Avoiding a giant commit is going to make it slightly easier to partially roll back if something goes wrong --- Gemfile.lock | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e4f44e233c..7eba484635 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -67,12 +67,12 @@ GEM rack (>= 0.9.0) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) - bootsnap (1.3.0) + bootsnap (1.3.2) msgpack (~> 1.0) builder (3.2.3) - bullet (5.7.5) + bullet (5.9.0) activesupport (>= 3.0.0) - uniform_notifier (~> 1.11.0) + uniform_notifier (~> 1.11) byebug (10.0.2) certified (1.0.0) chunky_png (1.3.10) @@ -91,7 +91,7 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) - danger (5.11.1) + danger (5.13.0) claide (~> 1.0) claide-plugins (>= 0.9.2) colored2 (~> 3.1) @@ -157,7 +157,7 @@ GEM hkdf (0.3.0) htmlentities (4.3.4) http_accept_language (2.0.5) - i18n (1.1.1) + i18n (1.5.3) concurrent-ruby (~> 1.0) image_size (1.5.0) in_threads (1.5.0) @@ -210,7 +210,7 @@ GEM metaclass (~> 0.0.1) mock_redis (0.18.0) moneta (1.0.0) - msgpack (1.2.4) + msgpack (1.2.6) multi_json (1.13.1) multi_xml (0.6.0) multipart-post (2.0.0) @@ -230,7 +230,7 @@ GEM rack (>= 1.2, < 3) octokit (4.13.0) sawyer (~> 0.8.0, >= 0.5.3) - oj (3.6.2) + oj (3.7.8) omniauth (1.9.0) hashie (>= 3.4.6, < 3.7.0) rack (>= 1.6.2, < 3) @@ -421,7 +421,7 @@ GEM trollop (2.1.2) tzinfo (1.2.5) thread_safe (~> 0.1) - uglifier (4.1.11) + uglifier (4.1.20) execjs (>= 0.3.0, < 3) unf (0.1.4) unf_ext @@ -430,7 +430,7 @@ GEM unicorn (5.4.1) kgio (~> 2.6) raindrops (~> 0.7) - uniform_notifier (1.11.0) + uniform_notifier (1.12.1) webmock (3.4.2) addressable (>= 2.3.6) crack (>= 0.3.2) From 033762242027a5c5c9adce0badec10028139c16f Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 Feb 2019 18:11:09 +1100 Subject: [PATCH 23/74] Revert "DEV: more low risk gem updates" This reverts commit 2c53dde9181d42 Turns out redis upgrade breaks our usage of redis, more internal fixes are required --- Gemfile.lock | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7eba484635..3632fe0d0d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -197,7 +197,7 @@ GEM metaclass (0.0.4) method_source (0.8.2) mini_mime (1.0.1) - mini_portile2 (2.4.0) + mini_portile2 (2.3.0) mini_racer (0.2.4) libv8 (>= 6.3) mini_scheduler (0.9.1) @@ -217,8 +217,8 @@ GEM mustache (1.0.5) nap (1.1.0) no_proxy_fix (0.1.2) - nokogiri (1.10.1) - mini_portile2 (~> 2.4.0) + nokogiri (1.8.5) + mini_portile2 (~> 2.3.0) nokogumbo (1.5.0) nokogiri oauth (0.5.4) @@ -272,7 +272,7 @@ GEM parallel (1.13.0) parser (2.6.0.0) ast (~> 2.4.0) - pg (1.1.4) + pg (1.1.3) powerpack (0.1.2) progress (3.4.0) pry (0.10.4) @@ -292,7 +292,7 @@ GEM rack-openid (1.3.1) rack (>= 1.1.0) ruby-openid (>= 2.1.8) - rack-protection (2.0.5) + rack-protection (2.0.3) rack rack-test (1.1.0) rack (>= 1.0, < 3) @@ -323,7 +323,7 @@ GEM msgpack (>= 0.4.3) trollop (>= 1.16.2) rchardet (1.8.0) - redis (4.1.0) + redis (4.0.1) redis-namespace (1.6.0) redis (>= 3.0.4) request_store (1.4.1) @@ -398,9 +398,9 @@ GEM shoulda-context (1.2.2) shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (5.2.5) - connection_pool (~> 2.2, >= 2.2.2) - rack (>= 1.5.0) + sidekiq (5.1.3) + concurrent-ruby (~> 1.0) + connection_pool (~> 2.2, >= 2.2.0) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) slop (3.6.0) @@ -427,7 +427,7 @@ GEM unf_ext unf_ext (0.0.7.5) unicode-display_width (1.4.1) - unicorn (5.4.1) + unicorn (5.4.0) kgio (~> 2.6) raindrops (~> 0.7) uniform_notifier (1.12.1) From e0e91fad877ffca71cc3baa94984fe0d5efaaf74 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 4 Feb 2019 13:46:39 +0200 Subject: [PATCH 24/74] FIX: Fix delete button for Tag Groups. (#6965) --- .../discourse/models/tag-group.js.es6 | 5 +++ .../discourse/templates/tag-groups-show.hbs | 2 +- .../acceptance/tag-groups-test.js.es6 | 36 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 test/javascripts/acceptance/tag-groups-test.js.es6 diff --git a/app/assets/javascripts/discourse/models/tag-group.js.es6 b/app/assets/javascripts/discourse/models/tag-group.js.es6 index 4d85f65be2..4c46aa6f9a 100644 --- a/app/assets/javascripts/discourse/models/tag-group.js.es6 +++ b/app/assets/javascripts/discourse/models/tag-group.js.es6 @@ -9,6 +9,11 @@ export default RestModel.extend({ return saving || Ember.isEmpty(name) || Ember.isEmpty(tagNames); }, + @computed("id") + disableDelete(id) { + return !parseInt(id); + }, + @computed("permissions") permissionName: { get(permissions) { diff --git a/app/assets/javascripts/discourse/templates/tag-groups-show.hbs b/app/assets/javascripts/discourse/templates/tag-groups-show.hbs index 65e1b8e6d6..6e289180a9 100644 --- a/app/assets/javascripts/discourse/templates/tag-groups-show.hbs +++ b/app/assets/javascripts/discourse/templates/tag-groups-show.hbs @@ -44,6 +44,6 @@ - + {{model.savingStatus}} diff --git a/test/javascripts/acceptance/tag-groups-test.js.es6 b/test/javascripts/acceptance/tag-groups-test.js.es6 new file mode 100644 index 0000000000..95556f09dd --- /dev/null +++ b/test/javascripts/acceptance/tag-groups-test.js.es6 @@ -0,0 +1,36 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("Tag Groups", { + loggedIn: true, + settings: { tagging_enabled: true }, + pretend(server, helper) { + server.post("/tag_groups", () => { + return helper.response({ + tag_group: { + id: 42, + name: "test tag group", + tag_names: ["monkey"], + parent_tag_name: [], + one_per_topic: false, + permissions: { everyone: 1 } + } + }); + }); + } +}); + +QUnit.test("tag groups can be saved and deleted", async assert => { + const tags = selectKit(".tag-chooser"); + + await visit("/tag_groups"); + await click(".content-list .btn"); + + await fillIn(".tag-group-content h1 input", "test tag group"); + await tags.expand(); + await tags.selectRowByValue("monkey"); + + await click(".tag-group-content .btn.btn-default"); + + await click(".tag-chooser .choice:first"); + assert.ok(!find(".tag-group-content .btn.btn-danger")[0].disabled); +}); From 9de906ddab3e440ef5ee1e9cf04aa1ae20757d31 Mon Sep 17 00:00:00 2001 From: Jeff Wong Date: Mon, 4 Feb 2019 08:27:40 -0800 Subject: [PATCH 25/74] FIX: Register pan events for touch only * touch events - only register touch, not pointer events * immediately request redraw frame, do not wait for after render to fire. --- .../discourse/components/site-header.js.es6 | 19 +++----- .../discourse/mixins/pan-events.js.es6 | 44 ++++++++----------- 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/discourse/components/site-header.js.es6 b/app/assets/javascripts/discourse/components/site-header.js.es6 index 61211e8515..85913e6a71 100644 --- a/app/assets/javascripts/discourse/components/site-header.js.es6 +++ b/app/assets/javascripts/discourse/components/site-header.js.es6 @@ -137,6 +137,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { this._isPanning = true; $("header.d-header").removeClass("scroll-down scroll-up"); this.eventDispatched(this._leftMenuAction(), "header"); + window.requestAnimationFrame(() => this.panMove(e)); } else if ( windowWidth - center.x < SCREEN_EDGE_MARGIN && !this.$(".menu-panel").length && @@ -148,6 +149,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { this._isPanning = true; $("header.d-header").removeClass("scroll-down scroll-up"); this.eventDispatched(this._rightMenuAction(), "header"); + window.requestAnimationFrame(() => this.panMove(e)); } else { this._isPanning = false; } @@ -242,13 +244,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { } }); - if (this.site.mobileView) { - $("body") - .on("pointerdown", e => this._panStart(e)) - .on("pointermove", e => this._panMove(e)) - .on("pointerup", e => this._panMove(e)) - .on("pointercancel", e => this._panMove(e)); - } + this.addTouchListeners($("body")); }, willDestroyElement() { @@ -260,13 +256,8 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { this.appEvents.off("header:hide-topic"); this.appEvents.off("dom:clean"); - if (this.site.mobileView) { - $("body") - .off("pointerdown") - .off("pointerup") - .off("pointermove") - .off("pointercancel"); - } + this.removeTouchListeners($("body")); + Ember.run.cancel(this._scheduledRemoveAnimate); window.cancelAnimationFrame(this._scheduledMovingAnimation); }, diff --git a/app/assets/javascripts/discourse/mixins/pan-events.js.es6 b/app/assets/javascripts/discourse/mixins/pan-events.js.es6 index a83cffbc30..840a6aa31d 100644 --- a/app/assets/javascripts/discourse/mixins/pan-events.js.es6 +++ b/app/assets/javascripts/discourse/mixins/pan-events.js.es6 @@ -12,37 +12,31 @@ export default Ember.Mixin.create({ didInsertElement() { this._super(...arguments); - - if (this.site.mobileView) { - if ("onpointerdown" in document.documentElement) { - this.$() - .on("pointerdown", e => this._panStart(e)) - .on("pointermove", e => this._panMove(e, e)) - .on("pointerup", e => this._panMove(e, e)) - .on("pointercancel", e => this._panMove(e, e)); - } else if ("ontouchstart" in document.documentElement) { - this.$() - .on("touchstart", e => this._panStart(e.touches[0])) - .on("touchmove", e => { - const touchEvent = e.touches[0]; - touchEvent.type = "pointermove"; - this._panMove(touchEvent, e); - }) - .on("touchend", e => this._panMove({ type: "pointerup" }, e)) - .on("touchcancel", e => this._panMove({ type: "pointercancel" }, e)); - } - } + this.addTouchListeners(this.$()); }, willDestroyElement() { this._super(...arguments); + this.removeTouchListeners(this.$()); + }, + addTouchListeners($element) { if (this.site.mobileView) { - this.$() - .off("pointerdown") - .off("pointerup") - .off("pointermove") - .off("pointercancel") + $element + .on("touchstart", e => this._panStart(e.touches[0])) + .on("touchmove", e => { + const touchEvent = e.touches[0]; + touchEvent.type = "pointermove"; + this._panMove(touchEvent, e); + }) + .on("touchend", e => this._panMove({ type: "pointerup" }, e)) + .on("touchcancel", e => this._panMove({ type: "pointercancel" }, e)); + } + }, + + removeTouchListeners($element) { + if (this.site.mobileView) { + $element .off("touchstart") .off("touchmove") .off("touchend") From 58d5fc3632c956d5a51cd72d49c62f0ee21657fa Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 4 Feb 2019 12:46:12 -0500 Subject: [PATCH 26/74] UX: Minor button icon color fixes --- .../javascripts/discourse/templates/selected-posts.hbs | 6 +++--- app/assets/stylesheets/common/admin/backups.scss | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/selected-posts.hbs b/app/assets/javascripts/discourse/templates/selected-posts.hbs index 517bd3a00a..e2de58c845 100644 --- a/app/assets/javascripts/discourse/templates/selected-posts.hbs +++ b/app/assets/javascripts/discourse/templates/selected-posts.hbs @@ -13,15 +13,15 @@ {{/if}} {{#if canMergeTopic}} - {{d-button action=(route-action "moveToTopic") icon="sign-out-alt" label="topic.move_to.action" class="move-to-topic"}} + {{d-button action=(route-action "moveToTopic") icon="sign-out-alt" label="topic.move_to.action" class="btn-primary move-to-topic"}} {{/if}} {{#if canChangeOwner}} - {{d-button action=(route-action "changeOwner") icon="user" label="topic.change_owner.action"}} + {{d-button action=(route-action "changeOwner") icon="user" label="topic.change_owner.action" class="btn-primary"}} {{/if}} {{#if canMergePosts}} - {{d-button action=(action "mergePosts") icon="arrows-v" label="topic.merge_posts.action"}} + {{d-button action=(action "mergePosts") icon="arrows-v" label="topic.merge_posts.action" class="btn-primary"}} {{/if}}

{{i18n 'topic.multi_select.cancel'}}

diff --git a/app/assets/stylesheets/common/admin/backups.scss b/app/assets/stylesheets/common/admin/backups.scss index 20b9836986..51fc9f46e5 100644 --- a/app/assets/stylesheets/common/admin/backups.scss +++ b/app/assets/stylesheets/common/admin/backups.scss @@ -6,8 +6,14 @@ $rollback-darker: darken($rollback, 20%) !default; .btn-rollback { color: $secondary; background: $rollback; + .d-icon { + color: $secondary; + } &:hover { background: $rollback-dark; + .d-icon { + color: currentColor; + } } &:active { @include linear-gradient($rollback-darker, $rollback-dark); From be24220e95225720d5e619d739f79aef0a52d2e5 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 5 Feb 2019 06:54:10 +1100 Subject: [PATCH 27/74] DEV: update low risk gems This follows up on 03376224 which reverted the redis update which is not supported, rest of the gems should be fine.(unicorn / nokogiri / sidekiq / pg) --- Gemfile.lock | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3632fe0d0d..7eba484635 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -197,7 +197,7 @@ GEM metaclass (0.0.4) method_source (0.8.2) mini_mime (1.0.1) - mini_portile2 (2.3.0) + mini_portile2 (2.4.0) mini_racer (0.2.4) libv8 (>= 6.3) mini_scheduler (0.9.1) @@ -217,8 +217,8 @@ GEM mustache (1.0.5) nap (1.1.0) no_proxy_fix (0.1.2) - nokogiri (1.8.5) - mini_portile2 (~> 2.3.0) + nokogiri (1.10.1) + mini_portile2 (~> 2.4.0) nokogumbo (1.5.0) nokogiri oauth (0.5.4) @@ -272,7 +272,7 @@ GEM parallel (1.13.0) parser (2.6.0.0) ast (~> 2.4.0) - pg (1.1.3) + pg (1.1.4) powerpack (0.1.2) progress (3.4.0) pry (0.10.4) @@ -292,7 +292,7 @@ GEM rack-openid (1.3.1) rack (>= 1.1.0) ruby-openid (>= 2.1.8) - rack-protection (2.0.3) + rack-protection (2.0.5) rack rack-test (1.1.0) rack (>= 1.0, < 3) @@ -323,7 +323,7 @@ GEM msgpack (>= 0.4.3) trollop (>= 1.16.2) rchardet (1.8.0) - redis (4.0.1) + redis (4.1.0) redis-namespace (1.6.0) redis (>= 3.0.4) request_store (1.4.1) @@ -398,9 +398,9 @@ GEM shoulda-context (1.2.2) shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (5.1.3) - concurrent-ruby (~> 1.0) - connection_pool (~> 2.2, >= 2.2.0) + sidekiq (5.2.5) + connection_pool (~> 2.2, >= 2.2.2) + rack (>= 1.5.0) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) slop (3.6.0) @@ -427,7 +427,7 @@ GEM unf_ext unf_ext (0.0.7.5) unicode-display_width (1.4.1) - unicorn (5.4.0) + unicorn (5.4.1) kgio (~> 2.6) raindrops (~> 0.7) uniform_notifier (1.12.1) From e5a81aeb6e6c35ff20c0d816c020e705b7b40ba9 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 4 Feb 2019 15:05:37 -0500 Subject: [PATCH 28/74] REFACTOR: Remove stubbed methods in tests --- spec/services/flag_sockpuppets_spec.rb | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/spec/services/flag_sockpuppets_spec.rb b/spec/services/flag_sockpuppets_spec.rb index 8dcb69fa37..ea875e4b3b 100644 --- a/spec/services/flag_sockpuppets_spec.rb +++ b/spec/services/flag_sockpuppets_spec.rb @@ -112,34 +112,41 @@ describe SpamRule::FlagSockpuppets do describe 'flag_sockpuppet_users' do let(:post2) { Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) } + let(:system) { Discourse.system_user } + let(:spam) { PostActionType.types[:spam] } it 'flags post and first post if both users are new' do - PostAction.expects(:act).with(Discourse.system_user, post1, PostActionType.types[:spam], anything).once - PostAction.expects(:act).with(Discourse.system_user, post2, PostActionType.types[:spam], anything).once described_class.new(post2).flag_sockpuppet_users + + expect(PostAction.where(user: system, post: post1, post_action_type_id: spam).exists?).to eq(true) + expect(PostAction.where(user: system, post: post2, post_action_type_id: spam).exists?).to eq(true) end it "doesn't flag the first post more than once" do - PostAction.expects(:act).with(Discourse.system_user, post2, PostActionType.types[:spam], anything).once - PostAction.stubs(:act).with(Discourse.system_user, post1, PostActionType.types[:spam], anything).raises(PostAction::AlreadyActed) described_class.new(post2).flag_sockpuppet_users + + expect(PostAction.where(user: system, post: post2, post_action_type_id: spam).exists?).to eq(true) + expect(PostAction.where(post: post2, post_action_type_id: spam).count).to eq(1) end it "doesn't flag the first post if the user is not new" do old_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 25.hours.ago, trust_level: TrustLevel[1]) first_post = Fabricate(:post, user: old_user, topic: Fabricate(:topic, user: old_user)) post2 = Fabricate(:post, user: Fabricate(:user, ip_address: old_user.ip_address), topic: first_post.topic) - PostAction.expects(:act).with(anything, post2, anything, anything).once - PostAction.expects(:act).with(anything, first_post, anything, anything).never + described_class.new(post2).flag_sockpuppet_users + + expect(PostAction.where(user: system, post: post2, post_action_type_id: spam).exists?).to eq(true) + expect(PostAction.where(user: system, post: first_post, post_action_type_id: spam).exists?).to eq(false) end it "doesn't create a flag if user is nil on first post" do post1.user_id = nil post1.save - PostAction.expects(:act).with(anything, post2, anything, anything).once - PostAction.expects(:act).with(anything, post1, anything, anything).never described_class.new(post2).flag_sockpuppet_users + + expect(PostAction.where(user: system, post: post2, post_action_type_id: spam).exists?).to eq(true) + expect(PostAction.where(user: system, post: post1, post_action_type_id: spam).exists?).to eq(false) end end end From edcdbe1946dc56969c9c8e1f4a9c777dd5e540a7 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 4 Feb 2019 15:41:58 -0500 Subject: [PATCH 29/74] DEV: Restore textarea type for site settings Currenty, no settings in core use this, but textareas will be useful in theme settings and plugins. --- lib/site_settings/type_supervisor.rb | 9 ++++++++- spec/components/site_settings/type_supervisor_spec.rb | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index 0f4f483faa..93da66333e 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -6,7 +6,7 @@ module SiteSettings; end class SiteSettings::TypeSupervisor include SiteSettings::Validations - CONSUMED_OPTS = %i[enum choices type validator min max regex hidden regex_error allow_any list_type].freeze + CONSUMED_OPTS = %i[enum choices type validator min max regex hidden regex_error allow_any list_type textarea].freeze VALIDATOR_OPTS = %i[min max regex hidden regex_error].freeze # For plugins, so they can tell if a feature is supported @@ -65,11 +65,16 @@ class SiteSettings::TypeSupervisor @types = {} @allow_any = {} @list_type = {} + @textareas = {} end def load_setting(name_arg, opts = {}) name = name_arg.to_sym + if opts[:textarea] + @textareas[name] = opts[:textarea] + end + if (enum = opts[:enum]) @enums[name] = enum.is_a?(String) ? enum.constantize : enum opts[:type] ||= :enum @@ -148,6 +153,8 @@ class SiteSettings::TypeSupervisor result[:choices] = @choices[name] if @choices.has_key? name result[:list_type] = @list_type[name] if @list_type.has_key? name + result[:textarea] = @textareas[name] if @textareas.has_key? name + result end diff --git a/spec/components/site_settings/type_supervisor_spec.rb b/spec/components/site_settings/type_supervisor_spec.rb index 9648b0ec27..181a06a146 100644 --- a/spec/components/site_settings/type_supervisor_spec.rb +++ b/spec/components/site_settings/type_supervisor_spec.rb @@ -331,6 +331,7 @@ describe SiteSettings::TypeSupervisor do settings.setting(:type_float, 2.3232) settings.setting(:type_string, 'string') settings.setting(:type_url_list, 'string', type: 'url_list') + settings.setting(:type_textarea, 'string', textarea: true) settings.setting(:type_enum_choices, '2', type: 'enum', choices: ['1', '2']) settings.setting(:type_enum_class, 'a', enum: 'TestEnumClass2') settings.setting(:type_list, 'a', type: 'list', choices: ['a', 'b'], list_type: 'compact') @@ -355,6 +356,9 @@ describe SiteSettings::TypeSupervisor do it 'returns url_list type' do expect(settings.type_supervisor.type_hash(:type_url_list)[:type]).to eq 'url_list' end + it 'returns textarea type' do + expect(settings.type_supervisor.type_hash(:type_textarea)[:textarea]).to eq true + end it 'returns enum type' do expect(settings.type_supervisor.type_hash(:type_enum_choices)[:type]).to eq 'enum' end From 8b88c738cf3da2aa12b67989e934c9649452dbd6 Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 4 Feb 2019 16:52:11 -0500 Subject: [PATCH 30/74] Updating README images --- README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index ea0716a668..a00c5b9643 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,8 @@ -![Logo](images/discourse.png) + + + Discourse is the 100% open source discussion platform built for the next decade of the Internet. Use it as a: @@ -11,12 +15,12 @@ To learn more about the philosophy and goals of the project, [visit **discourse. ## Screenshots -Boing Boing - - - +Boing Boing + + + -Mobile +Mobile Browse [lots more notable Discourse instances](https://www.discourse.org/customers). From 635bc72ec5ec5c13718bcb468bf9fd1833fc26a9 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 5 Feb 2019 09:08:44 +1100 Subject: [PATCH 31/74] DEV: pin redis to version 4.0.1 Version 4.1.0 returns frozen hashes which conflict with our monkey patch We will follow up unpinning this later --- Gemfile | 10 +++++++++- Gemfile.lock | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index f5bdeb3c16..24f5f69b86 100644 --- a/Gemfile +++ b/Gemfile @@ -31,7 +31,15 @@ gem 'mini_mime' gem 'mini_suffix' gem 'hiredis' -gem 'redis', require: ["redis", "redis/connection/hiredis"] + +# holding off redis upgrade temporarily as it is having issues with our current +# freedom patch, we will follow this up. +# +# FrozenError: can't modify frozen Hash +# /var/www/discourse/vendor/bundle/ruby/2.5.0/gems/redis-4.1.0/lib/redis/client.rb:93:in `delete' +# /var/www/discourse/vendor/bundle/ruby/2.5.0/gems/redis-4.1.0/lib/redis/client.rb:93:in `initialize' +# /var/www/discourse/lib/freedom_patches/redis.rb:7:in `initialize' +gem 'redis', '4.0.1', require: ["redis", "redis/connection/hiredis"] gem 'redis-namespace' gem 'active_model_serializers', '~> 0.8.3' diff --git a/Gemfile.lock b/Gemfile.lock index 7eba484635..d1a6f4cdcc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -323,7 +323,7 @@ GEM msgpack (>= 0.4.3) trollop (>= 1.16.2) rchardet (1.8.0) - redis (4.1.0) + redis (4.0.1) redis-namespace (1.6.0) redis (>= 3.0.4) request_store (1.4.1) @@ -527,7 +527,7 @@ DEPENDENCIES rb-inotify (~> 0.9) rbtrace rchardet - redis + redis (= 4.0.1) redis-namespace rinku rotp From 27c8688f028fcc3e4c53c57b7494ba4d961f7bf6 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 5 Feb 2019 11:23:21 +1100 Subject: [PATCH 32/74] DEV: update dependencies This updates some more low risk gems, maxmind, hiredis (c wrapper for redis), puma, rbtrace and stackprof. --- Gemfile.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d1a6f4cdcc..24e96885ab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -153,7 +153,7 @@ GEM hashdiff (0.3.7) hashie (3.6.0) highline (1.7.10) - hiredis (0.6.1) + hiredis (0.6.3) hkdf (0.3.0) htmlentities (4.3.4) http_accept_language (2.0.5) @@ -190,7 +190,7 @@ GEM lru_redux (1.1.0) mail (2.7.1) mini_mime (>= 0.1.1) - maxminddb (0.1.21) + maxminddb (0.1.22) memory_profiler (0.9.12) message_bus (2.2.0) rack (>= 1.1.3) @@ -269,12 +269,13 @@ GEM openid-redis-store (0.0.2) redis ruby-openid + optimist (3.0.0) parallel (1.13.0) parser (2.6.0.0) ast (~> 2.4.0) pg (1.1.4) powerpack (0.1.2) - progress (3.4.0) + progress (3.5.0) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -284,7 +285,7 @@ GEM pry-rails (0.3.6) pry (>= 0.10.4) public_suffix (3.0.3) - puma (3.11.4) + puma (3.12.0) r2 (0.2.7) rack (2.0.6) rack-mini-profiler (1.0.1) @@ -318,10 +319,10 @@ GEM rb-fsevent (0.10.3) rb-inotify (0.9.10) ffi (>= 0.5.0, < 2) - rbtrace (0.4.10) + rbtrace (0.4.11) ffi (>= 1.0.6) msgpack (>= 0.4.3) - trollop (>= 1.16.2) + optimist (>= 3.0.0) rchardet (1.8.0) redis (4.0.1) redis-namespace (1.6.0) @@ -412,13 +413,12 @@ GEM activesupport (>= 4.0) sprockets (>= 3.0.0) sshkey (1.9.0) - stackprof (0.2.11) + stackprof (0.2.12) terminal-table (1.8.0) unicode-display_width (~> 1.1, >= 1.1.1) thor (0.20.3) thread_safe (0.3.6) tilt (2.0.8) - trollop (2.1.2) tzinfo (1.2.5) thread_safe (~> 0.1) uglifier (4.1.20) From 1748ec421e5049f23103f69aad8180b03a81c067 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 5 Feb 2019 12:35:42 +1100 Subject: [PATCH 33/74] DEV: gem updates Another group of gems updated, better_errors is fairly safe as its for dev chunky_png fairly safe, used for getting image info from pngs, erubi update is very safe. Sanitize is used by onebox and should always be on latest. Long term we should think of making sanitize an optional dependency on onebox cause we are happy to just provide methods from core to do this and it would remove nokogumbo and sanitize deps. --- Gemfile.lock | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 24e96885ab..716c4e901f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -61,7 +61,7 @@ GEM barber (0.12.0) ember-source (>= 1.0, < 3.1) execjs (>= 1.2, < 3) - better_errors (2.4.0) + better_errors (2.5.0) coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) @@ -75,7 +75,7 @@ GEM uniform_notifier (~> 1.11) byebug (10.0.2) certified (1.0.0) - chunky_png (1.3.10) + chunky_png (1.3.11) claide (1.0.2) claide-plugins (0.9.2) cork @@ -87,7 +87,7 @@ GEM connection_pool (2.2.2) cork (0.3.0) colored2 (~> 3.1) - cppjieba_rb (0.3.0) + cppjieba_rb (0.3.3) crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) @@ -126,10 +126,10 @@ GEM jquery-rails (>= 1.0.17) railties (>= 3.1) ember-source (2.18.2) - erubi (1.7.1) + erubi (1.8.0) excon (0.62.0) execjs (2.7.0) - exifr (1.3.4) + exifr (1.3.5) fabrication (2.20.1) fakeweb (1.3.0) faraday (0.15.4) @@ -141,7 +141,7 @@ GEM rake rake-compiler fast_xs (0.8.0) - fastimage (2.1.3) + fastimage (2.1.5) ffi (1.9.25) flamegraph (0.9.5) fspath (3.1.0) @@ -219,8 +219,8 @@ GEM no_proxy_fix (0.1.2) nokogiri (1.10.1) mini_portile2 (~> 2.4.0) - nokogumbo (1.5.0) - nokogiri + nokogumbo (2.0.1) + nokogiri (~> 1.8, >= 1.8.4) oauth (0.5.4) oauth2 (1.4.1) faraday (>= 0.8, < 0.16.0) @@ -374,10 +374,10 @@ GEM nokogiri (>= 1.6.0) ruby_dep (1.5.0) safe_yaml (1.0.4) - sanitize (4.6.5) + sanitize (5.0.0) crass (~> 1.0.2) - nokogiri (>= 1.4.4) - nokogumbo (~> 1.4) + nokogiri (>= 1.8.0) + nokogumbo (~> 2.0) sass (3.5.6) sass-listen (~> 4.0.0) sass-listen (4.0.0) From b6e3ef4a9051fb95e6cfd28a5c208274e2a37d69 Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 4 Feb 2019 22:14:18 -0500 Subject: [PATCH 34/74] Minor icon color fix --- app/assets/stylesheets/common/base/compose.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss index f57cac9d6e..e09bd7bd6a 100644 --- a/app/assets/stylesheets/common/base/compose.scss +++ b/app/assets/stylesheets/common/base/compose.scss @@ -68,7 +68,7 @@ display: none; } .d-icon { - opacity: 1; + color: $secondary; } } } From a342d2f1508af0bf11f70f780d672dc8e9a09aa0 Mon Sep 17 00:00:00 2001 From: Joe <33972521+hnb-ku@users.noreply.github.com> Date: Tue, 5 Feb 2019 17:47:22 +0800 Subject: [PATCH 35/74] UX: reduces white-space in polls (#6956) * reduces white-space in polls and uses font-size variables --- .../poll/assets/stylesheets/common/poll.scss | 30 +++++++++---------- .../poll/assets/stylesheets/desktop/poll.scss | 4 +-- .../poll/assets/stylesheets/mobile/poll.scss | 11 ++++--- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/plugins/poll/assets/stylesheets/common/poll.scss b/plugins/poll/assets/stylesheets/common/poll.scss index 8510fe6c56..63f671d9ee 100644 --- a/plugins/poll/assets/stylesheets/common/poll.scss +++ b/plugins/poll/assets/stylesheets/common/poll.scss @@ -16,20 +16,17 @@ div.poll { li { cursor: pointer; font-size: $font-up-1; - &:not(:last-of-type) { - margin-bottom: 0.5em; - } } li[data-poll-option-id] { color: $primary; - padding: 0.5em 0.7em 0.7em 0.5em; + padding: 0.5em 0; } img { - max-width: 100% !important; /* needed to override internal styles */ + // needed to override internal styles in image-sizing hack + max-width: 100% !important; height: auto; - margin-top: 0.25em; } .poll-info { @@ -38,34 +35,34 @@ div.poll { padding: 1em 0; .info-label { - font-size: 1.7em; + font-size: $font-up-4; line-height: $line-height-medium; } .info-text { - margin: 5px 0; + margin: 0.25em 0; display: block; } } .poll-container { vertical-align: middle; - padding: 0.8em; + padding: 0.5em 1em; .poll-results-number-rating { - font-size: 2em; + font-size: $font-up-5; } } .poll-buttons { .info-text { - margin: 0 5px; + margin: 0.25em 0; color: $primary-medium; } } - .poll-voters-list { - margin-top: 0.25em; + .poll-voters:not(:empty) { + margin-bottom: 0.25em; li { display: inline; } @@ -79,17 +76,18 @@ div.poll { .results { > li { cursor: default; - padding: 0.5em 0.7em 0.7em 0.5em; + padding: 0.25em 0; + &:last-child { + padding-bottom: 0; + } } .option { - padding-bottom: 0.25em; p { margin: 0; } } .percentage { - font-size: $font-up-1; float: right; color: $primary-medium; margin-left: 0.25em; diff --git a/plugins/poll/assets/stylesheets/desktop/poll.scss b/plugins/poll/assets/stylesheets/desktop/poll.scss index 0c1b15273c..6926e4a702 100644 --- a/plugins/poll/assets/stylesheets/desktop/poll.scss +++ b/plugins/poll/assets/stylesheets/desktop/poll.scss @@ -15,7 +15,7 @@ div.poll { } p { - margin: 1.5em; + margin: 0.5em 0; } .info-label { @@ -30,7 +30,7 @@ div.poll { .poll-buttons { border-top: 1px solid $primary-low; - padding: 1em 1.25em; + padding: 1em; .info-text { line-height: 2em; diff --git a/plugins/poll/assets/stylesheets/mobile/poll.scss b/plugins/poll/assets/stylesheets/mobile/poll.scss index a7efc3f483..fd0178e020 100644 --- a/plugins/poll/assets/stylesheets/mobile/poll.scss +++ b/plugins/poll/assets/stylesheets/mobile/poll.scss @@ -1,20 +1,19 @@ div.poll { .poll-buttons { - padding: 0.5em 1.25em 1em 1.25em; + padding: 0.5em 1em 1em 1em; button { - margin: 0.25em; + margin-right: 0.5em; } } .poll-info { - padding: 0.5em 1.25em 0 1.25em; + padding: 0 1em; display: flex; flex-wrap: wrap; border-top: 1px solid $primary-low; .info-text { display: inline; - margin: 0 0.5em 0.25em 0; } .info-number { font-size: $font-up-6; @@ -23,8 +22,8 @@ div.poll { margin: 0; width: 100%; } - .info-label:before { - content: "\00a0"; //nbsp + .info-label { + margin-left: 0.25em; } } } From 2c222e16fefe1b2ae17f7748360c97d3978194f5 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 5 Feb 2019 11:54:11 +0200 Subject: [PATCH 36/74] FEATURE: Returning falsy value from upload handler stops upload. (#6969) --- .../javascripts/discourse/components/composer-editor.js.es6 | 5 +++-- app/assets/javascripts/discourse/lib/plugin-api.js.es6 | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 059bddbb54..db6cde6c0f 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -681,8 +681,9 @@ export default Ember.Component.extend({ const matchingHandler = uploadHandlers.find(matcher); if (data.files.length === 1 && matchingHandler) { - matchingHandler.method(data.files[0]); - return false; + if (!matchingHandler.method(data.files[0])) { + return false; + } } // If no plugin, continue as normal diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index a44548c175..ecce02061d 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -786,7 +786,8 @@ class PluginApi { /** * * Registers a function to handle uploads for specified file types - * The normal uploading functionality will be bypassed + * The normal uploading functionality will be bypassed if function returns + * a falsy value. * This only for uploads of individual files * * Example: From 4f3ee86bbd0b2c33af4863c77ba18c7a03994187 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 5 Feb 2019 11:54:52 +0200 Subject: [PATCH 37/74] FIX: in:title should work irrespective of the order. (#6968) --- lib/search.rb | 3 +-- spec/components/search_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index 61ccf79d91..04f7315098 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -157,6 +157,7 @@ class Search term.gsub!(/[\u201c\u201d]/, '"') @clean_term = term + @in_title = false term = process_advanced_search!(term) @@ -551,8 +552,6 @@ class Search end end - @in_title = false - if word == 'order:latest' || word == 'l' @order = :latest nil diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index ce6088a0a9..70b9ea8ab2 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -1053,6 +1053,19 @@ describe Search do results = Search.execute('first in:title') expect(results.posts.length).to eq(0) end + + it 'works irrespective of the order' do + topic = Fabricate(:topic, title: "A topic about Discourse") + Fabricate(:post, topic: topic, raw: "This is another post") + topic2 = Fabricate(:topic, title: "This is another topic") + Fabricate(:post, topic: topic2, raw: "Discourse is awesome") + + results = Search.execute('Discourse in:title status:open') + expect(results.posts.length).to eq(1) + + results = Search.execute('in:title status:open Discourse') + expect(results.posts.length).to eq(1) + end end context 'ignore_diacritics' do From 31ffcf989ce63e451baaed7deb61bf3e3126f197 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 5 Feb 2019 13:10:28 +0200 Subject: [PATCH 38/74] UX: Use translatedLabel for aria-label in buttons. --- app/assets/javascripts/discourse/components/d-button.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/components/d-button.js.es6 b/app/assets/javascripts/discourse/components/d-button.js.es6 index 70a409327e..67132bfc11 100644 --- a/app/assets/javascripts/discourse/components/d-button.js.es6 +++ b/app/assets/javascripts/discourse/components/d-button.js.es6 @@ -9,7 +9,7 @@ export default Ember.Component.extend({ attributeBindings: [ "disabled", "translatedTitle:title", - "translatedTitle:aria-label", + "translatedLabel:aria-label", "tabindex" ], From d42139dfaab87127e9c1942e64664a55bd37645d Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Tue, 5 Feb 2019 12:59:20 +0100 Subject: [PATCH 39/74] fix typo --- config/locales/server.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2af15b9479..386b06b808 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1328,7 +1328,7 @@ en: summary_posts_required: "Minimum posts in a topic before 'Summarize This Topic' is enabled" summary_likes_required: "Minimum likes in a topic before 'Summarize This Topic' is enabled" summary_percent_filter: "When a user clicks 'Summarize This Topic', show the top % of posts" - summary_max_results: "Maximum posts returned by 'Summary This Topic'" + summary_max_results: "Maximum posts returned by 'Summarize This Topic'" enable_personal_messages: "Allow trust level 1 (configurable via min trust level to send messages) users to create messages and reply to messages. Note that staff can always send messages no matter what." enable_system_message_replies: "Allows users to reply to system messages, even if personal messages are disabled" From 7b7bc3db39fc4ea5f91c5de71acebea36e2b8fe7 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Feb 2019 13:49:16 +0000 Subject: [PATCH 40/74] FIX: Rescue and display import errors when updating theme via git --- app/controllers/admin/themes_controller.rb | 2 ++ spec/requests/admin/themes_controller_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index f927eee337..31bda272de 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -209,6 +209,8 @@ class Admin::ThemesController < Admin::AdminController end end end + rescue RemoteTheme::ImportError => e + render_json_error e.message end def destroy diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index dc25e12513..c20b1284a0 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -311,7 +311,19 @@ describe Admin::ThemesController do # Database correct theme.reload expect(theme.theme_translation_overrides.count).to eq(0) + end + it 'handles import errors on update' do + theme.create_remote_theme!(remote_url: "https://example.com/repository") + + # RemoteTheme is extensively tested, and setting up the test scaffold is a large overhead + # So use a stub here to test the controller + RemoteTheme.any_instance.stubs(:update_from_remote).raises(RemoteTheme::ImportError.new("error message")) + put "/admin/themes/#{theme.id}.json", params: { + theme: { remote_update: true } + } + expect(response.status).to eq(422) + expect(JSON.parse(response.body)["errors"].first).to eq("error message") end it 'returns the right error message' do From a3b47c1dd1cbc5a3abc8b56909751c1558150847 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Feb 2019 14:14:53 +0000 Subject: [PATCH 41/74] FEATURE: Allow string theme settings to display with multiple lines To use, add `textarea: true` to the theme settings.yml. For example: ``` my_setting: default: "some string" textarea: true ``` --- app/serializers/theme_settings_serializer.rb | 11 ++++++++++- lib/theme_settings_manager.rb | 4 ++++ lib/theme_settings_parser.rb | 2 ++ spec/components/theme_settings_manager_spec.rb | 6 ++++++ spec/fixtures/theme_settings/valid_settings.yaml | 4 ++++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/serializers/theme_settings_serializer.rb b/app/serializers/theme_settings_serializer.rb index d7fe44c140..1bd892f755 100644 --- a/app/serializers/theme_settings_serializer.rb +++ b/app/serializers/theme_settings_serializer.rb @@ -1,6 +1,6 @@ class ThemeSettingsSerializer < ApplicationSerializer attributes :setting, :type, :default, :value, :description, :valid_values, - :list_type + :list_type, :textarea def setting object.name @@ -41,4 +41,13 @@ class ThemeSettingsSerializer < ApplicationSerializer def include_list_type? object.type == ThemeSetting.types[:list] end + + def textarea + object.textarea + end + + def include_textarea? + object.type == ThemeSetting.types[:string] + end + end diff --git a/lib/theme_settings_manager.rb b/lib/theme_settings_manager.rb index 81c2a7ef0c..c59b1481e6 100644 --- a/lib/theme_settings_manager.rb +++ b/lib/theme_settings_manager.rb @@ -105,6 +105,10 @@ class ThemeSettingsManager def is_valid_value?(new_value) (@opts[:min]..@opts[:max]).include? new_value.to_s.length end + + def textarea + @opts[:textarea] + end end class Bool < self diff --git a/lib/theme_settings_parser.rb b/lib/theme_settings_parser.rb index 064dd7ef14..ab2ee4396c 100644 --- a/lib/theme_settings_parser.rb +++ b/lib/theme_settings_parser.rb @@ -38,6 +38,8 @@ class ThemeSettingsParser opts[:list_type] = raw_opts[:list_type] end + opts[:textarea] = !!raw_opts[:textarea] + opts end diff --git a/spec/components/theme_settings_manager_spec.rb b/spec/components/theme_settings_manager_spec.rb index feb0382298..525d98d507 100644 --- a/spec/components/theme_settings_manager_spec.rb +++ b/spec/components/theme_settings_manager_spec.rb @@ -128,6 +128,12 @@ describe ThemeSettingsManager do expect { string_setting.value = ("a" * 21) }.to raise_error(Discourse::InvalidParameters) end + + it "can be a textarea" do + string_setting = find_by_name(:string_setting_02) + expect(find_by_name(:string_setting_02).textarea).to eq(false) + expect(find_by_name(:string_setting_03).textarea).to eq(true) + end end context "List" do diff --git a/spec/fixtures/theme_settings/valid_settings.yaml b/spec/fixtures/theme_settings/valid_settings.yaml index 8250f34f38..2885fba1b8 100644 --- a/spec/fixtures/theme_settings/valid_settings.yaml +++ b/spec/fixtures/theme_settings/valid_settings.yaml @@ -10,6 +10,10 @@ string_setting_02: min: 2 max: 20 +string_setting_03: + default: "string value" + textarea: true + integer_setting: 51 integer_setting_02: From fc999c04b34edb93cb3538718bcd22d295509a4a Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Feb 2019 15:12:39 +0000 Subject: [PATCH 42/74] Drop facebook_user_infos and twitter_user_infos (#6970) Data was migrated to user_associated_accounts in 208005f and 160d29b --- .../20190205104116_drop_unused_auth_tables.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 db/post_migrate/20190205104116_drop_unused_auth_tables.rb diff --git a/db/post_migrate/20190205104116_drop_unused_auth_tables.rb b/db/post_migrate/20190205104116_drop_unused_auth_tables.rb new file mode 100644 index 0000000000..45b5864d2c --- /dev/null +++ b/db/post_migrate/20190205104116_drop_unused_auth_tables.rb @@ -0,0 +1,18 @@ +require 'migration/table_dropper' + +class DropUnusedAuthTables < ActiveRecord::Migration[5.2] + def change + def up + %i{ + facebook_user_infos + twitter_user_infos + }.each do |table| + Migration::TableDropper.execute_drop(table) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end + end +end From ba724d7f25d9df8c4903663d308be17de4408262 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 5 Feb 2019 17:50:27 +0100 Subject: [PATCH 43/74] FIX: S3 endpoint broke bucket creation in non-default region --- app/models/site_setting.rb | 2 +- config/locales/server.en.yml | 4 ++-- config/site_settings.yml | 2 +- lib/s3_helper.rb | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 0bdf7b3000..49b9281c3d 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -152,7 +152,7 @@ class SiteSetting < ActiveRecord::Base bucket = SiteSetting.enable_s3_uploads ? Discourse.store.s3_bucket_name : GlobalSetting.s3_bucket_name # cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region - if SiteSetting.s3_endpoint == "https://s3.amazonaws.com" + if SiteSetting.s3_endpoint.blank? || SiteSetting.s3_endpoint.end_with?("amazonaws.com") if SiteSetting.Upload.s3_region.start_with?("cn-") "//#{bucket}.s3.#{SiteSetting.Upload.s3_region}.amazonaws.com.cn" else diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 386b06b808..286a8f2553 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1485,7 +1485,7 @@ en: automatic_backups_enabled: "Run automatic backups as defined in backup frequency" backup_frequency: "The number of days between backups." s3_backup_bucket: "The remote bucket to hold backups. WARNING: Make sure it is a private bucket." - s3_endpoint: "The endpoint can be modified to backup to an S3 compatible service like DigitalOcean Spaces or Minio. WARNING: Use default if using AWS S3" + s3_endpoint: "The endpoint can be modified to backup to an S3 compatible service like DigitalOcean Spaces or Minio. WARNING: Leave blank if using AWS S3." s3_force_path_style: "Enforce path-style addressing for your custom endpoint. IMPORTANT: Required for using Minio uploads and backups." s3_configure_tombstone_policy: "Enable automatic deletion policy for tombstone uploads. IMPORTANT: If disabled, no space will be reclaimed after uploads are deleted." s3_disable_cleanup: "Disable the removal of backups from S3 when removed locally." @@ -1542,7 +1542,7 @@ en: s3_upload_bucket: "The Amazon S3 bucket name that files will be uploaded into. WARNING: must be lowercase, no periods, no underscores." s3_access_key_id: "The Amazon S3 access key id that will be used to upload images." s3_secret_access_key: "The Amazon S3 secret access key that will be used to upload images." - s3_region: "The Amazon S3 region name that will be used to upload images." + s3_region: "The Amazon S3 region name that will be used to upload images and backups." s3_cdn_url: "The CDN URL to use for all s3 assets (for example: https://cdn.somewhere.com). WARNING: after changing this setting you must rebake all old posts." avatar_sizes: "List of automatically generated avatar sizes." diff --git a/config/site_settings.yml b/config/site_settings.yml index 5a1fcd1afb..7b13e73b02 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1038,7 +1038,7 @@ files: default: "" regex: '^[a-z0-9\-\/]+$' # can't use '.' when using HTTPS s3_endpoint: - default: "https://s3.amazonaws.com" + default: "" regex: '^https?:\/\/.+[^\/]$' shadowed_by_global: true s3_cdn_url: diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index 380ba86e5a..3e7b6c81d6 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -193,10 +193,11 @@ class S3Helper def self.s3_options(obj) opts = { region: obj.s3_region, - endpoint: SiteSetting.s3_endpoint, force_path_style: SiteSetting.s3_force_path_style } + opts[:endpoint] = SiteSetting.s3_endpoint if SiteSetting.s3_endpoint.present? + unless obj.s3_use_iam_profile opts[:access_key_id] = obj.s3_access_key_id opts[:secret_access_key] = obj.s3_secret_access_key From e7821a63e71edd1524d90343066255d478c4c24a Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Tue, 5 Feb 2019 23:31:19 +0530 Subject: [PATCH 44/74] FIX: Users should able check the emails for self --- app/controllers/users_controller.rb | 6 ++++-- spec/requests/users_controller_spec.rb | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index fe87d7d506..5d7c99b13c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -148,9 +148,11 @@ class UsersController < ApplicationController def check_emails user = fetch_user_from_params(include_inactive: true) - guardian.ensure_can_check_emails!(user) - StaffActionLogger.new(current_user).log_check_email(user, context: params[:context]) + unless user == current_user + guardian.ensure_can_check_emails!(user) + StaffActionLogger.new(current_user).log_check_email(user, context: params[:context]) + end email, *secondary_emails = user.emails diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 6752147159..11dda380c0 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2072,6 +2072,19 @@ describe UsersController do expect(response).to be_forbidden end + it "returns emails and associated_accounts for self" do + user = Fabricate(:user) + sign_in(user) + + get "/u/#{user.username}/emails.json" + + expect(response.status).to eq(200) + json = JSON.parse(response.body) + expect(json["email"]).to eq(user.email) + expect(json["secondary_emails"]).to eq(user.secondary_emails) + expect(json["associated_accounts"]).to eq([]) + end + it "returns emails and associated_accounts when you're allowed to see them" do user = Fabricate(:user) sign_in_admin From bdbf77dc38f93c5314458f213bbd67b601828089 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 5 Feb 2019 20:47:00 +0100 Subject: [PATCH 45/74] FIX: Unpause Sidekiq before uploading backup to S3 No need to pause Sidekiq longer than really needed. Uploads to S3 can take a long time. --- lib/backup_restore/backuper.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb index bbdf0c561a..cdda8a1de2 100644 --- a/lib/backup_restore/backuper.rb +++ b/lib/backup_restore/backuper.rb @@ -43,6 +43,8 @@ module BackupRestore log "Finalizing backup..." @with_uploads ? create_archive : move_dump_backup + + unpause_sidekiq upload_archive after_create_hook @@ -334,6 +336,7 @@ module BackupRestore end def unpause_sidekiq + return unless Sidekiq.paused? log "Unpausing sidekiq..." Sidekiq.unpause! rescue => ex From a52b2c962519ab7eb57a40980d58ea97a14e4d91 Mon Sep 17 00:00:00 2001 From: Kris Date: Tue, 5 Feb 2019 21:40:17 -0500 Subject: [PATCH 46/74] UX: Moving the create theme buttons, adding buttons to theme index --- .../javascripts/admin/templates/customize-themes-index.hbs | 6 +++++- app/assets/stylesheets/common/admin/customize.scss | 7 +++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/admin/templates/customize-themes-index.hbs b/app/assets/javascripts/admin/templates/customize-themes-index.hbs index 565d0024db..62d563e3cf 100644 --- a/app/assets/javascripts/admin/templates/customize-themes-index.hbs +++ b/app/assets/javascripts/admin/templates/customize-themes-index.hbs @@ -2,13 +2,17 @@

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

+
+ {{d-button label="admin.customize.new" icon="plus" action=(route-action "showCreateModal") class="btn-primary"}} + {{d-button action=(route-action "importModal") icon="upload" label="admin.customize.import" class="btn-default"}} +
{{#each externalResources as |resource|}} {{d-icon resource.icon}} {{I18n resource.key}} - {{/each}} + {{/each}}
diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index e6654ac80e..0217c7b19b 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -71,10 +71,6 @@ margin-bottom: 0; } } - - .create-actions { - margin-left: auto; - } } .admin-container { padding: 0; @@ -203,6 +199,9 @@ vertical-align: top; margin-top: 5px; } + .create-actions { + margin-bottom: 1em; + } .content-wrapper { display: inline-block; vertical-align: top; From 1a72242746136c6872c8822f33ebf148568e3a46 Mon Sep 17 00:00:00 2001 From: Kris Date: Tue, 5 Feb 2019 21:44:06 -0500 Subject: [PATCH 47/74] line-height adjustment --- app/assets/stylesheets/common/admin/badges.scss | 3 +++ app/assets/stylesheets/common/admin/customize.scss | 1 + 2 files changed, 4 insertions(+) diff --git a/app/assets/stylesheets/common/admin/badges.scss b/app/assets/stylesheets/common/admin/badges.scss index ee8b99cd84..495eb24a77 100644 --- a/app/assets/stylesheets/common/admin/badges.scss +++ b/app/assets/stylesheets/common/admin/badges.scss @@ -66,6 +66,9 @@ } .badge-intro { margin-top: 10%; + h1 { + line-height: $line-height-medium; + } .badge-intro-emoji { // it's an emoji so we want fixed deminsions height: 55px; diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index 0217c7b19b..dcbaf0212e 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -208,6 +208,7 @@ width: 65%; h1 { display: inline-block; + line-height: $line-height-medium; } } .external-link { From 0098b3072e126136cb4bc9adf764485a98e9aeab Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 6 Feb 2019 16:51:45 +1100 Subject: [PATCH 48/74] DEV: update rack-mini-profiler This gem update fixes an issue with upcoming Rails 6 (without this fix mini profiler will not work on rails 6 and simply renders text) --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 716c4e901f..9904a8f9c6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -288,7 +288,7 @@ GEM puma (3.12.0) r2 (0.2.7) rack (2.0.6) - rack-mini-profiler (1.0.1) + rack-mini-profiler (1.0.2) rack (>= 1.2.0) rack-openid (1.3.1) rack (>= 1.1.0) From 448ea663c312b0cf208c5fb066ed040303089bd2 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 6 Feb 2019 16:54:06 +1100 Subject: [PATCH 49/74] DEV: remove seed-fu pinning from rails master This pinning should no longer be needed --- Gemfile | 1 - 1 file changed, 1 deletion(-) diff --git a/Gemfile b/Gemfile index 24f5f69b86..29797ed7bd 100644 --- a/Gemfile +++ b/Gemfile @@ -11,7 +11,6 @@ end if rails_master? gem 'arel', git: 'https://github.com/rails/arel.git' gem 'rails', git: 'https://github.com/rails/rails.git' - gem 'seed-fu', git: 'https://github.com/SamSaffron/seed-fu.git', branch: 'discourse' else # until rubygems gives us optional dependencies we are stuck with this # bundle update actionmailer actionpack actionview activemodel activerecord activesupport railties From 4cfc201604fae75d39ee86cb97ae3b3016832488 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 6 Feb 2019 16:54:42 +1100 Subject: [PATCH 50/74] DEV: update logster to stable release This update logster to the stable 2.0.1 release instead of running a pre release --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9904a8f9c6..8f894e2438 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -183,7 +183,7 @@ GEM logstash-event (1.2.02) logstash-logger (0.26.1) logstash-event (~> 1.2) - logster (2.0.0.pre) + logster (2.0.1) loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) From fe7c10b40931025d55dc4674085052e6d3ed64d3 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 6 Feb 2019 17:33:36 +1100 Subject: [PATCH 51/74] DEV: fix seed-fu require for rails 6 --- Gemfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 29797ed7bd..04de71ec54 100644 --- a/Gemfile +++ b/Gemfile @@ -22,9 +22,10 @@ else gem 'activesupport', '5.2.2' gem 'railties', '5.2.2' gem 'sprockets-rails' - gem 'seed-fu' end +gem 'seed-fu' + gem 'mail', require: false gem 'mini_mime' gem 'mini_suffix' From 15857b900a3fe325064a47ae89b4ee7662880d53 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 6 Feb 2019 17:45:48 +1100 Subject: [PATCH 52/74] DEV: explicitly require Rails components `rails/all` includes too much stuff per: https://github.com/rails/rails/blob/master/railties/lib/rails/all.rb This commit makes it explicit what pieces of Rails Discourse depends on. Previously the LoadError was protecting us and we were excluding components, using the Gemfile, this method ensures that even if we add `rails` meta gem as a dependency only the parts of Rails Discourse uses will be used. --- config/application.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 65233ef4d8..a3ecce58ac 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,7 +14,11 @@ rescue end require File.expand_path('../boot', __FILE__) -require 'rails/all' +require 'active_record/railtie' +require 'action_controller/railtie' +require 'action_view/railtie' +require 'action_mailer/railtie' +require 'sprockets/railtie' # Plugin related stuff require_relative '../lib/discourse_event' From ff12c4b2d42b910eb3d73740d6f6e351521305b9 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 6 Feb 2019 19:16:08 +0530 Subject: [PATCH 53/74] FIX: Bucket name is missing in S3 inventory data path --- lib/s3_inventory.rb | 27 +++++++++---- spec/components/s3_inventory_spec.rb | 57 +++++++++++++++------------ spec/jobs/update_s3_inventory_spec.rb | 2 +- 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/lib/s3_inventory.rb b/lib/s3_inventory.rb index 321182f3d6..3f069673b3 100644 --- a/lib/s3_inventory.rb +++ b/lib/s3_inventory.rb @@ -10,6 +10,7 @@ class S3Inventory CSV_KEY_INDEX ||= 1 CSV_ETAG_INDEX ||= 2 INVENTORY_PREFIX ||= "inventory" + INVENTORY_VERSION ||= "1" def initialize(s3_helper, type) @s3_helper = s3_helper @@ -96,10 +97,10 @@ class S3Inventory "Effect": "Allow", "Principal": { "Service": "s3.amazonaws.com" }, "Action": ["s3:PutObject"], - "Resource": ["arn:aws:s3:::#{inventory_root_path}/*"], + "Resource": ["#{inventory_path_arn}/*"], "Condition": { "ArnLike": { - "aws:SourceArn": "arn:aws:s3:::#{bucket_name}" + "aws:SourceArn": bucket_arn }, "StringEquals": { "s3:x-amz-acl": "bucket-owner-full-control" @@ -134,7 +135,7 @@ class S3Inventory { destination: { s3_bucket_destination: { - bucket: "arn:aws:s3:::#{bucket_name}", + bucket: bucket_arn, prefix: destination_prefix, format: "CSV" } @@ -163,7 +164,7 @@ class S3Inventory def unsorted_files objects = [] - @s3_helper.list(File.join(inventory_path, "data")).each do |obj| + @s3_helper.list(inventory_data_path).each do |obj| if obj.key.match?(/\.csv\.gz$/i) objects << obj end @@ -174,12 +175,22 @@ class S3Inventory log("Failed to list inventory from S3", e) end - def inventory_path - File.join(inventory_root_path, inventory_id) + def inventory_data_path + File.join(inventory_path, bucket_name, inventory_id, "data") end - def inventory_root_path - File.join(bucket_name, bucket_folder_path || "", INVENTORY_PREFIX) + def inventory_path_arn + File.join(bucket_arn, inventory_path) + end + + def inventory_path + path = File.join(INVENTORY_PREFIX, INVENTORY_VERSION) + path = File.join(bucket_folder_path, path) if bucket_folder_path.present? + path + end + + def bucket_arn + "arn:aws:s3:::#{bucket_name}" end def log(message, ex = nil) diff --git a/spec/components/s3_inventory_spec.rb b/spec/components/s3_inventory_spec.rb index 5f147f370f..fdb4a793a6 100644 --- a/spec/components/s3_inventory_spec.rb +++ b/spec/components/s3_inventory_spec.rb @@ -16,33 +16,38 @@ describe "S3Inventory" do SiteSetting.s3_secret_access_key = "def" SiteSetting.enable_s3_inventory = true - client.stub_responses(:list_objects, - contents: [ - { - etag: "\"70ee1738b6b21e2c8a43f3a5ab0eee71\"", - key: "example1.csv.gz", - last_modified: Time.parse("2014-11-21T19:40:05.000Z"), - owner: { - display_name: "myname", - id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", + client.stub_responses(:list_objects, -> (context) { + inventory_data_path = "#{S3Inventory::INVENTORY_PREFIX}/#{S3Inventory::INVENTORY_VERSION}/bucket/original/data" + expect(context.params[:prefix]).to eq(inventory_data_path) + + { + contents: [ + { + etag: "\"70ee1738b6b21e2c8a43f3a5ab0eee71\"", + key: "example1.csv.gz", + last_modified: Time.parse("2014-11-21T19:40:05.000Z"), + owner: { + display_name: "myname", + id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", + }, + size: 11, + storage_class: "STANDARD", }, - size: 11, - storage_class: "STANDARD", - }, - { - etag: "\"9c8af9a76df052144598c115ef33e511\"", - key: "example2.csv.gz", - last_modified: Time.parse("2013-11-15T01:10:49.000Z"), - owner: { - display_name: "myname", - id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", - }, - size: 713193, - storage_class: "STANDARD", - } - ], - next_marker: "eyJNYXJrZXIiOiBudWxsLCAiYm90b190cnVuY2F0ZV9hbW91bnQiOiAyfQ==" - ) + { + etag: "\"9c8af9a76df052144598c115ef33e511\"", + key: "example2.csv.gz", + last_modified: Time.parse("2013-11-15T01:10:49.000Z"), + owner: { + display_name: "myname", + id: "12345example25102679df27bb0ae12b3f85be6f290b936c4393484be31bebcc", + }, + size: 713193, + storage_class: "STANDARD", + } + ], + next_marker: "eyJNYXJrZXIiOiBudWxsLCAiYm90b190cnVuY2F0ZV9hbW91bnQiOiAyfQ==" + } + }) end it "should return the latest inventory file name" do diff --git a/spec/jobs/update_s3_inventory_spec.rb b/spec/jobs/update_s3_inventory_spec.rb index 1bcb8985a7..ec26f950b1 100644 --- a/spec/jobs/update_s3_inventory_spec.rb +++ b/spec/jobs/update_s3_inventory_spec.rb @@ -18,7 +18,7 @@ describe Jobs::UpdateS3Inventory do id = "original" @client.expects(:put_bucket_policy).with( bucket: "bucket", - policy: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"InventoryAndAnalyticsPolicy\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"s3.amazonaws.com\"},\"Action\":[\"s3:PutObject\"],\"Resource\":[\"arn:aws:s3:::bucket/inventory/*\"],\"Condition\":{\"ArnLike\":{\"aws:SourceArn\":\"arn:aws:s3:::bucket\"},\"StringEquals\":{\"s3:x-amz-acl\":\"bucket-owner-full-control\"}}}]}" + policy: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"InventoryAndAnalyticsPolicy\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"s3.amazonaws.com\"},\"Action\":[\"s3:PutObject\"],\"Resource\":[\"arn:aws:s3:::bucket/#{S3Inventory::INVENTORY_PREFIX}/#{S3Inventory::INVENTORY_VERSION}/*\"],\"Condition\":{\"ArnLike\":{\"aws:SourceArn\":\"arn:aws:s3:::bucket\"},\"StringEquals\":{\"s3:x-amz-acl\":\"bucket-owner-full-control\"}}}]}" ) @client.expects(:put_bucket_inventory_configuration) @client.expects(:put_bucket_inventory_configuration).with( From 381793243e0af6a8083be34ca0e720cdc8fa717a Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 6 Feb 2019 19:19:00 +0530 Subject: [PATCH 54/74] FIX: include error message if the "accept invite" process fails --- app/controllers/invites_controller.rb | 3 ++- config/locales/server.en.yml | 1 + spec/requests/invites_controller_spec.rb | 12 ++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index a178e401da..884f67ec8f 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -63,7 +63,8 @@ class InvitesController < ApplicationController rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e render json: { success: false, - errors: e.record&.errors&.to_hash || {} + errors: e.record&.errors&.to_hash || {}, + message: I18n.t('invite.error_message') } end else diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 286a8f2553..c525fe1f90 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -208,6 +208,7 @@ en:

If you remember your password you can Login.

Otherwise please Reset Password.

+ error_message: "There was an error accepting invite. Please contact the site's administrator." user_exists: "There's no need to invite %{email}, they already have an account!" confirm_email: "

You’re almost done! We sent an activation mail to your email address. Please follow the instructions in the mail to activate your account.

If it doesn’t arrive, check your spam folder.

" diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 037d2aa047..b2b7f4152c 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -213,6 +213,18 @@ describe InvitesController do end end + context 'with an invalid invite record' do + let(:invite) { Fabricate(:invite, email: "John Doe ") } + it "responds with error message" do + put "/invites/show/#{invite.invite_key}.json" + expect(response.status).to eq(200) + json = JSON.parse(response.body) + expect(json["success"]).to eq(false) + expect(json["message"]).to eq(I18n.t('invite.error_message')) + expect(session[:current_user_id]).to be_blank + end + end + context 'with a deleted invite' do let(:topic) { Fabricate(:topic) } From ba9cc83d4cf946582d756d728b3bb224b24f291d Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 6 Feb 2019 20:51:28 +0530 Subject: [PATCH 55/74] FIX: Destination prefix in S3 inventory configuration is incorrect --- lib/s3_inventory.rb | 9 ++------- spec/jobs/update_s3_inventory_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/s3_inventory.rb b/lib/s3_inventory.rb index 3f069673b3..763f1d1e46 100644 --- a/lib/s3_inventory.rb +++ b/lib/s3_inventory.rb @@ -125,18 +125,13 @@ class S3Inventory def inventory_configuration filter_prefix = inventory_id - destination_prefix = File.join(INVENTORY_PREFIX, inventory_id) - - if bucket_folder_path.present? - filter_prefix = File.join(bucket_folder_path, filter_prefix) - destination_prefix = File.join(bucket_folder_path, destination_prefix) - end + filter_prefix = File.join(bucket_folder_path, filter_prefix) if bucket_folder_path.present? { destination: { s3_bucket_destination: { bucket: bucket_arn, - prefix: destination_prefix, + prefix: inventory_path, format: "CSV" } }, diff --git a/spec/jobs/update_s3_inventory_spec.rb b/spec/jobs/update_s3_inventory_spec.rb index ec26f950b1..07241cea0b 100644 --- a/spec/jobs/update_s3_inventory_spec.rb +++ b/spec/jobs/update_s3_inventory_spec.rb @@ -16,9 +16,11 @@ describe Jobs::UpdateS3Inventory do it "updates the bucket policy and inventory configuration in S3" do id = "original" + path = File.join(S3Inventory::INVENTORY_PREFIX, S3Inventory::INVENTORY_VERSION) + @client.expects(:put_bucket_policy).with( bucket: "bucket", - policy: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"InventoryAndAnalyticsPolicy\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"s3.amazonaws.com\"},\"Action\":[\"s3:PutObject\"],\"Resource\":[\"arn:aws:s3:::bucket/#{S3Inventory::INVENTORY_PREFIX}/#{S3Inventory::INVENTORY_VERSION}/*\"],\"Condition\":{\"ArnLike\":{\"aws:SourceArn\":\"arn:aws:s3:::bucket\"},\"StringEquals\":{\"s3:x-amz-acl\":\"bucket-owner-full-control\"}}}]}" + policy: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"InventoryAndAnalyticsPolicy\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"s3.amazonaws.com\"},\"Action\":[\"s3:PutObject\"],\"Resource\":[\"arn:aws:s3:::bucket/#{path}/*\"],\"Condition\":{\"ArnLike\":{\"aws:SourceArn\":\"arn:aws:s3:::bucket\"},\"StringEquals\":{\"s3:x-amz-acl\":\"bucket-owner-full-control\"}}}]}" ) @client.expects(:put_bucket_inventory_configuration) @client.expects(:put_bucket_inventory_configuration).with( @@ -28,7 +30,7 @@ describe Jobs::UpdateS3Inventory do destination: { s3_bucket_destination: { bucket: "arn:aws:s3:::bucket", - prefix: "inventory/#{id}", + prefix: path, format: "CSV" } }, From f3cfce4a93240fce264a25356d27303c749c472c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 6 Feb 2019 15:51:23 +0000 Subject: [PATCH 56/74] FEATURE: Calculate sprite-sheet based on currently active themes (#6973) Previously there was only one sprite sheet, which always included icons from all themes even if they were disabled --- app/controllers/svg_sprite_controller.rb | 7 +- app/helpers/application_helper.rb | 6 +- app/models/theme.rb | 1 + app/models/theme_field.rb | 4 ++ config/routes.rb | 2 +- lib/svg_sprite/svg_sprite.rb | 65 ++++++++++--------- spec/components/svg_sprite/svg_sprite_spec.rb | 46 ++++++++++--- spec/requests/svg_sprite_controller_spec.rb | 12 +++- 8 files changed, 92 insertions(+), 51 deletions(-) diff --git a/app/controllers/svg_sprite_controller.rb b/app/controllers/svg_sprite_controller.rb index 2a8e05958b..266db14013 100644 --- a/app/controllers/svg_sprite_controller.rb +++ b/app/controllers/svg_sprite_controller.rb @@ -10,12 +10,13 @@ class SvgSpriteController < ApplicationController no_cookies RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do + theme_ids = params[:theme_ids].split(",").map(&:to_i) - if SvgSprite.version != params[:version] - return redirect_to path(SvgSprite.path) + if SvgSprite.version(theme_ids) != params[:version] + return redirect_to path(SvgSprite.path(theme_ids)) end - svg_sprite = "window.__svg_sprite = #{SvgSprite.bundle.inspect};" + svg_sprite = "window.__svg_sprite = #{SvgSprite.bundle(theme_ids).inspect};" response.headers["Last-Modified"] = 10.years.ago.httpdate response.headers["Content-Length"] = svg_sprite.bytesize.to_s diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4963b95832..38b826b9a5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -382,7 +382,7 @@ module ApplicationHelper def theme_ids if customization_disabled? - nil + [nil] else request.env[:resolved_theme_ids] end @@ -454,11 +454,11 @@ module ApplicationHelper asset_version: Discourse.assets_digest, disable_custom_css: loading_admin?, highlight_js_path: HighlightJs.path, - svg_sprite_path: SvgSprite.path, + svg_sprite_path: SvgSprite.path(theme_ids), } if Rails.env.development? - setup_data[:svg_icon_list] = SvgSprite.all_icons + setup_data[:svg_icon_list] = SvgSprite.all_icons(theme_ids) end if guardian.can_enable_safe_mode? && params["safe_mode"] diff --git a/app/models/theme.rb b/app/models/theme.rb index 3b0be21fef..6a7854ede1 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -121,6 +121,7 @@ class Theme < ActiveRecord::Base end def self.transform_ids(ids, extend: true) + return [] if ids.nil? get_set_cache "#{extend ? "extended_" : ""}transformed_ids_#{ids.join("_")}" do next [] if ids.blank? diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index 34c24f1f49..740eaf4dd6 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -7,6 +7,10 @@ class ThemeField < ActiveRecord::Base belongs_to :upload has_one :javascript_cache, dependent: :destroy + after_commit do |field| + SvgSprite.expire_cache if field.target_id == Theme.targets[:settings] + end + scope :find_by_theme_ids, ->(theme_ids) { return none unless theme_ids.present? diff --git a/config/routes.rb b/config/routes.rb index 955d0b1c16..dca765bef8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -453,7 +453,7 @@ Discourse::Application.routes.draw do # in most production settings this is bypassed get "letter_avatar_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter" - get "svg-sprite/:hostname/svg-:version.js" => "svg_sprite#show", format: false, constraints: { hostname: /[\w\.-]+/, version: /\h{40}/ } + get "svg-sprite/:hostname/svg-:theme_ids-:version.js" => "svg_sprite#show", format: false, constraints: { hostname: /[\w\.-]+/, version: /\h{40}/, theme_ids: /([0-9]+(,[0-9]+)*)?/ } get "svg-sprite/search/:keyword" => "svg_sprite#search", format: false, constraints: { keyword: /[-a-z0-9\s\%]+/ } get "highlight-js/:hostname/:version.js" => "highlight_js#show", format: false, constraints: { hostname: /[\w\.-]+/ } diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index 3717d59944..217fa9772c 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -188,39 +188,37 @@ module SvgSprite SVG_SPRITE_PATHS = Dir.glob(["#{Rails.root}/vendor/assets/svg-icons/**/*.svg", "#{Rails.root}/plugins/*/svg-icons/*.svg"]) - def self.svg_sprite_cache - @svg_sprite_cache ||= DistributedCache.new('svg_sprite') + def self.all_icons(theme_ids = []) + get_set_cache("icons_#{Theme.transform_ids(theme_ids).join(',')}") do + Set.new() + .merge(settings_icons) + .merge(plugin_icons) + .merge(badge_icons) + .merge(group_icons) + .merge(theme_icons(theme_ids)) + .delete_if { |i| i.blank? || i.include?("/") } + .map! { |i| process(i.dup) } + .merge(SVG_ICONS) + .sort + end end - def self.all_icons - Set.new() - .merge(settings_icons) - .merge(plugin_icons) - .merge(badge_icons) - .merge(group_icons) - .merge(theme_icons) - .delete_if { |i| i.blank? || i.include?("/") } - .map! { |i| process(i.dup) } - .merge(SVG_ICONS) - .sort + def self.version(theme_ids = []) + get_set_cache("version_#{Theme.transform_ids(theme_ids).join(',')}") do + Digest::SHA1.hexdigest(all_icons(theme_ids).join('|')) + end end - def self.rebuild_cache - icons = all_icons - svg_sprite_cache['icons'] = icons - svg_sprite_cache['version'] = Digest::SHA1.hexdigest(icons.join('|')) + def self.path(theme_ids = []) + "/svg-sprite/#{Discourse.current_hostname}/svg-#{theme_ids&.join(",")}-#{version(theme_ids)}.js" end def self.expire_cache - svg_sprite_cache.clear + cache&.clear end - def self.version - svg_sprite_cache['version'] || rebuild_cache - end - - def self.bundle - icons = svg_sprite_cache['icons'] || all_icons + def self.bundle(theme_ids = []) + icons = all_icons(theme_ids) doc = File.open("#{Rails.root}/vendor/assets/svg-icons/fontawesome/solid.svg") { |f| Nokogiri::XML(f) } fa_license = doc.at('//comment()').text @@ -240,7 +238,6 @@ Discourse SVG subset of #{fa_license} svg_file.css('symbol').each do |sym| icon_id = prepare_symbol(sym, svg_filename) - if icons.include? icon_id sym.attributes['id'].value = icon_id sym.css('title').each(&:remove) @@ -286,10 +283,6 @@ Discourse SVG subset of #{fa_license} icon_id end - def self.path - "/svg-sprite/#{Discourse.current_hostname}/svg-#{version}.js" - end - def self.settings_icons # includes svg_icon_subset and any settings containing _icon (incl. plugin settings) site_setting_icons = [] @@ -319,11 +312,11 @@ Discourse SVG subset of #{fa_license} Group.where("flair_url LIKE '%fa-%'").pluck(:flair_url).uniq end - def self.theme_icons + def self.theme_icons(theme_ids) theme_icon_settings = [] - # Theme.all includes default values - Theme.all.each do |theme| + # Need to load full records for default values + Theme.where(id: Theme.transform_ids(theme_ids)).each do |theme| settings = theme.cached_settings.each do |key, value| if key.to_s.include?("_icon") && String === value theme_icon_settings |= value.split('|') @@ -347,4 +340,12 @@ Discourse SVG subset of #{fa_license} FA_ICON_MAP.each { |k, v| icon_name.sub!(k, v) } fa4_to_fa5_names[icon_name] || icon_name end + + def self.get_set_cache(key) + cache[key] ||= yield + end + + def self.cache + @cache ||= DistributedCache.new('svg_sprite') + end end diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb index 1d0fad90cc..b33fc70d41 100644 --- a/spec/components/svg_sprite/svg_sprite_spec.rb +++ b/spec/components/svg_sprite/svg_sprite_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe SvgSprite do before do - SvgSprite.rebuild_cache + SvgSprite.expire_cache end it 'can generate a bundle' do @@ -12,6 +12,15 @@ describe SvgSprite do expect(bundle).to match(/angle-double-down/) end + it 'can generate paths' do + version = SvgSprite.version # Icons won't change for this test + expect(SvgSprite.path).to eq("/svg-sprite/#{Discourse.current_hostname}/svg--#{version}.js") + expect(SvgSprite.path([1, 2])).to eq("/svg-sprite/#{Discourse.current_hostname}/svg-1,2-#{version}.js") + + # Safe mode + expect(SvgSprite.path([nil])).to eq("/svg-sprite/#{Discourse.current_hostname}/svg--#{version}.js") + end + it 'can search for a specific FA icon' do expect(SvgSprite.search("fa-heart")).to match(/heart/) expect(SvgSprite.search("poo-storm")).to match(/poo-storm/) @@ -51,26 +60,43 @@ describe SvgSprite do it 'includes icons defined in theme settings' do theme = Fabricate(:theme) - theme.set_field(target: :settings, name: :yaml, value: "custom_icon: magic") + + # Works for default settings: + theme.set_field(target: :settings, name: :yaml, value: "custom_icon: dragon") theme.save! + expect(SvgSprite.all_icons([theme.id])).to include("dragon") - # TODO: add test for default settings values + # Automatically purges cache when default changes: + theme.set_field(target: :settings, name: :yaml, value: "custom_icon: gamepad") + theme.save! + expect(SvgSprite.all_icons([theme.id])).to include("gamepad") + # Works when applying override theme.update_setting(:custom_icon, "gas-pump") - expect(SvgSprite.all_icons).to include("gas-pump") + expect(SvgSprite.all_icons([theme.id])).to include("gas-pump") + # Works when changing override theme.update_setting(:custom_icon, "gamepad") - expect(SvgSprite.all_icons).to include("gamepad") - expect(SvgSprite.all_icons).not_to include("gas-pump") + expect(SvgSprite.all_icons([theme.id])).to include("gamepad") + expect(SvgSprite.all_icons([theme.id])).not_to include("gas-pump") # FA5 syntax theme.update_setting(:custom_icon, "fab fa-bandcamp") - expect(SvgSprite.all_icons).to include("fab-bandcamp") + expect(SvgSprite.all_icons([theme.id])).to include("fab-bandcamp") # Internal Discourse syntax + multiple icons theme.update_setting(:custom_icon, "fab-android|dragon") - expect(SvgSprite.all_icons).to include("fab-android") - expect(SvgSprite.all_icons).to include("dragon") + expect(SvgSprite.all_icons([theme.id])).to include("fab-android") + expect(SvgSprite.all_icons([theme.id])).to include("dragon") + + # Check themes don't leak into non-theme sprite sheet + expect(SvgSprite.all_icons).not_to include("dragon") + + # Check components are included + theme.update(component: true) + parent_theme = Fabricate(:theme) + parent_theme.add_child_theme!(theme) + expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon") end it 'includes icons from SiteSettings' do @@ -82,10 +108,12 @@ describe SvgSprite do expect(all_icons).to include("fab-bandcamp") SiteSetting.svg_icon_subset = nil + SvgSprite.expire_cache expect(SvgSprite.all_icons).not_to include("drafting-compass") # does not fail on non-string setting SiteSetting.svg_icon_subset = false + SvgSprite.expire_cache expect(SvgSprite.all_icons).to be_truthy end diff --git a/spec/requests/svg_sprite_controller_spec.rb b/spec/requests/svg_sprite_controller_spec.rb index 1004c3b11d..8eec7bcaf4 100644 --- a/spec/requests/svg_sprite_controller_spec.rb +++ b/spec/requests/svg_sprite_controller_spec.rb @@ -4,17 +4,23 @@ describe SvgSpriteController do context 'show' do before do - SvgSprite.rebuild_cache + SvgSprite.expire_cache end it "should return bundle when version is current" do - get "/svg-sprite/#{Discourse.current_hostname}/svg-#{SvgSprite.version}.js" + get "/svg-sprite/#{Discourse.current_hostname}/svg--#{SvgSprite.version}.js" + expect(response.status).to eq(200) + + theme = Fabricate(:theme) + theme.set_field(target: :settings, name: :yaml, value: "custom_icon: dragon") + theme.save! + get "/svg-sprite/#{Discourse.current_hostname}/svg-#{theme.id}-#{SvgSprite.version([theme.id])}.js" expect(response.status).to eq(200) end it "should redirect to current version" do random_hash = Digest::SHA1.hexdigest("somerandomstring") - get "/svg-sprite/#{Discourse.current_hostname}/svg-#{random_hash}.js" + get "/svg-sprite/#{Discourse.current_hostname}/svg--#{random_hash}.js" expect(response.status).to eq(302) expect(response.location).to include(SvgSprite.version) From 057d1dc077cb507987d7e1018a3fa73d6897ed1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 6 Feb 2019 17:06:23 +0100 Subject: [PATCH 57/74] UX: disable browser's autocomplete in search menu --- .../javascripts/discourse/widgets/search-menu-controls.js.es6 | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 b/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 index f30e5a61f8..b9128fbe23 100644 --- a/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 +++ b/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 @@ -19,6 +19,7 @@ createWidget("search-term", { return { type: "text", value: attrs.value || "", + autocomplete: "off", placeholder: attrs.contextEnabled ? "" : I18n.t("search.title") }; }, From ca03b2ff3045f8b0ade6ad1043ffb52cbda6b59f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 6 Feb 2019 11:37:18 -0500 Subject: [PATCH 58/74] Move SCSS variables for topic post width This allows them to be re-used by other components, for example the upcoming review queue. --- app/assets/stylesheets/common/foundation/variables.scss | 3 +++ app/assets/stylesheets/desktop/topic-post.scss | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/common/foundation/variables.scss b/app/assets/stylesheets/common/foundation/variables.scss index fe706f9231..29e1ebbba4 100644 --- a/app/assets/stylesheets/common/foundation/variables.scss +++ b/app/assets/stylesheets/common/foundation/variables.scss @@ -10,6 +10,9 @@ $medium-width: 995px !default; $large-width: 1110px !default; $input-padding: 4px 10px; +$topic-body-width: 690px; +$topic-body-width-padding: 11px; +$topic-avatar-width: 45px; // Brand color variables // -------------------------------------------------- diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index a6d836b956..0a742e0755 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -633,9 +633,6 @@ blockquote { } // variables are used to calculate the width of .gap -$topic-body-width: 690px; -$topic-body-width-padding: 11px; -$topic-avatar-width: 45px; .topic-body { width: calc(#{$topic-body-width} + (#{$topic-body-width-padding} * 2)); float: left; From ab2c2ea60543f70621f930122b641e38e6913e06 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 6 Feb 2019 22:38:06 +0530 Subject: [PATCH 59/74] FIX: validate Invite email against `EmailValidator.email_regex` (#6975) --- app/models/invite.rb | 2 +- spec/models/invite_spec.rb | 8 +++++++- spec/requests/invites_controller_spec.rb | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/models/invite.rb b/app/models/invite.rb index 2a7f96876a..b201240536 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -16,7 +16,7 @@ class Invite < ActiveRecord::Base has_many :topic_invites has_many :topics, through: :topic_invites, source: :topic validates_presence_of :invited_by_id - validates :email, email: true + validates :email, email: true, format: { with: EmailValidator.email_regex } before_create do self.invite_key ||= SecureRandom.hex diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 6ed73ee538..15ebea3f32 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -25,9 +25,15 @@ describe Invite do context 'email validators' do let(:coding_horror) { Fabricate(:coding_horror) } - let(:invite) { Invite.create(email: "test@mailinator.com", invited_by: coding_horror) } + + it "should not allow an invite with unformatted email address" do + expect { + Fabricate(:invite, email: "John Doe ") + }.to raise_error(ActiveRecord::RecordInvalid) + end it "should not allow an invite with blacklisted email" do + invite = Invite.create(email: "test@mailinator.com", invited_by: coding_horror) expect(invite).not_to be_valid end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index b2b7f4152c..9da4714338 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -214,8 +214,9 @@ describe InvitesController do end context 'with an invalid invite record' do - let(:invite) { Fabricate(:invite, email: "John Doe ") } + let(:invite) { Fabricate(:invite) } it "responds with error message" do + invite.update_attribute(:email, "John Doe ") put "/invites/show/#{invite.invite_key}.json" expect(response.status).to eq(200) json = JSON.parse(response.body) From 56a9f777cb54fdf1ce21fd605297d8f51a1398dd Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 6 Feb 2019 14:36:27 -0500 Subject: [PATCH 60/74] UX: Turn off autocomplete on composer title --- .../discourse/templates/components/composer-title.hbs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/templates/components/composer-title.hbs b/app/assets/javascripts/discourse/templates/components/composer-title.hbs index f4a824f57a..63d81f88a6 100644 --- a/app/assets/javascripts/discourse/templates/components/composer-title.hbs +++ b/app/assets/javascripts/discourse/templates/components/composer-title.hbs @@ -3,6 +3,7 @@ id="reply-title" maxLength=titleMaxLength placeholderKey=composer.titlePlaceholder - disabled=composer.loading}} + disabled=composer.loading + autocomplete="off"}} {{popup-input-tip validation=validation}} From b88aa4a592b737de59960d91a5675979cd697056 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 6 Feb 2019 21:09:21 +0000 Subject: [PATCH 61/74] FIX: Correctly process {{each}} in raw handlebars templates for themes --- lib/theme_javascript_compiler.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/theme_javascript_compiler.rb b/lib/theme_javascript_compiler.rb index d5a11ca291..e01d7a6e84 100644 --- a/lib/theme_javascript_compiler.rb +++ b/lib/theme_javascript_compiler.rb @@ -54,7 +54,7 @@ class ThemeJavascriptCompiler function manipulatePath(path) { // Override old themeSetting syntax when it's a param inside another node - if(path.parts[0] == "themeSettings"){ + if(path.parts && path.parts[0] == "themeSettings"){ const settingParts = path.parts.slice(1); path.type = "SubExpression"; Object.assign(path, generateHelper(settingParts)) @@ -63,7 +63,7 @@ class ThemeJavascriptCompiler function manipulateNode(node) { // Magically add theme id as the first param for each of these helpers - if (["theme-i18n", "theme-prefix", "theme-setting"].includes(node.path.parts[0])) { + if (node.path.parts && ["theme-i18n", "theme-prefix", "theme-setting"].includes(node.path.parts[0])) { node.params.unshift({ type: "NumberLiteral", value: #{@theme_id}, @@ -72,7 +72,7 @@ class ThemeJavascriptCompiler } // Override old themeSetting syntax when it's in its own node - if (node.path.parts[0] == "themeSettings") { + if (node.path.parts && node.path.parts[0] == "themeSettings") { Object.assign(node, generateHelper(node.path.parts.slice(1))) } } @@ -100,6 +100,7 @@ class ThemeJavascriptCompiler if(node.path.original == 'get' && node.params && node.params[0] + && node.params[0].parts && node.params[0].parts[0] == 'themeSettings'){ node.path.parts = node.params[0].parts node.params = [] From fc4c015beec9179d4c71ae799546ad73225cbb3e Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 6 Feb 2019 23:43:07 +0200 Subject: [PATCH 62/74] FEATURE: Shortcut 'g s' goes to first suggested topic --- .../discourse/lib/keyboard-shortcuts.js.es6 | 22 + .../acceptance/keyboard-shortcuts-test.js.es6 | 64 +++ test/javascripts/fixtures/topic.js.es6 | 430 +++++++++--------- 3 files changed, 300 insertions(+), 216 deletions(-) create mode 100644 test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 diff --git a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 index 615949c23a..fb690805c6 100644 --- a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 +++ b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 @@ -1,6 +1,7 @@ import DiscourseURL from "discourse/lib/url"; import Composer from "discourse/models/composer"; import { minimumOffset } from "discourse/lib/offset-calculator"; +import { ajax } from "discourse/lib/ajax"; const bindings = { "!": { postAction: "showFlags" }, @@ -32,6 +33,7 @@ const bindings = { "g p": { path: "/my/activity" }, "g m": { path: "/my/messages" }, "g d": { path: "/my/activity/drafts" }, + "g s": { handler: "goToFirstSuggestedTopic", anonymous: true }, home: { handler: "goToFirstPost", anonymous: true }, "command+up": { handler: "goToFirstPost", anonymous: true }, j: { handler: "selectDown", anonymous: true }, @@ -133,6 +135,26 @@ export default { Ember.run.later(() => $(".d-editor .quote").click(), 500); }, + goToFirstSuggestedTopic() { + const $el = $(".suggested-topics a.raw-topic-link:first"); + if ($el.length) { + $el.click(); + } else { + const controller = this.container.lookup("controller:topic"); + // Only the last page contains list of suggested topics. + const url = `/t/${controller.get("model.id")}/last.json`; + ajax(url).then(result => { + if (result.suggested_topics && result.suggested_topics.length > 0) { + const topic = controller.store.createRecord( + "topic", + result.suggested_topics[0] + ); + DiscourseURL.routeTo(topic.get("url")); + } + }); + } + }, + goToFirstPost() { this._jumpTo("jumpTop"); }, diff --git a/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 b/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 new file mode 100644 index 0000000000..ae77149d80 --- /dev/null +++ b/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 @@ -0,0 +1,64 @@ +import { acceptance } from "helpers/qunit-helpers"; +import DiscourseURL from "discourse/lib/url"; + +acceptance("Keyboard Shortcuts", { loggedIn: true }); + +test("go to first suggested topic", async assert => { + server.get("/t/27331/4.json", () => [ + 200, + { "Content-Type": "application/json" }, + {} + ]); + + server.get("/t/27331.json", () => [ + 200, + { "Content-Type": "application/json" }, + {} + ]); + + /* + * No suggested topics exist. + */ + + server.get("/t/9/last.json", () => [ + 200, + { "Content-Type": "application/json" }, + {} + ]); + + await visit("/t/this-is-a-test-topic/9"); + await keyEvent(document, "keypress", "g".charCodeAt(0)); + await keyEvent(document, "keypress", "s".charCodeAt(0)); + assert.equal(currentURL(), "/t/this-is-a-test-topic/9"); + + /* + * Suggested topics elements exist. + */ + + await visit("/t/internationalization-localization/280"); + await keyEvent(document, "keypress", "g".charCodeAt(0)); + await keyEvent(document, "keypress", "s".charCodeAt(0)); + assert.equal(currentURL(), "/t/polls-are-still-very-buggy/27331/4"); + + /* + * Suggested topic is returned by server. + */ + + server.get("/t/28830/last.json", () => [ + 200, + { "Content-Type": "application/json" }, + { + suggested_topics: [ + { + id: 27331, + slug: "keyboard-shortcuts-are-awesome" + } + ] + } + ]); + + await visit("/t/1-3-0beta9-no-rate-limit-popups/28830"); + await keyEvent(document, "keypress", "g".charCodeAt(0)); + await keyEvent(document, "keypress", "s".charCodeAt(0)); + assert.equal(currentURL(), "/t/keyboard-shortcuts-are-awesome/27331"); +}); diff --git a/test/javascripts/fixtures/topic.js.es6 b/test/javascripts/fixtures/topic.js.es6 index 1a71861e40..e0869104d4 100644 --- a/test/javascripts/fixtures/topic.js.es6 +++ b/test/javascripts/fixtures/topic.js.es6 @@ -2321,222 +2321,6 @@ export default { post_count: 1 } ], - suggested_topics: [ - { - id: 27331, - title: "Polls are still very buggy", - fancy_title: "Polls are still very buggy", - slug: "polls-are-still-very-buggy", - posts_count: 4, - reply_count: 1, - highest_post_number: 4, - image_url: - "/uploads/default/_optimized/cd1/b8c/c162528887_690x401.png", - created_at: "2015-04-08T09:51:00.357Z", - last_posted_at: "2015-04-08T15:59:16.258Z", - bumped: true, - bumped_at: "2015-04-08T16:05:09.842Z", - unseen: false, - last_read_post_number: 3, - unread: 0, - new_posts: 1, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 2, - bookmarked: false, - liked: false, - archetype: "regular", - like_count: 11, - views: 55, - category_id: 1 - }, - { - id: 27343, - title: - "Mobile theme doesn't show last activity time for topics on category page", - fancy_title: - "Mobile theme doesn’t show last activity time for topics on category page", - slug: - "mobile-theme-doesnt-show-last-activity-time-for-topics-on-category-page", - posts_count: 4, - reply_count: 2, - highest_post_number: 4, - image_url: - "/uploads/default/_optimized/13e/25c/bd30b466be_281x500.png", - created_at: "2015-04-08T14:20:51.177Z", - last_posted_at: "2015-04-08T15:40:30.037Z", - bumped: true, - bumped_at: "2015-04-08T15:40:30.037Z", - unseen: false, - last_read_post_number: 2, - unread: 0, - new_posts: 2, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 2, - bookmarked: false, - liked: false, - archetype: "regular", - like_count: 3, - views: 23, - category_id: 9 - }, - { - id: 27346, - title: - 'Reply+{messagekey}@... optionaly in header "from" in addition to "reply-to"', - fancy_title: - "Reply+{messagekey}@… optionaly in header “from” in addition to “reply-to”", - slug: - "reply-messagekey-optionaly-in-header-from-in-addition-to-reply-to", - posts_count: 1, - reply_count: 0, - highest_post_number: 1, - image_url: null, - created_at: "2015-04-08T16:05:13.103Z", - last_posted_at: "2015-04-08T16:05:13.415Z", - bumped: true, - bumped_at: "2015-04-08T16:05:13.415Z", - unseen: true, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - bookmarked: null, - liked: null, - archetype: "regular", - like_count: 0, - views: 8, - category_id: 2 - }, - { - id: 19670, - title: "Parsing (Oneboxing) IMDB links", - fancy_title: "Parsing (Oneboxing) IMDB links", - slug: "parsing-oneboxing-imdb-links", - posts_count: 8, - reply_count: 1, - highest_post_number: 8, - image_url: null, - created_at: "2014-09-05T07:19:26.161Z", - last_posted_at: "2015-04-07T09:21:21.570Z", - bumped: true, - bumped_at: "2015-04-07T09:21:21.570Z", - unseen: false, - last_read_post_number: 8, - unread: 0, - new_posts: 0, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 2, - bookmarked: false, - liked: false, - archetype: "regular", - like_count: 4, - views: 253, - category_id: 2 - }, - { - id: 7512, - title: - "Support for Piwik Analytics as an alternative to Google Analytics", - fancy_title: - "Support for Piwik Analytics as an alternative to Google Analytics", - slug: - "support-for-piwik-analytics-as-an-alternative-to-google-analytics", - posts_count: 53, - reply_count: 41, - highest_post_number: 65, - image_url: "/plugins/emoji/images/smile.png", - created_at: "2013-06-16T01:32:30.596Z", - last_posted_at: "2015-02-22T13:46:26.845Z", - bumped: true, - bumped_at: "2015-02-22T13:46:26.845Z", - unseen: false, - last_read_post_number: 65, - unread: 0, - new_posts: 0, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 2, - bookmarked: false, - liked: false, - archetype: "regular", - like_count: 62, - views: 1877, - category_id: 2 - }, - { - id: 25480, - title: "CSS admin-contents reloaded", - fancy_title: "CSS admin-contents reloaded", - slug: "css-admin-contents-reloaded", - posts_count: 22, - reply_count: 15, - highest_post_number: 22, - image_url: null, - created_at: "2015-02-21T12:15:57.707Z", - last_posted_at: "2015-03-02T23:24:18.899Z", - bumped: true, - bumped_at: "2015-03-02T23:24:18.899Z", - unseen: false, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - bookmarked: null, - liked: null, - archetype: "regular", - like_count: 21, - views: 185, - category_id: 2 - }, - { - id: 26576, - title: "Badge timestamp should be the time the badge was granted?", - fancy_title: - "Badge timestamp should be the time the badge was granted?", - slug: "badge-timestamp-should-be-the-time-the-badge-was-granted", - posts_count: 2, - reply_count: 0, - highest_post_number: 2, - image_url: null, - created_at: "2015-03-20T13:22:08.266Z", - last_posted_at: "2015-03-21T00:33:52.243Z", - bumped: true, - bumped_at: "2015-03-21T00:33:52.243Z", - unseen: false, - last_read_post_number: 1, - unread: 0, - new_posts: 0, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 1, - bookmarked: false, - liked: false, - archetype: "regular", - like_count: 9, - views: 87, - category_id: 2 - } - ], links: [ { url: @@ -3140,6 +2924,220 @@ export default { ], chunk_size: 20, bookmarked: false, + suggested_topics: [ + { + id: 27331, + title: "Polls are still very buggy", + fancy_title: "Polls are still very buggy", + slug: "polls-are-still-very-buggy", + posts_count: 4, + reply_count: 1, + highest_post_number: 4, + image_url: "/uploads/default/_optimized/cd1/b8c/c162528887_690x401.png", + created_at: "2015-04-08T09:51:00.357Z", + last_posted_at: "2015-04-08T15:59:16.258Z", + bumped: true, + bumped_at: "2015-04-08T16:05:09.842Z", + unseen: false, + last_read_post_number: 3, + unread: 0, + new_posts: 1, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + archetype: "regular", + like_count: 11, + views: 55, + category_id: 1 + }, + { + id: 27343, + title: + "Mobile theme doesn't show last activity time for topics on category page", + fancy_title: + "Mobile theme doesn’t show last activity time for topics on category page", + slug: + "mobile-theme-doesnt-show-last-activity-time-for-topics-on-category-page", + posts_count: 4, + reply_count: 2, + highest_post_number: 4, + image_url: "/uploads/default/_optimized/13e/25c/bd30b466be_281x500.png", + created_at: "2015-04-08T14:20:51.177Z", + last_posted_at: "2015-04-08T15:40:30.037Z", + bumped: true, + bumped_at: "2015-04-08T15:40:30.037Z", + unseen: false, + last_read_post_number: 2, + unread: 0, + new_posts: 2, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + archetype: "regular", + like_count: 3, + views: 23, + category_id: 9 + }, + { + id: 27346, + title: + 'Reply+{messagekey}@... optionaly in header "from" in addition to "reply-to"', + fancy_title: + "Reply+{messagekey}@… optionaly in header “from” in addition to “reply-to”", + slug: + "reply-messagekey-optionaly-in-header-from-in-addition-to-reply-to", + posts_count: 1, + reply_count: 0, + highest_post_number: 1, + image_url: null, + created_at: "2015-04-08T16:05:13.103Z", + last_posted_at: "2015-04-08T16:05:13.415Z", + bumped: true, + bumped_at: "2015-04-08T16:05:13.415Z", + unseen: true, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + bookmarked: null, + liked: null, + archetype: "regular", + like_count: 0, + views: 8, + category_id: 2 + }, + { + id: 19670, + title: "Parsing (Oneboxing) IMDB links", + fancy_title: "Parsing (Oneboxing) IMDB links", + slug: "parsing-oneboxing-imdb-links", + posts_count: 8, + reply_count: 1, + highest_post_number: 8, + image_url: null, + created_at: "2014-09-05T07:19:26.161Z", + last_posted_at: "2015-04-07T09:21:21.570Z", + bumped: true, + bumped_at: "2015-04-07T09:21:21.570Z", + unseen: false, + last_read_post_number: 8, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + archetype: "regular", + like_count: 4, + views: 253, + category_id: 2 + }, + { + id: 7512, + title: + "Support for Piwik Analytics as an alternative to Google Analytics", + fancy_title: + "Support for Piwik Analytics as an alternative to Google Analytics", + slug: + "support-for-piwik-analytics-as-an-alternative-to-google-analytics", + posts_count: 53, + reply_count: 41, + highest_post_number: 65, + image_url: "/plugins/emoji/images/smile.png", + created_at: "2013-06-16T01:32:30.596Z", + last_posted_at: "2015-02-22T13:46:26.845Z", + bumped: true, + bumped_at: "2015-02-22T13:46:26.845Z", + unseen: false, + last_read_post_number: 65, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + liked: false, + archetype: "regular", + like_count: 62, + views: 1877, + category_id: 2 + }, + { + id: 25480, + title: "CSS admin-contents reloaded", + fancy_title: "CSS admin-contents reloaded", + slug: "css-admin-contents-reloaded", + posts_count: 22, + reply_count: 15, + highest_post_number: 22, + image_url: null, + created_at: "2015-02-21T12:15:57.707Z", + last_posted_at: "2015-03-02T23:24:18.899Z", + bumped: true, + bumped_at: "2015-03-02T23:24:18.899Z", + unseen: false, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + bookmarked: null, + liked: null, + archetype: "regular", + like_count: 21, + views: 185, + category_id: 2 + }, + { + id: 26576, + title: "Badge timestamp should be the time the badge was granted?", + fancy_title: + "Badge timestamp should be the time the badge was granted?", + slug: "badge-timestamp-should-be-the-time-the-badge-was-granted", + posts_count: 2, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2015-03-20T13:22:08.266Z", + last_posted_at: "2015-03-21T00:33:52.243Z", + bumped: true, + bumped_at: "2015-03-21T00:33:52.243Z", + unseen: false, + last_read_post_number: 1, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 1, + bookmarked: false, + liked: false, + archetype: "regular", + like_count: 9, + views: 87, + category_id: 2 + } + ], tags: null }, "/t/28830/1.json": { From 6c2fe7eaecc910f5da01ab41b53d3cfa2a3a7a5a Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 6 Feb 2019 23:55:05 +0200 Subject: [PATCH 63/74] DEV: Fix lint. --- test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 b/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 index ae77149d80..89f8fd5a4c 100644 --- a/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 +++ b/test/javascripts/acceptance/keyboard-shortcuts-test.js.es6 @@ -1,9 +1,9 @@ import { acceptance } from "helpers/qunit-helpers"; -import DiscourseURL from "discourse/lib/url"; acceptance("Keyboard Shortcuts", { loggedIn: true }); test("go to first suggested topic", async assert => { + /* global server */ server.get("/t/27331/4.json", () => [ 200, { "Content-Type": "application/json" }, From 94f8c4ecacc98fbb84603c4ec4a2086c0b8185ae Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 6 Feb 2019 20:18:12 -0500 Subject: [PATCH 64/74] UX: Truncate (don't wrap) badges in user cards if the text is long --- app/assets/stylesheets/desktop/user-card.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/desktop/user-card.scss b/app/assets/stylesheets/desktop/user-card.scss index 62b989c49f..9c00506e2c 100644 --- a/app/assets/stylesheets/desktop/user-card.scss +++ b/app/assets/stylesheets/desktop/user-card.scss @@ -289,6 +289,10 @@ $user_card_background: $secondary; margin-top: 5px; .user-badge { + display: flex; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; margin-right: 0.5em; background: $primary-very-low; border: 1px solid $primary-low; @@ -305,6 +309,7 @@ $user_card_background: $secondary; .more-user-badges { @extend .user-badge; padding: 3px 8px; + white-space: nowrap; } .suspended { From 984f1d96b0fe5498b092d795b1903939bb8eca65 Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 6 Feb 2019 20:51:54 -0500 Subject: [PATCH 65/74] Add max-width so long badges don't force short badges to truncate --- app/assets/stylesheets/desktop/user-card.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/desktop/user-card.scss b/app/assets/stylesheets/desktop/user-card.scss index 9c00506e2c..4fd021e55c 100644 --- a/app/assets/stylesheets/desktop/user-card.scss +++ b/app/assets/stylesheets/desktop/user-card.scss @@ -294,6 +294,7 @@ $user_card_background: $secondary; overflow: hidden; text-overflow: ellipsis; margin-right: 0.5em; + max-width: 150px; background: $primary-very-low; border: 1px solid $primary-low; color: $user_card_primary; From 1cacfddda45c414216e148ea72cc40c897271ace Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 6 Feb 2019 22:39:10 -0500 Subject: [PATCH 66/74] Prevent usercard badge section from overflowing --- app/assets/stylesheets/desktop/user-card.scss | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/desktop/user-card.scss b/app/assets/stylesheets/desktop/user-card.scss index 4fd021e55c..84990e84db 100644 --- a/app/assets/stylesheets/desktop/user-card.scss +++ b/app/assets/stylesheets/desktop/user-card.scss @@ -287,14 +287,15 @@ $user_card_background: $secondary; align-items: flex-start; padding-bottom: 10px; margin-top: 5px; - + span { + overflow: hidden; + text-overflow: ellipsis; + max-width: 185px; + } .user-badge { display: flex; white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; margin-right: 0.5em; - max-width: 150px; background: $primary-very-low; border: 1px solid $primary-low; color: $user_card_primary; From 19386ec2ea8dacb5574e5218c672370e12c83723 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 7 Feb 2019 15:16:28 +1100 Subject: [PATCH 67/74] FIX: old migration was loading up invalid model schema Generally we should never be touching AR objects in migrations, this is super risky as we may end up with invalid schema cache. This code from 2013 did it unconditionally. This change amends it so: 1. We only load up schema if we have no choice 2. We flush the cache before and after This makes this migration far less risky. --- ...0221215017_add_description_to_categories.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/db/migrate/20130221215017_add_description_to_categories.rb b/db/migrate/20130221215017_add_description_to_categories.rb index aa22568014..24514a4720 100644 --- a/db/migrate/20130221215017_add_description_to_categories.rb +++ b/db/migrate/20130221215017_add_description_to_categories.rb @@ -8,10 +8,20 @@ class AddDescriptionToCategories < ActiveRecord::Migration[4.2] remove_column :categories, :top1_user_id remove_column :categories, :top2_user_id - # Migrate excerpts over - Category.order('id').each do |c| - post = c.topic.posts.order(:post_number).first - PostRevisor.new(post).send(:update_category_description) + # some ancient installs may have bad category descriptions + # attempt to fix + if !DB.query_single("SELECT 1 FROM categories limit 1").empty? + + # Reaching into post revisor is not ideal here, but this code + # should almost never run, so bypass it + Discourse.reset_active_record_cache + + Category.order('id').each do |c| + post = c.topic.ordered_posts.first + PostRevisor.new(post).update_category_description + end + + Discourse.reset_active_record_cache end end From 0c14f0d0a0380c3993611185ba5977fe7d688aba Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 7 Feb 2019 11:04:49 +0000 Subject: [PATCH 68/74] UX: Rename color scheme to color palette in UI The word 'scheme' was very easy to get confused with 'theme', this provides a better distinction. --- config/locales/client.en.yml | 18 +++++++++--------- config/locales/server.en.yml | 28 ++++++++++------------------ 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 42f0aa68af..23bf69cde7 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3347,7 +3347,7 @@ en: preview: "Preview" is_default: "Theme is enabled by default" user_selectable: "Theme can be selected by users" - color_scheme: "Color Scheme" + color_scheme: "Color Palette" color_scheme_select: "Select colors to be used by theme" custom_sections: "Custom sections:" theme_components: "Theme Components" @@ -3438,19 +3438,19 @@ en: title: "Define theme settings in YAML format" colors: select_base: - title: "Select base color scheme" - description: "Base scheme:" + title: "Select base color palette" + description: "Base palette:" title: "Colors" - edit: "Edit Color Schemes" - long_title: "Color Schemes" - about: "Modify the colors used by your themes. Create a new color scheme to start." - new_name: "New Color Scheme" + edit: "Edit Color Palettes" + long_title: "Color Palettes" + about: "Modify the colors used by your themes. Create a new color palette to start." + new_name: "New Color Palette" copy_name_prefix: "Copy of" - delete_confirm: "Delete this color scheme?" + delete_confirm: "Delete this color palette?" undo: "undo" undo_title: "Undo your changes to this color since the last time it was saved." revert: "revert" - revert_title: "Reset this color to Discourse's default color scheme." + revert_title: "Reset this color to Discourse's default color palette." primary: name: "primary" description: "Most text, icons, and borders." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c525fe1f90..cf74939b93 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -72,7 +72,7 @@ en: on_another_topic: "On another topic" themes: - bad_color_scheme: "Can not update theme, invalid color scheme" + bad_color_scheme: "Can not update theme, invalid color palette" other_error: "Something went wrong updating theme" import_error: generic: An error occured while importing that theme @@ -84,7 +84,7 @@ en: errors: component_no_user_selectable: "Theme components can't be user-selectable" component_no_default: "Theme components can't be default theme" - component_no_color_scheme: "Theme components can't have color scheme" + component_no_color_scheme: "Theme components can't have color palettes" no_multilevels_components: "Themes with child themes can't be child themes themselves" settings_errors: invalid_yaml: "Provided YAML is invalid." @@ -3436,14 +3436,14 @@ en: color_schemes: base_theme_name: "Base" - light: "Light Scheme" - dark: "Dark Scheme" - neutral: "Neutral Scheme" - grey_amber: "Grey Amber Scheme" - shades_of_blue: "Shades of Blue Scheme" - latte: "Latte Scheme" - summer: "Summer Scheme" - dark_rose: "Dark Rose Scheme" + light: "Light" + dark: "Dark" + neutral: "Neutral" + grey_amber: "Grey Amber" + shades_of_blue: "Shades of Blue" + latte: "Latte" + summer: "Summer" + dark_rose: "Dark Rose" default_theme_name: "Light" light_theme_name: "Light" dark_theme_name: "Dark" @@ -4212,14 +4212,6 @@ en: colors: title: "Theme" - fields: - theme_id: - description: "Do you prefer a light or dark color scheme to start with? You can further customize the look and feel of your site via Admin, Customize." - choices: - default: - label: "Simple Light" - dark: - label: "Simple Dark" logos: title: "Logos" From c256121833008057f9f6834f9268aa77e0fc9ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 7 Feb 2019 12:09:06 +0100 Subject: [PATCH 69/74] FIX: add support for style element in SVGs --- lib/upload_creator.rb | 2 +- spec/fixtures/images/image.svg | 5 ++++- spec/models/upload_spec.rb | 10 ++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index a1386feb2e..0aa4af84b1 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -9,7 +9,7 @@ class UploadCreator WHITELISTED_SVG_ELEMENTS ||= %w{ circle clippath defs ellipse g line linearGradient path polygon polyline - radialGradient rect stop svg text textpath tref tspan use + radialGradient rect stop style svg text textpath tref tspan use }.each(&:freeze) # Available options diff --git a/spec/fixtures/images/image.svg b/spec/fixtures/images/image.svg index b95c0f4a79..f997dcfde2 100644 --- a/spec/fixtures/images/image.svg +++ b/spec/fixtures/images/image.svg @@ -1,3 +1,6 @@ - Discourse + + Discourse diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 1fe964da54..95ed94f7dc 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -45,6 +45,16 @@ describe Upload do end end + it "supports