Move the meta attribute to ChatChannelSerializer

I'm aware of N+1

lib/guardian/category_guardian.rb

@@ -49,7 +49,6 @@ module CategoryGuardian
     return false unless category
     return false if is_anonymous?
     return true if is_admin?
-    return true if !category.read_restricted
     Category.post_create_allowed(self).exists?(id: category.id)
   end
 

plugins/chat/app/jobs/regular/auto_join_channel_batch.rb

@@ -30,10 +30,11 @@ module Jobs
         last_seen_at: 3.months.ago,
         channel_category: channel.chatable_id,
         mode: UserChatChannelMembership.join_modes[:automatic],
+        permission_type: CategoryGroup.permission_types[:create_post],
+        everyone: Group::AUTO_GROUPS[:everyone],
       }
 
       new_member_ids = DB.query_single(create_memberships_query(category), query_args)
-
       # Only do this if we are running auto-join for a single user, if we
       # are doing it for many then we should do it after all batches are
       # complete for the channel in Jobs::AutoManageChannelMemberships
@@ -54,28 +55,25 @@ module Jobs
         INNER JOIN user_options uo ON uo.user_id = users.id
         LEFT OUTER JOIN user_chat_channel_memberships uccm ON
           uccm.chat_channel_id = :chat_channel_id AND uccm.user_id = users.id
-      SQL
 
-      query += <<~SQL if category.read_restricted?
-          INNER JOIN group_users gu ON gu.user_id = users.id
-          LEFT OUTER JOIN category_groups cg ON cg.group_id = gu.group_id
-        SQL
+        LEFT OUTER JOIN group_users gu ON gu.user_id = users.id
+        LEFT OUTER JOIN category_groups cg ON cg.group_id = gu.group_id AND
+        cg.permission_type <= :permission_type
 
-      query += <<~SQL
         WHERE (users.id >= :start AND users.id <= :end) AND
-          users.staged IS FALSE AND users.active AND
+          users.staged IS FALSE AND
+          users.active AND
           NOT EXISTS(SELECT 1 FROM anonymous_users a WHERE a.user_id = users.id) AND
           (suspended_till IS NULL OR suspended_till <= :suspended_until) AND
-          (last_seen_at > :last_seen_at) AND
+          (last_seen_at IS NULL OR last_seen_at > :last_seen_at) AND
           uo.chat_enabled AND
-          uccm.id IS NULL
+          uccm.id IS NULL AND
+          (NOT EXISTS(SELECT 1 FROM category_groups WHERE category_id = :channel_category) 
+            OR EXISTS (SELECT 1 FROM category_groups WHERE category_id = :channel_category AND group_id = :everyone AND permission_type <= :permission_type)
+            OR cg.category_id = :channel_category)
+
+        RETURNING user_chat_channel_memberships.user_id
       SQL
-
-      query += <<~SQL if category.read_restricted?
-          AND cg.category_id = :channel_category
-        SQL
-
-      query += "RETURNING user_chat_channel_memberships.user_id"
     end
   end
 end

plugins/chat/app/serializers/chat_channel_serializer.rb

@@ -117,6 +117,7 @@ class ChatChannelSerializer < ApplicationSerializer
           @opts[:new_mentions_message_bus_last_id] ||
             MessageBus.last_id(ChatPublisher.new_mentions_message_bus_channel(object.id)),
       },
+      can_join_chat_channel: scope.can_join_chat_channel?(object),
     }
   end
 

plugins/chat/assets/javascripts/discourse/components/chat-channel-preview-card.hbs

@@ -2,6 +2,7 @@
   class={{concat-class
     "chat-channel-preview-card"
     (unless this.hasDescription "-no-description")
+    (unless (and this.isOpen this.canJoin) "-no-button")
   }}
 >
   <ChatChannelTitle @channel={{this.channel}} />
@@ -10,7 +11,7 @@
       {{this.channel.description}}
     </p>
   {{/if}}
-  {{#if this.showJoinButton}}
+  {{#if (and this.isOpen this.canJoin)}}
     <ToggleChannelMembershipButton
       @channel={{this.channel}}
       @options={{hash joinClass="btn-primary"}}

plugins/chat/assets/javascripts/discourse/components/chat-channel-preview-card.js

@@ -10,7 +10,8 @@ export default class ChatChannelPreviewCard extends Component {
 
   channel = null;
 
-  @readOnly("channel.isOpen") showJoinButton;
+  @readOnly("channel.isOpen") isOpen;
+  @readOnly("channel.canJoin") canJoin;
 
   @computed("channel.description")
   get hasDescription() {

plugins/chat/assets/javascripts/discourse/models/chat-channel.js

@@ -119,6 +119,10 @@ export default class ChatChannel extends RestModel {
     return this.currentUserMembership.following;
   }
 
+  get canJoin() {
+    return this.meta.can_join_chat_channel;
+  }
+
   canModifyMessages(user) {
     if (user.staff) {
       return !STAFF_READONLY_STATUSES.includes(this.status);

plugins/chat/assets/stylesheets/common/chat-channel-preview-card.scss

@@ -12,6 +12,12 @@
     }
   }
 
+  &.-no-button {
+    .chat-channel-preview-card__browse-all {
+      margin-top: 0;
+    }
+  }
+
   &__description {
     color: var(--primary-600);
     text-align: center;

plugins/chat/lib/chat_channel_fetcher.rb

@@ -251,7 +251,7 @@ module Chat::ChatChannelFetcher
     end
 
     raise Discourse::NotFound if chat_channel.blank?
-    raise Discourse::InvalidAccess if !guardian.can_join_chat_channel?(chat_channel)
+    raise Discourse::InvalidAccess if !guardian.can_preview_chat_channel?(chat_channel)
     chat_channel
   end
 end

plugins/chat/spec/jobs/regular/auto_join_channel_batch_spec.rb

@@ -175,11 +175,35 @@ describe Jobs::AutoJoinChannelBatch do
 
         assert_users_follows_channel(channel, [user, another_user])
       end
+
+      it "doesn't join users with read-only access to the category" do
+        restricted_category = Fabricate(:category, read_restricted: true)
+        another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
+        non_chatters_group = Fabricate(:group)
+        readonly_channel =
+          Fabricate(:category_channel, chatable: restricted_category, auto_join_users: true)
+        Fabricate(
+          :category_group,
+          category: restricted_category,
+          group: non_chatters_group,
+          permission_type: CategoryGroup.permission_types[:readonly],
+        )
+        non_chatters_group.add(another_user)
+
+        subject.execute(
+          chat_channel_id: readonly_channel.id,
+          starts_at: another_user.id,
+          ends_at: another_user.id,
+        )
+
+        assert_user_skipped(readonly_channel, another_user)
+      end
     end
   end
 
   def assert_users_follows_channel(channel, users)
     new_memberships = UserChatChannelMembership.where(user: users, chat_channel: channel)
+    expect(new_memberships.length).to eq(users.length)
     expect(new_memberships.all?(&:following)).to eq(true)
   end
 

plugins/chat/spec/requests/chat_controller_spec.rb

@@ -1312,7 +1312,7 @@ RSpec.describe Chat::ChatController do
       channel = Fabricate(:category_channel, chatable: Fabricate(:category))
       message = Fabricate(:chat_message, chat_channel: channel)
 
-      Guardian.any_instance.expects(:can_join_chat_channel?).with(channel)
+      Guardian.any_instance.expects(:can_preview_chat_channel?).with(channel)
 
       sign_in(Fabricate(:user))
       get "/chat/message/#{message.id}.json"
@@ -1328,7 +1328,7 @@ RSpec.describe Chat::ChatController do
     before { sign_in(user) }
 
     it "ensures message's channel can be seen" do
-      Guardian.any_instance.expects(:can_join_chat_channel?).with(channel)
+      Guardian.any_instance.expects(:can_preview_chat_channel?).with(channel)
       get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
     end
 

plugins/chat/spec/support/api/schemas/user_chat_channel_membership.json

@@ -26,6 +26,9 @@
         "avatar_template": { "type": "string" },
         "username": { "type": "string" }
       }
+    },
+    "meta": {
+      "can_join_chat_channel": { "type": "boolean" }
     }
   }
 }

plugins/chat/spec/system/jit_messages_spec.rb

@@ -14,21 +14,74 @@ RSpec.describe "JIT messages", type: :system, js: true do
     sign_in(current_user)
   end
 
-  context "when mentioning a user not on the channel" do
-    it "displays a mention warning" do
-      Jobs.run_immediately!
+  context "when mentioning a user" do
+    context "when user is not on the channel" do
+      it "displays a mention warning" do
+        Jobs.run_immediately!
 
-      chat.visit_channel(channel_1)
-      channel.send_message("hi @#{other_user.username}")
+        chat.visit_channel(channel_1)
+        channel.send_message("hi @#{other_user.username}")
 
-      expect(page).to have_content(
-        I18n.t("js.chat.mention_warning.without_membership.one", username: other_user.username),
-        wait: 5,
-      )
+        expect(page).to have_content(
+          I18n.t("js.chat.mention_warning.without_membership.one", username: other_user.username),
+          wait: 5,
+        )
+      end
+    end
+
+    context "when user can’t access the channel" do
+      fab!(:group_1) { Fabricate(:group) }
+      fab!(:private_channel_1) { Fabricate(:private_category_channel, group: group_1) }
+
+      before do
+        group_1.add(current_user)
+        private_channel_1.add(current_user)
+      end
+
+      it "displays a mention warning" do
+        Jobs.run_immediately!
+
+        chat.visit_channel(private_channel_1)
+        channel.send_message("hi @#{other_user.username}")
+
+        expect(page).to have_content(
+          I18n.t("js.chat.mention_warning.cannot_see.one", username: other_user.username),
+          wait: 5,
+        )
+      end
+    end
+
+    context "when user can’t access a non read_restrictd channel" do
+      let!(:everyone) { Group.find(Group::AUTO_GROUPS[:everyone]) }
+      fab!(:category) { Fabricate(:category) }
+      fab!(:readonly_channel) { Fabricate(:category_channel, chatable: category) }
+
+      before do
+        Fabricate(
+          :category_group,
+          category: category,
+          group: everyone,
+          permission_type: CategoryGroup.permission_types[:readonly],
+        )
+        everyone.add(other_user)
+        readonly_channel.add(current_user)
+      end
+
+      it "displays a mention warning" do
+        Jobs.run_immediately!
+
+        chat.visit_channel(readonly_channel)
+        channel.send_message("hi @#{other_user.username}")
+
+        expect(page).to have_content(
+          I18n.t("js.chat.mention_warning.cannot_see.one", username: other_user.username),
+          wait: 5,
+        )
+      end
     end
   end
 
-  context "when mentioning a user who can’t access the channel" do
+  context "when category channel permission is readonly for everyone" do
     fab!(:group_1) { Fabricate(:group) }
     fab!(:private_channel_1) { Fabricate(:private_category_channel, group: group_1) }
 

plugins/chat/spec/system/visit_channel_spec.rb

@@ -93,6 +93,37 @@ RSpec.describe "Visit channel", type: :system, js: true do
         end
       end
 
+      context "when category channel is read-only" do
+        fab!(:restricted_category) { Fabricate(:category, read_restricted: true) }
+        fab!(:readonly_group_1) { Fabricate(:group, users: [current_user]) }
+        fab!(:readonly_category_channel_1) do
+          Fabricate(:category_channel, chatable: restricted_category)
+        end
+        fab!(:message_1) { Fabricate(:chat_message, chat_channel: readonly_category_channel_1) }
+
+        before do
+          Fabricate(
+            :category_group,
+            category: restricted_category,
+            group: readonly_group_1,
+            permission_type: CategoryGroup.permission_types[:readonly],
+          )
+        end
+
+        it "doesn't allow user to join it" do
+          chat.visit_channel(readonly_category_channel_1)
+
+          expect(page).not_to have_content(I18n.t("js.chat.channel_settings.join_channel"))
+        end
+
+        it "shows a preview of the channel" do
+          chat.visit_channel(readonly_category_channel_1)
+
+          expect(page).to have_content(readonly_category_channel_1.name)
+          expect(chat).to have_message(message_1)
+        end
+      end
+
       context "when current user is not member of the channel" do
         context "when category channel" do
           fab!(:message_1) { Fabricate(:chat_message, chat_channel: category_channel_1) }