FEATURE: Show error if invite to topic is invalid (#15959)

This can happen if the topic to which a user is invited is in a private
category and the user was not invited to one of the groups that can see
that specific category.

This used to be a warning and this commit makes it an error.

app/assets/javascripts/discourse/app/controllers/create-invite.js

@@ -121,7 +121,7 @@ export default Controller.extend(
 
       return this.invite
         .save(data)
-        .then((result) => {
+        .then(() => {
           this.rollbackBuffer();
 
           if (
@@ -131,22 +131,14 @@ export default Controller.extend(
             this.invites.unshiftObject(this.invite);
           }
 
-          if (result.warnings) {
+          if (this.isEmail && opts.sendEmail) {
+            this.send("closeModal");
+          } else {
             this.setProperties({
-              flashText: sanitize(result.warnings.join(",")),
-              flashClass: "warning",
+              flashText: sanitize(I18n.t("user.invited.invite.invite_saved")),
+              flashClass: "success",
               flashLink: !this.editing,
             });
-          } else {
-            if (this.isEmail && opts.sendEmail) {
-              this.send("closeModal");
-            } else {
-              this.setProperties({
-                flashText: sanitize(I18n.t("user.invited.invite.invite_saved")),
-                flashClass: "success",
-                flashLink: !this.editing,
-              });
-            }
           }
         })
         .catch((e) =>

app/assets/javascripts/discourse/app/controllers/share-topic.js

@@ -105,7 +105,10 @@ export default Controller.extend(
     inviteUsers() {
       this.set("showNotifyUsers", false);
       const controller = showModal("create-invite");
-      controller.set("inviteToTopic", true);
+      controller.setProperties({
+        inviteToTopic: true,
+        topics: [this.topic],
+      });
       controller.buffered.setProperties({
         topicId: this.topic.id,
         topicTitle: this.topic.title,

app/controllers/invites_controller.rb

@@ -138,6 +138,11 @@ class InvitesController < ApplicationController
 
     guardian.ensure_can_invite_to_forum!(groups)
 
+    if !groups_can_see_topic?(groups, topic)
+      editable_topic_groups = topic.category.groups.filter { |g| guardian.can_edit_group?(g) }
+      return render_json_error(I18n.t("invite.requires_groups", groups: editable_topic_groups.pluck(:name).join(", ")))
+    end
+
     begin
       invite = Invite.generate(current_user,
         email: params[:email],
@@ -201,6 +206,11 @@ class InvitesController < ApplicationController
         groups.each { |group| invite.invited_groups.find_or_create_by!(group_id: group.id) } if groups.present?
       end
 
+      if !groups_can_see_topic?(invite.groups, invite.topics.first)
+        editable_topic_groups = invite.topics.first.category.groups.filter { |g| guardian.can_edit_group?(g) }
+        return render_json_error(I18n.t("invite.requires_groups", groups: editable_topic_groups.pluck(:name).join(", ")))
+      end
+
       if params.has_key?(:email)
         old_email = invite.email.presence
         new_email = params[:email].presence
@@ -448,6 +458,15 @@ class InvitesController < ApplicationController
     end
   end
 
+  def groups_can_see_topic?(groups, topic)
+    if topic&.read_restricted_category?
+      topic_groups = topic.category.groups
+      return false if (groups & topic_groups).blank?
+    end
+
+    true
+  end
+
   def post_process_invite(user)
     user.enqueue_welcome_message('welcome_invite') if user.send_welcome_message
 

app/models/invite.rb

@@ -239,23 +239,6 @@ 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

app/serializers/invite_serializer.rb

@@ -13,8 +13,7 @@ class InviteSerializer < ApplicationSerializer
              :created_at,
              :updated_at,
              :expires_at,
-             :expired,
-             :warnings
+             :expired
 
   has_many :topics, embed: :object, serializer: BasicTopicSerializer
   has_many :groups, embed: :object, serializer: BasicGroupSerializer
@@ -46,12 +45,4 @@ class InviteSerializer < ApplicationSerializer
   def expired
     object.expired?
   end
-
-  def warnings
-    object.warnings(scope)
-  end
-
-  def include_warnings?
-    object.warnings(scope).present?
-  end
 end

config/locales/server.en.yml

@@ -258,7 +258,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}."
+    requires_groups: "Invite was not saved because the specified topic is inaccessible. Add one of the following groups: %{groups}."
     domain_not_allowed: "Your email cannot be used to redeem this invite."
 
   bulk_invite:

spec/models/invite_spec.rb

@@ -429,27 +429,4 @@ 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

spec/requests/invites_controller_spec.rb

@@ -203,6 +203,35 @@ describe InvitesController do
         post '/invites.json', params: { email: 'test@example.com', topic_id: -9999 }
         expect(response.status).to eq(400)
       end
+
+      context 'topic is private' do
+        fab!(:group) { Fabricate(:group) }
+
+        fab!(:secured_category) do |category|
+          category = Fabricate(:category)
+          category.permissions = { group.name => :full }
+          category.save!
+          category
+        end
+
+        fab!(:topic) { Fabricate(:topic, category: secured_category) }
+
+        it 'does not work and returns a list of required groups' do
+          sign_in(admin)
+
+          post '/invites.json', params: { email: 'test@example.com', topic_id: topic.id }
+          expect(response.status).to eq(422)
+          expect(response.parsed_body["errors"]).to contain_exactly(I18n.t("invite.requires_groups", groups: group.name))
+        end
+
+        it 'does not work if user cannot edit groups' do
+          group.add(user)
+          sign_in(user)
+
+          post '/invites.json', params: { email: 'test@example.com', topic_id: topic.id }
+          expect(response.status).to eq(403)
+        end
+      end
     end
 
     context 'invite to group' do