From d1d9a4f1289e35189d89a10df90bdf493d7523bc Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 11 Mar 2019 16:58:35 -0400 Subject: [PATCH 01/47] Add new `run_jobs_synchronously!` helper for tests Previously if you wanted to have jobs execute in test mode, you'd have to do `SiteSetting.queue_jobs = false`, because the opposite of queue is to execute. I found this very confusing, so I created a test helper called `run_jobs_synchronously!` which is much more clear about what it does. --- lib/generators/plugin/templates/controller_spec.rb.erb | 2 +- plugins/discourse-local-dates/spec/models/post_spec.rb | 2 +- .../advanced_user_narrative_spec.rb | 2 +- .../discourse_narrative_bot/new_user_narrative_spec.rb | 2 +- .../spec/discourse_narrative_bot/track_selector_spec.rb | 2 +- plugins/discourse-narrative-bot/spec/user_spec.rb | 2 +- spec/components/post_creator_spec.rb | 2 +- spec/components/post_destroyer_spec.rb | 2 +- spec/components/post_revisor_spec.rb | 2 +- spec/integration/watched_words_spec.rb | 2 +- spec/jobs/pull_hotlinked_images_spec.rb | 2 +- spec/lib/upload_recovery_spec.rb | 2 +- spec/models/category_user_spec.rb | 2 +- spec/models/discourse_single_sign_on_spec.rb | 2 +- spec/models/post_action_spec.rb | 4 ++-- spec/models/post_mover_spec.rb | 4 ++-- spec/models/post_spec.rb | 2 +- spec/models/quoted_post_spec.rb | 4 ++-- spec/models/tag_user_spec.rb | 2 +- spec/models/topic_spec.rb | 4 ++-- spec/models/topic_timer_spec.rb | 4 ++-- spec/models/topic_user_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/requests/admin/flags_controller_spec.rb | 8 ++++---- spec/requests/admin/groups_controller_spec.rb | 2 +- spec/requests/categories_controller_spec.rb | 4 ++-- spec/requests/embed_controller_spec.rb | 2 +- spec/requests/posts_controller_spec.rb | 2 +- spec/services/group_mentions_updater_spec.rb | 2 +- spec/services/post_alerter_spec.rb | 6 +++--- spec/services/user_anonymizer_spec.rb | 6 +++--- spec/services/username_changer_spec.rb | 2 +- spec/support/helpers.rb | 6 ++++++ 33 files changed, 51 insertions(+), 45 deletions(-) diff --git a/lib/generators/plugin/templates/controller_spec.rb.erb b/lib/generators/plugin/templates/controller_spec.rb.erb index 88935a5dfe..c73e0a87e9 100644 --- a/lib/generators/plugin/templates/controller_spec.rb.erb +++ b/lib/generators/plugin/templates/controller_spec.rb.erb @@ -2,7 +2,7 @@ require 'rails_helper' describe <%= name %>::ActionsController do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it 'can list' do diff --git a/plugins/discourse-local-dates/spec/models/post_spec.rb b/plugins/discourse-local-dates/spec/models/post_spec.rb index 13e7e9a5ac..9591607cc9 100644 --- a/plugins/discourse-local-dates/spec/models/post_spec.rb +++ b/plugins/discourse-local-dates/spec/models/post_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe Post do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end describe '#local_dates' do diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb index 32a5a4e3f4..91b0c15255 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb +++ b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/advanced_user_narrative_spec.rb @@ -22,7 +22,7 @@ RSpec.describe DiscourseNarrativeBot::AdvancedUserNarrative do let(:reset_trigger) { DiscourseNarrativeBot::TrackSelector.reset_trigger } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! SiteSetting.discourse_narrative_bot_enabled = true end diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb index 3a7e5d8aa3..7ecc727076 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb +++ b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb @@ -25,7 +25,7 @@ describe DiscourseNarrativeBot::NewUserNarrative do let(:reset_trigger) { DiscourseNarrativeBot::TrackSelector.reset_trigger } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! SiteSetting.discourse_narrative_bot_enabled = true end diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb index 0e6cb7a43f..4d76b06566 100644 --- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb +++ b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb @@ -37,7 +37,7 @@ describe DiscourseNarrativeBot::TrackSelector do end before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end describe '#select' do diff --git a/plugins/discourse-narrative-bot/spec/user_spec.rb b/plugins/discourse-narrative-bot/spec/user_spec.rb index 9779c2414f..de52f0eafe 100644 --- a/plugins/discourse-narrative-bot/spec/user_spec.rb +++ b/plugins/discourse-narrative-bot/spec/user_spec.rb @@ -13,7 +13,7 @@ describe User do end before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! SiteSetting.discourse_narrative_bot_enabled = true end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 78b2d6437f..0e609c9358 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -902,7 +902,7 @@ describe PostCreator do end it 'can post to a group correctly' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! expect(post.topic.archetype).to eq(Archetype.private_message) expect(post.topic.topic_allowed_users.count).to eq(1) diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index ba12c82f9c..d5da412659 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -617,7 +617,7 @@ describe PostDestroyer do context '@mentions' do it 'removes notifications when deleted' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! user = Fabricate(:evil_trout) post = create_post(raw: 'Hello @eviltrout') expect { diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 4f5df63762..d721692f26 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -608,7 +608,7 @@ describe PostRevisor do let(:mentioned_user) { Fabricate(:user) } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it "generates a notification for a mention" do diff --git a/spec/integration/watched_words_spec.rb b/spec/integration/watched_words_spec.rb index b165b175a2..7620237b1c 100644 --- a/spec/integration/watched_words_spec.rb +++ b/spec/integration/watched_words_spec.rb @@ -163,7 +163,7 @@ describe WatchedWord do end it "flags on revisions" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! post = Fabricate(:post, topic: Fabricate(:topic, user: tl2_user), user: tl2_user) expect { PostRevisor.new(post).revise!(post.user, { raw: "Want some #{flag_word.word} for cheap?" }, revised_at: post.updated_at + 10.seconds) diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index fc258ada1e..2415696ff0 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -32,7 +32,7 @@ describe Jobs::PullHotlinkedImages do describe '#execute' do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! FastImage.expects(:size).returns([100, 100]).at_least_once end diff --git a/spec/lib/upload_recovery_spec.rb b/spec/lib/upload_recovery_spec.rb index 5e8e4500a9..b9a6b911ee 100644 --- a/spec/lib/upload_recovery_spec.rb +++ b/spec/lib/upload_recovery_spec.rb @@ -31,7 +31,7 @@ RSpec.describe UploadRecovery do before do SiteSetting.authorized_extensions = 'png|pdf' - SiteSetting.queue_jobs = false + run_jobs_synchronously! end after do diff --git a/spec/models/category_user_spec.rb b/spec/models/category_user_spec.rb index 8313c0b715..048a19a2d0 100644 --- a/spec/models/category_user_spec.rb +++ b/spec/models/category_user_spec.rb @@ -68,7 +68,7 @@ describe CategoryUser do context 'integration' do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! NotificationEmailer.enable end diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index 68c72d1193..95f71d0f41 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -8,7 +8,7 @@ describe DiscourseSingleSignOn do SiteSetting.sso_url = @sso_url SiteSetting.enable_sso = true SiteSetting.sso_secret = @sso_secret - SiteSetting.queue_jobs = false + run_jobs_synchronously! end def make_sso diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 0de0efd6bc..3739283528 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -1072,7 +1072,7 @@ describe PostAction do end it "should create a notification in the related topic" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! post = Fabricate(:post) user = Fabricate(:user) action = PostAction.act(user, post, PostActionType.types[:spam], message: "WAT") @@ -1089,7 +1089,7 @@ describe PostAction do end it "should not add a moderator post when post is flagged via private message" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! post = Fabricate(:post) user = Fabricate(:user) action = PostAction.act(user, post, PostActionType.types[:notify_user], message: "WAT") diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 80e707b25f..4e117d9dc8 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -41,7 +41,7 @@ describe PostMover do before do SiteSetting.tagging_enabled = true - SiteSetting.queue_jobs = false + run_jobs_synchronously! p1.replies << p3 p2.replies << p4 UserActionCreator.enable @@ -570,7 +570,7 @@ describe PostMover do before do SiteSetting.tagging_enabled = true - SiteSetting.queue_jobs = false + run_jobs_synchronously! p1.replies << p3 p2.replies << p4 UserActionCreator.enable diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 93eaf3600f..fe827d16b8 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -995,7 +995,7 @@ describe Post do end before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end describe 'when user can not mention a group' do diff --git a/spec/models/quoted_post_spec.rb b/spec/models/quoted_post_spec.rb index 7636a29d05..538a3f21ca 100644 --- a/spec/models/quoted_post_spec.rb +++ b/spec/models/quoted_post_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe QuotedPost do it 'correctly extracts quotes' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! topic = Fabricate(:topic) post1 = create_post(topic: topic, post_number: 1, raw: "foo bar") @@ -34,7 +34,7 @@ describe QuotedPost do end it "doesn't count quotes from the same post" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! topic = Fabricate(:topic) post = create_post(topic: topic, post_number: 1, raw: "foo bar") diff --git a/spec/models/tag_user_spec.rb b/spec/models/tag_user_spec.rb index 9a120f2890..a57373ddbc 100644 --- a/spec/models/tag_user_spec.rb +++ b/spec/models/tag_user_spec.rb @@ -75,7 +75,7 @@ describe TagUser do context "with some tag notification settings" do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end let :watched_post do diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 82b850788c..d0ecd98a43 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1280,7 +1280,7 @@ describe Topic do describe 'user that is watching the new category' do it 'should generate the notification for the topic' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! topic.posts << Fabricate(:post) @@ -1602,7 +1602,7 @@ describe Topic do let(:topic) { Fabricate(:topic, category: category) } it "should be able to override category's default auto close" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! expect(topic.topic_timers.first.duration).to eq(4) diff --git a/spec/models/topic_timer_spec.rb b/spec/models/topic_timer_spec.rb index cd6f2e2b70..e49d51a3b0 100644 --- a/spec/models/topic_timer_spec.rb +++ b/spec/models/topic_timer_spec.rb @@ -190,7 +190,7 @@ RSpec.describe TopicTimer, type: :model do end before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it 'should close the topic' do @@ -219,7 +219,7 @@ RSpec.describe TopicTimer, type: :model do end before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it 'should open the topic' do diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index fca57e7804..ebe3e561f8 100644 --- a/spec/models/topic_user_spec.rb +++ b/spec/models/topic_user_spec.rb @@ -451,7 +451,7 @@ describe TopicUser do it "will receive email notification for every topic" do user1 = Fabricate(:user) - SiteSetting.queue_jobs = false + run_jobs_synchronously! SiteSetting.default_email_mailing_list_mode = true SiteSetting.default_email_mailing_list_mode_frequency = 1 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1fbdcc019..dfce0b76ef 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1088,7 +1088,7 @@ describe User do context "with a reply" do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! PostCreator.new(Fabricate(:user), raw: 'whatever this is a raw post', topic_id: topic.id, diff --git a/spec/requests/admin/flags_controller_spec.rb b/spec/requests/admin/flags_controller_spec.rb index 1c572fe765..cba8d017e6 100644 --- a/spec/requests/admin/flags_controller_spec.rb +++ b/spec/requests/admin/flags_controller_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Admin::FlagsController do context '#agree' do it 'should raise a reasonable error if a flag was deferred and then someone else agreed' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! _post_action = PostAction.act(user, post_1, PostActionType.types[:spam], message: 'bad') @@ -52,7 +52,7 @@ RSpec.describe Admin::FlagsController do end it 'should be able to agree and keep content' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! post_action = PostAction.act(user, post_1, PostActionType.types[:spam], message: 'bad') @@ -69,7 +69,7 @@ RSpec.describe Admin::FlagsController do it 'should be able to hide spam' do SiteSetting.allow_user_locale = true - SiteSetting.queue_jobs = false + run_jobs_synchronously! post_action = PostAction.act(user, post_1, PostActionType.types[:spam], message: 'bad') admin.update!(locale: 'ja') @@ -90,7 +90,7 @@ RSpec.describe Admin::FlagsController do end it 'should not delete category topic' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! category.update_column(:topic_id, first_post.topic_id) PostAction.act(user, first_post, PostActionType.types[:spam], message: 'bad') diff --git a/spec/requests/admin/groups_controller_spec.rb b/spec/requests/admin/groups_controller_spec.rb index 32ff704d45..326a301290 100644 --- a/spec/requests/admin/groups_controller_spec.rb +++ b/spec/requests/admin/groups_controller_spec.rb @@ -78,7 +78,7 @@ RSpec.describe Admin::GroupsController do let(:user2) { Fabricate(:user, trust_level: 4) } it "can assign users to a group by email or username" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! put "/admin/groups/bulk.json", params: { group_id: group.id, users: [user.username.upcase, user2.email, 'doesnt_exist'] diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index df387c56ab..92f0a6dc53 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -78,7 +78,7 @@ describe CategoriesController do describe "logged in" do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! sign_in(admin) end @@ -226,7 +226,7 @@ describe CategoriesController do context '#update' do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it "requires the user to be logged in" do diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb index 85601b0b10..6c79ab9840 100644 --- a/spec/requests/embed_controller_spec.rb +++ b/spec/requests/embed_controller_spec.rb @@ -74,7 +74,7 @@ describe EmbedController do let(:headers) { { 'REFERER' => embed_url } } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it "raises an error with no referer" do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 0829f3ee0e..20a58b3a86 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -730,7 +730,7 @@ describe PostsController do end it 'allows to create posts in import_mode' do - SiteSetting.queue_jobs = false + run_jobs_synchronously! NotificationEmailer.enable post_1 = Fabricate(:post) user = Fabricate(:user) diff --git a/spec/services/group_mentions_updater_spec.rb b/spec/services/group_mentions_updater_spec.rb index 5487bd1886..0491dbdbe8 100644 --- a/spec/services/group_mentions_updater_spec.rb +++ b/spec/services/group_mentions_updater_spec.rb @@ -4,7 +4,7 @@ RSpec.describe GroupMentionsUpdater do let(:post) { Fabricate(:post) } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end describe '.update' do diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index f4888711b7..c2f414041c 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -213,7 +213,7 @@ describe PostAlerter do let(:linking_post) { create_post(raw: "my magic topic\n##{Discourse.base_url}#{post1.url}") } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it "will notify correctly on linking" do @@ -289,7 +289,7 @@ describe PostAlerter do let(:topic) { mention_post.topic } before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it 'notifies a user' do @@ -591,7 +591,7 @@ describe PostAlerter do end it "correctly pushes notifications if configured correctly" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! SiteSetting.allowed_user_api_push_urls = "https://site.com/push|https://site2.com/push" 2.times do |i| diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index 87a652925e..8e3048b953 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/spec/services/user_anonymizer_spec.rb @@ -136,7 +136,7 @@ describe UserAnonymizer do end it "updates the avatar in posts" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! upload = Fabricate(:upload, user: user) user.user_avatar = UserAvatar.new(user_id: user.id, custom_upload_id: upload.id) user.uploaded_avatar_id = upload.id # chosen in user preferences @@ -214,7 +214,7 @@ describe UserAnonymizer do context "executes job" do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end it "removes invites" do @@ -302,7 +302,7 @@ describe UserAnonymizer do end it "exhaustively replaces all user ips" do - SiteSetting.queue_jobs = false + run_jobs_synchronously! link = IncomingLink.create!(current_user_id: user.id, ip_address: old_ip, post_id: post.id) screened_email = ScreenedEmail.create!(email: user.email, ip_address: old_ip) diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index c4281a403a..8ae83f191d 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe UsernameChanger do before do - SiteSetting.queue_jobs = false + run_jobs_synchronously! end describe '#change' do diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 455230e1b4..9d709d3e1d 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -5,6 +5,12 @@ module Helpers @next_seq = (@next_seq || 0) + 1 end + # If you don't `queue_jobs` it means you want to run them synchronously. This method + # makes that more clear in tests. It is automatically reset after every test. + def run_jobs_synchronously! + SiteSetting.queue_jobs = false + end + def log_in(fabricator = nil) user = Fabricate(fabricator || :user) log_in_user(user) From 32bae48fd37142eada28a017e27047bbc8057609 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 12 Mar 2019 01:58:14 +0200 Subject: [PATCH 02/47] DEV: Use User#human? User#bot? (#7140) --- app/jobs/scheduled/pending_users_reminder.rb | 2 +- app/models/user.rb | 4 ++++ app/services/post_alerter.rb | 5 ++--- lib/post_creator.rb | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/jobs/scheduled/pending_users_reminder.rb b/app/jobs/scheduled/pending_users_reminder.rb index 88c2fa4db3..85bedc6781 100644 --- a/app/jobs/scheduled/pending_users_reminder.rb +++ b/app/jobs/scheduled/pending_users_reminder.rb @@ -20,7 +20,7 @@ module Jobs if count > 0 target_usernames = Group[:moderators].users.map do |user| - next if user.id < 0 + next if user.bot? unseen_count = user.notifications.joins(:topic) .where("notifications.id > ?", user.seen_notification_id) diff --git a/app/models/user.rb b/app/models/user.rb index 97c8440a9a..d47cc92c37 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -294,6 +294,10 @@ class User < ActiveRecord::Base self.id > 0 end + def bot? + !self.human? + end + def effective_locale if SiteSetting.allow_user_locale && self.locale.present? self.locale diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 632029fbdd..454f981c07 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -13,7 +13,7 @@ class PostAlerter def not_allowed?(user, post) user.blank? || - user.id < 0 || + user.bot? || user.id == post.user_id end @@ -279,8 +279,7 @@ class PostAlerter DiscourseEvent.trigger(:before_create_notification, user, type, post, opts) - return if user.blank? - return if user.id < 0 + return if user.blank? || user.bot? is_liked = type == Notification.types[:liked] return if is_liked && user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never] diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 6d88d1adf2..77cc5d0407 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -514,7 +514,7 @@ class PostCreator end def create_post_notice - return if @user.id < 0 || @user.staged + return if @user.bot? || @user.staged last_post_time = Post.where(user_id: @user.id) .order(created_at: :desc) From 34b29f62db68b01cc2dcaf58b965de643992b8d2 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 12 Mar 2019 09:39:16 +0800 Subject: [PATCH 03/47] DEV: Remove the use of stubs and mocks in `Jobs::UserEmail` tests. We can only be sure that an email is sent when we get a mailer in `ActionMailer::Deliveries`. A couple of tests were actually incorrect because it didn't flow through our email sender where there are more conditions in determining whether an email is sent or not. --- app/jobs/regular/user_email.rb | 5 +- lib/email/sender.rb | 9 +- spec/jobs/user_email_spec.rb | 177 +++++++++++++++++++++++---------- 3 files changed, 134 insertions(+), 57 deletions(-) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 4b0ae6b1f5..322d9c01d8 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -54,7 +54,10 @@ module Jobs ) if message - Email::Sender.new(message, type, user).send + Email::Sender.new(message, type, user).send( + is_critical: self.class == Jobs::CriticalUserEmail + ) + if (b = user.user_stat.bounce_score) > SiteSetting.bounce_score_erode_on_send # erode bounce score each time we send an email # this means that we are punished a lot less for bounces diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 4cc6bcf25e..40217f3d05 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -21,8 +21,13 @@ module Email @user = user end - def send - return if SiteSetting.disable_emails == "yes" && @email_type.to_s != "admin_login" + def send(is_critical: false) + if SiteSetting.disable_emails == "yes" && + @email_type.to_s != "admin_login" && + !is_critical + + return + end return if ActionMailer::Base::NullMail === @message return if ActionMailer::Base::NullMail === (@message.message rescue nil) diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index 299aaba6c9..d35844bc89 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -11,7 +11,6 @@ describe Jobs::UserEmail do let(:staged) { Fabricate(:user, staged: true, last_seen_at: 11.minutes.ago) } let(:suspended) { Fabricate(:user, last_seen_at: 10.minutes.ago, suspended_at: 5.minutes.ago, suspended_till: 7.days.from_now) } let(:anonymous) { Fabricate(:anonymous, last_seen_at: 11.minutes.ago) } - let(:mailer) { Mail::Message.new(to: user.email) } it "raises an error when there is no user" do expect { Jobs::UserEmail.new.execute(type: :digest) }.to raise_error(Discourse::InvalidParameters) @@ -26,13 +25,15 @@ describe Jobs::UserEmail do end it "doesn't call the mailer when the user is missing" do - UserNotifications.expects(:digest).never Jobs::UserEmail.new.execute(type: :digest, user_id: 1234) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't call the mailer when the user is staged" do - UserNotifications.expects(:digest).never Jobs::UserEmail.new.execute(type: :digest, user_id: staged.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end context "bounce score" do @@ -45,55 +46,48 @@ describe Jobs::UserEmail do email_log = EmailLog.where(user_id: user.id).last expect(email_log.email_type).to eq("signup") + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) end end context 'to_address' do it 'overwrites a to_address when present' do - UserNotifications.expects(:confirm_new_email).returns(mailer) - Email::Sender.any_instance.expects(:send) Jobs::UserEmail.new.execute(type: :confirm_new_email, user_id: user.id, to_address: 'jake@adventuretime.ooo') - expect(mailer.to).to eq(['jake@adventuretime.ooo']) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + 'jake@adventuretime.ooo' + ) end end context "disable_emails setting" do it "sends when no" do SiteSetting.disable_emails = 'no' - Email::Sender.any_instance.expects(:send).once Jobs::UserEmail.new.execute(type: :confirm_new_email, user_id: user.id) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) end it "does not send an email when yes" do SiteSetting.disable_emails = 'yes' - Email::Sender.any_instance.expects(:send).never Jobs::UserEmail.new.execute(type: :confirm_new_email, user_id: user.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "sends when critical" do SiteSetting.disable_emails = 'yes' - Email::Sender.any_instance.expects(:send) Jobs::CriticalUserEmail.new.execute(type: :confirm_new_email, user_id: user.id) - end - end - context "recently seen" do - let(:post) { Fabricate(:post, user: user) } - - it "doesn't send an email to a user that's been recently seen" do - user.update_column(:last_seen_at, 9.minutes.ago) - Email::Sender.any_instance.expects(:send).never - Jobs::UserEmail.new.execute(type: :user_replied, user_id: user.id, post_id: post.id) - end - - it "does send an email to a user that's been recently seen but has email_always set" do - user.update_attributes(last_seen_at: 9.minutes.ago) - user.user_option.update_attributes(email_always: true) - PostTiming.create!(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id, msecs: 100) - - Email::Sender.any_instance.expects(:send) - Jobs::UserEmail.new.execute(type: :user_replied, user_id: user.id, post_id: post.id) + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) end end @@ -145,49 +139,65 @@ describe Jobs::UserEmail do context 'args' do it 'passes a token as an argument when a token is present' do - UserNotifications.expects(:forgot_password).with(user, email_token: 'asdfasdf').returns(mailer) - Email::Sender.any_instance.expects(:send) Jobs::UserEmail.new.execute(type: :forgot_password, user_id: user.id, email_token: 'asdfasdf') + + mail = ActionMailer::Base.deliveries.first + + expect(mail.to).to contain_exactly(user.email) + expect(mail.body).to include("asdfasdf") end context "post" do let(:post) { Fabricate(:post, user: user) } - it 'passes a post as an argument when a post_id is present' do - UserNotifications.expects(:user_private_message).with(user, post: post).returns(mailer) - Email::Sender.any_instance.expects(:send) - Jobs::UserEmail.new.execute(type: :user_private_message, user_id: user.id, post_id: post.id) - end - it "doesn't send the email if you've seen the post" do - Email::Sender.any_instance.expects(:send).never PostTiming.record_timing(topic_id: post.topic_id, user_id: user.id, post_number: post.post_number, msecs: 6666) Jobs::UserEmail.new.execute(type: :user_private_message, user_id: user.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send the email if the user deleted the post" do - Email::Sender.any_instance.expects(:send).never post.update_column(:user_deleted, true) Jobs::UserEmail.new.execute(type: :user_private_message, user_id: user.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send the email if user of the post has been deleted" do - Email::Sender.any_instance.expects(:send).never post.update_attributes!(user_id: nil) Jobs::UserEmail.new.execute(type: :user_replied, user_id: user.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end context 'user is suspended' do it "doesn't send email for a pm from a regular user" do - Email::Sender.any_instance.expects(:send).never Jobs::UserEmail.new.execute(type: :user_private_message, user_id: suspended.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "does send an email for a pm from a staff user" do pm_from_staff = Fabricate(:post, user: Fabricate(:moderator)) pm_from_staff.topic.topic_allowed_users.create!(user_id: suspended.id) - Email::Sender.any_instance.expects(:send) - Jobs::UserEmail.new.execute(type: :user_private_message, user_id: suspended.id, post_id: pm_from_staff.id) + + pm_notification = Fabricate(:notification, + user: suspended, + topic: pm_from_staff.topic, + post_number: pm_from_staff.post_number, + data: { original_post_id: pm_from_staff.id }.to_json + ) + + Jobs::UserEmail.new.execute(type: :user_private_message, + user_id: suspended.id, + post_id: pm_from_staff.id, + notification_id: pm_notification.id + ) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + suspended.email + ) end end @@ -195,15 +205,17 @@ describe Jobs::UserEmail do before { SiteSetting.allow_anonymous_posting = true } it "doesn't send email for a pm from a regular user" do - Email::Sender.any_instance.expects(:send).never Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send email for a pm from a staff user" do pm_from_staff = Fabricate(:post, user: Fabricate(:moderator)) pm_from_staff.topic.topic_allowed_users.create!(user_id: anonymous.id) - Email::Sender.any_instance.expects(:send).never Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, post_id: pm_from_staff.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end end end @@ -244,17 +256,61 @@ describe Jobs::UserEmail do end it "does send the email if the notification has been seen but the user is set for email_always" do - Email::Sender.any_instance.expects(:send) notification.update_column(:read, true) user.user_option.update_column(:email_always, true) - Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) + + Jobs::UserEmail.new.execute(type: :user_mentioned, + user_id: user.id, + post_id: post.id, + notification_id: notification.id + ) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) end it "does send the email if the user is using daily mailing list mode" do - Email::Sender.any_instance.expects(:send) user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0) - Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) + Jobs::UserEmail.new.execute(type: :user_mentioned, + user_id: user.id, + post_id: post.id, + notification_id: notification.id + ) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) + end + + context "recently seen" do + it "doesn't send an email to a user that's been recently seen" do + user.update!(last_seen_at: 9.minutes.ago) + + Jobs::UserEmail.new.execute(type: :user_replied, + user_id: user.id, + post_id: post.id, + notification_id: notification.id + ) + + expect(ActionMailer::Base.deliveries).to eq([]) + end + + it "does send an email to a user that's been recently seen but has email_always set" do + user.update!(last_seen_at: 9.minutes.ago) + user.user_option.update!(email_always: true) + + Jobs::UserEmail.new.execute(type: :user_replied, + user_id: user.id, + post_id: post.id, + notification_id: notification.id + ) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly( + user.email + ) + end end context 'max_emails_per_day_per_user limit is reached' do @@ -361,7 +417,6 @@ describe Jobs::UserEmail do end it "doesn't send the mail if the user is using individual mailing list mode" do - Email::Sender.any_instance.expects(:send).never user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1) # sometimes, we pass the notification_id Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id) @@ -373,10 +428,11 @@ describe Jobs::UserEmail do post = Fabricate(:post) post.topic.destroy Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send the mail if the user is using individual mailing list mode with no echo" do - Email::Sender.any_instance.expects(:send).never user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2) # sometimes, we pass the notification_id Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id) @@ -388,12 +444,15 @@ describe Jobs::UserEmail do post = Fabricate(:post) post.topic.destroy Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send the email if the post has been user deleted" do - Email::Sender.any_instance.expects(:send).never post.update_column(:user_deleted, true) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id) + + expect(ActionMailer::Base.deliveries).to eq([]) end context 'user is suspended' do @@ -450,8 +509,13 @@ describe Jobs::UserEmail do before { SiteSetting.allow_anonymous_posting = true } it "doesn't send email for a pm from a regular user" do - Email::Sender.any_instance.expects(:send).never - Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, notification_id: notification.id) + Jobs::UserEmail.new.execute(type: :user_private_message, + user_id: anonymous.id, + post_id: post.id, + notification_id: notification.id + ) + + expect(ActionMailer::Base.deliveries).to eq([]) end it "doesn't send email for a pm from staff" do @@ -463,8 +527,13 @@ describe Jobs::UserEmail do post_number: pm_from_staff.post_number, data: { original_post_id: pm_from_staff.id }.to_json ) - Email::Sender.any_instance.expects(:send).never - Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, notification_id: pm_notification.id) + Jobs::UserEmail.new.execute(type: :user_private_message, + user_id: anonymous.id, + post_id: pm_from_staff.id, + notification_id: pm_notification.id + ) + + expect(ActionMailer::Base.deliveries).to eq([]) end end end From 49cdb072d737cc71b2af809dc4cf19c8bd71ca5a Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 11 Mar 2019 22:04:47 -0400 Subject: [PATCH 04/47] DEV: Use `--profile` and `--fail-fast` in CI only --- .rspec | 2 -- lib/tasks/docker.rake | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.rspec b/.rspec index 49e9bca297..53607ea52b 100644 --- a/.rspec +++ b/.rspec @@ -1,3 +1 @@ --colour ---profile ---fail-fast diff --git a/lib/tasks/docker.rake b/lib/tasks/docker.rake index e689c7986b..c6d3497833 100644 --- a/lib/tasks/docker.rake +++ b/lib/tasks/docker.rake @@ -118,6 +118,8 @@ task 'docker:test' do puts "travis_fold:start:ruby_tests" if ENV["TRAVIS"] unless ENV["SKIP_CORE"] params = [] + params << "--profile" + params << "--fail-fast" if ENV["BISECT"] params << "--bisect" end From 95532814df06ba5716df91138f37966e8bcce9af Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 11 Mar 2019 22:33:24 -0400 Subject: [PATCH 05/47] DEV: Make Rubocop happy --- spec/jobs/user_email_spec.rb | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index d35844bc89..0a05e4ccde 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -189,7 +189,8 @@ describe Jobs::UserEmail do data: { original_post_id: pm_from_staff.id }.to_json ) - Jobs::UserEmail.new.execute(type: :user_private_message, + Jobs::UserEmail.new.execute( + type: :user_private_message, user_id: suspended.id, post_id: pm_from_staff.id, notification_id: pm_notification.id @@ -259,7 +260,8 @@ describe Jobs::UserEmail do notification.update_column(:read, true) user.user_option.update_column(:email_always, true) - Jobs::UserEmail.new.execute(type: :user_mentioned, + Jobs::UserEmail.new.execute( + type: :user_mentioned, user_id: user.id, post_id: post.id, notification_id: notification.id @@ -273,7 +275,8 @@ describe Jobs::UserEmail do it "does send the email if the user is using daily mailing list mode" do user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0) - Jobs::UserEmail.new.execute(type: :user_mentioned, + Jobs::UserEmail.new.execute( + type: :user_mentioned, user_id: user.id, post_id: post.id, notification_id: notification.id @@ -288,7 +291,8 @@ describe Jobs::UserEmail do it "doesn't send an email to a user that's been recently seen" do user.update!(last_seen_at: 9.minutes.ago) - Jobs::UserEmail.new.execute(type: :user_replied, + Jobs::UserEmail.new.execute( + type: :user_replied, user_id: user.id, post_id: post.id, notification_id: notification.id @@ -301,7 +305,8 @@ describe Jobs::UserEmail do user.update!(last_seen_at: 9.minutes.ago) user.user_option.update!(email_always: true) - Jobs::UserEmail.new.execute(type: :user_replied, + Jobs::UserEmail.new.execute( + type: :user_replied, user_id: user.id, post_id: post.id, notification_id: notification.id @@ -509,7 +514,8 @@ describe Jobs::UserEmail do before { SiteSetting.allow_anonymous_posting = true } it "doesn't send email for a pm from a regular user" do - Jobs::UserEmail.new.execute(type: :user_private_message, + Jobs::UserEmail.new.execute( + type: :user_private_message, user_id: anonymous.id, post_id: post.id, notification_id: notification.id @@ -527,7 +533,8 @@ describe Jobs::UserEmail do post_number: pm_from_staff.post_number, data: { original_post_id: pm_from_staff.id }.to_json ) - Jobs::UserEmail.new.execute(type: :user_private_message, + Jobs::UserEmail.new.execute( + type: :user_private_message, user_id: anonymous.id, post_id: pm_from_staff.id, notification_id: pm_notification.id From da941840d47db055a5d6ec407993bf017767bc45 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 12 Mar 2019 14:11:21 +0800 Subject: [PATCH 06/47] FIX: Advanced search category term should be case insensitive. --- lib/search.rb | 15 ++++++++++++--- spec/components/search_spec.rb | 13 ++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index 3ffbda872f..07ddba622e 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -399,8 +399,17 @@ class Search if slug[1] # sub category - parent_category_id = Category.where(slug: slug[0].downcase, parent_category_id: nil).pluck(:id).first - category_id = Category.where(slug: slug[1].downcase, parent_category_id: parent_category_id).pluck(:id).first + parent_category_id = Category + .where("lower(slug) = ? AND parent_category_id IS NULL", slug[0].downcase) + .pluck(:id) + .first + + category_id = Category + .where("lower(slug) = ? AND parent_category_id = ?", + slug[1].downcase, parent_category_id + ) + .pluck(:id) + .first else # main category if slug[0][0] == "=" @@ -409,7 +418,7 @@ class Search exact = false end - category_id = Category.where(slug: slug[0].downcase) + category_id = Category.where("lower(slug) = ?", slug[0].downcase) .order('case when parent_category_id is null then 0 else 1 end') .pluck(:id) .first diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 993a259719..287fa8e7f6 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -873,25 +873,28 @@ describe Search do it 'supports category slug and tags' do # main category - category = Fabricate(:category, name: 'category 24', slug: 'category-24') + category = Fabricate(:category, name: 'category 24', slug: 'cateGory-24') topic = Fabricate(:topic, created_at: 3.months.ago, category: category) post = Fabricate(:post, raw: 'Sams first post', topic: topic) - expect(Search.execute('sams post #category-24').posts.length).to eq(1) + expect(Search.execute('sams post #categoRy-24').posts.length).to eq(1) expect(Search.execute("sams post category:#{category.id}").posts.length).to eq(1) - expect(Search.execute('sams post #category-25').posts.length).to eq(0) + expect(Search.execute('sams post #categoRy-25').posts.length).to eq(0) sub_category = Fabricate(:category, name: 'sub category', slug: 'sub-category', parent_category_id: category.id) second_topic = Fabricate(:topic, created_at: 3.months.ago, category: sub_category) Fabricate(:post, raw: 'sams second post', topic: second_topic) - expect(Search.execute("sams post category:category-24").posts.length).to eq(2) - expect(Search.execute("sams post category:=category-24").posts.length).to eq(1) + expect(Search.execute("sams post category:categoRY-24").posts.length).to eq(2) + expect(Search.execute("sams post category:=cAtegory-24").posts.length).to eq(1) expect(Search.execute("sams post #category-24").posts.length).to eq(2) expect(Search.execute("sams post #=category-24").posts.length).to eq(1) expect(Search.execute("sams post #sub-category").posts.length).to eq(1) + expect(Search.execute("sams post #categoRY-24:SUB-category").posts.length) + .to eq(1) + # tags topic.tags = [Fabricate(:tag, name: 'alpha'), Fabricate(:tag, name: 'привет'), Fabricate(:tag, name: 'HeLlO')] expect(Search.execute('this is a test #alpha').posts.map(&:id)).to eq([post.id]) From 191e31dccf5abd84759fb2867310935c84b742b9 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 12 Mar 2019 10:16:56 +0200 Subject: [PATCH 07/47] FEATURE: Log user approvals. (#7121) --- app/models/user.rb | 2 ++ app/models/user_history.rb | 6 ++++-- app/services/staff_action_logger.rb | 7 +++++++ config/locales/client.en.yml | 1 + spec/requests/admin/users_controller_spec.rb | 1 + 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d47cc92c37..09ae5c6c2e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -421,6 +421,8 @@ class User < ActiveRecord::Base DiscourseEvent.trigger(:user_approved, self) end + StaffActionLogger.new(approved_by).log_user_approve(self) + result end diff --git a/app/models/user_history.rb b/app/models/user_history.rb index c8ec875439..75a7bd1b1f 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -84,7 +84,8 @@ class UserHistory < ActiveRecord::Base merge_user: 65, entity_export: 66, change_password: 67, - topic_timestamps_changed: 68 + topic_timestamps_changed: 68, + approve_user: 69 ) end @@ -147,7 +148,8 @@ class UserHistory < ActiveRecord::Base :merge_user, :entity_export, :change_name, - :topic_timestamps_changed + :topic_timestamps_changed, + :approve_user ] end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index a270dee353..5ae6266738 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -503,6 +503,13 @@ class StaffActionLogger )) end + def log_user_approve(user, opts = {}) + UserHistory.create!(params(opts).merge( + action: UserHistory.actions[:approve_user], + target_user_id: user.id + )) + end + def log_user_deactivate(user, reason, opts = {}) raise Discourse::InvalidParameters.new(:user) unless user UserHistory.create!(params(opts).merge( diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5e23831ce5..c77a2e821f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3689,6 +3689,7 @@ en: entity_export: "export entity" change_name: "change name" topic_timestamps_changed: "topic timestamps changed" + approve_user: "approved user" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 33d8e7b19e..48324b8493 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -71,6 +71,7 @@ RSpec.describe Admin::UsersController do expect(response.status).to eq(200) evil_trout.reload expect(evil_trout.approved).to eq(true) + expect(UserHistory.where(action: UserHistory.actions[:approve_user], target_user_id: evil_trout.id).count).to eq(1) end end From e6c2faf186c3c1fa01ad9fdee59a80e951592c79 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 12 Mar 2019 10:23:36 +0200 Subject: [PATCH 08/47] FIX: Disable 'Create Topic' button if tag is staff-only. (#6984) * FIX: Disable 'Create Topic' button if tag is staff-only. * FIX: Staff-only tags should always return 404. --- .../discourse/controllers/tags-show.js.es6 | 22 ++++- .../discourse/routes/tags-show.js.es6 | 8 +- app/controllers/tags_controller.rb | 2 + app/models/tag_group.rb | 2 + app/serializers/tag_serializer.rb | 6 +- lib/discourse_tagging.rb | 22 +++-- spec/components/discourse_tagging_spec.rb | 25 ++++++ spec/requests/tags_controller_spec.rb | 12 +++ test/javascripts/acceptance/tags-test.js.es6 | 82 ++++++++++++++++++- 9 files changed, 169 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 b/app/assets/javascripts/discourse/controllers/tags-show.js.es6 index 65062dfbc4..32141ceaa9 100644 --- a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/controllers/tags-show.js.es6 @@ -74,9 +74,25 @@ export default Ember.Controller.extend(BulkTopicSelection, { return listDraft ? "topic.open_draft" : "topic.create"; }, - @computed("canCreateTopic", "category", "canCreateTopicOnCategory") - createTopicDisabled(canCreateTopic, category, canCreateTopicOnCategory) { - return !canCreateTopic || (category && !canCreateTopicOnCategory); + @computed( + "canCreateTopic", + "category", + "canCreateTopicOnCategory", + "tag", + "canCreateTopicOnTag" + ) + createTopicDisabled( + canCreateTopic, + category, + canCreateTopicOnCategory, + tag, + canCreateTopicOnTag + ) { + return ( + !canCreateTopic || + (category && !canCreateTopicOnCategory) || + (tag && !canCreateTopicOnTag) + ); }, queryParams: [ diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index 88ec2a05d4..978816b7df 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -111,14 +111,18 @@ export default Discourse.Route.extend({ ).then(list => { if (list.topic_list.tags && list.topic_list.tags.length === 1) { // Update name of tag (case might be different) - tag.set("id", list.topic_list.tags[0].name); + tag.setProperties({ + id: list.topic_list.tags[0].name, + staff: list.topic_list.tags[0].staff + }); } controller.setProperties({ list, canCreateTopic: list.get("can_create_topic"), loading: false, canCreateTopicOnCategory: - this.get("category.permission") === PermissionType.FULL + this.get("category.permission") === PermissionType.FULL, + canCreateTopicOnTag: !tag.get("staff") || this.get("currentUser.staff") }); }); }, diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index f1e49da290..4c9bfc2a7f 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -98,6 +98,8 @@ class TagsController < ::ApplicationController end def show + raise Discourse::NotFound if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id]) + show_latest end diff --git a/app/models/tag_group.rb b/app/models/tag_group.rb index 5a5a74af02..91018c523d 100644 --- a/app/models/tag_group.rb +++ b/app/models/tag_group.rb @@ -12,6 +12,8 @@ class TagGroup < ActiveRecord::Base before_create :init_permissions before_save :apply_permissions + after_commit { DiscourseTagging.clear_cache! } + attr_accessor :permissions def tag_names=(tag_names_arg) diff --git a/app/serializers/tag_serializer.rb b/app/serializers/tag_serializer.rb index 9f05080029..5169ea1a65 100644 --- a/app/serializers/tag_serializer.rb +++ b/app/serializers/tag_serializer.rb @@ -1,3 +1,7 @@ class TagSerializer < ApplicationSerializer - attributes :id, :name, :topic_count + attributes :id, :name, :topic_count, :staff + + def staff + DiscourseTagging.staff_tag_names.include?(name) + end end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index ac2061b41f..5ba2563653 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -2,6 +2,7 @@ module DiscourseTagging TAGS_FIELD_NAME = "tags" TAGS_FILTER_REGEXP = /[\/\?#\[\]@!\$&'\(\)\*\+,;=\.%\\`^\s|\{\}"<>]+/ # /?#[]@!$&'()*+,;=.%\`^|{}"<> + TAGS_STAFF_CACHE_KEY = "staff_tag_names" def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false) if guardian.can_tag?(topic) @@ -194,11 +195,22 @@ module DiscourseTagging end def self.staff_tag_names - Tag.joins(tag_groups: :tag_group_permissions) - .where('tag_group_permissions.group_id = ? AND tag_group_permissions.permission_type = ?', - Group::AUTO_GROUPS[:everyone], - TagGroupPermission.permission_types[:readonly] - ).pluck(:name) + tag_names = Discourse.cache.read(TAGS_STAFF_CACHE_KEY, tag_names) + + if !tag_names + tag_names = Tag.joins(tag_groups: :tag_group_permissions) + .where('tag_group_permissions.group_id = ? AND tag_group_permissions.permission_type = ?', + Group::AUTO_GROUPS[:everyone], + TagGroupPermission.permission_types[:readonly] + ).pluck(:name) + Discourse.cache.write(TAGS_STAFF_CACHE_KEY, tag_names, expires_in: 1.hour) + end + + tag_names + end + + def self.clear_cache! + Discourse.cache.delete(TAGS_STAFF_CACHE_KEY) end def self.clean_tag(tag) diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb index f080a03f8c..c9ee958a70 100644 --- a/spec/components/discourse_tagging_spec.rb +++ b/spec/components/discourse_tagging_spec.rb @@ -216,4 +216,29 @@ describe DiscourseTagging do end end end + + describe "staff_tag_names" do + let(:tag) { Fabricate(:tag) } + + let(:staff_tag) { Fabricate(:tag) } + let(:other_staff_tag) { Fabricate(:tag) } + + let!(:staff_tag_group) { + Fabricate( + :tag_group, + permissions: { "staff" => 1, "everyone" => 3 }, + tag_names: [staff_tag.name] + ) + } + + it "returns all staff tags" do + expect(DiscourseTagging.staff_tag_names).to contain_exactly(staff_tag.name) + + staff_tag_group.update(tag_names: [staff_tag.name, other_staff_tag.name]) + expect(DiscourseTagging.staff_tag_names).to contain_exactly(staff_tag.name, other_staff_tag.name) + + staff_tag_group.update(tag_names: [other_staff_tag.name]) + expect(DiscourseTagging.staff_tag_names).to contain_exactly(other_staff_tag.name) + end + end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index ade557a66e..82c53eaaf2 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -64,6 +64,18 @@ describe TagsController do get "/tags/%2ftest%2f" expect(response.status).to eq(404) end + + it "does not show staff-only tags" do + tag_group = Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["test"]) + + get "/tags/test" + expect(response.status).to eq(404) + + sign_in(Fabricate(:admin)) + + get "/tags/test" + expect(response.status).to eq(200) + end end describe '#check_hashtag' do diff --git a/test/javascripts/acceptance/tags-test.js.es6 b/test/javascripts/acceptance/tags-test.js.es6 index b32fabca8a..81b9e49ed4 100644 --- a/test/javascripts/acceptance/tags-test.js.es6 +++ b/test/javascripts/acceptance/tags-test.js.es6 @@ -1,4 +1,4 @@ -import { acceptance } from "helpers/qunit-helpers"; +import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers"; acceptance("Tags", { loggedIn: true }); QUnit.test("list the tags", async assert => { @@ -92,3 +92,83 @@ QUnit.test("list the tags in groups", async assert => { "always uses lowercase URLs for mixed case tags" ); }); + +test("new topic button is not available for staff-only tags", async assert => { + /* global server */ + server.get("/tags/regular-tag/notifications", () => [ + 200, + { "Content-Type": "application/json" }, + { tag_notification: { id: "regular-tag", notification_level: 1 } } + ]); + + server.get("/tags/regular-tag/l/latest.json", () => [ + 200, + { "Content-Type": "application/json" }, + { + users: [], + primary_groups: [], + topic_list: { + can_create_topic: true, + draft: null, + draft_key: "new_topic", + draft_sequence: 1, + per_page: 30, + tags: [ + { + id: 1, + name: "regular-tag", + topic_count: 1 + } + ], + topics: [] + } + } + ]); + + server.get("/tags/staff-only-tag/notifications", () => [ + 200, + { "Content-Type": "application/json" }, + { tag_notification: { id: "staff-only-tag", notification_level: 1 } } + ]); + + server.get("/tags/staff-only-tag/l/latest.json", () => [ + 200, + { "Content-Type": "application/json" }, + { + users: [], + primary_groups: [], + topic_list: { + can_create_topic: true, + draft: null, + draft_key: "new_topic", + draft_sequence: 1, + per_page: 30, + tags: [ + { + id: 1, + name: "staff-only-tag", + topic_count: 1, + staff: true + } + ], + topics: [] + } + } + ]); + + replaceCurrentUser({ staff: false }); + + await visit("/tags/regular-tag"); + assert.ok(find("#create-topic:disabled").length === 0); + + await visit("/tags/staff-only-tag"); + assert.ok(find("#create-topic:disabled").length === 1); + + replaceCurrentUser({ staff: true }); + + await visit("/tags/regular-tag"); + assert.ok(find("#create-topic:disabled").length === 0); + + await visit("/tags/staff-only-tag"); + assert.ok(find("#create-topic:disabled").length === 0); +}); From 90965e1c182a87b94a97d4a174e823e8e60bc60f Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 12 Mar 2019 09:46:58 +0100 Subject: [PATCH 09/47] FIX: error with two inputs having the same id (#7147) --- .../discourse/templates/components/edit-category-settings.hbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs index b68c0603dc..f9628a3fa4 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs @@ -85,9 +85,9 @@ {{i18n "category.sort_order"}}
- {{combo-box valueAttribute="value" id="category-sort-order" content=availableSorts value=category.sort_order none="category.sort_options.default"}} + {{combo-box valueAttribute="value" content=availableSorts value=category.sort_order none="category.sort_options.default"}} {{#unless isDefaultSortOrder}} - {{combo-box castBoolean=true valueAttribute="value" id="category-sort-order" content=sortAscendingOptions value=category.sort_ascending none="category.sort_options.default"}} + {{combo-box castBoolean=true valueAttribute="value" content=sortAscendingOptions value=category.sort_ascending none="category.sort_options.default"}} {{/unless}}
From db08b59eb2fbcef1488962d29ca3f8798d0d25d7 Mon Sep 17 00:00:00 2001 From: Tarek Khalil <45508821+khalilovcmded@users.noreply.github.com> Date: Tue, 12 Mar 2019 11:10:18 +0000 Subject: [PATCH 10/47] UX: Change `ignore` button color (#7150) --- app/assets/javascripts/discourse/templates/user.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/templates/user.hbs b/app/assets/javascripts/discourse/templates/user.hbs index b702fd9b46..2279b2d6ba 100644 --- a/app/assets/javascripts/discourse/templates/user.hbs +++ b/app/assets/javascripts/discourse/templates/user.hbs @@ -57,7 +57,7 @@ icon="far-eye" label="user.unignore"}} {{else}} - {{d-button class="btn-danger" + {{d-button class="btn-default" action=(action "ignoreUser") icon="eye-slash" label="user.ignore"}} From 6d0528687d4bafa594c5e0ac20a1f17d42fdd270 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 12 Mar 2019 20:56:18 +0800 Subject: [PATCH 11/47] DEV: Remove redundant assertion. If the post is not present, the test will fail with an error. --- spec/components/system_message_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/components/system_message_spec.rb b/spec/components/system_message_spec.rb index c35f029a9d..6a77cfb25a 100644 --- a/spec/components/system_message_spec.rb +++ b/spec/components/system_message_spec.rb @@ -15,8 +15,7 @@ describe SystemMessage do post = system_message.create(:welcome_invite) topic = post.topic - expect(post).to be_present - expect(post).to be_valid + expect(post.valid?).to eq(true) expect(topic).to be_private_message expect(topic).to be_valid expect(topic.subtype).to eq(TopicSubtype.system_message) From 4a00772c199b18c49c4b16c2299a08712ec83ca7 Mon Sep 17 00:00:00 2001 From: Tarek Khalil <45508821+khalilovcmded@users.noreply.github.com> Date: Tue, 12 Mar 2019 13:32:25 +0000 Subject: [PATCH 12/47] FIX: invite approval `StaffActionLogger` bug (#7151) * FIX: invite approval `StaffActionLogger` bug --- app/models/invite_redeemer.rb | 2 +- app/models/user.rb | 12 +++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index bbf40ef0d2..2f63d50805 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -133,7 +133,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f def approve_account_if_needed if get_existing_user - invited_user.approve(invite.invited_by_id, false) + invited_user.approve(invite.invited_by, false) end end diff --git a/app/models/user.rb b/app/models/user.rb index 09ae5c6c2e..4c2710358c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -405,23 +405,17 @@ class User < ActiveRecord::Base end # Approve this user - def approve(approved_by, send_mail = true) + def approve(approver, send_mail = true) self.approved = true - - if approved_by.is_a?(Integer) - self.approved_by_id = approved_by - else - self.approved_by = approved_by - end - self.approved_at = Time.zone.now + self.approved_by = approver if result = save send_approval_email if send_mail DiscourseEvent.trigger(:user_approved, self) end - StaffActionLogger.new(approved_by).log_user_approve(self) + StaffActionLogger.new(approver).log_user_approve(self) result end From 28384ba62cdc8d9df9e1575b87eef87db285c1d5 Mon Sep 17 00:00:00 2001 From: Tarek Khalil <45508821+khalilovcmded@users.noreply.github.com> Date: Tue, 12 Mar 2019 16:01:58 +0000 Subject: [PATCH 13/47] FEATURE: Add `Top Ignored Users` report (#7153) * FEATURE: Add `Top Ignored Users` report ## Why? This is part of the [Ability to ignore a user feature](https://meta.discourse.org/t/ability-to-ignore-a-user/110254/8), and also part of [this PR](https://github.com/discourse/discourse/pull/7144). We want to send a System Message daily when a specific count threshold for an ignored is reached. To make this system message informative, we want to link to a report for the Top Ignored Users too. --- app/models/report.rb | 48 ++++++++++++++++++++++++++++++++++++ config/locales/server.en.yml | 6 +++++ spec/models/report_spec.rb | 29 ++++++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/app/models/report.rb b/app/models/report.rb index d820ad5e39..e34efd5351 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1548,6 +1548,54 @@ class Report end end + def self.report_top_ignored_users(report) + report.modes = [:table] + + report.labels = [ + { + type: :user, + properties: { + id: :ignored_user_id, + username: :ignored_username, + avatar: :ignored_user_avatar_template, + }, + title: I18n.t("reports.top_ignored_users.labels.ignored_user") + }, + { + type: :number, + properties: [ + :ignores_count, + ], + title: I18n.t("reports.top_ignored_users.labels.ignores_count") + } + ] + + report.data = [] + + sql = <<~SQL + SELECT + u.id AS user_id, + u.username, + u.uploaded_avatar_id, + COUNT(*) AS ignores_count + FROM users AS u + INNER JOIN ignored_users AS ig ON ig.ignored_user_id = u.id + WHERE ig.created_at >= '#{report.start_date}' AND ig.created_at <= '#{report.end_date}' + GROUP BY u.id + ORDER BY COUNT(*) DESC + LIMIT #{report.limit || 250} + SQL + + DB.query(sql).each do |row| + report.data << { + ignored_user_id: row.user_id, + ignored_username: row.username, + ignored_user_avatar_template: User.avatar_template(row.username, row.uploaded_avatar_id), + ignores_count: row.ignores_count, + } + end + end + DiscourseEvent.on(:site_setting_saved) do |site_setting| if ["backup_location", "s3_backup_bucket"].include?(site_setting.name.to_s) clear_cache(:storage_stats) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 182b0a0705..5cd4c04e45 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1227,6 +1227,12 @@ en: author: Author filesize: File size description: "List all uploads by extension, filesize and author." + top_ignored_users: + title: "Top Ignored Users" + labels: + ignored_user: Ignored User + ignores_count: Ignores count + description: "List top ignored users." dashboard: rails_env_warning: "Your server is running in %{env} mode." diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 7061cdff12..42e252e451 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -1094,6 +1094,35 @@ describe Report do include_examples "no data" end + describe "report_top_ignored_users" do + let(:report) { Report.find("top_ignored_users") } + let(:tarek) { Fabricate(:user, username: "tarek") } + let(:john) { Fabricate(:user, username: "john") } + let(:matt) { Fabricate(:user, username: "matt") } + + context "with data" do + before do + Fabricate(:ignored_user, user: tarek, ignored_user: john) + Fabricate(:ignored_user, user: tarek, ignored_user: matt) + end + + it "works" do + expect(report.data.length).to eq(2) + expect_row_to_be_equal(report.data[0], john) + expect_row_to_be_equal(report.data[1], matt) + end + + def expect_row_to_be_equal(row, user) + expect(row[:ignored_user_id]).to eq(user.id) + expect(row[:ignored_username]).to eq(user.username) + expect(row[:ignored_user_avatar_template]).to eq(User.avatar_template(user.username, user.uploaded_avatar_id)) + expect(row[:ignores_count]).to eq(1) + end + end + + include_examples "no data" + end + describe "consolidated_page_views" do before do freeze_time(Time.now.at_midnight) From 154f503d2e99f904356b52f2fae9edcc495708fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 12 Mar 2019 17:13:21 +0100 Subject: [PATCH 14/47] REFACTOR: remove unnecessary parentheses --- app/models/post.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8e03538f52..8020ddbb8f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -215,8 +215,7 @@ class Post < ActiveRecord::Base end def matches_recent_post? - post_id = $redis.get(unique_post_key) - post_id != (nil) && post_id.to_i != (id) + $redis.get(unique_post_key).to_i != id end def raw_hash From a901527057f3f1dc6758abe5b7b0c172d683fe35 Mon Sep 17 00:00:00 2001 From: Saurabh Patel Date: Tue, 12 Mar 2019 22:21:09 +0530 Subject: [PATCH 15/47] DEV: remove user_profile events added for akismet (#7044) --- app/models/user_profile.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index deb4adf66f..4d36ab5565 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -8,8 +8,6 @@ class UserProfile < ActiveRecord::Base validates :user, presence: true before_save :cook after_save :trigger_badges - after_commit :trigger_profile_created_event, on: :create - after_commit :trigger_profile_updated_event, on: :update validates :profile_background, upload_url: true, if: :profile_background_changed? validates :card_background, upload_url: true, if: :card_background_changed? @@ -108,14 +106,6 @@ class UserProfile < ActiveRecord::Base tempfile.close! if tempfile && tempfile.respond_to?(:close!) end - def trigger_profile_created_event - DiscourseEvent.trigger(:user_profile_created, self) - end - - def trigger_profile_updated_event - DiscourseEvent.trigger(:user_profile_updated, self) - end - protected def trigger_badges From e5e2fa4064110907a6138c4d10a84e11a1909827 Mon Sep 17 00:00:00 2001 From: Simon Cossar Date: Tue, 12 Mar 2019 10:08:56 -0700 Subject: [PATCH 16/47] FEATURE: unhide the embed_whitelist_selector setting (#7137) * Unhide embed_whitelist_selector Site Setting and move it to Posting section * Add i18n key for the embed_whitelist_selector setting --- config/locales/server.en.yml | 1 + config/site_settings.yml | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5cd4c04e45..feb1d292ae 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1876,6 +1876,7 @@ en: embed_truncate: "Truncate the embedded posts." embed_support_markdown: "Support Markdown formatting for embedded posts." + embed_whitelist_selector: "A comma separated list of CSS elements that are allowed in embeds." allowed_href_schemes: "Schemes allowed in links in addition to http and https." embed_post_limit: "Maximum number of posts to embed." embed_username_required: "The username for topic creation is required." diff --git a/config/site_settings.yml b/config/site_settings.yml index 79b08d0023..7b0aeb843f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -790,6 +790,8 @@ posting: default: true embed_support_markdown: default: false + embed_whitelist_selector: + default: "" allowed_href_schemes: client: true default: "" @@ -1473,9 +1475,6 @@ embedding: embed_title_scrubber: default: "" hidden: true - embed_whitelist_selector: - default: "" - hidden: true embed_blacklist_selector: default: "" hidden: true From c6ed86220e475671ae3c4380e8d346a8f4aa3dcd Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 12 Mar 2019 19:09:34 +0200 Subject: [PATCH 17/47] FIX: Notify on tag change. (#7119) --- app/jobs/regular/notify_tag_change.rb | 15 +++++++++++++++ lib/post_revisor.rb | 9 ++++++++- spec/services/post_alerter_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 app/jobs/regular/notify_tag_change.rb diff --git a/app/jobs/regular/notify_tag_change.rb b/app/jobs/regular/notify_tag_change.rb new file mode 100644 index 0000000000..b0fcb390c8 --- /dev/null +++ b/app/jobs/regular/notify_tag_change.rb @@ -0,0 +1,15 @@ +require_dependency "post_alerter" + +module Jobs + class NotifyTagChange < Jobs::Base + def execute(args) + post = Post.find_by(id: args[:post_id]) + + if post&.topic + post_alerter = PostAlerter.new + post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids])) + post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic)) + end + end + end +end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index e3c2407b0e..bd0a800596 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -83,7 +83,14 @@ class PostRevisor tc.check_result(false) next end - tc.record_change('tags', prev_tags, tags) unless prev_tags.sort == tags.sort + if prev_tags.sort != tags.sort + tc.record_change('tags', prev_tags, tags) + DB.after_commit do + post = tc.topic.ordered_posts.first + notified_user_ids = [post.user_id, post.last_editor_id].uniq + Jobs.enqueue(:notify_tag_change, post_id: post.id, notified_user_ids: notified_user_ids) + end + end end end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index c2f414041c..7108917c69 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -926,6 +926,29 @@ describe PostAlerter do expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user], post]) end end + + context "on change" do + let(:user) { Fabricate(:user) } + let(:other_tag) { Fabricate(:tag) } + let(:watched_tag) { Fabricate(:tag) } + let(:post) { Fabricate(:post) } + + before do + SiteSetting.tagging_enabled = true + SiteSetting.queue_jobs = false + end + + it "triggers a notification" do + TagUser.change(user.id, watched_tag.id, TagUser.notification_levels[:watching_first_post]) + expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(0) + + PostRevisor.new(post).revise!(Fabricate(:user), tags: [other_tag.name, watched_tag.name]) + expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(1) + + PostRevisor.new(post).revise!(Fabricate(:user), tags: [watched_tag.name, other_tag.name]) + expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(1) + end + end end describe '#extract_linked_users' do From e9ba4c74f7fc341826383c90c8ab371df42b422b Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 12 Mar 2019 15:00:58 -0400 Subject: [PATCH 18/47] Revert "REFACTOR: remove unnecessary parentheses" Specs fail --- app/models/post.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8020ddbb8f..8e03538f52 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -215,7 +215,8 @@ class Post < ActiveRecord::Base end def matches_recent_post? - $redis.get(unique_post_key).to_i != id + post_id = $redis.get(unique_post_key) + post_id != (nil) && post_id.to_i != (id) end def raw_hash From 7310ee3ef1660d7f05140066fac790aa10a51e34 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 12 Mar 2019 23:06:28 +0200 Subject: [PATCH 19/47] FEATURE: Add more control over post notices. (#7148) --- app/serializers/post_serializer.rb | 2 +- config/locales/server.en.yml | 1 + config/site_settings.yml | 3 +++ spec/serializers/post_serializer_spec.rb | 6 ++++++ 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index f3456648fc..e06e067bec 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -370,7 +370,7 @@ class PostSerializer < BasicPostSerializer end def include_post_notice_type? - return false if scope.user&.id == object.user_id || !scope.user&.has_trust_level?(TrustLevel[2]) + return false if scope.user&.id == object.user_id || !scope.user&.has_trust_level?(SiteSetting.min_post_notice_tl) post_notice_type.present? end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index feb1d292ae..4114136a96 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1910,6 +1910,7 @@ en: max_allowed_message_recipients: "Maximum recipients allowed in a message." watched_words_regular_expressions: "Watched words are regular expressions." + min_post_notice_tl: "Minimum trust level required to see post notices." returning_users_days: "How many days should pass before a user is considered to be returning." default_email_digest_frequency: "How often users receive summary emails by default." diff --git a/config/site_settings.yml b/config/site_settings.yml index 7b0aeb843f..dfac7fd48c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -808,6 +808,9 @@ posting: default: false client: true shadowed_by_global: true + min_post_notice_tl: + default: 2 + enum: "TrustLevelSetting" returning_users_days: default: 60 diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 649f1ec4f6..723ab53bcf 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -194,8 +194,14 @@ describe PostSerializer do it "will not show for poster and TL2+ users" do expect(json_for_user(nil)[:post_notice_type]).to eq(nil) expect(json_for_user(user)[:post_notice_type]).to eq(nil) + + SiteSetting.min_post_notice_tl = 2 expect(json_for_user(user_tl1)[:post_notice_type]).to eq(nil) expect(json_for_user(user_tl2)[:post_notice_type]).to eq("returning") + + SiteSetting.min_post_notice_tl = 1 + expect(json_for_user(user_tl1)[:post_notice_type]).to eq("returning") + expect(json_for_user(user_tl2)[:post_notice_type]).to eq("returning") end end From 476d0050ab80b2dfcaee6368c529f5094f14d79a Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 13 Mar 2019 00:18:58 +0200 Subject: [PATCH 20/47] FIX: Disable notices for posts by anonymous. --- app/serializers/post_serializer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index e06e067bec..9df5e31184 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -370,7 +370,9 @@ class PostSerializer < BasicPostSerializer end def include_post_notice_type? - return false if scope.user&.id == object.user_id || !scope.user&.has_trust_level?(SiteSetting.min_post_notice_tl) + return false if !scope.user || !scope.user.id || scope.user.id == object.user_id || + !object.user || object.user.anonymous? || + !scope.user.has_trust_level?(SiteSetting.min_post_notice_tl) post_notice_type.present? end From 7ac394f51f147a7dd2db1467ef42bff0d41ee6fa Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Tue, 12 Mar 2019 17:16:42 -0600 Subject: [PATCH 21/47] FIX: prevent mixed api auth headers & query params When using the api and you provide an http header based api key any other auth based information (username, external_id, or user_id) passed in as query params will not be used and vice versa. Followup to f03b293e6a5f12f12ba2a61ab2bc2cfb8a7f1a63 --- lib/auth/default_current_user_provider.rb | 12 ++-- .../default_current_user_provider_spec.rb | 56 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 8404ee6694..18e2384875 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -45,7 +45,7 @@ class Auth::DefaultCurrentUserProvider request = @request user_api_key = @env[USER_API_KEY] - api_key = @env.blank? ? nil : request[API_KEY] || @env[HEADER_API_KEY] + api_key = @env.blank? ? nil : @env[HEADER_API_KEY] || request[API_KEY] auth_token = request.cookies[TOKEN_COOKIE] unless user_api_key || api_key @@ -284,7 +284,7 @@ class Auth::DefaultCurrentUserProvider def lookup_api_user(api_key_value, request) if api_key = ApiKey.where(key: api_key_value).includes(:user).first - api_username = request[API_USERNAME] || @env[HEADER_API_USERNAME] + api_username = header_api_key? ? @env[HEADER_API_USERNAME] : request[API_USERNAME] if api_key.allowed_ips.present? && !api_key.allowed_ips.any? { |ip| ip.include?(request.ip) } Rails.logger.warn("[Unauthorized API Access] username: #{api_username}, IP address: #{request.ip}") @@ -295,9 +295,9 @@ class Auth::DefaultCurrentUserProvider api_key.user if !api_username || (api_key.user.username_lower == api_username.downcase) elsif api_username User.find_by(username_lower: api_username.downcase) - elsif user_id = request["api_user_id"] || @env[HEADER_API_USER_ID] + elsif user_id = header_api_key? ? @env[HEADER_API_USER_ID] : request["api_user_id"] User.find_by(id: user_id.to_i) - elsif external_id = request["api_user_external_id"] || @env[HEADER_API_USER_EXTERNAL_ID] + elsif external_id = header_api_key? ? @env[HEADER_API_USER_EXTERNAL_ID] : request["api_user_external_id"] SingleSignOnRecord.find_by(external_id: external_id.to_s).try(:user) end end @@ -305,6 +305,10 @@ class Auth::DefaultCurrentUserProvider private + def header_api_key? + !!@env[HEADER_API_KEY] + end + def rate_limit_admin_api_requests(api_key) return if Rails.env == "profile" diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index ed582333d8..5d188b66e3 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -92,6 +92,15 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/?api_key=hello&api_username=#{user.username.downcase}").current_user.id).to eq(user.id) end + it "raises for a mismatched api_key param and header username" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + params = { "HTTP_API_USERNAME" => user.username.downcase } + expect { + provider("/?api_key=hello", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + it "finds a user for a correct system api key with external id" do user = Fabricate(:user) ApiKey.create!(key: "hello", created_by_id: -1) @@ -99,12 +108,31 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/?api_key=hello&api_user_external_id=abc").current_user.id).to eq(user.id) end + it "raises for a mismatched api_key param and header external id" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + SingleSignOnRecord.create(user_id: user.id, external_id: "abc", last_payload: '') + params = { "HTTP_API_USER_EXTERNAL_ID" => "abc" } + expect { + provider("/?api_key=hello", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + it "finds a user for a correct system api key with id" do user = Fabricate(:user) ApiKey.create!(key: "hello", created_by_id: -1) expect(provider("/?api_key=hello&api_user_id=#{user.id}").current_user.id).to eq(user.id) end + it "raises for a mismatched api_key param and header user id" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + params = { "HTTP_API_USER_ID" => user.id } + expect { + provider("/?api_key=hello", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + context "rate limiting" do before do RateLimiter.enable @@ -243,6 +271,15 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/", params).current_user.id).to eq(user.id) end + it "raises for a mismatched api_key header and param username" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + params = { "HTTP_API_KEY" => "hello" } + expect { + provider("/?api_username=#{user.username.downcase}", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + it "finds a user for a correct system api key with external id" do user = Fabricate(:user) ApiKey.create!(key: "hello", created_by_id: -1) @@ -251,6 +288,16 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/", params).current_user.id).to eq(user.id) end + it "raises for a mismatched api_key header and param external id" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + SingleSignOnRecord.create(user_id: user.id, external_id: "abc", last_payload: '') + params = { "HTTP_API_KEY" => "hello" } + expect { + provider("/?api_user_external_id=abc", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + it "finds a user for a correct system api key with id" do user = Fabricate(:user) ApiKey.create!(key: "hello", created_by_id: -1) @@ -258,6 +305,15 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/", params).current_user.id).to eq(user.id) end + it "raises for a mismatched api_key header and param user id" do + user = Fabricate(:user) + ApiKey.create!(key: "hello", created_by_id: -1) + params = { "HTTP_API_KEY" => "hello" } + expect { + provider("/?api_user_id=#{user.id}", params).current_user + }.to raise_error(Discourse::InvalidAccess) + end + context "rate limiting" do before do RateLimiter.enable From edc6d878628b7472a04ac287945162c1ae26360c Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 13 Mar 2019 06:34:23 +0100 Subject: [PATCH 22/47] FIX: invite-panels regressions on private topics (#7157) --- .../javascripts/discourse/components/invite-panel.js.es6 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/components/invite-panel.js.es6 b/app/assets/javascripts/discourse/components/invite-panel.js.es6 index 2f0fc23539..4427410a7d 100644 --- a/app/assets/javascripts/discourse/components/invite-panel.js.es6 +++ b/app/assets/javascripts/discourse/components/invite-panel.js.es6 @@ -32,9 +32,9 @@ export default Ember.Component.extend({ "emailOrUsername", "invitingToTopic", "isPrivateTopic", - "topic.groupNames", - "topic.saving", - "topic.details.can_invite_to" + "inviteModel.groupNames.[]", + "inviteModel.saving", + "inviteModel.details.can_invite_to" ) disabled( isAdmin, @@ -79,7 +79,7 @@ export default Ember.Component.extend({ "emailOrUsername", "inviteModel.saving", "isPrivateTopic", - "inviteModel.groupNames", + "inviteModel.groupNames.[]", "hasCustomMessage" ) disabledCopyLink( From d1c4981f6544740407dcca5fc39bbc737e13a9ee Mon Sep 17 00:00:00 2001 From: Tim Lange Date: Wed, 13 Mar 2019 07:11:36 +0100 Subject: [PATCH 23/47] UX: Added tooltips to topic admin menu (#7146) * UX: Added tooltips to topic admin menu * FIX: Prettyfied code * FIX: Fixed typo * Update config/locales/client.en.yml Co-Authored-By: venarius --- .../discourse/widgets/topic-admin-menu.js.es6 | 50 ++++++++++++++----- config/locales/client.en.yml | 17 +++++++ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 index 2d6e06502b..e0f94d7b8c 100644 --- a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 @@ -16,6 +16,7 @@ createWidget("admin-menu-button", { action: attrs.action, icon: attrs.icon, label: attrs.fullLabel || `topic.${attrs.label}`, + title: attrs.title, secondaryAction: "hideAdminMenu" }) ); @@ -136,7 +137,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "toggleMultiSelect", icon: "tasks", - label: "actions.multi_select" + label: "actions.multi_select", + title: "topic.actions.multi_select_tooltip" }); const topic = attrs.topic; @@ -148,7 +150,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-danger", action: "deleteTopic", icon: "far-trash-alt", - label: "actions.delete" + label: "actions.delete", + title: "topic.actions.delete_tooltip" }); } @@ -158,7 +161,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "recoverTopic", icon: "undo", - label: "actions.recover" + label: "actions.recover", + title: "topic.actions.recover_tooltip" }); } @@ -168,7 +172,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "toggleClosed", icon: "unlock", - label: "actions.open" + label: "actions.open", + title: "topic.actions.open_tooltip" }); } else { buttons.push({ @@ -176,7 +181,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "toggleClosed", icon: "lock", - label: "actions.close" + label: "actions.close", + title: "topic.actions.close_tooltip" }); } @@ -185,7 +191,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "showTopicStatusUpdate", icon: "far-clock", - label: "actions.timed_update" + label: "actions.timed_update", + title: "topic.actions.timed_update_tooltip" }); const isPrivateMessage = topic.get("isPrivateMessage"); @@ -197,7 +204,10 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "showFeatureTopic", icon: "thumbtack", - label: featured ? "actions.unpin" : "actions.pin" + label: featured ? "actions.unpin" : "actions.pin", + title: featured + ? "topic.actions.unpin_tooltip" + : "topic.actions.pin_tooltip" }); } @@ -207,7 +217,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "showChangeTimestamp", icon: "calendar-alt", - label: "change_timestamp.title" + label: "change_timestamp.title", + title: "topic.change_timestamp.tooltip" }); } @@ -216,7 +227,8 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "resetBumpDate", icon: "anchor", - label: "actions.reset_bump_date" + label: "actions.reset_bump_date", + title: "topic.actions.reset_bump_date_tooltip" }); if (!isPrivateMessage) { @@ -225,7 +237,10 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "toggleArchived", icon: "folder", - label: topic.get("archived") ? "actions.unarchive" : "actions.archive" + label: topic.get("archived") ? "actions.unarchive" : "actions.archive", + title: topic.get("archived") + ? "topic.actions.unarchive_tooltip" + : "topic.actions.archive_tooltip" }); } @@ -235,7 +250,10 @@ export default createWidget("topic-admin-menu", { buttonClass: "btn-default", action: "toggleVisibility", icon: visible ? "far-eye-slash" : "far-eye", - label: visible ? "actions.invisible" : "actions.visible" + label: visible ? "actions.invisible" : "actions.visible", + title: visible + ? "topic.actions.invisible_tooltip" + : "topic.actions.visible_tooltip" }); if (details.get("can_convert_topic")) { @@ -246,7 +264,12 @@ export default createWidget("topic-admin-menu", { ? "convertToPublicTopic" : "convertToPrivateMessage", icon: isPrivateMessage ? "comment" : "envelope", - label: isPrivateMessage ? "actions.make_public" : "actions.make_private" + label: isPrivateMessage + ? "actions.make_public" + : "actions.make_private", + title: isPrivateMessage + ? "topic.actions.make_public_tooltip" + : "topic.actions.make_private_tooltip" }); } @@ -255,7 +278,8 @@ export default createWidget("topic-admin-menu", { action: "showModerationHistory", buttonClass: "btn-default", icon: "list", - fullLabel: "admin.flags.moderation_history" + fullLabel: "admin.flags.moderation_history", + title: "admin.flags.moderation_history_tooltip" }); } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c77a2e821f..10e3d2f13d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1927,21 +1927,36 @@ en: actions: recover: "Un-Delete Topic" + recover_tooltip: "Restores the topic." delete: "Delete Topic" + delete_tooltip: "Marks the topic as deleted." open: "Open Topic" + open_tooltip: "Marks the topic as open and allows new replies." close: "Close Topic" + close_tooltip: "Marks the topic as closed and prevents new replies." multi_select: "Select Posts…" + multi_select_tooltip: "Toggles multiselect mode in which multiple posts can be selected." timed_update: "Set Topic Timer..." + timed_update_tooltip: "Opens a new window in which a timer for different actions can be set." pin: "Pin Topic…" + pin_tooltip: "Opens a window in which the topic can be featured in different ways." unpin: "Un-Pin Topic…" + unpin_tooltip: "Unpins the topic and it will no longer be featured." unarchive: "Unarchive Topic" + unarchive_tooltip: "Unfreezes the topic and allows interacting with it." archive: "Archive Topic" + archive_tooltip: "Freezes the topic and prevents interacting with it." invisible: "Make Unlisted" + invisible_tooltip: "Prevents the inclusion of the topic in lists and digest emails." visible: "Make Listed" + visible_tooltip: "Marks the topic as visible and includes the topic in lists and digest emails." reset_read: "Reset Read Data" make_public: "Make Public Topic" + make_public_tooltip: "Converts to public topic and makes it visible for other users." make_private: "Make Personal Message" + make_private_tooltip: "Converts to personal message and makes it invisible for other users." reset_bump_date: "Reset Bump Date" + reset_bump_date_tooltip: "Puts the topic back into chronological order according to when it was originally created." feature: pin: "Pin Topic" @@ -2099,6 +2114,7 @@ en: change_timestamp: title: "Change Timestamp..." + tooltip: "Opens a window in which the timestamp can be changed." action: "change timestamp" invalid_timestamp: "Timestamp cannot be in the future." error: "There was an error changing the timestamp of the topic." @@ -2974,6 +2990,7 @@ en: old_posts: "Old Flagged Posts" topics: "Flagged Topics" moderation_history: "Moderation History" + moderation_history_tooltip: "Shows the moderation history." agree: "Agree" agree_title: "Confirm this flag as valid and correct" From bd2edbb243e28bec52f8b4455a4680966f36f98a Mon Sep 17 00:00:00 2001 From: Erick Guan Date: Wed, 13 Mar 2019 07:22:46 +0100 Subject: [PATCH 24/47] FIX: a temporary fix when CJK user tries to add a long title (#7045) Discourse doesn't analyze the sentence components. So it counts the whole sentence as a word for CJK. https://meta.discoursecn.org/t/topic/3033 --- config/site_settings.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/config/site_settings.yml b/config/site_settings.yml index dfac7fd48c..cc8a362f3d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -691,7 +691,13 @@ posting: max_users_notified_per_group_mention: 100 newuser_max_replies_per_topic: 3 newuser_max_mentions_per_post: 2 - title_max_word_length: 30 + title_max_word_length: + default: 30 + locale_default: + ja: 50 + ko: 50 + zh_CN: 50 + zh_TW: 50 whitelisted_link_domains: default: "" type: list From 684eef71c77a3a7be0f91a069a41a5235544a5de Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 13 Mar 2019 15:23:01 +0800 Subject: [PATCH 25/47] REFACTOR: Better variable name. --- lib/search.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index 07ddba622e..2c478c5845 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -394,31 +394,33 @@ class Search exact = true - slug = match.to_s.split(":") - next if slug.empty? + category_slug, subcategory_slug = match.to_s.split(":") + next unless category_slug - if slug[1] + if subcategory_slug # sub category parent_category_id = Category - .where("lower(slug) = ? AND parent_category_id IS NULL", slug[0].downcase) + .where( + "lower(slug) = ? AND parent_category_id IS NULL", category_slug.downcase + ) .pluck(:id) .first category_id = Category .where("lower(slug) = ? AND parent_category_id = ?", - slug[1].downcase, parent_category_id + subcategory_slug.downcase, parent_category_id ) .pluck(:id) .first else # main category - if slug[0][0] == "=" - slug[0] = slug[0][1..-1] + if category_slug[0] == "=" + category_slug = category_slug[1..-1] else exact = false end - category_id = Category.where("lower(slug) = ?", slug[0].downcase) + category_id = Category.where("lower(slug) = ?", category_slug.downcase) .order('case when parent_category_id is null then 0 else 1 end') .pluck(:id) .first @@ -434,7 +436,7 @@ class Search posts.where("topics.category_id IN (?)", category_ids) else # try a possible tag match - tag_id = Tag.where_name(slug[0]).pluck(:id).first + tag_id = Tag.where_name(category_slug).pluck(:id).first if (tag_id) posts.where("topics.id IN ( SELECT DISTINCT(tt.topic_id) From b0c8fdd7da20fc4eeb30575050360fbf47ec4c8f Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 2 Jan 2019 15:29:17 +0800 Subject: [PATCH 26/47] FIX: Properly support defaults for upload site settings. --- .../discourse/widgets/home-logo.js.es6 | 2 +- .../admin/site_settings_controller.rb | 2 +- app/helpers/application_helper.rb | 4 +- app/helpers/user_notifications_helper.rb | 3 +- app/jobs/scheduled/clean_up_uploads.rb | 4 +- app/models/report.rb | 4 +- app/models/site_setting.rb | 73 ++++--------------- app/models/upload.rb | 3 + app/services/push_notification_pusher.rb | 4 +- config/site_settings.yml | 10 +-- db/fixtures/010_uploads.rb | 17 +++++ lib/file_store/base_store.rb | 10 ++- lib/site_setting_extension.rb | 22 +++++- lib/site_settings/type_supervisor.rb | 2 +- .../requests/discobot_certificate_spec.rb | 2 +- spec/components/file_store/base_store_spec.rb | 9 +++ .../components/site_setting_extension_spec.rb | 22 ++++++ .../site_settings/type_supervisor_spec.rb | 3 + spec/jobs/clean_up_uploads_spec.rb | 4 +- spec/models/emoji_spec.rb | 2 +- spec/models/site_setting_spec.rb | 34 --------- .../admin/site_settings_controller_spec.rb | 54 +++++++++----- spec/requests/exceptions_controller_spec.rb | 8 +- .../services/push_notification_pusher_spec.rb | 7 +- test/javascripts/helpers/site-settings.js | 2 +- .../javascripts/widgets/home-logo-test.js.es6 | 14 ++-- 26 files changed, 170 insertions(+), 151 deletions(-) create mode 100644 db/fixtures/010_uploads.rb diff --git a/app/assets/javascripts/discourse/widgets/home-logo.js.es6 b/app/assets/javascripts/discourse/widgets/home-logo.js.es6 index 86bc33966a..5c7454df11 100644 --- a/app/assets/javascripts/discourse/widgets/home-logo.js.es6 +++ b/app/assets/javascripts/discourse/widgets/home-logo.js.es6 @@ -17,7 +17,7 @@ export default createWidget("home-logo", { }, logoUrl() { - return this.siteSettings.site_home_logo_url || ""; + return this.siteSettings.site_logo_url || ""; }, mobileLogoUrl() { diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index d12405ff05..3a894e1e2b 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -15,7 +15,7 @@ class Admin::SiteSettingsController < Admin::AdminController raise_access_hidden_setting(id) if SiteSetting.type_supervisor.get_type(id) == :upload - value = Upload.get_from_url(value) || '' + value = Upload.find_by(url: value) || '' end SiteSetting.set_and_log(id, value, current_user) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 38b826b9a5..81d28e90f3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -219,7 +219,7 @@ module ApplicationHelper opts[:twitter_summary_large_image] = twitter_summary_large_image_url end - opts[:image] = SiteSetting.opengraph_image_url.presence || + opts[:image] = SiteSetting.site_opengraph_image_url.presence || twitter_summary_large_image_url.presence || SiteSetting.site_large_icon_url.presence || SiteSetting.site_apple_touch_icon_url.presence || @@ -295,7 +295,7 @@ module ApplicationHelper if mobile_view? && SiteSetting.site_mobile_logo_url SiteSetting.site_mobile_logo_url else - SiteSetting.site_home_logo_url + SiteSetting.site_logo_url end end end diff --git a/app/helpers/user_notifications_helper.rb b/app/helpers/user_notifications_helper.rb index e3aed77955..575635f13a 100644 --- a/app/helpers/user_notifications_helper.rb +++ b/app/helpers/user_notifications_helper.rb @@ -22,8 +22,7 @@ module UserNotificationsHelper logo_url = SiteSetting.site_digest_logo_url logo_url = SiteSetting.site_logo_url if logo_url.blank? || logo_url =~ /\.svg$/i return nil if logo_url.blank? || logo_url =~ /\.svg$/i - - full_cdn_url(logo_url) + logo_url end def html_site_link(color) diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 3c4f37cc60..2043c43bcc 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -7,6 +7,7 @@ module Jobs # always remove invalid upload records Upload + .by_users .where("retain_hours IS NULL OR created_at < current_timestamp - interval '1 hour' * retain_hours") .where("created_at < ?", grace_period.hour.ago) .where(url: "") @@ -44,7 +45,8 @@ module Jobs end end.compact.uniq - result = Upload.where("uploads.retain_hours IS NULL OR uploads.created_at < current_timestamp - interval '1 hour' * uploads.retain_hours") + result = Upload.by_users + .where("uploads.retain_hours IS NULL OR uploads.created_at < current_timestamp - interval '1 hour' * uploads.retain_hours") .where("uploads.created_at < ?", grace_period.hour.ago) .joins(<<~SQL) LEFT JOIN site_settings ss diff --git a/app/models/report.rb b/app/models/report.rb index e34efd5351..e0410effa5 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1529,7 +1529,9 @@ class Report FROM uploads up JOIN users u ON u.id = up.user_id - WHERE up.created_at >= '#{report.start_date}' AND up.created_at <= '#{report.end_date}' + WHERE up.created_at >= '#{report.start_date}' + AND up.created_at <= '#{report.end_date}' + AND up.id > #{Upload::SEEDED_ID_THRESHOLD} ORDER BY up.filesize DESC LIMIT #{report.limit || 250} SQL diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 49b9281c3d..031fd5a5b7 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -175,69 +175,26 @@ class SiteSetting < ActiveRecord::Base site_logo_small_url site_mobile_logo_url site_favicon_url - site_home_logo_url }.each { |client_setting| client_settings << client_setting } - def self.site_home_logo_url - upload = SiteSetting.logo - - if SiteSetting.defaults.get(:title) != SiteSetting.title && !upload - '' - else - full_cdn_url(upload ? upload.url : '/images/d-logo-sketch.png') + %i{ + logo + logo_small + digest_logo + mobile_logo + large_icon + favicon + apple_touch_icon + twitter_summary_large_image + opengraph_image + push_notifications_icon + }.each do |setting_name| + define_singleton_method("site_#{setting_name}_url") do + upload = self.public_send(setting_name) + upload ? full_cdn_url(upload.url) : '' end end - def self.site_logo_url - upload = self.logo - upload ? full_cdn_url(upload.url) : self.logo_url(warn: false) - end - - def self.site_logo_small_url - upload = self.logo_small - upload ? full_cdn_url(upload.url) : self.logo_small_url(warn: false) - end - - def self.site_digest_logo_url - upload = self.digest_logo - upload ? full_cdn_url(upload.url) : self.digest_logo_url(warn: false) - end - - def self.site_mobile_logo_url - upload = self.mobile_logo - upload ? full_cdn_url(upload.url) : self.mobile_logo_url(warn: false) - end - - def self.site_large_icon_url - upload = self.large_icon - upload ? full_cdn_url(upload.url) : self.large_icon_url(warn: false) - end - - def self.site_favicon_url - upload = self.favicon - upload ? full_cdn_url(upload.url) : self.favicon_url(warn: false) - end - - def self.site_apple_touch_icon_url - upload = self.apple_touch_icon - upload ? full_cdn_url(upload.url) : self.apple_touch_icon_url(warn: false) - end - - def self.opengraph_image_url - upload = self.opengraph_image - upload ? full_cdn_url(upload.url) : self.default_opengraph_image_url(warn: false) - end - - def self.site_twitter_summary_large_image_url - self.twitter_summary_large_image&.url || - self.twitter_summary_large_image_url(warn: false) - end - - def self.site_push_notifications_icon_url - SiteSetting.push_notifications_icon&.url || - SiteSetting.push_notifications_icon_url(warn: false) - end - def self.shared_drafts_enabled? c = SiteSetting.shared_drafts_category c.present? && c.to_i != SiteSetting.uncategorized_category_id.to_i diff --git a/app/models/upload.rb b/app/models/upload.rb index 3cf5fb6b46..262d0e2c60 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -10,6 +10,7 @@ class Upload < ActiveRecord::Base include ActionView::Helpers::NumberHelper SHA1_LENGTH = 40 + SEEDED_ID_THRESHOLD = 0 belongs_to :user @@ -36,6 +37,8 @@ class Upload < ActiveRecord::Base UserAvatar.where(custom_upload_id: self.id).update_all(custom_upload_id: nil) end + scope :by_users, -> { where("uploads.id > ?", SEEDED_ID_THRESHOLD) } + def to_s self.url end diff --git a/app/services/push_notification_pusher.rb b/app/services/push_notification_pusher.rb index a7d562f238..447324011a 100644 --- a/app/services/push_notification_pusher.rb +++ b/app/services/push_notification_pusher.rb @@ -67,8 +67,8 @@ class PushNotificationPusher protected def self.get_badge - if SiteSetting.site_push_notifications_icon_url.present? - SiteSetting.site_push_notifications_icon_url + if (url = SiteSetting.site_push_notifications_icon_url).present? + url else ActionController::Base.helpers.image_url("push-notifications/discourse.png") end diff --git a/config/site_settings.yml b/config/site_settings.yml index cc8a362f3d..840dfd6864 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1,6 +1,6 @@ # Available options: # -# default - The default value of the setting. +# default - The default value of the setting. For upload site settings, use the id of the upload seeded in db/fixtures/010_uploads.rb. # client - Set to true if the javascript should have access to this setting's value. # refresh - Set to true if clients should refresh when the setting is changed. # min - For a string setting, the minimum length. For an integer setting, the minimum value. @@ -48,14 +48,14 @@ required: default: "" type: username logo: - default: "" + default: -1 client: true type: upload logo_url: hidden: true default: "/images/d-logo-sketch.png" logo_small: - default: "" + default: -2 client: true type: upload logo_small_url: @@ -83,14 +83,14 @@ required: hidden: true default: "" favicon: - default: "" + default: -3 client: true type: upload favicon_url: hidden: true default: "/images/default-favicon.ico" apple_touch_icon: - default: "" + default: -4 client: true type: upload apple_touch_icon_url: diff --git a/db/fixtures/010_uploads.rb b/db/fixtures/010_uploads.rb new file mode 100644 index 0000000000..630ac04a5c --- /dev/null +++ b/db/fixtures/010_uploads.rb @@ -0,0 +1,17 @@ +{ + -1 => "d-logo-sketch.png", + -2 => "d-logo-sketch-small.png", + -3 => "default-favicon.ico", + -4 => "default-apple-touch-icon.png" +}.each do |id, filename| + path = Rails.root.join("public/images/#{filename}") + + Upload.seed do |upload| + upload.id = id + upload.user_id = Discourse.system_user.id + upload.original_filename = filename + upload.url = "/images/#{filename}" + upload.filesize = File.size(path) + upload.extension = File.extname(path)[1..10] + end +end diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 2c8b416eff..37226070ab 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -92,10 +92,6 @@ module FileStore def purge_tombstone(grace_period) end - def get_depth_for(id) - [0, Math.log(id / 1_000.0, 16).ceil].max - end - def get_path_for(type, id, sha, extension) depth = get_depth_for(id) tree = File.join(*sha[0, depth].chars, "") @@ -148,6 +144,12 @@ module FileStore raise "Not implemented." end + def get_depth_for(id) + depths = [0] + depths << Math.log(id / 1_000.0, 16).ceil if id.positive? + depths.max + end + end end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 08c775d1d9..9c545245ee 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -230,17 +230,25 @@ module SiteSettingExtension .map do |s, v| value = send(s) + type_hash = type_supervisor.type_hash(s) + default = defaults.get(s, default_locale).to_s + + if type_hash[:type].to_s == "upload" && + default.to_i < Upload::SEEDED_ID_THRESHOLD + + default = default_uploads[default.to_i] + end opts = { setting: s, description: description(s), - default: defaults.get(s, default_locale).to_s, + default: default, value: value.to_s, category: categories[s], preview: previews[s], secret: secret_settings.include?(s), placeholder: placeholder(s) - }.merge(type_supervisor.type_hash(s)) + }.merge!(type_hash) opts end.unshift(locale_setting_hash) @@ -450,7 +458,7 @@ module SiteSettingExtension value = value.to_i - if value > 0 + if value != Upload::SEEDED_ID_THRESHOLD upload = Upload.find_by(id: value) uploads[name] = upload if upload end @@ -495,6 +503,14 @@ module SiteSettingExtension private + def default_uploads + @default_uploads ||= {} + + @default_uploads[provider.current_site] ||= begin + Upload.where("id < ?", Upload::SEEDED_ID_THRESHOLD).pluck(:id, :url).to_h + end + end + def uploads @uploads ||= {} @uploads[provider.current_site] ||= {} diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index 93da66333e..bed7c4e6e3 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -177,7 +177,7 @@ class SiteSettings::TypeSupervisor elsif type == self.class.types[:enum] val = @defaults_provider[name].is_a?(Integer) ? val.to_i : val.to_s elsif type == self.class.types[:upload] && val.present? - val = val.id + val = val.is_a?(Integer) ? val : val.id end [val, type] diff --git a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb index 119b2b8a8d..f9c4f35819 100644 --- a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb +++ b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb @@ -27,7 +27,7 @@ describe "Discobot Certificate" do it 'should return the right text' do stub_request(:get, /letter_avatar_proxy/).to_return(status: 200) - stub_request(:get, "http://test.localhost//images/d-logo-sketch-small.png") + stub_request(:get, SiteSetting.site_logo_small_url) .to_return(status: 200) get '/discobot/certificate.svg', params: params diff --git a/spec/components/file_store/base_store_spec.rb b/spec/components/file_store/base_store_spec.rb index 623b164478..df6c62c119 100644 --- a/spec/components/file_store/base_store_spec.rb +++ b/spec/components/file_store/base_store_spec.rb @@ -18,6 +18,15 @@ RSpec.describe FileStore::BaseStore do .to eq('original/2X/4/4170ac2a2782a1516fe9e13d7322ae482c1bd594.png') end end + + describe 'when id is negative' do + it 'should return the right depth' do + upload.update!(id: -999) + + expect(FileStore::BaseStore.new.get_path_for_upload(upload)) + .to eq('original/1X/4170ac2a2782a1516fe9e13d7322ae482c1bd594.png') + end + end end describe '#get_path_for_optimized_image' do diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index a4a33e6f17..daf158bbe8 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -711,6 +711,28 @@ describe SiteSettingExtension do end + describe '.all_settings' do + describe 'uploads settings' do + it 'should return the right values' do + system_upload = Fabricate(:upload, id: -999) + settings.setting(:logo, system_upload.id, type: :upload) + settings.refresh! + setting = settings.all_settings.last + + expect(setting[:value]).to eq(system_upload.url) + expect(setting[:default]).to eq(system_upload.url) + + upload = Fabricate(:upload) + settings.logo = upload + settings.refresh! + setting = settings.all_settings.last + + expect(setting[:value]).to eq(upload.url) + expect(setting[:default]).to eq(system_upload.url) + end + end + end + describe '.client_settings_json_uncached' do it 'should return the right json value' do upload = Fabricate(:upload) diff --git a/spec/components/site_settings/type_supervisor_spec.rb b/spec/components/site_settings/type_supervisor_spec.rb index 181a06a146..692923f8c6 100644 --- a/spec/components/site_settings/type_supervisor_spec.rb +++ b/spec/components/site_settings/type_supervisor_spec.rb @@ -190,6 +190,9 @@ describe SiteSettings::TypeSupervisor do expect(settings.type_supervisor.to_db_value(:type_upload, upload)) .to eq([upload.id, SiteSetting.types[:upload]]) + + expect(settings.type_supervisor.to_db_value(:type_upload, 1)) + .to eq([1, SiteSetting.types[:upload]]) end it 'returns enum value with string default' do diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index fe0aa1c550..e76398a123 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -50,6 +50,7 @@ describe Jobs::CleanUpUploads do SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting) SiteSetting.clean_orphan_uploads_grace_period_hours = 1 + system_upload = fabricate_upload(id: -999) logo_upload = fabricate_upload logo_small_upload = fabricate_upload digest_logo_upload = fabricate_upload @@ -84,7 +85,8 @@ describe Jobs::CleanUpUploads do opengraph_image_upload, twitter_summary_large_image_upload, favicon_upload, - apple_touch_icon_upload + apple_touch_icon_upload, + system_upload ].each { |record| expect(Upload.exists?(id: record.id)).to eq(true) } fabricate_upload diff --git a/spec/models/emoji_spec.rb b/spec/models/emoji_spec.rb index d1b7430f76..228fdda14b 100644 --- a/spec/models/emoji_spec.rb +++ b/spec/models/emoji_spec.rb @@ -18,7 +18,7 @@ describe Emoji do describe '.load_custom' do describe 'when a custom emoji has an invalid upload_id' do it 'should return the custom emoji without a URL' do - CustomEmoji.create!(name: 'test', upload_id: -1) + CustomEmoji.create!(name: 'test', upload_id: 9999) emoji = Emoji.load_custom.first diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index 6f4cf60782..6c6f556c29 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -150,40 +150,6 @@ describe SiteSetting do end end - describe '.site_home_logo_url' do - describe 'when logo site setting is set' do - let(:upload) { Fabricate(:upload) } - - before do - SiteSetting.logo = upload - end - - it 'should return the right URL' do - expect(SiteSetting.site_home_logo_url) - .to eq("#{Discourse.base_url}#{upload.url}") - end - end - - describe 'when logo site setting is not set' do - describe 'when there is a custom title' do - before do - SiteSetting.title = "test" - end - - it 'should return a blank string' do - expect(SiteSetting.site_home_logo_url).to eq('') - end - end - - describe 'when title has not been set' do - it 'should return the default logo url' do - expect(SiteSetting.site_home_logo_url) - .to eq("#{Discourse.base_url}/images/d-logo-sketch.png") - end - end - end - end - context 'deprecated site settings' do before do SiteSetting.force_https = true diff --git a/spec/requests/admin/site_settings_controller_spec.rb b/spec/requests/admin/site_settings_controller_spec.rb index 98c4586c3b..56428afaa6 100644 --- a/spec/requests/admin/site_settings_controller_spec.rb +++ b/spec/requests/admin/site_settings_controller_spec.rb @@ -51,31 +51,49 @@ describe Admin::SiteSettingsController do expect(SiteSetting.test_setting).to eq('') end - it 'allows upload site settings to be updated' do - upload = Fabricate(:upload) + describe 'upload site settings' do + it 'can remove the site setting' do + SiteSetting.test_upload = Fabricate(:upload) - put "/admin/site_settings/test_upload.json", params: { - test_upload: upload.url - } + put "/admin/site_settings/test_upload.json", params: { + test_upload: nil + } - expect(response.status).to eq(200) - expect(SiteSetting.test_upload).to eq(upload) + expect(response.status).to eq(200) + expect(SiteSetting.test_upload).to eq(nil) + end - user_history = UserHistory.last + it 'can reset the site setting to the default' do + SiteSetting.test_upload = nil + default_upload = Upload.find(-1) - expect(user_history.action).to eq( - UserHistory.actions[:change_site_setting] - ) + put "/admin/site_settings/test_upload.json", params: { + test_upload: default_upload.url + } - expect(user_history.previous_value).to eq(nil) - expect(user_history.new_value).to eq(upload.url) + expect(response.status).to eq(200) + expect(SiteSetting.test_upload).to eq(default_upload) + end - put "/admin/site_settings/test_upload.json", params: { - test_upload: nil - } + it 'can update the site setting' do + upload = Fabricate(:upload) - expect(response.status).to eq(200) - expect(SiteSetting.test_upload).to eq(nil) + put "/admin/site_settings/test_upload.json", params: { + test_upload: upload.url + } + + expect(response.status).to eq(200) + expect(SiteSetting.test_upload).to eq(upload) + + user_history = UserHistory.last + + expect(user_history.action).to eq( + UserHistory.actions[:change_site_setting] + ) + + expect(user_history.previous_value).to eq(nil) + expect(user_history.new_value).to eq(upload.url) + end end it 'logs the change' do diff --git a/spec/requests/exceptions_controller_spec.rb b/spec/requests/exceptions_controller_spec.rb index 3ede1182ef..a19a05a626 100644 --- a/spec/requests/exceptions_controller_spec.rb +++ b/spec/requests/exceptions_controller_spec.rb @@ -10,16 +10,14 @@ RSpec.describe ExceptionsController do expect(response.body).to have_tag( "img", with: { - src: SiteSetting.site_home_logo_url + src: SiteSetting.site_logo_url } ) end describe "text site logo" do - let(:title) { "some awesome title" } - before do - SiteSetting.title = title + SiteSetting.logo = nil end it "should return the right response" do @@ -29,7 +27,7 @@ RSpec.describe ExceptionsController do expect(response.body).to have_tag( "h2", - text: title + text: SiteSetting.title ) end end diff --git a/spec/services/push_notification_pusher_spec.rb b/spec/services/push_notification_pusher_spec.rb index 33ec16c625..5bfb947f6a 100644 --- a/spec/services/push_notification_pusher_spec.rb +++ b/spec/services/push_notification_pusher_spec.rb @@ -7,8 +7,11 @@ RSpec.describe PushNotificationPusher do end it "returns custom badges url" do - SiteSetting.push_notifications_icon_url = "/test.png" - expect(PushNotificationPusher.get_badge).to eq("/test.png") + upload = Fabricate(:upload) + SiteSetting.push_notifications_icon = upload + + expect(PushNotificationPusher.get_badge) + .to eq(UrlHelper.absolute(upload.url)) end end diff --git a/test/javascripts/helpers/site-settings.js b/test/javascripts/helpers/site-settings.js index 47da5ec9e5..70f6ab8604 100644 --- a/test/javascripts/helpers/site-settings.js +++ b/test/javascripts/helpers/site-settings.js @@ -2,7 +2,7 @@ Discourse.SiteSettingsOriginal = { title: "Discourse Meta", site_logo_url: "/assets/logo.png", - site_home_logo_url: "/assets/logo.png", + site_logo_url: "/assets/logo.png", site_logo_small_url: "/assets/logo-single.png", site_mobile_logo_url: "", site_favicon_url: diff --git a/test/javascripts/widgets/home-logo-test.js.es6 b/test/javascripts/widgets/home-logo-test.js.es6 index 9e8ff18306..c68ad0885c 100644 --- a/test/javascripts/widgets/home-logo-test.js.es6 +++ b/test/javascripts/widgets/home-logo-test.js.es6 @@ -10,7 +10,7 @@ const title = "Cool Forum"; widgetTest("basics", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { - this.siteSettings.site_home_logo_url = bigLogo; + this.siteSettings.site_logo_url = bigLogo; this.siteSettings.site_logo_small_url = smallLogo; this.siteSettings.title = title; this.set("args", { minimized: false }); @@ -28,7 +28,7 @@ widgetTest("basics", { widgetTest("basics - minimized", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { - this.siteSettings.site_home_logo_url = bigLogo; + this.siteSettings.site_logo_url = bigLogo; this.siteSettings.site_logo_small_url = smallLogo; this.siteSettings.title = title; this.set("args", { minimized: true }); @@ -44,7 +44,7 @@ widgetTest("basics - minimized", { widgetTest("no logo", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { - this.siteSettings.site_home_logo_url = ""; + this.siteSettings.site_logo_url = ""; this.siteSettings.site_logo_small_url = ""; this.siteSettings.title = title; this.set("args", { minimized: false }); @@ -59,7 +59,7 @@ widgetTest("no logo", { widgetTest("no logo - minimized", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { - this.siteSettings.site_home_logo_url = ""; + this.siteSettings.site_logo_url = ""; this.siteSettings.site_logo_small_url = ""; this.siteSettings.title = title; this.set("args", { minimized: true }); @@ -87,7 +87,7 @@ widgetTest("mobile logo", { widgetTest("mobile without logo", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { - this.siteSettings.site_home_logo_url = bigLogo; + this.siteSettings.site_logo_url = bigLogo; this.site.mobileView = true; }, @@ -101,7 +101,7 @@ widgetTest("basics, subfolder", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { Discourse.BaseUri = "/forum"; - this.siteSettings.site_home_logo_url = bigLogo; + this.siteSettings.site_logo_url = bigLogo; this.siteSettings.site_logo_small_url = smallLogo; this.siteSettings.title = title; this.set("args", { minimized: false }); @@ -118,7 +118,7 @@ widgetTest("basics, subfolder - minimized", { template: '{{mount-widget widget="home-logo" args=args}}', beforeEach() { Discourse.BaseUri = "/forum"; - this.siteSettings.site_home_logo_url = bigLogo; + this.siteSettings.site_logo_url = bigLogo; this.siteSettings.site_logo_small_url = smallLogo; this.siteSettings.title = title; this.set("args", { minimized: true }); From 243fb8d9ad87c716bd75f1bf101378abff4cc65d Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 13 Mar 2019 17:39:07 +0800 Subject: [PATCH 27/47] Fix the build. --- lib/file_store/s3_store.rb | 2 +- lib/s3_inventory.rb | 2 +- spec/components/s3_inventory_spec.rb | 3 ++- spec/helpers/application_helper_spec.rb | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index aa38584899..7428879d91 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -129,7 +129,7 @@ module FileStore S3Inventory.new(s3_helper, :upload).backfill_etags_and_list_missing S3Inventory.new(s3_helper, :optimized).backfill_etags_and_list_missing unless skip_optimized else - list_missing(Upload, "original/") + list_missing(Upload.by_users, "original/") list_missing(OptimizedImage, "optimized/") unless skip_optimized end end diff --git a/lib/s3_inventory.rb b/lib/s3_inventory.rb index 2231d78e90..e0c8aed0cb 100644 --- a/lib/s3_inventory.rb +++ b/lib/s3_inventory.rb @@ -54,7 +54,7 @@ class S3Inventory WHERE #{model.table_name}.etag IS NULL AND url ILIKE '%' || #{table_name}.key") - uploads = (model == Upload) ? model.where("created_at < ?", inventory_date) : model + uploads = (model == Upload) ? model.by_users.where("created_at < ?", inventory_date) : model 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 diff --git a/spec/components/s3_inventory_spec.rb b/spec/components/s3_inventory_spec.rb index 1a6d9e5ee7..3f81fb677c 100644 --- a/spec/components/s3_inventory_spec.rb +++ b/spec/components/s3_inventory_spec.rb @@ -61,6 +61,7 @@ describe "S3Inventory" do 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) @@ -91,6 +92,6 @@ describe "S3Inventory" do expect { inventory.backfill_etags_and_list_missing }.to change { Upload.where(etag: nil).count }.by(-2) end - expect(Upload.order(:url).pluck(:url, :etag)).to eq(files) + expect(Upload.by_users.order(:url).pluck(:url, :etag)).to eq(files) end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 650b245fcb..cae84db2f4 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -274,7 +274,7 @@ describe ApplicationHelper do ).to include("some-image.png") expect(helper.crawlable_meta_data).to include( - SiteSetting.opengraph_image_url + SiteSetting.site_opengraph_image_url ) SiteSetting.opengraph_image = nil From 135191d1d194b1cd03eb0416b5c4f7bf1a319aaf Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 13 Mar 2019 15:17:26 +0530 Subject: [PATCH 28/47] DEV: Option to add extra topic status icons in its widget and component and did some refactoring --- .../discourse/components/topic-status.js.es6 | 33 ++++------------- .../helpers/topic-status-icons.js.es6 | 28 +++++++++++++++ .../discourse/widgets/topic-status.js.es6 | 36 +++++-------------- 3 files changed, 43 insertions(+), 54 deletions(-) create mode 100644 app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 diff --git a/app/assets/javascripts/discourse/components/topic-status.js.es6 b/app/assets/javascripts/discourse/components/topic-status.js.es6 index 1929cf15e0..a3e47b7597 100644 --- a/app/assets/javascripts/discourse/components/topic-status.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-status.js.es6 @@ -1,6 +1,7 @@ import { iconHTML } from "discourse-common/lib/icon-library"; import { bufferedRender } from "discourse-common/lib/buffered-render"; import { escapeExpression } from "discourse/lib/utilities"; +import TopicStatusIcons from "discourse/helpers/topic-status-icons"; export default Ember.Component.extend( bufferedRender({ @@ -30,7 +31,10 @@ export default Ember.Component.extend( }.property("disableActions"), buildBuffer(buffer) { - const renderIcon = function(name, key, actionable) { + const canAct = this.get("canAct"); + + TopicStatusIcons.render(this.get("topic"), function(name, key) { + const actionable = ["pinned", "unpinned"].includes(key) && canAct; const title = escapeExpression(I18n.t(`topic_statuses.${key}.help`)), startTag = actionable ? "a href" : "span", endTag = actionable ? "a" : "span", @@ -40,32 +44,7 @@ export default Ember.Component.extend( buffer.push( `<${startTag} title='${title}' class='topic-status'>${icon}` ); - }; - - const renderIconIf = (conditionProp, name, key, actionable) => { - if (!this.get(conditionProp)) { - return; - } - renderIcon(name, key, actionable); - }; - - renderIconIf("topic.is_warning", "envelope", "warning"); - - if (this.get("topic.closed") && this.get("topic.archived")) { - renderIcon("lock", "locked_and_archived"); - } else { - renderIconIf("topic.closed", "lock", "locked"); - renderIconIf("topic.archived", "lock", "archived"); - } - - renderIconIf("topic.pinned", "thumbtack", "pinned", this.get("canAct")); - renderIconIf( - "topic.unpinned", - "thumbtack", - "unpinned", - this.get("canAct") - ); - renderIconIf("topic.invisible", "far-eye-slash", "unlisted"); + }); } }) ); diff --git a/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 b/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 new file mode 100644 index 0000000000..f13973d375 --- /dev/null +++ b/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 @@ -0,0 +1,28 @@ +export default Ember.ArrayProxy.extend({ + render(topic, renderIcon) { + const renderIconIf = (conditionProp, name, key) => { + if (!topic.get(conditionProp)) { + return; + } + renderIcon(name, key); + }; + + if (topic.get("closed") && topic.get("archived")) { + renderIcon("lock", "locked_and_archived"); + } else { + renderIconIf("closed", "lock", "locked"); + renderIconIf("archived", "lock", "archived"); + } + + this.forEach(args => { + renderIconIf(...args); + }); + } +}).create({ + content: [ + ["is_warning", "envelope", "warning"], + ["pinned", "thumbtack", "pinned"], + ["unpinned", "thumbtack", "unpinned"], + ["invisible", "far-eye-slash", "unlisted"] + ] +}); diff --git a/app/assets/javascripts/discourse/widgets/topic-status.js.es6 b/app/assets/javascripts/discourse/widgets/topic-status.js.es6 index 2650563e1e..fd72606210 100644 --- a/app/assets/javascripts/discourse/widgets/topic-status.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-status.js.es6 @@ -2,16 +2,7 @@ import { createWidget } from "discourse/widgets/widget"; import { iconNode } from "discourse-common/lib/icon-library"; import { h } from "virtual-dom"; import { escapeExpression } from "discourse/lib/utilities"; - -function renderIcon(name, key, canAct) { - const iconArgs = key === "unpinned" ? { class: "unpinned" } : null, - icon = iconNode(name, iconArgs); - - const attributes = { - title: escapeExpression(I18n.t(`topic_statuses.${key}.help`)) - }; - return h(`${canAct ? "a" : "span"}.topic-status`, attributes, icon); -} +import TopicStatusIcons from "discourse/helpers/topic-status-icons"; export default createWidget("topic-status", { tagName: "div.topic-statuses", @@ -21,25 +12,16 @@ export default createWidget("topic-status", { const canAct = this.currentUser && !attrs.disableActions; const result = []; - const renderIconIf = (conditionProp, name, key) => { - if (!topic.get(conditionProp)) { - return; - } - result.push(renderIcon(name, key, canAct)); - }; - renderIconIf("is_warning", "envelope", "warning"); + TopicStatusIcons.render(topic, function(name, key) { + const iconArgs = key === "unpinned" ? { class: "unpinned" } : null, + icon = iconNode(name, iconArgs); - if (topic.get("closed") && topic.get("archived")) { - renderIcon("lock", "locked_and_archived"); - } else { - renderIconIf("closed", "lock", "locked"); - renderIconIf("archived", "lock", "archived"); - } - - renderIconIf("pinned", "thumbtack", "pinned"); - renderIconIf("unpinned", "thumbtack", "unpinned"); - renderIconIf("invisible", "far-eye-slash", "unlisted"); + const attributes = { + title: escapeExpression(I18n.t(`topic_statuses.${key}.help`)) + }; + result.push(h(`${canAct ? "a" : "span"}.topic-status`, attributes, icon)); + }); return result; } From 35426b5ad6b00c9047b9b7ba17d42d4f30bdb488 Mon Sep 17 00:00:00 2001 From: Tim Lange Date: Wed, 13 Mar 2019 11:17:59 +0100 Subject: [PATCH 29/47] FIX: Better emoji escaping for topic title This commit also puts emojiVersion in its own erb file. --- app/assets/javascripts/pretty-text-bundle.js | 1 + .../javascripts/pretty-text/emoji.js.es6 | 175 ++++++++++++++++++ .../javascripts/pretty-text/emoji.js.es6.erb | 127 ------------- .../pretty-text/emoji/data.js.es6.erb | 1 + .../pretty-text/emoji/version.js.es6.erb | 2 + app/models/emoji.rb | 8 +- lib/pretty_text.rb | 12 +- lib/pretty_text/shims.js | 1 + spec/models/topic_spec.rb | 5 + .../acceptance/emoji-picker-test.js.es6 | 2 +- test/javascripts/acceptance/emoji-test.js.es6 | 2 +- test/javascripts/acceptance/topic-test.js.es6 | 19 +- test/javascripts/lib/emoji-test.js.es6 | 3 +- test/javascripts/lib/pretty-text-test.js.es6 | 2 +- test/javascripts/models/topic-test.js.es6 | 2 +- 15 files changed, 221 insertions(+), 141 deletions(-) create mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6 delete mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6.erb create mode 100644 app/assets/javascripts/pretty-text/emoji/version.js.es6.erb diff --git a/app/assets/javascripts/pretty-text-bundle.js b/app/assets/javascripts/pretty-text-bundle.js index 41324ba31e..ed5331b643 100644 --- a/app/assets/javascripts/pretty-text-bundle.js +++ b/app/assets/javascripts/pretty-text-bundle.js @@ -2,6 +2,7 @@ //= require ./pretty-text/guid //= require ./pretty-text/censored-words //= require ./pretty-text/emoji/data +//= require ./pretty-text/emoji/version //= require ./pretty-text/emoji //= require ./pretty-text/engines/discourse-markdown-it //= require xss.min diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 new file mode 100644 index 0000000000..d2c8503d69 --- /dev/null +++ b/app/assets/javascripts/pretty-text/emoji.js.es6 @@ -0,0 +1,175 @@ +import { + emojis, + aliases, + searchAliases, + translations, + tonableEmojis, + replacements +} from "pretty-text/emoji/data"; +import { IMAGE_VERSION } from "pretty-text/emoji/version"; + +const extendedEmoji = {}; + +export function registerEmoji(code, url) { + code = code.toLowerCase(); + extendedEmoji[code] = url; +} + +export function extendedEmojiList() { + return extendedEmoji; +} + +const emojiHash = {}; + +// add all default emojis +emojis.forEach(code => (emojiHash[code] = true)); + +// and their aliases +const aliasHash = {}; +Object.keys(aliases).forEach(name => { + aliases[name].forEach(alias => (aliasHash[alias] = name)); +}); + +export function performEmojiUnescape(string, opts) { + if (!string) { + return; + } + + return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { + const isEmoticon = !!translations[m]; + const isUnicodeEmoticon = !!replacements[m]; + const emojiVal = isEmoticon + ? translations[m] + : isUnicodeEmoticon + ? replacements[m] + : m.slice(1, m.length - 1); + const hasEndingColon = m.lastIndexOf(":") === m.length - 1; + const url = buildEmojiUrl(emojiVal, opts); + const classes = isCustomEmoji(emojiVal, opts) + ? "emoji emoji-custom" + : "emoji"; + + return url && (isEmoticon || hasEndingColon || isUnicodeEmoticon) + ? `${emojiVal}` + : m; + }); + + return string; +} + +export function performEmojiEscape(string) { + if (!string) { + return; + } + + return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { + if (!!translations[m]) { + return ":" + translations[m] + ":"; + } else if (!!replacements[m]) { + return ":" + replacements[m] + ":"; + } else { + return m; + } + }); + + return string; +} + +export function isCustomEmoji(code, opts) { + code = code.toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) return true; + if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) + return true; + return false; +} + +export function buildEmojiUrl(code, opts) { + let url; + code = String(code).toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) { + url = extendedEmoji[code]; + } + + if (opts && opts.customEmoji && opts.customEmoji[code]) { + url = opts.customEmoji[code]; + } + + const noToneMatch = code.match(/([^:]+):?/); + if ( + noToneMatch && + !url && + (emojiHash.hasOwnProperty(noToneMatch[1]) || + aliasHash.hasOwnProperty(noToneMatch[1])) + ) { + url = opts.getURL( + `/images/emoji/${opts.emojiSet}/${code.replace(/:t/, "/")}.png` + ); + } + + if (url) { + url = url + "?v=" + IMAGE_VERSION; + } + + return url; +} + +export function emojiExists(code) { + code = code.toLowerCase(); + return !!( + extendedEmoji.hasOwnProperty(code) || + emojiHash.hasOwnProperty(code) || + aliasHash.hasOwnProperty(code) + ); +} + +let toSearch; +export function emojiSearch(term, options) { + const maxResults = (options && options["maxResults"]) || -1; + if (maxResults === 0) { + return []; + } + + toSearch = + toSearch || + _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); + + const results = []; + + function addResult(t) { + const val = aliasHash[t] || t; + if (results.indexOf(val) === -1) { + results.push(val); + } + } + + // if term matches from beginning + for (let i = 0; i < toSearch.length; i++) { + const item = toSearch[i]; + if (item.indexOf(term) === 0) addResult(item); + } + + if (searchAliases[term]) { + results.push.apply(results, searchAliases[term]); + } + + for (let i = 0; i < toSearch.length; i++) { + const item = toSearch[i]; + if (item.indexOf(term) > 0) addResult(item); + } + + if (maxResults === -1) { + return results; + } else { + return results.slice(0, maxResults); + } +} + +export function isSkinTonableEmoji(term) { + const match = _.compact(term.split(":"))[0]; + if (match) { + return tonableEmojis.indexOf(match) !== -1; + } + return false; +} diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6.erb b/app/assets/javascripts/pretty-text/emoji.js.es6.erb deleted file mode 100644 index 09ab4ce5ed..0000000000 --- a/app/assets/javascripts/pretty-text/emoji.js.es6.erb +++ /dev/null @@ -1,127 +0,0 @@ -import { emojis, aliases, searchAliases, translations, tonableEmojis } from 'pretty-text/emoji/data'; - -// bump up this number to expire all emojis -export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; - -const extendedEmoji = {}; - -export function registerEmoji(code, url) { - code = code.toLowerCase(); - extendedEmoji[code] = url; -} - -export function extendedEmojiList() { - return extendedEmoji; -} - -const emojiHash = {}; - -// add all default emojis -emojis.forEach(code => emojiHash[code] = true); - -// and their aliases -const aliasHash = {}; -Object.keys(aliases).forEach(name => { - aliases[name].forEach(alias => aliasHash[alias] = name); -}); - -export function performEmojiUnescape(string, opts) { - if (!string) { return; } - - // this can be further improved by supporting matches of emoticons that don't begin with a colon - if (string.indexOf(":") >= 0) { - return string.replace(/\B:[^\s:]+(?::t\d)?:?\B/g, m => { - const isEmoticon = !!translations[m]; - const emojiVal = isEmoticon ? translations[m] : m.slice(1, m.length - 1); - const hasEndingColon = m.lastIndexOf(":") === m.length - 1; - const url = buildEmojiUrl(emojiVal, opts); - const classes = isCustomEmoji(emojiVal, opts) ? "emoji emoji-custom" : "emoji"; - - return url && (isEmoticon || hasEndingColon) ? - `${emojiVal}` : m; - }); - } - - return string; -} - -export function isCustomEmoji(code, opts) { - code = code.toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) return true; - if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) return true; - return false; -} - -export function buildEmojiUrl(code, opts) { - let url; - code = String(code).toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) { - url = extendedEmoji[code]; - } - - if (opts && opts.customEmoji && opts.customEmoji[code]) { - url = opts.customEmoji[code]; - } - - const noToneMatch = code.match(/([^:]+):?/); - if (noToneMatch && !url && (emojiHash.hasOwnProperty(noToneMatch[1]) || aliasHash.hasOwnProperty(noToneMatch[1]))) { - url = opts.getURL(`/images/emoji/${opts.emojiSet}/${code.replace(/:t/, '/')}.png`); - } - - if (url) { - url = url + "?v=" + IMAGE_VERSION; - } - - return url; -} - -export function emojiExists(code) { - code = code.toLowerCase(); - return !!(extendedEmoji.hasOwnProperty(code) || emojiHash.hasOwnProperty(code) || aliasHash.hasOwnProperty(code)); -}; - -let toSearch; -export function emojiSearch(term, options) { - const maxResults = (options && options["maxResults"]) || -1; - if (maxResults === 0) { return []; } - - toSearch = toSearch || _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); - - const results = []; - - function addResult(t) { - const val = aliasHash[t] || t; - if (results.indexOf(val) === -1) { - results.push(val); - } - } - - // if term matches from beginning - for (let i=0; i 0) addResult(item); - } - - if (maxResults === -1) { - return results; - } else { - return results.slice(0, maxResults); - } -}; - -export function isSkinTonableEmoji(term) { - const match = _.compact(term.split(":"))[0]; - if (match) { - return tonableEmojis.indexOf(match) !== -1; - } - return false; -} diff --git a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb index 32a9b986bd..5825170790 100644 --- a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb +++ b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb @@ -3,3 +3,4 @@ export const tonableEmojis = <%= Emoji.tonable_emojis.flatten.inspect %>; export const aliases = <%= Emoji.aliases.inspect.gsub("=>", ":") %>; export const searchAliases = <%= Emoji.search_aliases.inspect.gsub("=>", ":") %>; export const translations = <%= Emoji.translations.inspect.gsub("=>", ":") %>; +export const replacements = <%= Emoji.unicode_replacements_json %>; diff --git a/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb new file mode 100644 index 0000000000..b846ca59d5 --- /dev/null +++ b/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb @@ -0,0 +1,2 @@ +// bump up this number to expire all emojis +export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; diff --git a/app/models/emoji.rb b/app/models/emoji.rb index c20d26ef06..ebe58ea311 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -157,13 +157,7 @@ class Emoji end def self.unicode_unescape(string) - string.each_char.map do |c| - if str = unicode_replacements[c] - ":#{str}:" - else - c - end - end.join + PrettyText.escape_emoji(string) end def self.gsub_emoji_to_unicode(str) diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index d4d8ce56f8..1943417ef2 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -227,7 +227,7 @@ module PrettyText end def self.unescape_emoji(title) - return title unless SiteSetting.enable_emoji? + return title unless SiteSetting.enable_emoji? && title set = SiteSetting.emoji_set.inspect custom = Emoji.custom.map { |e| [e.name, e.url] }.to_h.to_json @@ -239,6 +239,16 @@ module PrettyText end end + def self.escape_emoji(title) + return unless title + + protect do + v8.eval(<<~JS) + __performEmojiEscape(#{title.inspect}); + JS + end + end + def self.cook(text, opts = {}) options = opts.dup diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 649c3e4b39..929dafd13a 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -1,6 +1,7 @@ __PrettyText = require("pretty-text/pretty-text").default; __buildOptions = require("pretty-text/pretty-text").buildOptions; __performEmojiUnescape = require("pretty-text/emoji").performEmojiUnescape; +__performEmojiEscape = require("pretty-text/emoji").performEmojiEscape; __utils = require("discourse/lib/utilities"); diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index d0ecd98a43..634f403993 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -279,6 +279,7 @@ describe Topic do let(:topic_image) { build_topic_with_title("Topic with image in its title") } let(:topic_script) { build_topic_with_title("Topic with script in its title") } let(:topic_emoji) { build_topic_with_title("I 💖 candy alot") } + let(:topic_modifier_emoji) { build_topic_with_title("I 👨‍🌾 candy alot") } it "escapes script contents" do expect(topic_script.fancy_title).to eq("Topic with <script>alert(‘title’)</script> script in its title") @@ -288,6 +289,10 @@ describe Topic do expect(topic_emoji.fancy_title).to eq("I :sparkling_heart: candy alot") end + it "keeps combined emojis" do + expect(topic_modifier_emoji.fancy_title).to eq("I :man_farmer: candy alot") + end + it "escapes bold contents" do expect(topic_bold.fancy_title).to eq("Topic with <b>bold</b> text in its title") end diff --git a/test/javascripts/acceptance/emoji-picker-test.js.es6 b/test/javascripts/acceptance/emoji-picker-test.js.es6 index 3c2ebe7be4..9122cd8ea5 100644 --- a/test/javascripts/acceptance/emoji-picker-test.js.es6 +++ b/test/javascripts/acceptance/emoji-picker-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { resetCache } from "discourse/components/emoji-picker"; acceptance("EmojiPicker", { diff --git a/test/javascripts/acceptance/emoji-test.js.es6 b/test/javascripts/acceptance/emoji-test.js.es6 index 48ac7f9bcc..39dc37696e 100644 --- a/test/javascripts/acceptance/emoji-test.js.es6 +++ b/test/javascripts/acceptance/emoji-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { acceptance } from "helpers/qunit-helpers"; acceptance("Emoji", { loggedIn: true }); diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index c38e574345..1c371c41d1 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; acceptance("Topic", { loggedIn: true, @@ -121,6 +121,23 @@ QUnit.test("Updating the topic title with emojis", async assert => { ); }); +QUnit.test("Updating the topic title with unicode emojis", async assert => { + await visit("/t/internationalization-localization/280"); + await click("#topic-title .d-icon-pencil-alt"); + + await fillIn("#edit-title", "emojis title 👨‍🌾"); + + await click("#topic-title .submit-edit"); + + assert.equal( + find(".fancy-title") + .html() + .trim(), + `emojis title man_farmer`, + "it displays the new title with escaped unicode emojis" + ); +}); + acceptance("Topic featured links", { loggedIn: true, settings: { diff --git a/test/javascripts/lib/emoji-test.js.es6 b/test/javascripts/lib/emoji-test.js.es6 index 47665a8c8a..c3b8ccd563 100644 --- a/test/javascripts/lib/emoji-test.js.es6 +++ b/test/javascripts/lib/emoji-test.js.es6 @@ -1,4 +1,5 @@ -import { emojiSearch, IMAGE_VERSION as v } from "pretty-text/emoji"; +import { emojiSearch } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { emojiUnescape } from "discourse/lib/text"; QUnit.module("lib:emoji"); diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6 index 162ef49691..9495122a87 100644 --- a/test/javascripts/lib/pretty-text-test.js.es6 +++ b/test/javascripts/lib/pretty-text-test.js.es6 @@ -1,7 +1,7 @@ import Quote from "discourse/lib/quote"; import Post from "discourse/models/post"; import { default as PrettyText, buildOptions } from "pretty-text/pretty-text"; -import { IMAGE_VERSION as v } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { INLINE_ONEBOX_LOADING_CSS_CLASS } from "pretty-text/inline-oneboxer"; QUnit.module("lib:pretty-text"); diff --git a/test/javascripts/models/topic-test.js.es6 b/test/javascripts/models/topic-test.js.es6 index ca6bd1f7b5..e385e1e92f 100644 --- a/test/javascripts/models/topic-test.js.es6 +++ b/test/javascripts/models/topic-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji"; +import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; QUnit.module("model:topic"); From 76a14c47acb215bae9f867ed3b7c4c269a7f3870 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 13 Mar 2019 12:34:47 +0200 Subject: [PATCH 30/47] FEATURE: Add site contact group. (#7152) --- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 3 +++ lib/site_settings/type_supervisor.rb | 3 +++ lib/system_message.rb | 1 + lib/validators/group_setting_validator.rb | 14 +++++++++++++ spec/components/system_message_spec.rb | 21 ++++++++++++++++--- .../group_setting_validator_spec.rb | 21 +++++++++++++++++++ 7 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 lib/validators/group_setting_validator.rb create mode 100644 spec/components/validators/group_setting_validator_spec.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4114136a96..c1cba642b2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1406,6 +1406,7 @@ en: post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on." share_links: "Determine which items appear on the share dialog, and in what order." site_contact_username: "A valid staff username to send all automated messages from. If left blank the default System account will be used." + site_contact_group_name: "A valid group name to be invited to all automated messages." send_welcome_message: "Send all new users a welcome message with a quick start guide." send_tl1_welcome_message: "Send new trust level 1 users a welcome message." suppress_reply_directly_below: "Don't show the expandable reply count on a post when there is only a single reply directly below this post." @@ -1987,6 +1988,7 @@ en: errors: invalid_email: "Invalid email address." invalid_username: "There's no user with that username." + invalid_group: "There's no group with that name." invalid_integer_min_max: "Value must be between %{min} and %{max}." invalid_integer_min: "Value must be %{min} or greater." invalid_integer_max: "Value cannot be higher than %{max}." diff --git a/config/site_settings.yml b/config/site_settings.yml index 840dfd6864..24e69ece6f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -47,6 +47,9 @@ required: site_contact_username: default: "" type: username + site_contact_group_name: + default: "" + type: group logo: default: -1 client: true diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index bed7c4e6e3..d354946402 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -32,6 +32,7 @@ class SiteSettings::TypeSupervisor category: 16, uploaded_image_list: 17, upload: 18, + group: 19, ) end @@ -239,6 +240,8 @@ class SiteSettings::TypeSupervisor EmailSettingValidator when self.class.types[:username] UsernameSettingValidator + when self.class.types[:group] + GroupSettingValidator when self.class.types[:integer] IntegerSettingValidator when self.class.types[:regex] diff --git a/lib/system_message.rb b/lib/system_message.rb index 553f3b9338..4d10c554ae 100644 --- a/lib/system_message.rb +++ b/lib/system_message.rb @@ -28,6 +28,7 @@ class SystemMessage raw: raw, archetype: Archetype.private_message, target_usernames: @recipient.username, + target_group_names: Group.exists?(name: SiteSetting.site_contact_group_name) ? SiteSetting.site_contact_group_name : nil, subtype: TopicSubtype.system_message, skip_validations: true) diff --git a/lib/validators/group_setting_validator.rb b/lib/validators/group_setting_validator.rb new file mode 100644 index 0000000000..01ba575626 --- /dev/null +++ b/lib/validators/group_setting_validator.rb @@ -0,0 +1,14 @@ +class GroupSettingValidator + + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(val) + val.blank? || Group.exists?(name: val) + end + + def error_message + I18n.t('site_settings.errors.invalid_group') + end +end diff --git a/spec/components/system_message_spec.rb b/spec/components/system_message_spec.rb index 6a77cfb25a..35dd84d74c 100644 --- a/spec/components/system_message_spec.rb +++ b/spec/components/system_message_spec.rb @@ -6,11 +6,14 @@ describe SystemMessage do context 'send' do - it 'should create a post correctly' do + let(:admin) { Fabricate(:admin) } + let(:user) { Fabricate(:user) } - admin = Fabricate(:admin) - user = Fabricate(:user) + before do SiteSetting.site_contact_username = admin.username + end + + it 'should create a post correctly' do system_message = SystemMessage.new(user) post = system_message.create(:welcome_invite) topic = post.topic @@ -24,6 +27,18 @@ describe SystemMessage do expect(UserArchivedMessage.where(user_id: admin.id, topic_id: topic.id).length).to eq(1) end + + it 'should allow site_contact_group_name' do + group = Fabricate(:group) + SiteSetting.site_contact_group_name = group.name + + post = SystemMessage.create(user, :welcome_invite) + expect(post.topic.allowed_groups).to contain_exactly(group) + + group.update!(name: "anewname") + post = SystemMessage.create(user, :welcome_invite) + expect(post.topic.allowed_groups).to contain_exactly() + end end end diff --git a/spec/components/validators/group_setting_validator_spec.rb b/spec/components/validators/group_setting_validator_spec.rb new file mode 100644 index 0000000000..668d44badd --- /dev/null +++ b/spec/components/validators/group_setting_validator_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +describe GroupSettingValidator do + describe '#valid_value?' do + subject(:validator) { described_class.new } + + it "returns true for blank values" do + expect(validator.valid_value?('')).to eq(true) + expect(validator.valid_value?(nil)).to eq(true) + end + + it "returns true if value matches an existing group" do + Fabricate(:group, name: "hello") + expect(validator.valid_value?('hello')).to eq(true) + end + + it "returns false if value does not match a group" do + expect(validator.valid_value?('notagroup')).to eq(false) + end + end +end From 1b454c73ae07769d19c76828b972bd0525fa3769 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 13 Mar 2019 16:34:47 +0530 Subject: [PATCH 31/47] FIX: 'topic' can have null value --- Gemfile.lock | 2 +- .../javascripts/discourse/components/topic-status.js.es6 | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 299c7f91ac..1c78e4c720 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -560,4 +560,4 @@ DEPENDENCIES webpush BUNDLED WITH - 1.17.3 + 2.0.1 diff --git a/app/assets/javascripts/discourse/components/topic-status.js.es6 b/app/assets/javascripts/discourse/components/topic-status.js.es6 index a3e47b7597..9e92561ea6 100644 --- a/app/assets/javascripts/discourse/components/topic-status.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-status.js.es6 @@ -32,8 +32,13 @@ export default Ember.Component.extend( buildBuffer(buffer) { const canAct = this.get("canAct"); + const topic = this.get("topic"); - TopicStatusIcons.render(this.get("topic"), function(name, key) { + if (!topic) { + return; + } + + TopicStatusIcons.render(topic, function(name, key) { const actionable = ["pinned", "unpinned"].includes(key) && canAct; const title = escapeExpression(I18n.t(`topic_statuses.${key}.help`)), startTag = actionable ? "a href" : "span", From d4d67386c93962e2c559614b5cdfd589724577d5 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 13 Mar 2019 16:43:45 +0530 Subject: [PATCH 32/47] FIX: change to correct bundled version --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1c78e4c720..299c7f91ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -560,4 +560,4 @@ DEPENDENCIES webpush BUNDLED WITH - 2.0.1 + 1.17.3 From 3eebf8be733065996ff172adbcad85c254048274 Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Wed, 13 Mar 2019 12:16:06 +0100 Subject: [PATCH 33/47] DEV: Upgrade to Ember 3.7.0 (#6977) * Upgrade to Ember 3.7.0 * use ember source 3.7.0.2 * fix mobile header * fix navigation --- Gemfile | 2 +- Gemfile.lock | 4 +- .../admin/components/ace-editor.js.es6 | 2 +- .../admin/routes/admin-users-list-show.js.es6 | 7 +- .../discourse/components/d-editor.js.es6 | 26 +++---- .../discourse/components/d-modal-body.js.es6 | 8 +-- .../components/discourse-topic.js.es6 | 42 ++++++------ .../discourse/components/emoji-picker.js.es6 | 8 ++- .../discourse/components/login-modal.js.es6 | 2 +- .../discourse/components/nav-item.js.es6 | 4 +- .../components/scrolling-post-stream.js.es6 | 68 +++++++++++-------- .../discourse/components/site-header.js.es6 | 24 ++++--- .../components/topic-entrance.js.es6 | 4 +- .../discourse/components/watch-read.js.es6 | 2 +- .../discourse/helpers/route-action.js.es6 | 9 ++- .../initializers/page-tracking.js.es6 | 6 +- .../discourse/initializers/show-footer.js.es6 | 2 +- .../discourse/lib/app-events.js.es6 | 49 ++++++++++++- .../discourse/lib/page-tracker.js.es6 | 8 +-- .../mixins/card-contents-base.js.es6 | 12 ++-- .../discourse/mixins/url-refresh.js.es6 | 4 +- .../discourse/routes/badges-show.js.es6 | 3 +- .../routes/build-category-route.js.es6 | 2 +- .../routes/group-activity-posts.js.es6 | 2 +- .../discourse/routes/new-message.js.es6 | 3 +- .../discourse/routes/new-topic.js.es6 | 20 +++--- .../discourse/routes/tags-show.js.es6 | 4 +- .../javascripts/discourse/routes/topic.js.es6 | 2 +- .../routes/user-activity-stream.js.es6 | 2 +- .../components/category-chooser.js.es6 | 11 --- .../components/group-members-dropdown.js.es6 | 2 +- .../select-kit/components/select-kit.js.es6 | 6 +- .../topic-notifications-options.js.es6 | 16 ++--- 33 files changed, 214 insertions(+), 152 deletions(-) diff --git a/Gemfile b/Gemfile index 45af6c03c3..8b2b8978b3 100644 --- a/Gemfile +++ b/Gemfile @@ -49,7 +49,7 @@ gem 'onebox', '1.8.82' gem 'http_accept_language', '~>2.0.5', require: false gem 'ember-rails', '0.18.5' -gem 'discourse-ember-source', '~> 3.5.1' +gem 'discourse-ember-source', '~> 3.7.0' gem 'ember-handlebars-template', '0.8.0' gem 'barber' diff --git a/Gemfile.lock b/Gemfile.lock index 299c7f91ac..33227007f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -108,7 +108,7 @@ GEM terminal-table (~> 1) debug_inspector (0.0.3) diff-lcs (1.3) - discourse-ember-source (3.5.1.3) + discourse-ember-source (3.7.0.2) discourse_image_optim (0.26.2) exifr (~> 1.2, >= 1.2.2) fspath (~> 3.0) @@ -466,7 +466,7 @@ DEPENDENCIES colored2 cppjieba_rb danger - discourse-ember-source (~> 3.5.1) + discourse-ember-source (~> 3.7.0) discourse_image_optim email_reply_trimmer (~> 0.1) ember-handlebars-template (= 0.8.0) diff --git a/app/assets/javascripts/admin/components/ace-editor.js.es6 b/app/assets/javascripts/admin/components/ace-editor.js.es6 index b86c0e4cd7..b3576c8bea 100644 --- a/app/assets/javascripts/admin/components/ace-editor.js.es6 +++ b/app/assets/javascripts/admin/components/ace-editor.js.es6 @@ -100,7 +100,7 @@ export default Ember.Component.extend({ if (this.appEvents) { // xxx: don't run during qunit tests - this.appEvents.on("ace:resize", () => this.resize()); + this.appEvents.on("ace:resize", this, "resize"); } if (this.get("autofocus")) { diff --git a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 b/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 index e893139fc4..fba32bec28 100644 --- a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 @@ -10,12 +10,13 @@ export default Discourse.Route.extend({ const routeName = "adminUsersList.show"; if (transition.targetName === routeName) { - const params = transition.params[routeName]; + const params = transition.routeInfos.find(a => a.name === routeName) + .params; const controller = this.controllerFor(routeName); if (controller) { controller.setProperties({ - order: transition.queryParams.order, - ascending: transition.queryParams.ascending, + order: transition.to.queryParams.order, + ascending: transition.to.queryParams.ascending, query: params.filter, showEmails: false, refreshing: false diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index bc9240d53e..b066805374 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -290,25 +290,27 @@ export default Ember.Component.extend({ }); if (this.get("composerEvents")) { - this.appEvents.on("composer:insert-block", text => - this._addBlock(this._getSelected(), text) - ); - this.appEvents.on("composer:insert-text", (text, options) => - this._addText(this._getSelected(), text, options) - ); - this.appEvents.on("composer:replace-text", (oldVal, newVal, opts) => - this._replaceText(oldVal, newVal, opts) - ); + this.appEvents.on("composer:insert-block", this, "_insertBlock"); + this.appEvents.on("composer:insert-text", this, "_insertText"); + this.appEvents.on("composer:replace-text", this, "_replaceText"); } this._mouseTrap = mouseTrap; }, + _insertBlock(text) { + this._addBlock(this._getSelected(), text); + }, + + _insertText(text, options) { + this._addText(this._getSelected(), text, options); + }, + @on("willDestroyElement") _shutDown() { if (this.get("composerEvents")) { - this.appEvents.off("composer:insert-block"); - this.appEvents.off("composer:insert-text"); - this.appEvents.off("composer:replace-text"); + this.appEvents.off("composer:insert-block", this, "_insertBlock"); + this.appEvents.off("composer:insert-text", this, "_insertText"); + this.appEvents.off("composer:replace-text", this, "_replaceText"); } const mouseTrap = this._mouseTrap; diff --git a/app/assets/javascripts/discourse/components/d-modal-body.js.es6 b/app/assets/javascripts/discourse/components/d-modal-body.js.es6 index 60f0d9a82a..a49887519c 100644 --- a/app/assets/javascripts/discourse/components/d-modal-body.js.es6 +++ b/app/assets/javascripts/discourse/components/d-modal-body.js.es6 @@ -14,14 +14,14 @@ export default Ember.Component.extend({ } Ember.run.scheduleOnce("afterRender", this, this._afterFirstRender); - this.appEvents.on("modal-body:flash", msg => this._flash(msg)); - this.appEvents.on("modal-body:clearFlash", () => this._clearFlash()); + this.appEvents.on("modal-body:flash", this, "_flash"); + this.appEvents.on("modal-body:clearFlash", this, "_clearFlash"); }, willDestroyElement() { this._super(...arguments); - this.appEvents.off("modal-body:flash"); - this.appEvents.off("modal-body:clearFlash"); + this.appEvents.off("modal-body:flash", this, "_flash"); + this.appEvents.off("modal-body:clearFlash", this, "_clearFlash"); }, _afterFirstRender() { diff --git a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 index 4a112c7e20..d404252cd3 100644 --- a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 +++ b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 @@ -77,9 +77,7 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { } ); - this.appEvents.on("post:highlight", postNumber => { - Ember.run.scheduleOnce("afterRender", null, highlight, postNumber); - }); + this.appEvents.on("post:highlight", this, "_highlightPost"); this.appEvents.on("header:update-topic", topic => { if (topic === null) { @@ -99,23 +97,29 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { }); // setup mobile scroll logo if (this.site.mobileView) { - this.appEvents.on("topic:scrolled", offset => - this.mobileScrollGaurd(offset) - ); + this.appEvents.on("topic:scrolled", this, "mobileScrollGuard"); // used to animate header contents on scroll - this.appEvents.on("header:show-topic", () => { - $("header.d-header") - .removeClass("scroll-up") - .addClass("scroll-down"); - }); - this.appEvents.on("header:hide-topic", () => { - $("header.d-header") - .removeClass("scroll-down") - .addClass("scroll-up"); - }); + this.appEvents.on("header:show-topic", this, "_showTopic"); + this.appEvents.on("header:hide-topic", this, "_hideTopic"); } }, + _showTopic() { + $("header.d-header") + .removeClass("scroll-up") + .addClass("scroll-down"); + }, + + _hideTopic() { + $("header.d-header") + .removeClass("scroll-down") + .addClass("scroll-up"); + }, + + _highlightPost(postNumber) { + Ember.run.scheduleOnce("afterRender", null, highlight, postNumber); + }, + willDestroyElement() { this._super(...arguments); this.unbindScrolling("topic-view"); @@ -128,10 +132,10 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { // this happens after route exit, stuff could have trickled in this.appEvents.trigger("header:hide-topic"); - this.appEvents.off("post:highlight"); + this.appEvents.off("post:highlight", this, "_highlightPost"); // mobile scroll logo clean up. if (this.site.mobileView) { - this.appEvents.off("topic:scrolled"); + this.appEvents.off("topic:scrolled", this, "mobileScrollGuard"); $("header.d-header").removeClass("scroll-down scroll-up"); } }, @@ -199,7 +203,7 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { // determines scroll direction, triggers header topic info on mobile // and ensures that the switch happens only once per scroll direction change - mobileScrollGaurd(offset) { + mobileScrollGuard(offset) { // user hasn't scrolled past topic title. if (offset < this.dockAt) return; diff --git a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 index 22fac50e4f..ee8e6fc6dd 100644 --- a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 +++ b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 @@ -70,7 +70,11 @@ export default Ember.Component.extend({ @on("willDestroyElement") _unbindGlobalEvents() { - this.appEvents.off("emoji-picker:close"); + this.appEvents.off("emoji-picker:close", this, "_closeEmojiPicker"); + }, + + _closeEmojiPicker() { + this.set("active", false); }, @on("didInsertElement") @@ -78,7 +82,7 @@ export default Ember.Component.extend({ this.$picker = this.$(".emoji-picker"); this.$modal = this.$(".emoji-picker-modal"); - this.appEvents.on("emoji-picker:close", () => this.set("active", false)); + this.appEvents.on("emoji-picker:close", this, "_closeEmojiPicker"); if (!keyValueStore.getObject(EMOJI_USAGE)) { keyValueStore.setObject({ key: EMOJI_USAGE, value: [] }); diff --git a/app/assets/javascripts/discourse/components/login-modal.js.es6 b/app/assets/javascripts/discourse/components/login-modal.js.es6 index c1c452f663..59939542cb 100644 --- a/app/assets/javascripts/discourse/components/login-modal.js.es6 +++ b/app/assets/javascripts/discourse/components/login-modal.js.es6 @@ -18,7 +18,7 @@ export default Ember.Component.extend({ "#login-account-password, #login-account-name, #login-second-factor" ).keydown(e => { if (e.keyCode === 13) { - this.sendAction(); + this.action(); } }); }); diff --git a/app/assets/javascripts/discourse/components/nav-item.js.es6 b/app/assets/javascripts/discourse/components/nav-item.js.es6 index 3910ad6b64..729c22fb12 100644 --- a/app/assets/javascripts/discourse/components/nav-item.js.es6 +++ b/app/assets/javascripts/discourse/components/nav-item.js.es6 @@ -7,7 +7,7 @@ export default Ember.Component.extend({ tagName: "li", classNameBindings: ["active"], - @computed() + @computed router() { return getOwner(this).lookup("router:main"); }, @@ -27,7 +27,7 @@ export default Ember.Component.extend({ router = this.get("router"); return routeParam - ? router.isActive(route, routeParam) + ? router.currentRoute.params["filter"] === routeParam : router.isActive(route); } }); diff --git a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 index fc5f0a413a..94779cdebe 100644 --- a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 +++ b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 @@ -262,6 +262,38 @@ export default MountWidget.extend({ Ember.run.scheduleOnce("afterRender", this, this.scrolled); }, + _posted(staged) { + const disableJumpReply = this.currentUser.get("disable_jump_reply"); + + this.queueRerender(() => { + if (staged && !disableJumpReply) { + const postNumber = staged.get("post_number"); + DiscourseURL.jumpToPost(postNumber, { skipIfOnScreen: true }); + } + }); + }, + + _refresh(args) { + if (args) { + if (args.id) { + this.dirtyKeys.keyDirty(`post-${args.id}`); + + if (args.refreshLikes) { + this.dirtyKeys.keyDirty(`post-menu-${args.id}`, { + onRefresh: "refreshLikes" + }); + } + } else if (args.force) { + this.dirtyKeys.forceAll(); + } + } + this.queueRerender(); + }, + + _debouncedScroll() { + Ember.run.debounce(this, this._scrollTriggered, 10); + }, + didInsertElement() { this._super(...arguments); const debouncedScroll = () => @@ -269,21 +301,12 @@ export default MountWidget.extend({ this._previouslyNearby = {}; - this.appEvents.on("post-stream:refresh", debouncedScroll); + this.appEvents.on("post-stream:refresh", this, "_debouncedScroll"); $(document).bind("touchmove.post-stream", debouncedScroll); $(window).bind("scroll.post-stream", debouncedScroll); this._scrollTriggered(); - this.appEvents.on("post-stream:posted", staged => { - const disableJumpReply = this.currentUser.get("disable_jump_reply"); - - this.queueRerender(() => { - if (staged && !disableJumpReply) { - const postNumber = staged.get("post_number"); - DiscourseURL.jumpToPost(postNumber, { skipIfOnScreen: true }); - } - }); - }); + this.appEvents.on("post-stream:posted", this, "_posted"); this.$().on("mouseenter.post-stream", "button.widget-button", e => { $("button.widget-button").removeClass("d-hover"); @@ -294,33 +317,18 @@ export default MountWidget.extend({ $("button.widget-button").removeClass("d-hover"); }); - this.appEvents.on("post-stream:refresh", args => { - if (args) { - if (args.id) { - this.dirtyKeys.keyDirty(`post-${args.id}`); - - if (args.refreshLikes) { - this.dirtyKeys.keyDirty(`post-menu-${args.id}`, { - onRefresh: "refreshLikes" - }); - } - } else if (args.force) { - this.dirtyKeys.forceAll(); - } - } - this.queueRerender(); - }); + this.appEvents.on("post-stream:refresh", this, "_refresh"); }, willDestroyElement() { this._super(...arguments); $(document).unbind("touchmove.post-stream"); $(window).unbind("scroll.post-stream"); - this.appEvents.off("post-stream:refresh"); + this.appEvents.off("post-stream:refresh", this, "_debouncedScroll"); this.$().off("mouseenter.post-stream"); this.$().off("mouseleave.post-stream"); - this.appEvents.off("post-stream:refresh"); - this.appEvents.off("post-stream:posted"); + this.appEvents.off("post-stream:refresh", this, "_refresh"); + this.appEvents.off("post-stream:posted", this, "_posted"); }, showModerationHistory(post) { diff --git a/app/assets/javascripts/discourse/components/site-header.js.es6 b/app/assets/javascripts/discourse/components/site-header.js.es6 index 28627bff78..898ebcce2d 100644 --- a/app/assets/javascripts/discourse/components/site-header.js.es6 +++ b/app/assets/javascripts/discourse/components/site-header.js.es6 @@ -231,19 +231,14 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { const { isAndroid } = this.capabilities; $(window).on("resize.discourse-menu-panel", () => this.afterRender()); - this.appEvents.on("header:show-topic", topic => this.setTopic(topic)); - this.appEvents.on("header:hide-topic", () => this.setTopic(null)); + this.appEvents.on("header:show-topic", this, "setTopic"); + this.appEvents.on("header:hide-topic", this, "setTopic"); this.dispatch("notifications:changed", "user-notifications"); this.dispatch("header:keyboard-trigger", "header"); this.dispatch("search-autocomplete:after-complete", "search-term"); - this.appEvents.on("dom:clean", () => { - // For performance, only trigger a re-render if any menu panels are visible - if (this.$(".menu-panel").length) { - this.eventDispatched("dom:clean", "header"); - } - }); + this.appEvents.on("dom:clean", this, "_cleanDom"); // Only add listeners for opening menus by swiping them in on Android devices // iOS will respond to these events, but also does swiping for back/forward @@ -252,15 +247,22 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { } }, + _cleanDom() { + // For performance, only trigger a re-render if any menu panels are visible + if (this.$(".menu-panel").length) { + this.eventDispatched("dom:clean", "header"); + } + }, + willDestroyElement() { this._super(...arguments); const { isAndroid } = this.capabilities; $("body").off("keydown.header"); $(window).off("resize.discourse-menu-panel"); - this.appEvents.off("header:show-topic"); - this.appEvents.off("header:hide-topic"); - this.appEvents.off("dom:clean"); + this.appEvents.off("header:show-topic", this, "setTopic"); + this.appEvents.off("header:hide-topic", this, "setTopic"); + this.appEvents.off("dom:clean", this, "_cleanDom"); if (isAndroid) { this.removeTouchListeners($("body")); diff --git a/app/assets/javascripts/discourse/components/topic-entrance.js.es6 b/app/assets/javascripts/discourse/components/topic-entrance.js.es6 index 2ded0e7314..fa25a528a7 100644 --- a/app/assets/javascripts/discourse/components/topic-entrance.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-entrance.js.es6 @@ -53,7 +53,7 @@ export default Ember.Component.extend(CleansUp, { didInsertElement() { this._super(...arguments); - this.appEvents.on("topic-entrance:show", data => this._show(data)); + this.appEvents.on("topic-entrance:show", this, "_show"); }, _setCSS() { @@ -100,7 +100,7 @@ export default Ember.Component.extend(CleansUp, { }, willDestroyElement() { - this.appEvents.off("topic-entrance:show"); + this.appEvents.off("topic-entrance:show", this, "_show"); }, _jumpTo(destination) { diff --git a/app/assets/javascripts/discourse/components/watch-read.js.es6 b/app/assets/javascripts/discourse/components/watch-read.js.es6 index 7cca41ab8b..4b149d371c 100644 --- a/app/assets/javascripts/discourse/components/watch-read.js.es6 +++ b/app/assets/javascripts/discourse/components/watch-read.js.es6 @@ -13,7 +13,7 @@ export default Ember.Component.extend({ $(window).on("load.faq resize.faq scroll.faq", () => { const faqUnread = !currentUser.get("read_faq"); if (faqUnread && isElementInViewport($(".contents p").last())) { - this.sendAction(); + this.action(); } }); } diff --git a/app/assets/javascripts/discourse/helpers/route-action.js.es6 b/app/assets/javascripts/discourse/helpers/route-action.js.es6 index 946dd69ac8..4e010498bd 100644 --- a/app/assets/javascripts/discourse/helpers/route-action.js.es6 +++ b/app/assets/javascripts/discourse/helpers/route-action.js.es6 @@ -9,15 +9,14 @@ const { runInDebug } = Ember; -function getCurrentHandlerInfos(router) { +function getCurrentRouteInfos(router) { let routerLib = router._routerMicrolib || router.router; - - return routerLib.currentHandlerInfos; + return routerLib.currentRouteInfos; } function getRoutes(router) { - return emberArray(getCurrentHandlerInfos(router)) - .mapBy("handler") + return emberArray(getCurrentRouteInfos(router)) + .mapBy("_route") .reverse(); } diff --git a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 index b10cbe9b7d..84a9d51de7 100644 --- a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 +++ b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 @@ -12,10 +12,12 @@ export default { initialize(container) { // Tell our AJAX system to track a page transition const router = container.lookup("router:main"); - router.on("willTransition", viewTrackingRequired); - router.on("didTransition", cleanDOM); + + router.on("routeWillChange", viewTrackingRequired); + router.on("routeDidChange", cleanDOM); let appEvents = container.lookup("app-events:main"); + startPageTracking(router, appEvents); // Out of the box, Discourse tries to track google analytics diff --git a/app/assets/javascripts/discourse/initializers/show-footer.js.es6 b/app/assets/javascripts/discourse/initializers/show-footer.js.es6 index ce58457774..77040e907f 100644 --- a/app/assets/javascripts/discourse/initializers/show-footer.js.es6 +++ b/app/assets/javascripts/discourse/initializers/show-footer.js.es6 @@ -7,7 +7,7 @@ export default { // only take care of hiding the footer here // controllers MUST take care of displaying it - router.on("willTransition", () => { + router.on("routeWillChange", () => { application.set("showFooter", false); return true; }); diff --git a/app/assets/javascripts/discourse/lib/app-events.js.es6 b/app/assets/javascripts/discourse/lib/app-events.js.es6 index 59257700fd..aff71a2a40 100644 --- a/app/assets/javascripts/discourse/lib/app-events.js.es6 +++ b/app/assets/javascripts/discourse/lib/app-events.js.es6 @@ -1 +1,48 @@ -export default Ember.Object.extend(Ember.Evented); +import deprecated from "discourse-common/lib/deprecated"; + +export default Ember.Object.extend(Ember.Evented, { + _events: {}, + + on() { + if (arguments.length === 2) { + let [name, fn] = arguments; + let target = {}; + this._events[name] = this._events[name] || []; + this._events[name].push({ target, fn }); + + this._super(name, target, fn); + } else if (arguments.length === 3) { + let [name, target, fn] = arguments; + this._events[name] = this._events[name] || []; + this._events[name].push({ target, fn }); + + this._super(...arguments); + } + return this; + }, + + off() { + let name = arguments[0]; + let fn = arguments[2]; + + if (this._events[name]) { + if (arguments.length === 1) { + deprecated( + "Removing all event listeners at once is deprecated, please remove each listener individually." + ); + + this._events[name].forEach(ref => { + this._super(name, ref.target, ref.fn); + }); + delete this._events[name]; + } else if (arguments.length === 3) { + this._super(...arguments); + + this._events[name] = this._events[name].filter(e => e.fn !== fn); + if (this._events[name].length === 0) delete this._events[name]; + } + } + + return this; + } +}); diff --git a/app/assets/javascripts/discourse/lib/page-tracker.js.es6 b/app/assets/javascripts/discourse/lib/page-tracker.js.es6 index 0765ad50b1..6a36cc7cbc 100644 --- a/app/assets/javascripts/discourse/lib/page-tracker.js.es6 +++ b/app/assets/javascripts/discourse/lib/page-tracker.js.es6 @@ -15,10 +15,9 @@ export function startPageTracking(router, appEvents) { if (_started) { return; } - - router.on("didTransition", function() { - this.send("refreshTitle"); - const url = Discourse.getURL(this.get("url")); + router.on("routeDidChange", () => { + router.send("refreshTitle"); + const url = Discourse.getURL(router.get("url")); // Refreshing the title is debounced, so we need to trigger this in the // next runloop to have the correct title. @@ -39,6 +38,7 @@ export function startPageTracking(router, appEvents) { } }); }); + _started = true; } diff --git a/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 b/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 index ce110da855..756a24930f 100644 --- a/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 +++ b/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 @@ -122,10 +122,7 @@ export default Ember.Mixin.create({ return this._show($target.text().replace(/^@/, ""), $target); }); - this.appEvents.on(previewClickEvent, $target => { - this.set("isFixed", true); - return this._show($target.text().replace(/^@/, ""), $target); - }); + this.appEvents.on(previewClickEvent, this, "_previewClick"); this.appEvents.on(`topic-header:trigger-${id}`, (username, $target) => { this.setProperties({ isFixed: true, isDocked: true }); @@ -133,6 +130,11 @@ export default Ember.Mixin.create({ }); }, + _previewClick($target) { + this.set("isFixed", true); + return this._show($target.text().replace(/^@/, ""), $target); + }, + _positionCard(target) { const rtl = $("html").css("direction") === "rtl"; if (!target) { @@ -239,7 +241,7 @@ export default Ember.Mixin.create({ $("#main") .off(clickDataExpand) .off(clickMention); - this.appEvents.off(previewClickEvent); + this.appEvents.off(previewClickEvent, this, "_previewClick"); }, keyUp(e) { diff --git a/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 b/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 index 4b30762941..bf39cd084a 100644 --- a/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 +++ b/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 @@ -7,12 +7,12 @@ export default { didInsertElement() { this._super(...arguments); - this.appEvents.on("url:refresh", this.refresh); + this.appEvents.on("url:refresh", this, "refresh"); }, willDestroyElement() { this._super(...arguments); - this.appEvents.off("url:refresh"); + this.appEvents.off("url:refresh", this, "refresh"); } }; diff --git a/app/assets/javascripts/discourse/routes/badges-show.js.es6 b/app/assets/javascripts/discourse/routes/badges-show.js.es6 index e2330c3ac8..7a5b7f7830 100644 --- a/app/assets/javascripts/discourse/routes/badges-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/badges-show.js.es6 @@ -30,7 +30,8 @@ export default Discourse.Route.extend({ }, afterModel(model, transition) { - const username = transition.queryParams && transition.queryParams.username; + const username = + transition.to.queryParams && transition.to.queryParams.username; const userBadgesGrant = UserBadge.findByBadgeId(model.get("id"), { username diff --git a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 index eac9ea2017..182571cb8e 100644 --- a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 @@ -93,7 +93,7 @@ export default (filterArg, params) => { const listFilter = `c/${Discourse.Category.slugFor( category )}/l/${this.filter(category)}`, - findOpts = filterQueryParams(transition.queryParams, params), + findOpts = filterQueryParams(transition.to.queryParams, params), extras = { cached: this.isPoppedState(transition) }; return findTopicList( diff --git a/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 b/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 index bb8a600ab5..62aad28945 100644 --- a/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 +++ b/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 @@ -7,7 +7,7 @@ export function buildGroupPage(type) { }, model(params, transition) { - let categoryId = Ember.get(transition, "queryParams.category_id"); + let categoryId = Ember.get(transition.to, "queryParams.category_id"); return this.modelFor("group").findPosts({ type, categoryId }); }, diff --git a/app/assets/javascripts/discourse/routes/new-message.js.es6 b/app/assets/javascripts/discourse/routes/new-message.js.es6 index 7069151882..f926ba3967 100644 --- a/app/assets/javascripts/discourse/routes/new-message.js.es6 +++ b/app/assets/javascripts/discourse/routes/new-message.js.es6 @@ -3,7 +3,8 @@ import Group from "discourse/models/group"; export default Discourse.Route.extend({ beforeModel(transition) { - const params = transition.queryParams; + const params = transition.to.queryParams; + const groupName = params.groupname || params.group_name; if (this.currentUser) { diff --git a/app/assets/javascripts/discourse/routes/new-topic.js.es6 b/app/assets/javascripts/discourse/routes/new-topic.js.es6 index 46f38d4ef3..947193be38 100644 --- a/app/assets/javascripts/discourse/routes/new-topic.js.es6 +++ b/app/assets/javascripts/discourse/routes/new-topic.js.es6 @@ -6,11 +6,11 @@ export default Discourse.Route.extend({ if (Discourse.User.current()) { let category, category_id; - if (transition.queryParams.category_id) { - category_id = transition.queryParams.category_id; + if (transition.to.queryParams.category_id) { + category_id = transition.to.queryParams.category_id; category = Category.findById(category_id); - } else if (transition.queryParams.category) { - const splitCategory = transition.queryParams.category.split("/"); + } else if (transition.to.queryParams.category) { + const splitCategory = transition.to.queryParams.category.split("/"); category = this._getCategory( splitCategory[0], splitCategory[1], @@ -46,10 +46,10 @@ export default Discourse.Route.extend({ Ember.run.next(function() { e.send( "createNewTopicViaParams", - transition.queryParams.title, - transition.queryParams.body, + transition.to.queryParams.title, + transition.to.queryParams.body, category_id, - transition.queryParams.tags + transition.to.queryParams.tags ); }); } @@ -60,10 +60,10 @@ export default Discourse.Route.extend({ Ember.run.next(function() { e.send( "createNewTopicViaParams", - transition.queryParams.title, - transition.queryParams.body, + transition.to.queryParams.title, + transition.to.queryParams.body, null, - transition.queryParams.tags + transition.to.queryParams.tags ); }); } diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index 978816b7df..db3c2eb4f3 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -70,8 +70,8 @@ export default Discourse.Route.extend({ controller.set("loading", true); const params = controller.getProperties("order", "ascending"); - params.order = transition.queryParams.order || params.order; - params.ascending = transition.queryParams.ascending || params.ascending; + params.order = transition.to.queryParams.order || params.order; + params.ascending = transition.to.queryParams.ascending || params.ascending; const categorySlug = this.get("categorySlug"); const parentCategorySlug = this.get("parentCategorySlug"); diff --git a/app/assets/javascripts/discourse/routes/topic.js.es6 b/app/assets/javascripts/discourse/routes/topic.js.es6 index 06518c1e16..4e4865e165 100644 --- a/app/assets/javascripts/discourse/routes/topic.js.es6 +++ b/app/assets/javascripts/discourse/routes/topic.js.es6 @@ -231,7 +231,7 @@ const TopicRoute = Discourse.Route.extend({ }); } - const queryParams = transition.queryParams; + const queryParams = transition.to.queryParams; let topic = this.modelFor("topic"); if (topic && topic.get("id") === parseInt(params.id, 10)) { diff --git a/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 b/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 index e10629077d..5da6e94dc0 100644 --- a/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 +++ b/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 @@ -14,7 +14,7 @@ export default Discourse.Route.extend(ViewingActionType, { filter: this.get("userActionType"), noContentHelpKey: this.get("noContentHelpKey") || "user_activity.no_default", - actingUsername: transition.queryParams.acting_username + actingUsername: transition.to.queryParams.acting_username }); }, diff --git a/app/assets/javascripts/select-kit/components/category-chooser.js.es6 b/app/assets/javascripts/select-kit/components/category-chooser.js.es6 index 34cd38b0f8..f0ae60f693 100644 --- a/app/assets/javascripts/select-kit/components/category-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/category-chooser.js.es6 @@ -1,5 +1,4 @@ import ComboBoxComponent from "select-kit/components/combo-box"; -import { on } from "ember-addons/ember-computed-decorators"; import computed from "ember-addons/ember-computed-decorators"; import PermissionType from "discourse/models/permission-type"; import Category from "discourse/models/category"; @@ -100,16 +99,6 @@ export default ComboBoxComponent.extend({ return content; }, - @on("didRender") - _bindComposerResizing() { - this.appEvents.on("composer:resized", this, this.applyDirection); - }, - - @on("willDestroyElement") - _unbindComposerResizing() { - this.appEvents.off("composer:resized"); - }, - didSelect(computedContentItem) { if (this.attrs.onChooseCategory) { this.attrs.onChooseCategory(computedContentItem.originalContent); diff --git a/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 b/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 index 01b1f6906d..41e5445847 100644 --- a/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 +++ b/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 @@ -28,6 +28,6 @@ export default DropdownSelectBoxComponent.extend({ }, mutateValue(value) { - this.sendAction(value); + this.get(value)(); } }); diff --git a/app/assets/javascripts/select-kit/components/select-kit.js.es6 b/app/assets/javascripts/select-kit/components/select-kit.js.es6 index ccc5f49f8d..cad55bf463 100644 --- a/app/assets/javascripts/select-kit/components/select-kit.js.es6 +++ b/app/assets/javascripts/select-kit/components/select-kit.js.es6 @@ -134,10 +134,10 @@ export default Ember.Component.extend( this.removeObserver( `content.@each.${this.get("nameProperty")}`, this, - this._compute + "_compute" ); - this.removeObserver(`content.[]`, this, this._compute); - this.removeObserver(`asyncContent.[]`, this, this._compute); + this.removeObserver(`content.[]`, this, "_compute"); + this.removeObserver(`asyncContent.[]`, this, "_compute"); }, willComputeAttributes() {}, diff --git a/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 b/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 index 1bcd7df3fb..b15b688faa 100644 --- a/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 +++ b/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 @@ -17,20 +17,20 @@ export default NotificationOptionsComponent.extend({ return archetype === "private_message" ? "_pm" : ""; }, + _changed(msg) { + if (this.get("computedValue") !== msg.id) { + this.get("topic.details").updateNotifications(msg.id); + } + }, + @on("didInsertElement") _bindGlobalLevelChanged() { - this.appEvents.on("topic-notifications-button:changed", msg => { - if (msg.type === "notification") { - if (this.get("computedValue") !== msg.id) { - this.get("topic.details").updateNotifications(msg.id); - } - } - }); + this.appEvents.on("topic-notifications-button:changed", this, "_changed"); }, @on("willDestroyElement") _unbindGlobalLevelChanged() { - this.appEvents.off("topic-notifications-button:changed"); + this.appEvents.off("topic-notifications-button:changed", this, "_changed"); }, mutateValue(value) { From d32557ea326735c33a420dee7cc3de856c683dc7 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 13 Mar 2019 13:02:56 +0100 Subject: [PATCH 34/47] Revert "FIX: Better emoji escaping for topic title" This reverts commit 35426b5ad6b00c9047b9b7ba17d42d4f30bdb488. --- app/assets/javascripts/pretty-text-bundle.js | 1 - .../javascripts/pretty-text/emoji.js.es6 | 175 ------------------ .../javascripts/pretty-text/emoji.js.es6.erb | 127 +++++++++++++ .../pretty-text/emoji/data.js.es6.erb | 1 - .../pretty-text/emoji/version.js.es6.erb | 2 - app/models/emoji.rb | 8 +- lib/pretty_text.rb | 12 +- lib/pretty_text/shims.js | 1 - spec/models/topic_spec.rb | 5 - .../acceptance/emoji-picker-test.js.es6 | 2 +- test/javascripts/acceptance/emoji-test.js.es6 | 2 +- test/javascripts/acceptance/topic-test.js.es6 | 19 +- test/javascripts/lib/emoji-test.js.es6 | 3 +- test/javascripts/lib/pretty-text-test.js.es6 | 2 +- test/javascripts/models/topic-test.js.es6 | 2 +- 15 files changed, 141 insertions(+), 221 deletions(-) delete mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6 create mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6.erb delete mode 100644 app/assets/javascripts/pretty-text/emoji/version.js.es6.erb diff --git a/app/assets/javascripts/pretty-text-bundle.js b/app/assets/javascripts/pretty-text-bundle.js index ed5331b643..41324ba31e 100644 --- a/app/assets/javascripts/pretty-text-bundle.js +++ b/app/assets/javascripts/pretty-text-bundle.js @@ -2,7 +2,6 @@ //= require ./pretty-text/guid //= require ./pretty-text/censored-words //= require ./pretty-text/emoji/data -//= require ./pretty-text/emoji/version //= require ./pretty-text/emoji //= require ./pretty-text/engines/discourse-markdown-it //= require xss.min diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 deleted file mode 100644 index d2c8503d69..0000000000 --- a/app/assets/javascripts/pretty-text/emoji.js.es6 +++ /dev/null @@ -1,175 +0,0 @@ -import { - emojis, - aliases, - searchAliases, - translations, - tonableEmojis, - replacements -} from "pretty-text/emoji/data"; -import { IMAGE_VERSION } from "pretty-text/emoji/version"; - -const extendedEmoji = {}; - -export function registerEmoji(code, url) { - code = code.toLowerCase(); - extendedEmoji[code] = url; -} - -export function extendedEmojiList() { - return extendedEmoji; -} - -const emojiHash = {}; - -// add all default emojis -emojis.forEach(code => (emojiHash[code] = true)); - -// and their aliases -const aliasHash = {}; -Object.keys(aliases).forEach(name => { - aliases[name].forEach(alias => (aliasHash[alias] = name)); -}); - -export function performEmojiUnescape(string, opts) { - if (!string) { - return; - } - - return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { - const isEmoticon = !!translations[m]; - const isUnicodeEmoticon = !!replacements[m]; - const emojiVal = isEmoticon - ? translations[m] - : isUnicodeEmoticon - ? replacements[m] - : m.slice(1, m.length - 1); - const hasEndingColon = m.lastIndexOf(":") === m.length - 1; - const url = buildEmojiUrl(emojiVal, opts); - const classes = isCustomEmoji(emojiVal, opts) - ? "emoji emoji-custom" - : "emoji"; - - return url && (isEmoticon || hasEndingColon || isUnicodeEmoticon) - ? `${emojiVal}` - : m; - }); - - return string; -} - -export function performEmojiEscape(string) { - if (!string) { - return; - } - - return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { - if (!!translations[m]) { - return ":" + translations[m] + ":"; - } else if (!!replacements[m]) { - return ":" + replacements[m] + ":"; - } else { - return m; - } - }); - - return string; -} - -export function isCustomEmoji(code, opts) { - code = code.toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) return true; - if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) - return true; - return false; -} - -export function buildEmojiUrl(code, opts) { - let url; - code = String(code).toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) { - url = extendedEmoji[code]; - } - - if (opts && opts.customEmoji && opts.customEmoji[code]) { - url = opts.customEmoji[code]; - } - - const noToneMatch = code.match(/([^:]+):?/); - if ( - noToneMatch && - !url && - (emojiHash.hasOwnProperty(noToneMatch[1]) || - aliasHash.hasOwnProperty(noToneMatch[1])) - ) { - url = opts.getURL( - `/images/emoji/${opts.emojiSet}/${code.replace(/:t/, "/")}.png` - ); - } - - if (url) { - url = url + "?v=" + IMAGE_VERSION; - } - - return url; -} - -export function emojiExists(code) { - code = code.toLowerCase(); - return !!( - extendedEmoji.hasOwnProperty(code) || - emojiHash.hasOwnProperty(code) || - aliasHash.hasOwnProperty(code) - ); -} - -let toSearch; -export function emojiSearch(term, options) { - const maxResults = (options && options["maxResults"]) || -1; - if (maxResults === 0) { - return []; - } - - toSearch = - toSearch || - _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); - - const results = []; - - function addResult(t) { - const val = aliasHash[t] || t; - if (results.indexOf(val) === -1) { - results.push(val); - } - } - - // if term matches from beginning - for (let i = 0; i < toSearch.length; i++) { - const item = toSearch[i]; - if (item.indexOf(term) === 0) addResult(item); - } - - if (searchAliases[term]) { - results.push.apply(results, searchAliases[term]); - } - - for (let i = 0; i < toSearch.length; i++) { - const item = toSearch[i]; - if (item.indexOf(term) > 0) addResult(item); - } - - if (maxResults === -1) { - return results; - } else { - return results.slice(0, maxResults); - } -} - -export function isSkinTonableEmoji(term) { - const match = _.compact(term.split(":"))[0]; - if (match) { - return tonableEmojis.indexOf(match) !== -1; - } - return false; -} diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6.erb b/app/assets/javascripts/pretty-text/emoji.js.es6.erb new file mode 100644 index 0000000000..09ab4ce5ed --- /dev/null +++ b/app/assets/javascripts/pretty-text/emoji.js.es6.erb @@ -0,0 +1,127 @@ +import { emojis, aliases, searchAliases, translations, tonableEmojis } from 'pretty-text/emoji/data'; + +// bump up this number to expire all emojis +export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; + +const extendedEmoji = {}; + +export function registerEmoji(code, url) { + code = code.toLowerCase(); + extendedEmoji[code] = url; +} + +export function extendedEmojiList() { + return extendedEmoji; +} + +const emojiHash = {}; + +// add all default emojis +emojis.forEach(code => emojiHash[code] = true); + +// and their aliases +const aliasHash = {}; +Object.keys(aliases).forEach(name => { + aliases[name].forEach(alias => aliasHash[alias] = name); +}); + +export function performEmojiUnescape(string, opts) { + if (!string) { return; } + + // this can be further improved by supporting matches of emoticons that don't begin with a colon + if (string.indexOf(":") >= 0) { + return string.replace(/\B:[^\s:]+(?::t\d)?:?\B/g, m => { + const isEmoticon = !!translations[m]; + const emojiVal = isEmoticon ? translations[m] : m.slice(1, m.length - 1); + const hasEndingColon = m.lastIndexOf(":") === m.length - 1; + const url = buildEmojiUrl(emojiVal, opts); + const classes = isCustomEmoji(emojiVal, opts) ? "emoji emoji-custom" : "emoji"; + + return url && (isEmoticon || hasEndingColon) ? + `${emojiVal}` : m; + }); + } + + return string; +} + +export function isCustomEmoji(code, opts) { + code = code.toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) return true; + if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) return true; + return false; +} + +export function buildEmojiUrl(code, opts) { + let url; + code = String(code).toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) { + url = extendedEmoji[code]; + } + + if (opts && opts.customEmoji && opts.customEmoji[code]) { + url = opts.customEmoji[code]; + } + + const noToneMatch = code.match(/([^:]+):?/); + if (noToneMatch && !url && (emojiHash.hasOwnProperty(noToneMatch[1]) || aliasHash.hasOwnProperty(noToneMatch[1]))) { + url = opts.getURL(`/images/emoji/${opts.emojiSet}/${code.replace(/:t/, '/')}.png`); + } + + if (url) { + url = url + "?v=" + IMAGE_VERSION; + } + + return url; +} + +export function emojiExists(code) { + code = code.toLowerCase(); + return !!(extendedEmoji.hasOwnProperty(code) || emojiHash.hasOwnProperty(code) || aliasHash.hasOwnProperty(code)); +}; + +let toSearch; +export function emojiSearch(term, options) { + const maxResults = (options && options["maxResults"]) || -1; + if (maxResults === 0) { return []; } + + toSearch = toSearch || _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); + + const results = []; + + function addResult(t) { + const val = aliasHash[t] || t; + if (results.indexOf(val) === -1) { + results.push(val); + } + } + + // if term matches from beginning + for (let i=0; i 0) addResult(item); + } + + if (maxResults === -1) { + return results; + } else { + return results.slice(0, maxResults); + } +}; + +export function isSkinTonableEmoji(term) { + const match = _.compact(term.split(":"))[0]; + if (match) { + return tonableEmojis.indexOf(match) !== -1; + } + return false; +} diff --git a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb index 5825170790..32a9b986bd 100644 --- a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb +++ b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb @@ -3,4 +3,3 @@ export const tonableEmojis = <%= Emoji.tonable_emojis.flatten.inspect %>; export const aliases = <%= Emoji.aliases.inspect.gsub("=>", ":") %>; export const searchAliases = <%= Emoji.search_aliases.inspect.gsub("=>", ":") %>; export const translations = <%= Emoji.translations.inspect.gsub("=>", ":") %>; -export const replacements = <%= Emoji.unicode_replacements_json %>; diff --git a/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb deleted file mode 100644 index b846ca59d5..0000000000 --- a/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb +++ /dev/null @@ -1,2 +0,0 @@ -// bump up this number to expire all emojis -export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; diff --git a/app/models/emoji.rb b/app/models/emoji.rb index ebe58ea311..c20d26ef06 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -157,7 +157,13 @@ class Emoji end def self.unicode_unescape(string) - PrettyText.escape_emoji(string) + string.each_char.map do |c| + if str = unicode_replacements[c] + ":#{str}:" + else + c + end + end.join end def self.gsub_emoji_to_unicode(str) diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 1943417ef2..d4d8ce56f8 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -227,7 +227,7 @@ module PrettyText end def self.unescape_emoji(title) - return title unless SiteSetting.enable_emoji? && title + return title unless SiteSetting.enable_emoji? set = SiteSetting.emoji_set.inspect custom = Emoji.custom.map { |e| [e.name, e.url] }.to_h.to_json @@ -239,16 +239,6 @@ module PrettyText end end - def self.escape_emoji(title) - return unless title - - protect do - v8.eval(<<~JS) - __performEmojiEscape(#{title.inspect}); - JS - end - end - def self.cook(text, opts = {}) options = opts.dup diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 929dafd13a..649c3e4b39 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -1,7 +1,6 @@ __PrettyText = require("pretty-text/pretty-text").default; __buildOptions = require("pretty-text/pretty-text").buildOptions; __performEmojiUnescape = require("pretty-text/emoji").performEmojiUnescape; -__performEmojiEscape = require("pretty-text/emoji").performEmojiEscape; __utils = require("discourse/lib/utilities"); diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 634f403993..d0ecd98a43 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -279,7 +279,6 @@ describe Topic do let(:topic_image) { build_topic_with_title("Topic with image in its title") } let(:topic_script) { build_topic_with_title("Topic with script in its title") } let(:topic_emoji) { build_topic_with_title("I 💖 candy alot") } - let(:topic_modifier_emoji) { build_topic_with_title("I 👨‍🌾 candy alot") } it "escapes script contents" do expect(topic_script.fancy_title).to eq("Topic with <script>alert(‘title’)</script> script in its title") @@ -289,10 +288,6 @@ describe Topic do expect(topic_emoji.fancy_title).to eq("I :sparkling_heart: candy alot") end - it "keeps combined emojis" do - expect(topic_modifier_emoji.fancy_title).to eq("I :man_farmer: candy alot") - end - it "escapes bold contents" do expect(topic_bold.fancy_title).to eq("Topic with <b>bold</b> text in its title") end diff --git a/test/javascripts/acceptance/emoji-picker-test.js.es6 b/test/javascripts/acceptance/emoji-picker-test.js.es6 index 9122cd8ea5..3c2ebe7be4 100644 --- a/test/javascripts/acceptance/emoji-picker-test.js.es6 +++ b/test/javascripts/acceptance/emoji-picker-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { resetCache } from "discourse/components/emoji-picker"; acceptance("EmojiPicker", { diff --git a/test/javascripts/acceptance/emoji-test.js.es6 b/test/javascripts/acceptance/emoji-test.js.es6 index 39dc37696e..48ac7f9bcc 100644 --- a/test/javascripts/acceptance/emoji-test.js.es6 +++ b/test/javascripts/acceptance/emoji-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { acceptance } from "helpers/qunit-helpers"; acceptance("Emoji", { loggedIn: true }); diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index 1c371c41d1..c38e574345 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; acceptance("Topic", { loggedIn: true, @@ -121,23 +121,6 @@ QUnit.test("Updating the topic title with emojis", async assert => { ); }); -QUnit.test("Updating the topic title with unicode emojis", async assert => { - await visit("/t/internationalization-localization/280"); - await click("#topic-title .d-icon-pencil-alt"); - - await fillIn("#edit-title", "emojis title 👨‍🌾"); - - await click("#topic-title .submit-edit"); - - assert.equal( - find(".fancy-title") - .html() - .trim(), - `emojis title man_farmer`, - "it displays the new title with escaped unicode emojis" - ); -}); - acceptance("Topic featured links", { loggedIn: true, settings: { diff --git a/test/javascripts/lib/emoji-test.js.es6 b/test/javascripts/lib/emoji-test.js.es6 index c3b8ccd563..47665a8c8a 100644 --- a/test/javascripts/lib/emoji-test.js.es6 +++ b/test/javascripts/lib/emoji-test.js.es6 @@ -1,5 +1,4 @@ -import { emojiSearch } from "pretty-text/emoji"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { emojiSearch, IMAGE_VERSION as v } from "pretty-text/emoji"; import { emojiUnescape } from "discourse/lib/text"; QUnit.module("lib:emoji"); diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6 index 9495122a87..162ef49691 100644 --- a/test/javascripts/lib/pretty-text-test.js.es6 +++ b/test/javascripts/lib/pretty-text-test.js.es6 @@ -1,7 +1,7 @@ import Quote from "discourse/lib/quote"; import Post from "discourse/models/post"; import { default as PrettyText, buildOptions } from "pretty-text/pretty-text"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { INLINE_ONEBOX_LOADING_CSS_CLASS } from "pretty-text/inline-oneboxer"; QUnit.module("lib:pretty-text"); diff --git a/test/javascripts/models/topic-test.js.es6 b/test/javascripts/models/topic-test.js.es6 index e385e1e92f..ca6bd1f7b5 100644 --- a/test/javascripts/models/topic-test.js.es6 +++ b/test/javascripts/models/topic-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; QUnit.module("model:topic"); From c773f82ba1c3b75324a04c3de5b1f604e43ac905 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 13 Mar 2019 18:38:27 +0530 Subject: [PATCH 35/47] DEV: Added qunit test functions and did minor refactoring https://github.com/discourse/discourse/commit/135191d1d194b1cd03eb0416b5c4f7bf1a319aaf --- .../helpers/topic-status-icons.js.es6 | 4 +- .../discourse/widgets/topic-status.js.es6 | 4 +- .../widgets/topic-status-test.js.es6 | 37 +++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 test/javascripts/widgets/topic-status-test.js.es6 diff --git a/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 b/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 index f13973d375..80076d725d 100644 --- a/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 +++ b/app/assets/javascripts/discourse/helpers/topic-status-icons.js.es6 @@ -14,9 +14,7 @@ export default Ember.ArrayProxy.extend({ renderIconIf("archived", "lock", "archived"); } - this.forEach(args => { - renderIconIf(...args); - }); + this.forEach(args => renderIconIf(...args)); } }).create({ content: [ diff --git a/app/assets/javascripts/discourse/widgets/topic-status.js.es6 b/app/assets/javascripts/discourse/widgets/topic-status.js.es6 index fd72606210..366fe462a2 100644 --- a/app/assets/javascripts/discourse/widgets/topic-status.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-status.js.es6 @@ -14,8 +14,8 @@ export default createWidget("topic-status", { const result = []; TopicStatusIcons.render(topic, function(name, key) { - const iconArgs = key === "unpinned" ? { class: "unpinned" } : null, - icon = iconNode(name, iconArgs); + const iconArgs = key === "unpinned" ? { class: "unpinned" } : null; + const icon = iconNode(name, iconArgs); const attributes = { title: escapeExpression(I18n.t(`topic_statuses.${key}.help`)) diff --git a/test/javascripts/widgets/topic-status-test.js.es6 b/test/javascripts/widgets/topic-status-test.js.es6 new file mode 100644 index 0000000000..e0ed454fc3 --- /dev/null +++ b/test/javascripts/widgets/topic-status-test.js.es6 @@ -0,0 +1,37 @@ +import { moduleForWidget, widgetTest } from "helpers/widget-test"; +import TopicStatusIcons from "discourse/helpers/topic-status-icons"; + +moduleForWidget("topic-status"); + +widgetTest("basics", { + template: '{{mount-widget widget="topic-status" args=args}}', + beforeEach(store) { + this.set("args", { + topic: store.createRecord("topic", { closed: true }), + disableActions: true + }); + }, + test(assert) { + assert.ok(find(".topic-status .d-icon-lock").length); + } +}); + +widgetTest("extendability", { + template: '{{mount-widget widget="topic-status" args=args}}', + beforeEach(store) { + TopicStatusIcons.addObject([ + "has_accepted_answer", + "check-square-o", + "solved" + ]); + this.set("args", { + topic: store.createRecord("topic", { + has_accepted_answer: true + }), + disableActions: true + }); + }, + test(assert) { + assert.ok(find(".topic-status .d-icon-check-square-o").length); + } +}); From 91bd0becaa117eeb344fd75d2e95e832317c1ed9 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 13 Mar 2019 17:44:08 +0200 Subject: [PATCH 36/47] DEV: Allow topics to register default topic order criteria. (#7158) --- .../components/edit-category-settings.js.es6 | 10 +++++++--- .../javascripts/discourse/lib/plugin-api.js.es6 | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 index 455418f13c..712aa67be1 100644 --- a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 +++ b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 @@ -2,6 +2,11 @@ import { setting } from "discourse/lib/computed"; import { buildCategoryPanel } from "discourse/components/edit-category-panel"; import computed from "ember-addons/ember-computed-decorators"; +const categorySortCriteria = []; +export function addCategorySortCriteria(criteria) { + categorySortCriteria.push(criteria); +} + export default buildCategoryPanel("settings", { emailInEnabled: setting("email_in"), showPositionInput: setting("fixed_category_positions"), @@ -64,10 +69,9 @@ export default buildCategoryPanel("settings", { "category", "created" ] + .concat(categorySortCriteria) .map(s => ({ name: I18n.t("category.sort_options." + s), value: s })) - .sort((a, b) => { - return a.name > b.name; - }); + .sort((a, b) => a.name.localeCompare(b.name)); }, @computed diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index 910b55fbe4..62703f7c66 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -41,9 +41,10 @@ import { disableNameSuppression } from "discourse/widgets/poster-name"; import { registerCustomPostMessageCallback as registerCustomPostMessageCallback1 } from "discourse/controllers/topic"; import Sharing from "discourse/lib/sharing"; import { addComposerUploadHandler } from "discourse/components/composer-editor"; +import { addCategorySortCriteria } from "discourse/components/edit-category-settings"; // If you add any methods to the API ensure you bump up this number -const PLUGIN_API_VERSION = "0.8.29"; +const PLUGIN_API_VERSION = "0.8.30"; class PluginApi { constructor(version, container) { @@ -801,7 +802,6 @@ class PluginApi { } /** - * * Registers a function to handle uploads for specified file types * The normal uploading functionality will be bypassed if function returns * a falsy value. @@ -817,6 +817,18 @@ class PluginApi { addComposerUploadHandler(extensions, method); } + /** + * Registers a criteria that can be used as default topic order on category + * pages. + * + * Example: + * + * categorySortCriteria("votes"); + */ + addCategorySortCriteria(criteria) { + addCategorySortCriteria(criteria); + } + /** * Registers a renderer that overrides the display of category links. * From 9a5f08ee6188d126fbbe76e0a771de2fa1f7b4bd Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 13 Mar 2019 16:50:56 +0100 Subject: [PATCH 37/47] DEV: skip discourse-chat-integration tests (#7164) Suspected of causing timeout issues in our test suite with Ember 3.7 --- 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..4e5ab8b382 100644 --- a/lib/tasks/plugin.rake +++ b/lib/tasks/plugin.rake @@ -7,7 +7,8 @@ task 'plugin:install_all_official' do 'discourse-nginx-performance-report', 'lazyYT', 'poll', - 'discourse-calendar' + 'discourse-calendar', + 'discourse-chat-integration' ]) map = { From 420c6f81028302b0a538f234988c8a630e7cdc82 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 13 Mar 2019 16:17:59 +0000 Subject: [PATCH 38/47] FEATURE: Skip sending emails to domains on the `.invalid` TLD (#7162) This is a reserved TLD which we use when importing users without an email address. https://tools.ietf.org/html/rfc2606 --- app/models/skipped_email_log.rb | 3 ++- config/locales/server.en.yml | 1 + lib/email/sender.rb | 2 ++ spec/components/email/sender_spec.rb | 7 +++++++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/models/skipped_email_log.rb b/app/models/skipped_email_log.rb index 3e73b58b7d..0fc28fa141 100644 --- a/app/models/skipped_email_log.rb +++ b/app/models/skipped_email_log.rb @@ -32,7 +32,8 @@ class SkippedEmailLog < ActiveRecord::Base sender_message_to_blank: 17, sender_text_part_body_blank: 18, sender_body_blank: 19, - sender_post_deleted: 20 + sender_post_deleted: 20, + sender_message_to_invalid: 21 # you need to add the reason in server.en.yml below the "skipped_email_log" key # when you add a new enum value ) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c1cba642b2..7c40397134 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -3465,6 +3465,7 @@ en: sender_text_part_body_blank: "text_part.body is blank" sender_body_blank: "body is blank" sender_post_deleted: "post has been deleted" + sender_message_to_invalid: "recipient has invalid email address" color_schemes: base_theme_name: "Base" diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 40217f3d05..b8c62eb6ed 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -35,6 +35,8 @@ module Email return skip(SkippedEmailLog.reason_types[:sender_message_blank]) if @message.blank? return skip(SkippedEmailLog.reason_types[:sender_message_to_blank]) if @message.to.blank? + return skip(SkippedEmailLog.reason_types[:sender_message_to_invalid]) if to_address.end_with?(".invalid") + if @message.text_part if @message.text_part.body.to_s.blank? return skip(SkippedEmailLog.reason_types[:sender_text_part_body_blank]) diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb index 472edab7d2..5cd476aa0d 100644 --- a/spec/components/email/sender_spec.rb +++ b/spec/components/email/sender_spec.rb @@ -58,6 +58,13 @@ describe Email::Sender do Email::Sender.new(message, :hello).send end + it "doesn't deliver when the to address uses the .invalid tld" do + message = Mail::Message.new(body: 'hello', to: 'myemail@example.invalid') + message.expects(:deliver_now).never + expect { Email::Sender.new(message, :hello).send }. + to change { SkippedEmailLog.where(reason_type: SkippedEmailLog.reason_types[:sender_message_to_invalid]).count }.by(1) + end + it "doesn't deliver when the body is nil" do message = Mail::Message.new(to: 'eviltrout@test.domain') message.expects(:deliver_now).never From 12c37ada2e37bc51b5f503479d7da5377d5319ab Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Wed, 13 Mar 2019 17:48:40 +0100 Subject: [PATCH 39/47] UX: focus on search box when emoji picker is opened (#7098) --- .../discourse/components/d-editor.js.es6 | 34 +++++++++---------- .../discourse/components/emoji-picker.js.es6 | 7 ++++ .../templates/components/d-editor.hbs | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index b066805374..2b82d2ea17 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -378,24 +378,21 @@ export default Ember.Component.extend({ _applyCategoryHashtagAutocomplete() { const siteSettings = this.siteSettings; - const self = this; this.$(".d-editor-input").autocomplete({ template: findRawTemplate("category-tag-autocomplete"), key: "#", - afterComplete() { - self._focusTextArea(); - }, - transformComplete(obj) { + afterComplete: () => this._focusTextArea(), + transformComplete: obj => { return obj.text; }, - dataSource(term) { + dataSource: term => { if (term.match(/\s/)) { return null; } return searchCategoryTag(term, siteSettings); }, - triggerRule(textarea, opts) { + triggerRule: (textarea, opts) => { return categoryHashtagTriggerRule(textarea, opts); } }); @@ -406,17 +403,15 @@ export default Ember.Component.extend({ return; } - const self = this; - $editorInput.autocomplete({ template: findRawTemplate("emoji-selector-autocomplete"), key: ":", - afterComplete(text) { - self.set("value", text); - self._focusTextArea(); + afterComplete: text => { + this.set("value", text); + this._focusTextArea(); }, - onKeyUp(text, cp) { + onKeyUp: (text, cp) => { const matches = /(?:^|[^a-z])(:(?!:).?[\w-]*:?(?!:)(?:t\d?)?:?) ?$/gi.exec( text.substring(0, cp) ); @@ -426,22 +421,26 @@ export default Ember.Component.extend({ } }, - transformComplete(v) { + transformComplete: v => { if (v.code) { return `${v.code}:`; } else { $editorInput.autocomplete({ cancel: true }); - self.set("emojiPickerIsActive", true); + this.set( + "isEditorFocused", + $("textarea.d-editor-input").is(":focus") + ); + this.set("emojiPickerIsActive", true); return ""; } }, - dataSource(term) { + dataSource: term => { return new Ember.RSVP.Promise(resolve => { const full = `:${term}`; term = term.toLowerCase(); - if (term.length < self.siteSettings.emoji_autocomplete_min_chars) { + if (term.length < this.siteSettings.emoji_autocomplete_min_chars) { return resolve([]); } @@ -858,6 +857,7 @@ export default Ember.Component.extend({ return; } + this.set("isEditorFocused", $("textarea.d-editor-input").is(":focus")); this.set("emojiPickerIsActive", !this.get("emojiPickerIsActive")); }, diff --git a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 index ee8e6fc6dd..e8bdc4fb43 100644 --- a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 +++ b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 @@ -7,6 +7,7 @@ import { isSkinTonableEmoji, emojiSearch } from "pretty-text/emoji"; +import { safariHacksDisabled } from "discourse/lib/utilities"; const { run } = Ember; const keyValueStore = new KeyValueStore("discourse_emojis_"); @@ -58,6 +59,12 @@ export default Ember.Component.extend({ this._scrollTo(); this._updateSelectedDiversity(); this._checkVisibleSection(true); + + if ( + (!this.site.isMobileDevice || this.get("isEditorFocused")) && + !safariHacksDisabled() + ) + this.$filter.find("input[name='filter']").focus(); }); }, diff --git a/app/assets/javascripts/discourse/templates/components/d-editor.hbs b/app/assets/javascripts/discourse/templates/components/d-editor.hbs index b0aec1ede6..11b783648a 100644 --- a/app/assets/javascripts/discourse/templates/components/d-editor.hbs +++ b/app/assets/javascripts/discourse/templates/components/d-editor.hbs @@ -50,4 +50,4 @@ -{{emoji-picker active=emojiPickerIsActive emojiSelected=(action 'emojiSelected')}} +{{emoji-picker active=emojiPickerIsActive isEditorFocused=isEditorFocused emojiSelected=(action 'emojiSelected')}} From 65f3ed0689e3923c6114b8f43b80dbd146cfc9e2 Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Wed, 13 Mar 2019 18:40:43 +0100 Subject: [PATCH 40/47] UX: make name optional for confirmation user field (#7149) --- .../templates/components/user-fields/confirm.hbs | 15 ++++++++++++--- app/models/user_field.rb | 3 ++- spec/models/user_field_spec.rb | 13 +++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 spec/models/user_field_spec.rb diff --git a/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs b/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs index 18896a4dc3..f3bcf803c9 100644 --- a/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs +++ b/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs @@ -1,5 +1,14 @@ - +{{#if field.name}} + +{{/if}} +
- +
diff --git a/app/models/user_field.rb b/app/models/user_field.rb index 27789fb07d..65dcc6236a 100644 --- a/app/models/user_field.rb +++ b/app/models/user_field.rb @@ -2,7 +2,8 @@ class UserField < ActiveRecord::Base include AnonCacheInvalidator - validates_presence_of :name, :description, :field_type + validates_presence_of :description, :field_type + validates_presence_of :name, unless: -> { field_type == "confirm" } has_many :user_field_options, dependent: :destroy accepts_nested_attributes_for :user_field_options diff --git a/spec/models/user_field_spec.rb b/spec/models/user_field_spec.rb new file mode 100644 index 0000000000..f75dafe8f8 --- /dev/null +++ b/spec/models/user_field_spec.rb @@ -0,0 +1,13 @@ +require 'rails_helper' + +describe UserField do + describe "doesn't validate presence of name if field type is 'confirm'" do + subject { described_class.new(field_type: 'confirm') } + it { is_expected.not_to validate_presence_of :name } + end + + describe "validates presence of name for other field types" do + subject { described_class.new(field_type: 'dropdown') } + it { is_expected.to validate_presence_of :name } + end +end From 4178578e09d36cc6ef4324672a24e1c872f11e56 Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 13 Mar 2019 13:42:11 -0400 Subject: [PATCH 41/47] UX: Adhere to Facebook & Twitter brand guidelines for share icon colors --- app/assets/stylesheets/common/base/share_link.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/stylesheets/common/base/share_link.scss b/app/assets/stylesheets/common/base/share_link.scss index 3abd01db9e..5b2311c11c 100644 --- a/app/assets/stylesheets/common/base/share_link.scss +++ b/app/assets/stylesheets/common/base/share_link.scss @@ -54,6 +54,14 @@ .social-link { margin-right: s(2); font-size: $font-up-4; + .d-icon-fab-facebook-square { + // Adheres to Facebook brand guidelines + color: dark-light-choose($facebook, white); + } + .d-icon-fab-twitter-square { + // Adheres to Twitter brand guidelines + color: dark-light-choose($twitter, white); + } } .link { font-size: $font-up-3; From 77931b70c316c9d53f33dbbd0ff0be295397a050 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Wed, 13 Mar 2019 15:49:47 -0300 Subject: [PATCH 42/47] Revert "DEV: Upgrade to Ember 3.7.0 (#6977)" (#7165) This reverts commit 3eebf8be733065996ff172adbcad85c254048274. --- Gemfile | 2 +- Gemfile.lock | 4 +- .../admin/components/ace-editor.js.es6 | 2 +- .../admin/routes/admin-users-list-show.js.es6 | 7 +- .../discourse/components/d-editor.js.es6 | 26 ++++--- .../discourse/components/d-modal-body.js.es6 | 8 +-- .../components/discourse-topic.js.es6 | 42 ++++++------ .../discourse/components/emoji-picker.js.es6 | 8 +-- .../discourse/components/login-modal.js.es6 | 2 +- .../discourse/components/nav-item.js.es6 | 4 +- .../components/scrolling-post-stream.js.es6 | 68 ++++++++----------- .../discourse/components/site-header.js.es6 | 24 +++---- .../components/topic-entrance.js.es6 | 4 +- .../discourse/components/watch-read.js.es6 | 2 +- .../discourse/helpers/route-action.js.es6 | 9 +-- .../initializers/page-tracking.js.es6 | 6 +- .../discourse/initializers/show-footer.js.es6 | 2 +- .../discourse/lib/app-events.js.es6 | 49 +------------ .../discourse/lib/page-tracker.js.es6 | 8 +-- .../mixins/card-contents-base.js.es6 | 12 ++-- .../discourse/mixins/url-refresh.js.es6 | 4 +- .../discourse/routes/badges-show.js.es6 | 3 +- .../routes/build-category-route.js.es6 | 2 +- .../routes/group-activity-posts.js.es6 | 2 +- .../discourse/routes/new-message.js.es6 | 3 +- .../discourse/routes/new-topic.js.es6 | 20 +++--- .../discourse/routes/tags-show.js.es6 | 4 +- .../javascripts/discourse/routes/topic.js.es6 | 2 +- .../routes/user-activity-stream.js.es6 | 2 +- .../components/category-chooser.js.es6 | 11 +++ .../components/group-members-dropdown.js.es6 | 2 +- .../select-kit/components/select-kit.js.es6 | 6 +- .../topic-notifications-options.js.es6 | 16 ++--- 33 files changed, 152 insertions(+), 214 deletions(-) diff --git a/Gemfile b/Gemfile index 8b2b8978b3..45af6c03c3 100644 --- a/Gemfile +++ b/Gemfile @@ -49,7 +49,7 @@ gem 'onebox', '1.8.82' gem 'http_accept_language', '~>2.0.5', require: false gem 'ember-rails', '0.18.5' -gem 'discourse-ember-source', '~> 3.7.0' +gem 'discourse-ember-source', '~> 3.5.1' gem 'ember-handlebars-template', '0.8.0' gem 'barber' diff --git a/Gemfile.lock b/Gemfile.lock index 33227007f3..299c7f91ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -108,7 +108,7 @@ GEM terminal-table (~> 1) debug_inspector (0.0.3) diff-lcs (1.3) - discourse-ember-source (3.7.0.2) + discourse-ember-source (3.5.1.3) discourse_image_optim (0.26.2) exifr (~> 1.2, >= 1.2.2) fspath (~> 3.0) @@ -466,7 +466,7 @@ DEPENDENCIES colored2 cppjieba_rb danger - discourse-ember-source (~> 3.7.0) + discourse-ember-source (~> 3.5.1) discourse_image_optim email_reply_trimmer (~> 0.1) ember-handlebars-template (= 0.8.0) diff --git a/app/assets/javascripts/admin/components/ace-editor.js.es6 b/app/assets/javascripts/admin/components/ace-editor.js.es6 index b3576c8bea..b86c0e4cd7 100644 --- a/app/assets/javascripts/admin/components/ace-editor.js.es6 +++ b/app/assets/javascripts/admin/components/ace-editor.js.es6 @@ -100,7 +100,7 @@ export default Ember.Component.extend({ if (this.appEvents) { // xxx: don't run during qunit tests - this.appEvents.on("ace:resize", this, "resize"); + this.appEvents.on("ace:resize", () => this.resize()); } if (this.get("autofocus")) { diff --git a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 b/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 index fba32bec28..e893139fc4 100644 --- a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 @@ -10,13 +10,12 @@ export default Discourse.Route.extend({ const routeName = "adminUsersList.show"; if (transition.targetName === routeName) { - const params = transition.routeInfos.find(a => a.name === routeName) - .params; + const params = transition.params[routeName]; const controller = this.controllerFor(routeName); if (controller) { controller.setProperties({ - order: transition.to.queryParams.order, - ascending: transition.to.queryParams.ascending, + order: transition.queryParams.order, + ascending: transition.queryParams.ascending, query: params.filter, showEmails: false, refreshing: false diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 2b82d2ea17..9987cf5ec7 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -290,27 +290,25 @@ export default Ember.Component.extend({ }); if (this.get("composerEvents")) { - this.appEvents.on("composer:insert-block", this, "_insertBlock"); - this.appEvents.on("composer:insert-text", this, "_insertText"); - this.appEvents.on("composer:replace-text", this, "_replaceText"); + this.appEvents.on("composer:insert-block", text => + this._addBlock(this._getSelected(), text) + ); + this.appEvents.on("composer:insert-text", (text, options) => + this._addText(this._getSelected(), text, options) + ); + this.appEvents.on("composer:replace-text", (oldVal, newVal, opts) => + this._replaceText(oldVal, newVal, opts) + ); } this._mouseTrap = mouseTrap; }, - _insertBlock(text) { - this._addBlock(this._getSelected(), text); - }, - - _insertText(text, options) { - this._addText(this._getSelected(), text, options); - }, - @on("willDestroyElement") _shutDown() { if (this.get("composerEvents")) { - this.appEvents.off("composer:insert-block", this, "_insertBlock"); - this.appEvents.off("composer:insert-text", this, "_insertText"); - this.appEvents.off("composer:replace-text", this, "_replaceText"); + this.appEvents.off("composer:insert-block"); + this.appEvents.off("composer:insert-text"); + this.appEvents.off("composer:replace-text"); } const mouseTrap = this._mouseTrap; diff --git a/app/assets/javascripts/discourse/components/d-modal-body.js.es6 b/app/assets/javascripts/discourse/components/d-modal-body.js.es6 index a49887519c..60f0d9a82a 100644 --- a/app/assets/javascripts/discourse/components/d-modal-body.js.es6 +++ b/app/assets/javascripts/discourse/components/d-modal-body.js.es6 @@ -14,14 +14,14 @@ export default Ember.Component.extend({ } Ember.run.scheduleOnce("afterRender", this, this._afterFirstRender); - this.appEvents.on("modal-body:flash", this, "_flash"); - this.appEvents.on("modal-body:clearFlash", this, "_clearFlash"); + this.appEvents.on("modal-body:flash", msg => this._flash(msg)); + this.appEvents.on("modal-body:clearFlash", () => this._clearFlash()); }, willDestroyElement() { this._super(...arguments); - this.appEvents.off("modal-body:flash", this, "_flash"); - this.appEvents.off("modal-body:clearFlash", this, "_clearFlash"); + this.appEvents.off("modal-body:flash"); + this.appEvents.off("modal-body:clearFlash"); }, _afterFirstRender() { diff --git a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 index d404252cd3..4a112c7e20 100644 --- a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 +++ b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 @@ -77,7 +77,9 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { } ); - this.appEvents.on("post:highlight", this, "_highlightPost"); + this.appEvents.on("post:highlight", postNumber => { + Ember.run.scheduleOnce("afterRender", null, highlight, postNumber); + }); this.appEvents.on("header:update-topic", topic => { if (topic === null) { @@ -97,29 +99,23 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { }); // setup mobile scroll logo if (this.site.mobileView) { - this.appEvents.on("topic:scrolled", this, "mobileScrollGuard"); + this.appEvents.on("topic:scrolled", offset => + this.mobileScrollGaurd(offset) + ); // used to animate header contents on scroll - this.appEvents.on("header:show-topic", this, "_showTopic"); - this.appEvents.on("header:hide-topic", this, "_hideTopic"); + this.appEvents.on("header:show-topic", () => { + $("header.d-header") + .removeClass("scroll-up") + .addClass("scroll-down"); + }); + this.appEvents.on("header:hide-topic", () => { + $("header.d-header") + .removeClass("scroll-down") + .addClass("scroll-up"); + }); } }, - _showTopic() { - $("header.d-header") - .removeClass("scroll-up") - .addClass("scroll-down"); - }, - - _hideTopic() { - $("header.d-header") - .removeClass("scroll-down") - .addClass("scroll-up"); - }, - - _highlightPost(postNumber) { - Ember.run.scheduleOnce("afterRender", null, highlight, postNumber); - }, - willDestroyElement() { this._super(...arguments); this.unbindScrolling("topic-view"); @@ -132,10 +128,10 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { // this happens after route exit, stuff could have trickled in this.appEvents.trigger("header:hide-topic"); - this.appEvents.off("post:highlight", this, "_highlightPost"); + this.appEvents.off("post:highlight"); // mobile scroll logo clean up. if (this.site.mobileView) { - this.appEvents.off("topic:scrolled", this, "mobileScrollGuard"); + this.appEvents.off("topic:scrolled"); $("header.d-header").removeClass("scroll-down scroll-up"); } }, @@ -203,7 +199,7 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { // determines scroll direction, triggers header topic info on mobile // and ensures that the switch happens only once per scroll direction change - mobileScrollGuard(offset) { + mobileScrollGaurd(offset) { // user hasn't scrolled past topic title. if (offset < this.dockAt) return; diff --git a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 index e8bdc4fb43..ad588faf65 100644 --- a/app/assets/javascripts/discourse/components/emoji-picker.js.es6 +++ b/app/assets/javascripts/discourse/components/emoji-picker.js.es6 @@ -77,11 +77,7 @@ export default Ember.Component.extend({ @on("willDestroyElement") _unbindGlobalEvents() { - this.appEvents.off("emoji-picker:close", this, "_closeEmojiPicker"); - }, - - _closeEmojiPicker() { - this.set("active", false); + this.appEvents.off("emoji-picker:close"); }, @on("didInsertElement") @@ -89,7 +85,7 @@ export default Ember.Component.extend({ this.$picker = this.$(".emoji-picker"); this.$modal = this.$(".emoji-picker-modal"); - this.appEvents.on("emoji-picker:close", this, "_closeEmojiPicker"); + this.appEvents.on("emoji-picker:close", () => this.set("active", false)); if (!keyValueStore.getObject(EMOJI_USAGE)) { keyValueStore.setObject({ key: EMOJI_USAGE, value: [] }); diff --git a/app/assets/javascripts/discourse/components/login-modal.js.es6 b/app/assets/javascripts/discourse/components/login-modal.js.es6 index 59939542cb..c1c452f663 100644 --- a/app/assets/javascripts/discourse/components/login-modal.js.es6 +++ b/app/assets/javascripts/discourse/components/login-modal.js.es6 @@ -18,7 +18,7 @@ export default Ember.Component.extend({ "#login-account-password, #login-account-name, #login-second-factor" ).keydown(e => { if (e.keyCode === 13) { - this.action(); + this.sendAction(); } }); }); diff --git a/app/assets/javascripts/discourse/components/nav-item.js.es6 b/app/assets/javascripts/discourse/components/nav-item.js.es6 index 729c22fb12..3910ad6b64 100644 --- a/app/assets/javascripts/discourse/components/nav-item.js.es6 +++ b/app/assets/javascripts/discourse/components/nav-item.js.es6 @@ -7,7 +7,7 @@ export default Ember.Component.extend({ tagName: "li", classNameBindings: ["active"], - @computed + @computed() router() { return getOwner(this).lookup("router:main"); }, @@ -27,7 +27,7 @@ export default Ember.Component.extend({ router = this.get("router"); return routeParam - ? router.currentRoute.params["filter"] === routeParam + ? router.isActive(route, routeParam) : router.isActive(route); } }); diff --git a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 index 94779cdebe..fc5f0a413a 100644 --- a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 +++ b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 @@ -262,38 +262,6 @@ export default MountWidget.extend({ Ember.run.scheduleOnce("afterRender", this, this.scrolled); }, - _posted(staged) { - const disableJumpReply = this.currentUser.get("disable_jump_reply"); - - this.queueRerender(() => { - if (staged && !disableJumpReply) { - const postNumber = staged.get("post_number"); - DiscourseURL.jumpToPost(postNumber, { skipIfOnScreen: true }); - } - }); - }, - - _refresh(args) { - if (args) { - if (args.id) { - this.dirtyKeys.keyDirty(`post-${args.id}`); - - if (args.refreshLikes) { - this.dirtyKeys.keyDirty(`post-menu-${args.id}`, { - onRefresh: "refreshLikes" - }); - } - } else if (args.force) { - this.dirtyKeys.forceAll(); - } - } - this.queueRerender(); - }, - - _debouncedScroll() { - Ember.run.debounce(this, this._scrollTriggered, 10); - }, - didInsertElement() { this._super(...arguments); const debouncedScroll = () => @@ -301,12 +269,21 @@ export default MountWidget.extend({ this._previouslyNearby = {}; - this.appEvents.on("post-stream:refresh", this, "_debouncedScroll"); + this.appEvents.on("post-stream:refresh", debouncedScroll); $(document).bind("touchmove.post-stream", debouncedScroll); $(window).bind("scroll.post-stream", debouncedScroll); this._scrollTriggered(); - this.appEvents.on("post-stream:posted", this, "_posted"); + this.appEvents.on("post-stream:posted", staged => { + const disableJumpReply = this.currentUser.get("disable_jump_reply"); + + this.queueRerender(() => { + if (staged && !disableJumpReply) { + const postNumber = staged.get("post_number"); + DiscourseURL.jumpToPost(postNumber, { skipIfOnScreen: true }); + } + }); + }); this.$().on("mouseenter.post-stream", "button.widget-button", e => { $("button.widget-button").removeClass("d-hover"); @@ -317,18 +294,33 @@ export default MountWidget.extend({ $("button.widget-button").removeClass("d-hover"); }); - this.appEvents.on("post-stream:refresh", this, "_refresh"); + this.appEvents.on("post-stream:refresh", args => { + if (args) { + if (args.id) { + this.dirtyKeys.keyDirty(`post-${args.id}`); + + if (args.refreshLikes) { + this.dirtyKeys.keyDirty(`post-menu-${args.id}`, { + onRefresh: "refreshLikes" + }); + } + } else if (args.force) { + this.dirtyKeys.forceAll(); + } + } + this.queueRerender(); + }); }, willDestroyElement() { this._super(...arguments); $(document).unbind("touchmove.post-stream"); $(window).unbind("scroll.post-stream"); - this.appEvents.off("post-stream:refresh", this, "_debouncedScroll"); + this.appEvents.off("post-stream:refresh"); this.$().off("mouseenter.post-stream"); this.$().off("mouseleave.post-stream"); - this.appEvents.off("post-stream:refresh", this, "_refresh"); - this.appEvents.off("post-stream:posted", this, "_posted"); + this.appEvents.off("post-stream:refresh"); + this.appEvents.off("post-stream:posted"); }, showModerationHistory(post) { diff --git a/app/assets/javascripts/discourse/components/site-header.js.es6 b/app/assets/javascripts/discourse/components/site-header.js.es6 index 898ebcce2d..28627bff78 100644 --- a/app/assets/javascripts/discourse/components/site-header.js.es6 +++ b/app/assets/javascripts/discourse/components/site-header.js.es6 @@ -231,14 +231,19 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { const { isAndroid } = this.capabilities; $(window).on("resize.discourse-menu-panel", () => this.afterRender()); - this.appEvents.on("header:show-topic", this, "setTopic"); - this.appEvents.on("header:hide-topic", this, "setTopic"); + this.appEvents.on("header:show-topic", topic => this.setTopic(topic)); + this.appEvents.on("header:hide-topic", () => this.setTopic(null)); this.dispatch("notifications:changed", "user-notifications"); this.dispatch("header:keyboard-trigger", "header"); this.dispatch("search-autocomplete:after-complete", "search-term"); - this.appEvents.on("dom:clean", this, "_cleanDom"); + this.appEvents.on("dom:clean", () => { + // For performance, only trigger a re-render if any menu panels are visible + if (this.$(".menu-panel").length) { + this.eventDispatched("dom:clean", "header"); + } + }); // Only add listeners for opening menus by swiping them in on Android devices // iOS will respond to these events, but also does swiping for back/forward @@ -247,22 +252,15 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { } }, - _cleanDom() { - // For performance, only trigger a re-render if any menu panels are visible - if (this.$(".menu-panel").length) { - this.eventDispatched("dom:clean", "header"); - } - }, - willDestroyElement() { this._super(...arguments); const { isAndroid } = this.capabilities; $("body").off("keydown.header"); $(window).off("resize.discourse-menu-panel"); - this.appEvents.off("header:show-topic", this, "setTopic"); - this.appEvents.off("header:hide-topic", this, "setTopic"); - this.appEvents.off("dom:clean", this, "_cleanDom"); + this.appEvents.off("header:show-topic"); + this.appEvents.off("header:hide-topic"); + this.appEvents.off("dom:clean"); if (isAndroid) { this.removeTouchListeners($("body")); diff --git a/app/assets/javascripts/discourse/components/topic-entrance.js.es6 b/app/assets/javascripts/discourse/components/topic-entrance.js.es6 index fa25a528a7..2ded0e7314 100644 --- a/app/assets/javascripts/discourse/components/topic-entrance.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-entrance.js.es6 @@ -53,7 +53,7 @@ export default Ember.Component.extend(CleansUp, { didInsertElement() { this._super(...arguments); - this.appEvents.on("topic-entrance:show", this, "_show"); + this.appEvents.on("topic-entrance:show", data => this._show(data)); }, _setCSS() { @@ -100,7 +100,7 @@ export default Ember.Component.extend(CleansUp, { }, willDestroyElement() { - this.appEvents.off("topic-entrance:show", this, "_show"); + this.appEvents.off("topic-entrance:show"); }, _jumpTo(destination) { diff --git a/app/assets/javascripts/discourse/components/watch-read.js.es6 b/app/assets/javascripts/discourse/components/watch-read.js.es6 index 4b149d371c..7cca41ab8b 100644 --- a/app/assets/javascripts/discourse/components/watch-read.js.es6 +++ b/app/assets/javascripts/discourse/components/watch-read.js.es6 @@ -13,7 +13,7 @@ export default Ember.Component.extend({ $(window).on("load.faq resize.faq scroll.faq", () => { const faqUnread = !currentUser.get("read_faq"); if (faqUnread && isElementInViewport($(".contents p").last())) { - this.action(); + this.sendAction(); } }); } diff --git a/app/assets/javascripts/discourse/helpers/route-action.js.es6 b/app/assets/javascripts/discourse/helpers/route-action.js.es6 index 4e010498bd..946dd69ac8 100644 --- a/app/assets/javascripts/discourse/helpers/route-action.js.es6 +++ b/app/assets/javascripts/discourse/helpers/route-action.js.es6 @@ -9,14 +9,15 @@ const { runInDebug } = Ember; -function getCurrentRouteInfos(router) { +function getCurrentHandlerInfos(router) { let routerLib = router._routerMicrolib || router.router; - return routerLib.currentRouteInfos; + + return routerLib.currentHandlerInfos; } function getRoutes(router) { - return emberArray(getCurrentRouteInfos(router)) - .mapBy("_route") + return emberArray(getCurrentHandlerInfos(router)) + .mapBy("handler") .reverse(); } diff --git a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 index 84a9d51de7..b10cbe9b7d 100644 --- a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 +++ b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 @@ -12,12 +12,10 @@ export default { initialize(container) { // Tell our AJAX system to track a page transition const router = container.lookup("router:main"); - - router.on("routeWillChange", viewTrackingRequired); - router.on("routeDidChange", cleanDOM); + router.on("willTransition", viewTrackingRequired); + router.on("didTransition", cleanDOM); let appEvents = container.lookup("app-events:main"); - startPageTracking(router, appEvents); // Out of the box, Discourse tries to track google analytics diff --git a/app/assets/javascripts/discourse/initializers/show-footer.js.es6 b/app/assets/javascripts/discourse/initializers/show-footer.js.es6 index 77040e907f..ce58457774 100644 --- a/app/assets/javascripts/discourse/initializers/show-footer.js.es6 +++ b/app/assets/javascripts/discourse/initializers/show-footer.js.es6 @@ -7,7 +7,7 @@ export default { // only take care of hiding the footer here // controllers MUST take care of displaying it - router.on("routeWillChange", () => { + router.on("willTransition", () => { application.set("showFooter", false); return true; }); diff --git a/app/assets/javascripts/discourse/lib/app-events.js.es6 b/app/assets/javascripts/discourse/lib/app-events.js.es6 index aff71a2a40..59257700fd 100644 --- a/app/assets/javascripts/discourse/lib/app-events.js.es6 +++ b/app/assets/javascripts/discourse/lib/app-events.js.es6 @@ -1,48 +1 @@ -import deprecated from "discourse-common/lib/deprecated"; - -export default Ember.Object.extend(Ember.Evented, { - _events: {}, - - on() { - if (arguments.length === 2) { - let [name, fn] = arguments; - let target = {}; - this._events[name] = this._events[name] || []; - this._events[name].push({ target, fn }); - - this._super(name, target, fn); - } else if (arguments.length === 3) { - let [name, target, fn] = arguments; - this._events[name] = this._events[name] || []; - this._events[name].push({ target, fn }); - - this._super(...arguments); - } - return this; - }, - - off() { - let name = arguments[0]; - let fn = arguments[2]; - - if (this._events[name]) { - if (arguments.length === 1) { - deprecated( - "Removing all event listeners at once is deprecated, please remove each listener individually." - ); - - this._events[name].forEach(ref => { - this._super(name, ref.target, ref.fn); - }); - delete this._events[name]; - } else if (arguments.length === 3) { - this._super(...arguments); - - this._events[name] = this._events[name].filter(e => e.fn !== fn); - if (this._events[name].length === 0) delete this._events[name]; - } - } - - return this; - } -}); +export default Ember.Object.extend(Ember.Evented); diff --git a/app/assets/javascripts/discourse/lib/page-tracker.js.es6 b/app/assets/javascripts/discourse/lib/page-tracker.js.es6 index 6a36cc7cbc..0765ad50b1 100644 --- a/app/assets/javascripts/discourse/lib/page-tracker.js.es6 +++ b/app/assets/javascripts/discourse/lib/page-tracker.js.es6 @@ -15,9 +15,10 @@ export function startPageTracking(router, appEvents) { if (_started) { return; } - router.on("routeDidChange", () => { - router.send("refreshTitle"); - const url = Discourse.getURL(router.get("url")); + + router.on("didTransition", function() { + this.send("refreshTitle"); + const url = Discourse.getURL(this.get("url")); // Refreshing the title is debounced, so we need to trigger this in the // next runloop to have the correct title. @@ -38,7 +39,6 @@ export function startPageTracking(router, appEvents) { } }); }); - _started = true; } diff --git a/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 b/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 index 756a24930f..ce110da855 100644 --- a/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 +++ b/app/assets/javascripts/discourse/mixins/card-contents-base.js.es6 @@ -122,7 +122,10 @@ export default Ember.Mixin.create({ return this._show($target.text().replace(/^@/, ""), $target); }); - this.appEvents.on(previewClickEvent, this, "_previewClick"); + this.appEvents.on(previewClickEvent, $target => { + this.set("isFixed", true); + return this._show($target.text().replace(/^@/, ""), $target); + }); this.appEvents.on(`topic-header:trigger-${id}`, (username, $target) => { this.setProperties({ isFixed: true, isDocked: true }); @@ -130,11 +133,6 @@ export default Ember.Mixin.create({ }); }, - _previewClick($target) { - this.set("isFixed", true); - return this._show($target.text().replace(/^@/, ""), $target); - }, - _positionCard(target) { const rtl = $("html").css("direction") === "rtl"; if (!target) { @@ -241,7 +239,7 @@ export default Ember.Mixin.create({ $("#main") .off(clickDataExpand) .off(clickMention); - this.appEvents.off(previewClickEvent, this, "_previewClick"); + this.appEvents.off(previewClickEvent); }, keyUp(e) { diff --git a/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 b/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 index bf39cd084a..4b30762941 100644 --- a/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 +++ b/app/assets/javascripts/discourse/mixins/url-refresh.js.es6 @@ -7,12 +7,12 @@ export default { didInsertElement() { this._super(...arguments); - this.appEvents.on("url:refresh", this, "refresh"); + this.appEvents.on("url:refresh", this.refresh); }, willDestroyElement() { this._super(...arguments); - this.appEvents.off("url:refresh", this, "refresh"); + this.appEvents.off("url:refresh"); } }; diff --git a/app/assets/javascripts/discourse/routes/badges-show.js.es6 b/app/assets/javascripts/discourse/routes/badges-show.js.es6 index 7a5b7f7830..e2330c3ac8 100644 --- a/app/assets/javascripts/discourse/routes/badges-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/badges-show.js.es6 @@ -30,8 +30,7 @@ export default Discourse.Route.extend({ }, afterModel(model, transition) { - const username = - transition.to.queryParams && transition.to.queryParams.username; + const username = transition.queryParams && transition.queryParams.username; const userBadgesGrant = UserBadge.findByBadgeId(model.get("id"), { username diff --git a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 index 182571cb8e..eac9ea2017 100644 --- a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 @@ -93,7 +93,7 @@ export default (filterArg, params) => { const listFilter = `c/${Discourse.Category.slugFor( category )}/l/${this.filter(category)}`, - findOpts = filterQueryParams(transition.to.queryParams, params), + findOpts = filterQueryParams(transition.queryParams, params), extras = { cached: this.isPoppedState(transition) }; return findTopicList( diff --git a/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 b/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 index 62aad28945..bb8a600ab5 100644 --- a/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 +++ b/app/assets/javascripts/discourse/routes/group-activity-posts.js.es6 @@ -7,7 +7,7 @@ export function buildGroupPage(type) { }, model(params, transition) { - let categoryId = Ember.get(transition.to, "queryParams.category_id"); + let categoryId = Ember.get(transition, "queryParams.category_id"); return this.modelFor("group").findPosts({ type, categoryId }); }, diff --git a/app/assets/javascripts/discourse/routes/new-message.js.es6 b/app/assets/javascripts/discourse/routes/new-message.js.es6 index f926ba3967..7069151882 100644 --- a/app/assets/javascripts/discourse/routes/new-message.js.es6 +++ b/app/assets/javascripts/discourse/routes/new-message.js.es6 @@ -3,8 +3,7 @@ import Group from "discourse/models/group"; export default Discourse.Route.extend({ beforeModel(transition) { - const params = transition.to.queryParams; - + const params = transition.queryParams; const groupName = params.groupname || params.group_name; if (this.currentUser) { diff --git a/app/assets/javascripts/discourse/routes/new-topic.js.es6 b/app/assets/javascripts/discourse/routes/new-topic.js.es6 index 947193be38..46f38d4ef3 100644 --- a/app/assets/javascripts/discourse/routes/new-topic.js.es6 +++ b/app/assets/javascripts/discourse/routes/new-topic.js.es6 @@ -6,11 +6,11 @@ export default Discourse.Route.extend({ if (Discourse.User.current()) { let category, category_id; - if (transition.to.queryParams.category_id) { - category_id = transition.to.queryParams.category_id; + if (transition.queryParams.category_id) { + category_id = transition.queryParams.category_id; category = Category.findById(category_id); - } else if (transition.to.queryParams.category) { - const splitCategory = transition.to.queryParams.category.split("/"); + } else if (transition.queryParams.category) { + const splitCategory = transition.queryParams.category.split("/"); category = this._getCategory( splitCategory[0], splitCategory[1], @@ -46,10 +46,10 @@ export default Discourse.Route.extend({ Ember.run.next(function() { e.send( "createNewTopicViaParams", - transition.to.queryParams.title, - transition.to.queryParams.body, + transition.queryParams.title, + transition.queryParams.body, category_id, - transition.to.queryParams.tags + transition.queryParams.tags ); }); } @@ -60,10 +60,10 @@ export default Discourse.Route.extend({ Ember.run.next(function() { e.send( "createNewTopicViaParams", - transition.to.queryParams.title, - transition.to.queryParams.body, + transition.queryParams.title, + transition.queryParams.body, null, - transition.to.queryParams.tags + transition.queryParams.tags ); }); } diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index db3c2eb4f3..978816b7df 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -70,8 +70,8 @@ export default Discourse.Route.extend({ controller.set("loading", true); const params = controller.getProperties("order", "ascending"); - params.order = transition.to.queryParams.order || params.order; - params.ascending = transition.to.queryParams.ascending || params.ascending; + params.order = transition.queryParams.order || params.order; + params.ascending = transition.queryParams.ascending || params.ascending; const categorySlug = this.get("categorySlug"); const parentCategorySlug = this.get("parentCategorySlug"); diff --git a/app/assets/javascripts/discourse/routes/topic.js.es6 b/app/assets/javascripts/discourse/routes/topic.js.es6 index 4e4865e165..06518c1e16 100644 --- a/app/assets/javascripts/discourse/routes/topic.js.es6 +++ b/app/assets/javascripts/discourse/routes/topic.js.es6 @@ -231,7 +231,7 @@ const TopicRoute = Discourse.Route.extend({ }); } - const queryParams = transition.to.queryParams; + const queryParams = transition.queryParams; let topic = this.modelFor("topic"); if (topic && topic.get("id") === parseInt(params.id, 10)) { diff --git a/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 b/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 index 5da6e94dc0..e10629077d 100644 --- a/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 +++ b/app/assets/javascripts/discourse/routes/user-activity-stream.js.es6 @@ -14,7 +14,7 @@ export default Discourse.Route.extend(ViewingActionType, { filter: this.get("userActionType"), noContentHelpKey: this.get("noContentHelpKey") || "user_activity.no_default", - actingUsername: transition.to.queryParams.acting_username + actingUsername: transition.queryParams.acting_username }); }, diff --git a/app/assets/javascripts/select-kit/components/category-chooser.js.es6 b/app/assets/javascripts/select-kit/components/category-chooser.js.es6 index f0ae60f693..34cd38b0f8 100644 --- a/app/assets/javascripts/select-kit/components/category-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/category-chooser.js.es6 @@ -1,4 +1,5 @@ import ComboBoxComponent from "select-kit/components/combo-box"; +import { on } from "ember-addons/ember-computed-decorators"; import computed from "ember-addons/ember-computed-decorators"; import PermissionType from "discourse/models/permission-type"; import Category from "discourse/models/category"; @@ -99,6 +100,16 @@ export default ComboBoxComponent.extend({ return content; }, + @on("didRender") + _bindComposerResizing() { + this.appEvents.on("composer:resized", this, this.applyDirection); + }, + + @on("willDestroyElement") + _unbindComposerResizing() { + this.appEvents.off("composer:resized"); + }, + didSelect(computedContentItem) { if (this.attrs.onChooseCategory) { this.attrs.onChooseCategory(computedContentItem.originalContent); diff --git a/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 b/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 index 41e5445847..01b1f6906d 100644 --- a/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 +++ b/app/assets/javascripts/select-kit/components/group-members-dropdown.js.es6 @@ -28,6 +28,6 @@ export default DropdownSelectBoxComponent.extend({ }, mutateValue(value) { - this.get(value)(); + this.sendAction(value); } }); diff --git a/app/assets/javascripts/select-kit/components/select-kit.js.es6 b/app/assets/javascripts/select-kit/components/select-kit.js.es6 index cad55bf463..ccc5f49f8d 100644 --- a/app/assets/javascripts/select-kit/components/select-kit.js.es6 +++ b/app/assets/javascripts/select-kit/components/select-kit.js.es6 @@ -134,10 +134,10 @@ export default Ember.Component.extend( this.removeObserver( `content.@each.${this.get("nameProperty")}`, this, - "_compute" + this._compute ); - this.removeObserver(`content.[]`, this, "_compute"); - this.removeObserver(`asyncContent.[]`, this, "_compute"); + this.removeObserver(`content.[]`, this, this._compute); + this.removeObserver(`asyncContent.[]`, this, this._compute); }, willComputeAttributes() {}, diff --git a/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 b/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 index b15b688faa..1bcd7df3fb 100644 --- a/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 +++ b/app/assets/javascripts/select-kit/components/topic-notifications-options.js.es6 @@ -17,20 +17,20 @@ export default NotificationOptionsComponent.extend({ return archetype === "private_message" ? "_pm" : ""; }, - _changed(msg) { - if (this.get("computedValue") !== msg.id) { - this.get("topic.details").updateNotifications(msg.id); - } - }, - @on("didInsertElement") _bindGlobalLevelChanged() { - this.appEvents.on("topic-notifications-button:changed", this, "_changed"); + this.appEvents.on("topic-notifications-button:changed", msg => { + if (msg.type === "notification") { + if (this.get("computedValue") !== msg.id) { + this.get("topic.details").updateNotifications(msg.id); + } + } + }); }, @on("willDestroyElement") _unbindGlobalLevelChanged() { - this.appEvents.off("topic-notifications-button:changed", this, "_changed"); + this.appEvents.off("topic-notifications-button:changed"); }, mutateValue(value) { From 18ff790e79b3920504064763d1a8e44258924a3b Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 13 Mar 2019 21:16:09 +0200 Subject: [PATCH 43/47] FIX: Use prioritize_username_in_ux in post notices. --- .../javascripts/discourse/widgets/post.js.es6 | 14 ++++++++-- test/javascripts/widgets/post-test.js.es6 | 28 +++++++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/widgets/post.js.es6 b/app/assets/javascripts/discourse/widgets/post.js.es6 index 956dafd358..da63b16d97 100644 --- a/app/assets/javascripts/discourse/widgets/post.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post.js.es6 @@ -431,15 +431,25 @@ createWidget("post-contents", { createWidget("post-notice", { tagName: "div.post-notice", + buildClasses(attrs) { + if (attrs.postNoticeType === "first") { + return ["new-user"]; + } else if (attrs.postNoticeType === "returning") { + return ["returning-user"]; + } + return []; + }, + html(attrs) { + const user = this.siteSettings.prioritize_username_in_ux || !attrs.name ? attrs.username : attrs.name; let text, icon; if (attrs.postNoticeType === "first") { icon = "hands-helping"; - text = I18n.t("post.notice.first", { user: attrs.username }); + text = I18n.t("post.notice.first", { user }); } else if (attrs.postNoticeType === "returning") { icon = "far-smile"; text = I18n.t("post.notice.return", { - user: attrs.username, + user, time: relativeAge(attrs.postNoticeTime, { format: "tiny", addAgo: true diff --git a/test/javascripts/widgets/post-test.js.es6 b/test/javascripts/widgets/post-test.js.es6 index 5dc1b8ad50..6c0c7faad4 100644 --- a/test/javascripts/widgets/post-test.js.es6 +++ b/test/javascripts/widgets/post-test.js.es6 @@ -853,21 +853,43 @@ widgetTest("pm map", { } }); -widgetTest("post notice", { +widgetTest("post notice - with username", { template: '{{mount-widget widget="post" args=args}}', beforeEach() { + this.siteSettings.prioritize_username_in_ux = true; this.set("args", { postNoticeType: "returning", postNoticeTime: new Date("2010-01-01 12:00:00 UTC"), - username: "codinghorror" + username: "codinghorror", + name: "Jeff" }); }, test(assert) { assert.equal( - find(".post-notice") + find(".post-notice.returning-user") .text() .trim(), I18n.t("post.notice.return", { user: "codinghorror", time: "Jan '10" }) ); } }); + +widgetTest("post notice - with name", { + template: '{{mount-widget widget="post" args=args}}', + beforeEach() { + this.siteSettings.prioritize_username_in_ux = false; + this.set("args", { + postNoticeType: "first", + username: "codinghorror", + name: "Jeff" + }); + }, + test(assert) { + assert.equal( + find(".post-notice.new-user") + .text() + .trim(), + I18n.t("post.notice.first", { user: "Jeff", time: "Jan '10" }) + ); + } +}); From bd8e46a9c1275e70e297972c9be4db53a7e532c2 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Wed, 13 Mar 2019 16:24:54 -0300 Subject: [PATCH 44/47] SECURITY: Upgrading Rails version to 5.2.2.1 --- Gemfile | 14 +++++++------- Gemfile.lock | 54 ++++++++++++++++++++++++++-------------------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Gemfile b/Gemfile index 45af6c03c3..5ee5675cf3 100644 --- a/Gemfile +++ b/Gemfile @@ -14,13 +14,13 @@ if rails_master? else # until rubygems gives us optional dependencies we are stuck with this # bundle update actionmailer actionpack actionview activemodel activerecord activesupport railties - gem 'actionmailer', '5.2.2' - gem 'actionpack', '5.2.2' - gem 'actionview', '5.2.2' - gem 'activemodel', '5.2.2' - gem 'activerecord', '5.2.2' - gem 'activesupport', '5.2.2' - gem 'railties', '5.2.2' + gem 'actionmailer', '5.2.2.1' + gem 'actionpack', '5.2.2.1' + gem 'actionview', '5.2.2.1' + gem 'activemodel', '5.2.2.1' + gem 'activerecord', '5.2.2.1' + gem 'activesupport', '5.2.2.1' + gem 'railties', '5.2.2.1' gem 'sprockets-rails' end diff --git a/Gemfile.lock b/Gemfile.lock index 299c7f91ac..86c23f2778 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,37 +1,37 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (5.2.2) - actionpack (= 5.2.2) - actionview (= 5.2.2) - activejob (= 5.2.2) + actionmailer (5.2.2.1) + actionpack (= 5.2.2.1) + actionview (= 5.2.2.1) + activejob (= 5.2.2.1) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.2.2) - actionview (= 5.2.2) - activesupport (= 5.2.2) + actionpack (5.2.2.1) + actionview (= 5.2.2.1) + activesupport (= 5.2.2.1) rack (~> 2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.2.2) - activesupport (= 5.2.2) + actionview (5.2.2.1) + activesupport (= 5.2.2.1) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) active_model_serializers (0.8.4) activemodel (>= 3.0) - activejob (5.2.2) - activesupport (= 5.2.2) + activejob (5.2.2.1) + activesupport (= 5.2.2.1) globalid (>= 0.3.6) - activemodel (5.2.2) - activesupport (= 5.2.2) - activerecord (5.2.2) - activemodel (= 5.2.2) - activesupport (= 5.2.2) + activemodel (5.2.2.1) + activesupport (= 5.2.2.1) + activerecord (5.2.2.1) + activemodel (= 5.2.2.1) + activesupport (= 5.2.2.1) arel (>= 9.0) - activesupport (5.2.2) + activesupport (5.2.2.1) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -308,9 +308,9 @@ GEM rails_multisite (2.0.6) activerecord (> 4.2, < 6) railties (> 4.2, < 6) - railties (5.2.2) - actionpack (= 5.2.2) - activesupport (= 5.2.2) + railties (5.2.2.1) + actionpack (= 5.2.2.1) + activesupport (= 5.2.2.1) method_source rake (>= 0.8.7) thor (>= 0.19.0, < 2.0) @@ -446,13 +446,13 @@ PLATFORMS ruby DEPENDENCIES - actionmailer (= 5.2.2) - actionpack (= 5.2.2) - actionview (= 5.2.2) + actionmailer (= 5.2.2.1) + actionpack (= 5.2.2.1) + actionview (= 5.2.2.1) active_model_serializers (~> 0.8.3) - activemodel (= 5.2.2) - activerecord (= 5.2.2) - activesupport (= 5.2.2) + activemodel (= 5.2.2.1) + activerecord (= 5.2.2.1) + activesupport (= 5.2.2.1) annotate aws-sdk-s3 aws-sdk-sns @@ -525,7 +525,7 @@ DEPENDENCIES rack-mini-profiler rack-protection rails_multisite - railties (= 5.2.2) + railties (= 5.2.2.1) rake rb-fsevent rb-inotify (~> 0.9) From 81bf4df1a57802516cd41c4e13032d2c0eb9e030 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 13 Mar 2019 21:26:09 +0200 Subject: [PATCH 45/47] DEV: Fix lint. --- app/assets/javascripts/discourse/widgets/post.js.es6 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/widgets/post.js.es6 b/app/assets/javascripts/discourse/widgets/post.js.es6 index da63b16d97..a7c0c452d9 100644 --- a/app/assets/javascripts/discourse/widgets/post.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post.js.es6 @@ -441,7 +441,10 @@ createWidget("post-notice", { }, html(attrs) { - const user = this.siteSettings.prioritize_username_in_ux || !attrs.name ? attrs.username : attrs.name; + const user = + this.siteSettings.prioritize_username_in_ux || !attrs.name + ? attrs.username + : attrs.name; let text, icon; if (attrs.postNoticeType === "first") { icon = "hands-helping"; From 7db2dc717e672e831657183b3b069307aaa2a842 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 13 Mar 2019 15:33:18 -0400 Subject: [PATCH 46/47] REFACTOR: remove unnecessary parentheses attempt 2 (follow-up on 154f503d) --- app/models/post.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8e03538f52..0441bcb327 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -215,8 +215,7 @@ class Post < ActiveRecord::Base end def matches_recent_post? - post_id = $redis.get(unique_post_key) - post_id != (nil) && post_id.to_i != (id) + $redis.get(unique_post_key)&.to_i != id end def raw_hash From 4f74210949ea8329c0c4c5b25d9eef081a6640c2 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Wed, 13 Mar 2019 16:47:23 -0300 Subject: [PATCH 47/47] Version bump to v2.3.0.beta5 --- lib/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/version.rb b/lib/version.rb index 6c1a93c110..f9a01d38c4 100644 --- a/lib/version.rb +++ b/lib/version.rb @@ -7,7 +7,7 @@ module Discourse MAJOR = 2 MINOR = 3 TINY = 0 - PRE = 'beta4' + PRE = 'beta5' STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.') end