From 34387c5a389ff87ac768fa2eae3e63f59bf49f06 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 6 Jul 2021 12:49:26 +0300 Subject: [PATCH] FEATURE: Warn if invited user cannot see topic (#13548) Users can invite people to topics from secured category, but they will not be redirected to the topic after signing up unless they have the permissions to view the topic. This commit shows a warning when invite is saved if the topic is in a secured category and none of the invite groups are allowed to see it. --- .../discourse/app/components/d-modal-body.js | 7 +++++- .../app/controllers/create-invite.js | 9 ++++++-- app/controllers/invites_controller.rb | 6 ++--- app/models/invite.rb | 17 ++++++++++++++ app/serializers/invite_serializer.rb | 11 ++++++++- config/locales/server.en.yml | 1 + spec/models/invite_spec.rb | 23 +++++++++++++++++++ 7 files changed, 67 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-modal-body.js b/app/assets/javascripts/discourse/app/components/d-modal-body.js index 120d00e23cb..4f2f38292fc 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal-body.js +++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js @@ -62,7 +62,12 @@ export default Component.extend({ const modalAlert = document.getElementById("modal-alert"); if (modalAlert) { modalAlert.style.display = "none"; - modalAlert.classList.remove("alert-info", "alert-error", "alert-success"); + modalAlert.classList.remove( + "alert-error", + "alert-info", + "alert-success", + "alert-warning" + ); } }, diff --git a/app/assets/javascripts/discourse/app/controllers/create-invite.js b/app/assets/javascripts/discourse/app/controllers/create-invite.js index d24e4525685..1a774042373 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-invite.js +++ b/app/assets/javascripts/discourse/app/controllers/create-invite.js @@ -99,10 +99,15 @@ export default Controller.extend( return this.invite .save(data) - .then(() => { + .then((result) => { this.rollbackBuffer(); this.setAutogenerated(opts.autogenerated); - if (!this.autogenerated) { + if (result.warnings) { + this.appEvents.trigger("modal-body:flash", { + text: result.warnings.join(","), + messageClass: "warning", + }); + } else if (!this.autogenerated) { if (this.isEmail && opts.sendEmail) { this.send("closeModal"); } else { diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 0ef40298b82..acd5b49f4fd 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -102,7 +102,7 @@ class InvitesController < ApplicationController ) if invite.present? - render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email)) + render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email), show_warnings: true) else render json: failed_json, status: 422 end @@ -121,7 +121,7 @@ class InvitesController < ApplicationController guardian.ensure_can_invite_to_forum!(nil) - render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email)) + render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email), show_warnings: true) end def update @@ -194,7 +194,7 @@ class InvitesController < ApplicationController Jobs.enqueue(:invite_email, invite_id: invite.id, invite_to_topic: params[:invite_to_topic]) end - render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email)) + render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email), show_warnings: true) end def destroy diff --git a/app/models/invite.rb b/app/models/invite.rb index 704a62f0e28..a5c08d38347 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -249,6 +249,23 @@ class Invite < ActiveRecord::Base Jobs.enqueue(:invite_email, invite_id: self.id) end + def warnings(guardian) + @warnings ||= begin + warnings = [] + + topic = self.topics.first + if topic&.read_restricted_category? + topic_groups = topic.category.groups + if (self.groups & topic_groups).blank? + editable_topic_groups = topic_groups.filter { |g| guardian.can_edit_group?(g) } + warnings << I18n.t("invite.requires_groups", groups: editable_topic_groups.pluck(:name).join(", ")) + end + end + + warnings + end + end + def limit_invites_per_day RateLimiter.new(invited_by, "invites-per-day", SiteSetting.max_invites_per_day, 1.day.to_i) end diff --git a/app/serializers/invite_serializer.rb b/app/serializers/invite_serializer.rb index 0d8f0dbf6bd..51aa36939e3 100644 --- a/app/serializers/invite_serializer.rb +++ b/app/serializers/invite_serializer.rb @@ -12,7 +12,8 @@ class InviteSerializer < ApplicationSerializer :created_at, :updated_at, :expires_at, - :expired + :expired, + :warnings has_many :topics, embed: :object, serializer: BasicTopicSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer @@ -44,4 +45,12 @@ class InviteSerializer < ApplicationSerializer def expired object.expired? end + + def warnings + object.warnings(scope) + end + + def include_warnings? + object.warnings(scope).present? + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f8d75b4fcec..c815b967ef5 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -254,6 +254,7 @@ en: disabled_errors: discourse_connect_enabled: "Invites are disabled because DiscourseConnect is enabled." invalid_access: "You are not permitted to view the requested resource." + requires_groups: "Invite saved. To give access to the specified topic, add one of the following groups: %{groups}." bulk_invite: file_should_be_csv: "The uploaded file should be of csv format." diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 91316b4a86d..58f32775397 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -404,4 +404,27 @@ describe Invite do expect(invite.invalidated_at).to be_nil end end + + describe '#warnings' do + fab!(:admin) { Fabricate(:admin) } + fab!(:invite) { Fabricate(:invite) } + fab!(:group) { Fabricate(:group) } + fab!(:secured_category) do + secured_category = Fabricate(:category) + secured_category.permissions = { group.name => :full } + secured_category.save! + secured_category + end + + it 'does not return any warnings for simple invites' do + expect(invite.warnings(admin.guardian)).to be_blank + end + + it 'returns a warning if topic is private' do + topic = Fabricate(:topic, category: secured_category) + TopicInvite.create!(topic: topic, invite: invite) + + expect(invite.warnings(admin.guardian)).to contain_exactly(I18n.t("invite.requires_groups", groups: group.name)) + end + end end