From 6f15403f9165de6740d6f536e33971f91c655d48 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 09:30:27 +1000
Subject: [PATCH 1/9] WIP: Mark all chat channels read

---
 .../chat/app/controllers/api_controller.rb    |  13 +
 .../chat/api/chat_tracking_controller.rb      |   8 +
 .../app/controllers/chat_base_controller.rb   |  36 ++
 .../chat/app/controllers/chat_controller.rb   | 447 ++++++++++++++++++
 .../app/helpers/chat/with_service_helper.rb   |   3 +
 .../app/services/update_user_last_read.rb     |  84 ++++
 .../initializers/chat-keyboard-shortcuts.js   |  24 +
 .../discourse/services/chat-api.js            |   7 +
 .../services/chat-channels-manager.js         |  11 +
 plugins/chat/config/locales/client.en.yml     |   1 +
 plugins/chat/lib/service_runner.rb            |   2 +
 plugins/chat/plugin.rb                        |  19 -
 12 files changed, 636 insertions(+), 19 deletions(-)
 create mode 100644 plugins/chat/app/controllers/api_controller.rb
 create mode 100644 plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
 create mode 100644 plugins/chat/app/controllers/chat_base_controller.rb
 create mode 100644 plugins/chat/app/controllers/chat_controller.rb
 create mode 100644 plugins/chat/app/services/update_user_last_read.rb

diff --git a/plugins/chat/app/controllers/api_controller.rb b/plugins/chat/app/controllers/api_controller.rb
new file mode 100644
index 0000000000..fa27b825d8
--- /dev/null
+++ b/plugins/chat/app/controllers/api_controller.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class Chat::Api < Chat::ChatBaseController
+  before_action :ensure_logged_in
+  before_action :ensure_can_chat
+
+  private
+
+  def ensure_can_chat
+    raise Discourse::NotFound unless SiteSetting.chat_enabled
+    guardian.ensure_can_chat!
+  end
+end
diff --git a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
new file mode 100644
index 0000000000..3372de394a
--- /dev/null
+++ b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class Chat::Api::ChatTrackingController < Chat::Api
+  def read
+    channel_id = params[:channel_id]
+    message_id = params[:message_id]
+  end
+end
diff --git a/plugins/chat/app/controllers/chat_base_controller.rb b/plugins/chat/app/controllers/chat_base_controller.rb
new file mode 100644
index 0000000000..789491507a
--- /dev/null
+++ b/plugins/chat/app/controllers/chat_base_controller.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class Chat::ChatBaseController < ::ApplicationController
+  before_action :ensure_logged_in
+  before_action :ensure_can_chat
+
+  include Chat::WithServiceHelper
+
+  private
+
+  def ensure_can_chat
+    raise Discourse::NotFound unless SiteSetting.chat_enabled
+    guardian.ensure_can_chat!
+  end
+
+  def set_channel_and_chatable_with_access_check(chat_channel_id: nil)
+    params.require(:chat_channel_id) if chat_channel_id.blank?
+    id_or_name = chat_channel_id || params[:chat_channel_id]
+    @chat_channel = Chat::ChatChannelFetcher.find_with_access_check(id_or_name, guardian)
+    @chatable = @chat_channel.chatable
+  end
+
+  def default_actions_for_service
+    proc do
+      on_success { render(json: success_json) }
+      on_failure { render(json: failed_json, status: 422) }
+      on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+      on_failed_contract do
+        render(
+          json: failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
+          status: 400,
+        )
+      end
+    end
+  end
+end
diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb
new file mode 100644
index 0000000000..afe131e897
--- /dev/null
+++ b/plugins/chat/app/controllers/chat_controller.rb
@@ -0,0 +1,447 @@
+# frozen_string_literal: true
+
+class Chat::ChatController < Chat::ChatBaseController
+  PAST_MESSAGE_LIMIT = 40
+  FUTURE_MESSAGE_LIMIT = 40
+  PAST = "past"
+  FUTURE = "future"
+  CHAT_DIRECTIONS = [PAST, FUTURE]
+
+  # Other endpoints use set_channel_and_chatable_with_access_check, but
+  # these endpoints require a standalone find because they need to be
+  # able to get deleted channels and recover them.
+  before_action :find_chatable, only: %i[enable_chat disable_chat]
+  before_action :find_chat_message,
+                only: %i[delete restore lookup_message edit_message rebake message_link]
+  before_action :set_channel_and_chatable_with_access_check,
+                except: %i[
+                  respond
+                  enable_chat
+                  disable_chat
+                  message_link
+                  lookup_message
+                  set_user_chat_status
+                  dismiss_retention_reminder
+                  flag
+                ]
+
+  def respond
+    render
+  end
+
+  def enable_chat
+    chat_channel = ChatChannel.with_deleted.find_by(chatable: @chatable)
+
+    guardian.ensure_can_join_chat_channel!(chat_channel) if chat_channel
+
+    if chat_channel && chat_channel.trashed?
+      chat_channel.recover!
+    elsif chat_channel
+      return render_json_error I18n.t("chat.already_enabled")
+    else
+      chat_channel = @chatable.chat_channel
+      guardian.ensure_can_join_chat_channel!(chat_channel)
+    end
+
+    success = chat_channel.save
+    if success && chat_channel.chatable_has_custom_fields?
+      @chatable.custom_fields[Chat::HAS_CHAT_ENABLED] = true
+      @chatable.save!
+    end
+
+    if success
+      membership = Chat::ChatChannelMembershipManager.new(channel).follow(user)
+      render_serialized(chat_channel, ChatChannelSerializer, membership: membership)
+    else
+      render_json_error(chat_channel)
+    end
+
+    Chat::ChatChannelMembershipManager.new(channel).follow(user)
+  end
+
+  def disable_chat
+    chat_channel = ChatChannel.with_deleted.find_by(chatable: @chatable)
+    guardian.ensure_can_join_chat_channel!(chat_channel)
+    return render json: success_json if chat_channel.trashed?
+    chat_channel.trash!(current_user)
+
+    success = chat_channel.save
+    if success
+      if chat_channel.chatable_has_custom_fields?
+        @chatable.custom_fields.delete(Chat::HAS_CHAT_ENABLED)
+        @chatable.save!
+      end
+
+      render json: success_json
+    else
+      render_json_error(chat_channel)
+    end
+  end
+
+  def create_message
+    raise Discourse::InvalidAccess if current_user.silenced?
+
+    Chat::ChatMessageRateLimiter.run!(current_user)
+
+    @user_chat_channel_membership =
+      Chat::ChatChannelMembershipManager.new(@chat_channel).find_for_user(
+        current_user,
+        following: true,
+      )
+    raise Discourse::InvalidAccess unless @user_chat_channel_membership
+
+    reply_to_msg_id = params[:in_reply_to_id]
+    if reply_to_msg_id
+      rm = ChatMessage.find(reply_to_msg_id)
+      raise Discourse::NotFound if rm.chat_channel_id != @chat_channel.id
+    end
+
+    content = params[:message]
+
+    chat_message_creator =
+      Chat::ChatMessageCreator.create(
+        chat_channel: @chat_channel,
+        user: current_user,
+        in_reply_to_id: reply_to_msg_id,
+        content: content,
+        staged_id: params[:staged_id],
+        upload_ids: params[:upload_ids],
+      )
+
+    return render_json_error(chat_message_creator.error) if chat_message_creator.failed?
+
+    @user_chat_channel_membership.update!(
+      last_read_message_id: chat_message_creator.chat_message.id,
+    )
+
+    if @chat_channel.direct_message_channel?
+      # If any of the channel users is ignoring, muting, or preventing DMs from
+      # the current user then we shold not auto-follow the channel once again or
+      # publish the new channel.
+      user_ids_allowing_communication =
+        UserCommScreener.new(
+          acting_user: current_user,
+          target_user_ids: @chat_channel.user_chat_channel_memberships.pluck(:user_id),
+        ).allowing_actor_communication
+
+      if user_ids_allowing_communication.any?
+        ChatPublisher.publish_new_channel(
+          @chat_channel,
+          @chat_channel.chatable.users.where(id: user_ids_allowing_communication),
+        )
+
+        @chat_channel
+          .user_chat_channel_memberships
+          .where(user_id: user_ids_allowing_communication)
+          .update_all(following: true)
+      end
+    end
+
+    ChatPublisher.publish_user_tracking_state(
+      current_user,
+      @chat_channel.id,
+      chat_message_creator.chat_message.id,
+    )
+    render json: success_json
+  end
+
+  def edit_message
+    chat_message_updater =
+      Chat::ChatMessageUpdater.update(
+        guardian: guardian,
+        chat_message: @message,
+        new_content: params[:new_message],
+        upload_ids: params[:upload_ids] || [],
+      )
+
+    return render_json_error(chat_message_updater.error) if chat_message_updater.failed?
+
+    render json: success_json
+  end
+
+  def update_user_last_read
+    with_service(Chat::Service::UpdateUserLastRead, channel_id: params[:chat_channel_id]) do
+      on_failed_policy(:ensure_message_id_recency) do
+        raise Discourse::InvalidParameters.new(:message_id)
+      end
+      on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound }
+      on_model_not_found(:active_membership) { raise Discourse::NotFound }
+      on_model_not_found(:channel) { raise Discourse::NotFound }
+    end
+  end
+
+  def messages
+    page_size = params[:page_size]&.to_i || 1000
+    direction = params[:direction].to_s
+    message_id = params[:message_id]
+    if page_size > 50 ||
+         (
+           message_id.blank? ^ direction.blank? &&
+             (direction.present? && !CHAT_DIRECTIONS.include?(direction))
+         )
+      raise Discourse::InvalidParameters
+    end
+
+    messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
+    messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
+
+    if message_id.present?
+      condition = direction == PAST ? "<" : ">"
+      messages = messages.where("id #{condition} ?", message_id.to_i)
+    end
+
+    # NOTE: This order is reversed when we return the ChatView below if the direction
+    # is not FUTURE.
+    order = direction == FUTURE ? "ASC" : "DESC"
+    messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a
+
+    can_load_more_past = nil
+    can_load_more_future = nil
+
+    if direction == FUTURE
+      can_load_more_future = messages.size == page_size
+    elsif direction == PAST
+      can_load_more_past = messages.size == page_size
+    else
+      # When direction is blank, we'll return the latest messages.
+      can_load_more_future = false
+      can_load_more_past = messages.size == page_size
+    end
+
+    chat_view =
+      ChatView.new(
+        chat_channel: @chat_channel,
+        chat_messages: direction == FUTURE ? messages : messages.reverse,
+        user: current_user,
+        can_load_more_past: can_load_more_past,
+        can_load_more_future: can_load_more_future,
+      )
+    render_serialized(chat_view, ChatViewSerializer, root: false)
+  end
+
+  def react
+    params.require(%i[message_id emoji react_action])
+    guardian.ensure_can_react!
+
+    Chat::ChatMessageReactor.new(current_user, @chat_channel).react!(
+      message_id: params[:message_id],
+      react_action: params[:react_action].to_sym,
+      emoji: params[:emoji],
+    )
+
+    render json: success_json
+  end
+
+  def delete
+    guardian.ensure_can_delete_chat!(@message, @chatable)
+
+    ChatMessageDestroyer.new.trash_message(@message, current_user)
+
+    head :ok
+  end
+
+  def restore
+    chat_channel = @message.chat_channel
+    guardian.ensure_can_restore_chat!(@message, chat_channel.chatable)
+    updated = @message.recover!
+    if updated
+      ChatPublisher.publish_restore!(chat_channel, @message)
+      render json: success_json
+    else
+      render_json_error(@message)
+    end
+  end
+
+  def rebake
+    guardian.ensure_can_rebake_chat_message!(@message)
+    @message.rebake!(invalidate_oneboxes: true)
+    render json: success_json
+  end
+
+  def message_link
+    raise Discourse::NotFound if @message.blank? || @message.deleted_at.present?
+    raise Discourse::NotFound if @message.chat_channel.blank?
+    set_channel_and_chatable_with_access_check(chat_channel_id: @message.chat_channel_id)
+    render json:
+             success_json.merge(
+               chat_channel_id: @chat_channel.id,
+               chat_channel_title: @chat_channel.title(current_user),
+             )
+  end
+
+  def lookup_message
+    set_channel_and_chatable_with_access_check(chat_channel_id: @message.chat_channel_id)
+
+    messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
+    messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
+
+    past_messages =
+      messages
+        .where("created_at < ?", @message.created_at)
+        .order(created_at: :desc)
+        .limit(PAST_MESSAGE_LIMIT)
+
+    future_messages =
+      messages
+        .where("created_at > ?", @message.created_at)
+        .order(created_at: :asc)
+        .limit(FUTURE_MESSAGE_LIMIT)
+
+    can_load_more_past = past_messages.count == PAST_MESSAGE_LIMIT
+    can_load_more_future = future_messages.count == FUTURE_MESSAGE_LIMIT
+    messages = [past_messages.reverse, [@message], future_messages].reduce([], :concat)
+    chat_view =
+      ChatView.new(
+        chat_channel: @chat_channel,
+        chat_messages: messages,
+        user: current_user,
+        can_load_more_past: can_load_more_past,
+        can_load_more_future: can_load_more_future,
+      )
+    render_serialized(chat_view, ChatViewSerializer, root: false)
+  end
+
+  def set_user_chat_status
+    params.require(:chat_enabled)
+
+    current_user.user_option.update(chat_enabled: params[:chat_enabled])
+    render json: { chat_enabled: current_user.user_option.chat_enabled }
+  end
+
+  def invite_users
+    params.require(:user_ids)
+
+    users =
+      User
+        .includes(:groups)
+        .joins(:user_option)
+        .where(user_options: { chat_enabled: true })
+        .not_suspended
+        .where(id: params[:user_ids])
+    users.each do |user|
+      guardian = Guardian.new(user)
+      if guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
+        data = {
+          message: "chat.invitation_notification",
+          chat_channel_id: @chat_channel.id,
+          chat_channel_title: @chat_channel.title(user),
+          chat_channel_slug: @chat_channel.slug,
+          invited_by_username: current_user.username,
+        }
+        data[:chat_message_id] = params[:chat_message_id] if params[:chat_message_id]
+        user.notifications.create(
+          notification_type: Notification.types[:chat_invitation],
+          high_priority: true,
+          data: data.to_json,
+        )
+      end
+    end
+
+    render json: success_json
+  end
+
+  def dismiss_retention_reminder
+    params.require(:chatable_type)
+    guardian.ensure_can_chat!
+    unless ChatChannel.chatable_types.include?(params[:chatable_type])
+      raise Discourse::InvalidParameters
+    end
+
+    field =
+      (
+        if ChatChannel.public_channel_chatable_types.include?(params[:chatable_type])
+          :dismissed_channel_retention_reminder
+        else
+          :dismissed_dm_retention_reminder
+        end
+      )
+    current_user.user_option.update(field => true)
+    render json: success_json
+  end
+
+  def quote_messages
+    params.require(:message_ids)
+
+    message_ids = params[:message_ids].map(&:to_i)
+    markdown =
+      ChatTranscriptService.new(
+        @chat_channel,
+        current_user,
+        messages_or_ids: message_ids,
+      ).generate_markdown
+    render json: success_json.merge(markdown: markdown)
+  end
+
+  def flag
+    RateLimiter.new(current_user, "flag_chat_message", 4, 1.minutes).performed!
+
+    permitted_params =
+      params.permit(
+        %i[chat_message_id flag_type_id message is_warning take_action queue_for_review],
+      )
+
+    chat_message =
+      ChatMessage.includes(:chat_channel, :revisions).find(permitted_params[:chat_message_id])
+
+    flag_type_id = permitted_params[:flag_type_id].to_i
+
+    if !ReviewableScore.types.values.include?(flag_type_id)
+      raise Discourse::InvalidParameters.new(:flag_type_id)
+    end
+
+    set_channel_and_chatable_with_access_check(chat_channel_id: chat_message.chat_channel_id)
+
+    result =
+      Chat::ChatReviewQueue.new.flag_message(chat_message, guardian, flag_type_id, permitted_params)
+
+    if result[:success]
+      render json: success_json
+    else
+      render_json_error(result[:errors])
+    end
+  end
+
+  def set_draft
+    if params[:data].present?
+      ChatDraft.find_or_initialize_by(
+        user: current_user,
+        chat_channel_id: @chat_channel.id,
+      ).update!(data: params[:data])
+    else
+      ChatDraft.where(user: current_user, chat_channel_id: @chat_channel.id).destroy_all
+    end
+
+    render json: success_json
+  end
+
+  private
+
+  def preloaded_chat_message_query
+    query =
+      ChatMessage
+        .includes(in_reply_to: [:user, chat_webhook_event: [:incoming_chat_webhook]])
+        .includes(:revisions)
+        .includes(user: :primary_group)
+        .includes(chat_webhook_event: :incoming_chat_webhook)
+        .includes(reactions: :user)
+        .includes(:bookmarks)
+        .includes(:uploads)
+        .includes(chat_channel: :chatable)
+
+    query = query.includes(user: :user_status) if SiteSetting.enable_user_status
+
+    query
+  end
+
+  def find_chatable
+    @chatable = Category.find_by(id: params[:chatable_id])
+    guardian.ensure_can_moderate_chat!(@chatable)
+  end
+
+  def find_chat_message
+    @message = preloaded_chat_message_query.with_deleted
+    @message = @message.where(chat_channel_id: params[:chat_channel_id]) if params[:chat_channel_id]
+    @message = @message.find_by(id: params[:message_id])
+    raise Discourse::NotFound unless @message
+  end
+end
diff --git a/plugins/chat/app/helpers/chat/with_service_helper.rb b/plugins/chat/app/helpers/chat/with_service_helper.rb
index c8e820cc2c..e9c3191cf8 100644
--- a/plugins/chat/app/helpers/chat/with_service_helper.rb
+++ b/plugins/chat/app/helpers/chat/with_service_helper.rb
@@ -5,6 +5,9 @@ module Chat
       @_result
     end
 
+    # @param service [Class] A class including {Chat::Service::Base}
+    # @param dependencies [kwargs] Any additional params to load into the service context,
+    #                              in addition to controller @params.
     def with_service(service, default_actions: true, **dependencies, &block)
       object = self
       merged_block =
diff --git a/plugins/chat/app/services/update_user_last_read.rb b/plugins/chat/app/services/update_user_last_read.rb
new file mode 100644
index 0000000000..914b484602
--- /dev/null
+++ b/plugins/chat/app/services/update_user_last_read.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+module Chat
+  module Service
+    # Service responsible for updating the last read message id of a membership.
+    #
+    # @example
+    #  Chat::Service::UpdateUserLastRead.call(channel_id: 2, message_id: 3, guardian: guardian)
+    #
+    class UpdateUserLastRead
+      include Base
+
+      # @!method call(user_id:, channel_id:, message_id:, guardian:)
+      #   @param [Integer] channel_id
+      #   @param [Integer] message_id
+      #   @param [Guardian] guardian
+      #   @return [Chat::Service::Base::Context]
+
+      contract
+      model :channel
+      model :active_membership
+      policy :invalid_access
+      policy :ensure_message_exists
+      policy :ensure_message_id_recency
+      step :update_last_read_message_id
+      step :mark_associated_mentions_as_read
+      step :publish_new_last_read_to_clients
+
+      # @!visibility private
+      class Contract
+        attribute :message_id, :integer
+        attribute :channel_id, :integer
+
+        validates :message_id, :channel_id, presence: true
+      end
+
+      private
+
+      def fetch_channel(contract:, **)
+        ChatChannel.find_by(id: contract.channel_id)
+      end
+
+      def fetch_active_membership(guardian:, channel:, **)
+        Chat::ChatChannelMembershipManager.new(channel).find_for_user(
+          guardian.user,
+          following: true,
+        )
+      end
+
+      def invalid_access(guardian:, active_membership:, **)
+        guardian.can_join_chat_channel?(active_membership.chat_channel)
+      end
+
+      def ensure_message_exists(channel:, contract:, **)
+        ChatMessage.with_deleted.exists?(chat_channel_id: channel.id, id: contract.message_id)
+      end
+
+      def ensure_message_id_recency(contract:, active_membership:, **)
+        !active_membership.last_read_message_id ||
+          contract.message_id >= active_membership.last_read_message_id
+      end
+
+      def update_last_read_message_id(contract:, active_membership:, **)
+        active_membership.update!(last_read_message_id: contract.message_id)
+      end
+
+      def mark_associated_mentions_as_read(active_membership:, contract:, **)
+        Notification
+          .where(notification_type: Notification.types[:chat_mention])
+          .where(user: active_membership.user)
+          .where(read: false)
+          .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
+          .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
+          .where("chat_messages.id <= ?", contract.message_id)
+          .where("chat_messages.chat_channel_id = ?", active_membership.chat_channel.id)
+          .update_all(read: true)
+      end
+
+      def publish_new_last_read_to_clients(guardian:, channel:, contract:, **)
+        ChatPublisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id)
+      end
+    end
+  end
+end
diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
index ae70c3e454..34a00c5ccf 100644
--- a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
+++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
@@ -17,6 +17,9 @@ export default {
     const router = container.lookup("service:router");
     const appEvents = container.lookup("service:app-events");
     const chatStateManager = container.lookup("service:chat-state-manager");
+    const chatChannelsManager = container.lookup(
+      "service:chat-channels-manager"
+    );
     const openChannelSelector = (e) => {
       e.preventDefault();
       e.stopPropagation();
@@ -92,6 +95,12 @@ export default {
       appEvents.trigger("chat:toggle-close", event);
     };
 
+    const markAllChannelsRead = (event) => {
+      event.preventDefault();
+      event.stopPropagation();
+      chatChannelsManager.markAllChannelsRead();
+    };
+
     withPluginApi("0.12.1", (api) => {
       api.addKeyboardShortcut(`${KEY_MODIFIER}+k`, openChannelSelector, {
         global: true,
@@ -201,6 +210,21 @@ export default {
           },
         },
       });
+      api.addKeyboardShortcut(
+        `shift+esc`,
+        (event) => markAllChannelsRead(event),
+        {
+          global: true,
+          help: {
+            category: "chat",
+            name: "chat.keyboard_shortcuts.mark_all_channels_read",
+            definition: {
+              keys1: ["shift", "esc"],
+              keysDelimiter: "plus",
+            },
+          },
+        }
+      );
     });
   },
 };
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
index 44ad75dc5c..d1ca13f3d9 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
@@ -282,6 +282,13 @@ export default class ChatApi extends Service {
     );
   }
 
+  updateCurrentUserTracking({ channelId, messageId } = {}) {
+    return this.#putRequest(`/tracking/read/me`, {
+      channel_id: channelId,
+      message_id: messageId,
+    });
+  }
+
   get #basePath() {
     return "/chat/api";
   }
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
index 0905f46802..66463af524 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
@@ -1,4 +1,5 @@
 import Service, { inject as service } from "@ember/service";
+import { debounce } from "discourse-common/utils/decorators";
 import Promise from "rsvp";
 import ChatChannel from "discourse/plugins/chat/discourse/models/chat-channel";
 import { tracked } from "@glimmer/tracking";
@@ -82,6 +83,16 @@ export default class ChatChannelsManager extends Service {
     });
   }
 
+  @debounce(300)
+  async markAllChannelsRead() {
+    return this.chatApi.updateCurrentUserTracking().then(() => {
+      this.channels.forEach((channel) => {
+        channel.currentUserMembership.unread_count = 0;
+        channel.currentUserMembership.unread_mentions = 0;
+      });
+    });
+  }
+
   get unreadCount() {
     let count = 0;
     this.publicMessageChannels.forEach((channel) => {
diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml
index 5925fe1e6f..b9b5f112aa 100644
--- a/plugins/chat/config/locales/client.en.yml
+++ b/plugins/chat/config/locales/client.en.yml
@@ -598,6 +598,7 @@ en:
           composer_code: "%{shortcut} Code (composer only)"
           drawer_open: "%{shortcut} Open chat drawer"
           drawer_close: "%{shortcut} Close chat drawer"
+          mark_all_channels_read: "%{shortcut} Mark all channels read"
     topic_statuses:
       chat:
         help: "Chat is enabled for this topic"
diff --git a/plugins/chat/lib/service_runner.rb b/plugins/chat/lib/service_runner.rb
index b82ff5c9dc..ad3eeafdec 100644
--- a/plugins/chat/lib/service_runner.rb
+++ b/plugins/chat/lib/service_runner.rb
@@ -19,6 +19,8 @@
 # * +on_model_not_found(name)+: will execute the provided block if the service
 #   fails and its model is not present
 #
+# Default actions for each of these are defined in [Chat::BaseController#default_actions_for_service]
+#
 # @example In a controller
 #   def create
 #     with_service MyService do
diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb
index 1f2f5b81d9..b83ad11bb7 100644
--- a/plugins/chat/plugin.rb
+++ b/plugins/chat/plugin.rb
@@ -381,25 +381,6 @@ after_initialize do
     end
   end
 
-  Discourse::Application.routes.append do
-    mount ::Chat::Engine, at: "/chat"
-
-    get "/admin/plugins/chat" => "chat/admin/incoming_webhooks#index",
-        :constraints => StaffConstraint.new
-    post "/admin/plugins/chat/hooks" => "chat/admin/incoming_webhooks#create",
-         :constraints => StaffConstraint.new
-    put "/admin/plugins/chat/hooks/:incoming_chat_webhook_id" =>
-          "chat/admin/incoming_webhooks#update",
-        :constraints => StaffConstraint.new
-    delete "/admin/plugins/chat/hooks/:incoming_chat_webhook_id" =>
-             "chat/admin/incoming_webhooks#destroy",
-           :constraints => StaffConstraint.new
-    get "u/:username/preferences/chat" => "users#preferences",
-        :constraints => {
-          username: RouteFormat.username,
-        }
-  end
-
   if defined?(DiscourseAutomation)
     add_automation_scriptable("send_chat_message") do
       field :chat_channel_id, component: :text, required: true

From cbbed325745711b230749908d01ddf0d6cc69408 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 12:00:14 +1000
Subject: [PATCH 2/9] WIP: Mark all channels as read

---
 .../chat/app/controllers/api_controller.rb    |  16 +++
 .../chat/api/chat_tracking_controller.rb      |  19 +++
 .../app/controllers/chat_base_controller.rb   |  16 ---
 .../chat/app/controllers/chat_controller.rb   |  11 --
 .../services/mark_all_user_channels_read.rb   |  66 ++++++++++
 .../app/services/update_user_last_read.rb     |   2 +-
 .../discourse/models/chat-channel.js          |   5 +-
 .../discourse/services/chat-api.js            |   9 ++
 .../services/chat-channels-manager.js         |   5 +-
 .../chat/api/chat_tracking_controller_spec.rb | 114 ++++++++++++++++++
 .../spec/requests/chat_controller_spec.rb     | 111 -----------------
 11 files changed, 232 insertions(+), 142 deletions(-)
 create mode 100644 plugins/chat/app/services/mark_all_user_channels_read.rb
 create mode 100644 plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb

diff --git a/plugins/chat/app/controllers/api_controller.rb b/plugins/chat/app/controllers/api_controller.rb
index fa27b825d8..70bf35dc60 100644
--- a/plugins/chat/app/controllers/api_controller.rb
+++ b/plugins/chat/app/controllers/api_controller.rb
@@ -4,10 +4,26 @@ class Chat::Api < Chat::ChatBaseController
   before_action :ensure_logged_in
   before_action :ensure_can_chat
 
+  include Chat::WithServiceHelper
+
   private
 
   def ensure_can_chat
     raise Discourse::NotFound unless SiteSetting.chat_enabled
     guardian.ensure_can_chat!
   end
+
+  def default_actions_for_service
+    proc do
+      on_success { render(json: success_json) }
+      on_failure { render(json: failed_json, status: 422) }
+      on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+      on_failed_contract do
+        render(
+          json: failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
+          status: 400,
+        )
+      end
+    end
+  end
 end
diff --git a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
index 3372de394a..681b05d5f4 100644
--- a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
@@ -2,7 +2,26 @@
 
 class Chat::Api::ChatTrackingController < Chat::Api
   def read
+    params.permit(:channel_id, :message_id)
+
     channel_id = params[:channel_id]
     message_id = params[:message_id]
+
+    if channel_id.present? && message_id.present?
+      with_service(Chat::Service::UpdateUserLastRead) do
+        on_failed_policy(:ensure_message_id_recency) do
+          raise Discourse::InvalidParameters.new(:message_id)
+        end
+        on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound }
+        on_model_not_found(:active_membership) { raise Discourse::NotFound }
+        on_model_not_found(:channel) { raise Discourse::NotFound }
+      end
+    else
+      with_service(Chat::Service::MarkAllUserChannelsRead) do
+        on_success do
+          render(json: success_json.merge(updated_memberships: result.updated_memberships))
+        end
+      end
+    end
   end
 end
diff --git a/plugins/chat/app/controllers/chat_base_controller.rb b/plugins/chat/app/controllers/chat_base_controller.rb
index 789491507a..6e014502dd 100644
--- a/plugins/chat/app/controllers/chat_base_controller.rb
+++ b/plugins/chat/app/controllers/chat_base_controller.rb
@@ -4,8 +4,6 @@ class Chat::ChatBaseController < ::ApplicationController
   before_action :ensure_logged_in
   before_action :ensure_can_chat
 
-  include Chat::WithServiceHelper
-
   private
 
   def ensure_can_chat
@@ -19,18 +17,4 @@ class Chat::ChatBaseController < ::ApplicationController
     @chat_channel = Chat::ChatChannelFetcher.find_with_access_check(id_or_name, guardian)
     @chatable = @chat_channel.chatable
   end
-
-  def default_actions_for_service
-    proc do
-      on_success { render(json: success_json) }
-      on_failure { render(json: failed_json, status: 422) }
-      on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
-      on_failed_contract do
-        render(
-          json: failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
-          status: 400,
-        )
-      end
-    end
-  end
 end
diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb
index afe131e897..a4e30f7ccf 100644
--- a/plugins/chat/app/controllers/chat_controller.rb
+++ b/plugins/chat/app/controllers/chat_controller.rb
@@ -159,17 +159,6 @@ class Chat::ChatController < Chat::ChatBaseController
     render json: success_json
   end
 
-  def update_user_last_read
-    with_service(Chat::Service::UpdateUserLastRead, channel_id: params[:chat_channel_id]) do
-      on_failed_policy(:ensure_message_id_recency) do
-        raise Discourse::InvalidParameters.new(:message_id)
-      end
-      on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound }
-      on_model_not_found(:active_membership) { raise Discourse::NotFound }
-      on_model_not_found(:channel) { raise Discourse::NotFound }
-    end
-  end
-
   def messages
     page_size = params[:page_size]&.to_i || 1000
     direction = params[:direction].to_s
diff --git a/plugins/chat/app/services/mark_all_user_channels_read.rb b/plugins/chat/app/services/mark_all_user_channels_read.rb
new file mode 100644
index 0000000000..a11cebf443
--- /dev/null
+++ b/plugins/chat/app/services/mark_all_user_channels_read.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module Chat
+  module Service
+    # Service responsible for marking all the channels that a user is a
+    # member of as read, including mentions.
+    #
+    # @example
+    #  Chat::Service::MarkAllUserChannelsRead.call(guardian: guardian)
+    #
+    class MarkAllUserChannelsRead
+      include Base
+
+      # @!method call(guardian:)
+      #   @param [Integer] channel_id
+      #   @param [Integer] message_id
+      #   @param [Guardian] guardian
+      #   @return [Chat::Service::Base::Context]
+
+      transaction do
+        step :update_last_read_message_ids
+        step :mark_associated_mentions_as_read
+      end
+
+      step :publish_new_last_read_to_clients
+
+      private
+
+      def update_last_read_message_ids(guardian:, **)
+        updated_memberships = DB.query(<<~SQL, user_id: guardian.user.id)
+          UPDATE user_chat_channel_memberships
+          SET last_read_message_id = subquery.newest_message_id
+          FROM
+          (
+            SELECT chat_messages.chat_channel_id, MAX(chat_messages.id) AS newest_message_id
+            FROM chat_messages
+            WHERE chat_messages.deleted_at IS NULL
+            GROUP BY chat_messages.chat_channel_id
+          ) AS subquery
+          WHERE user_chat_channel_memberships.chat_channel_id = subquery.chat_channel_id AND
+            subquery.newest_message_id > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND
+            user_chat_channel_memberships.user_id = :user_id AND
+            user_chat_channel_memberships.following
+          RETURNING user_chat_channel_memberships.id AS membership_id, user_chat_channel_memberships.chat_channel_id,
+            user_chat_channel_memberships.last_read_message_id;
+        SQL
+        context[:updated_memberships] = updated_memberships
+      end
+
+      def mark_associated_mentions_as_read(guardian:, updated_memberships:, **)
+        Notification
+          .where(notification_type: Notification.types[:chat_mention])
+          .where(user: guardian.user)
+          .where(read: false)
+          .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
+          .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
+          .where("chat_messages.chat_channel_id IN (?)", updated_memberships.map(&:chat_channel_id))
+          .update_all(read: true)
+      end
+
+      def publish_new_last_read_to_clients(guardian:, **)
+        # ChatPublisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id)
+      end
+    end
+  end
+end
diff --git a/plugins/chat/app/services/update_user_last_read.rb b/plugins/chat/app/services/update_user_last_read.rb
index 914b484602..ac889d2ba6 100644
--- a/plugins/chat/app/services/update_user_last_read.rb
+++ b/plugins/chat/app/services/update_user_last_read.rb
@@ -10,7 +10,7 @@ module Chat
     class UpdateUserLastRead
       include Base
 
-      # @!method call(user_id:, channel_id:, message_id:, guardian:)
+      # @!method call(channel_id:, message_id:, guardian:)
       #   @param [Integer] channel_id
       #   @param [Integer] message_id
       #   @param [Guardian] guardian
diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
index e1eb2b5840..1dec46d201 100644
--- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
+++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
@@ -192,8 +192,11 @@ export default class ChatChannel extends RestModel {
       return;
     }
 
-    return ajax(`/chat/${this.id}/read/${messageId}.json`, {
+    // TODO (martin) Change this to use chatApi service once we change this
+    // class not to use RestModel
+    return ajax(`/chat/api/tracking/read/me`, {
       method: "PUT",
+      data: { message_id: messageId, channel_id: this.id },
     }).then(() => {
       this.currentUserMembership.last_read_message_id = messageId;
     });
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
index d1ca13f3d9..5227a0c2e4 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
@@ -282,6 +282,15 @@ export default class ChatApi extends Service {
     );
   }
 
+  /**
+   * Marks messages for a user's chat channel memberships as read. If no
+   * channel ID and no message ID are provided, then all of the user's
+   * followed chat channel memberships will be marked as read.
+   *
+   * @param {number} channelId - The ID of the channel for the message being marked as read.
+   * @param {number} messageId - The ID of the message being marked as read.
+   * @returns {Promise}
+   */
   updateCurrentUserTracking({ channelId, messageId } = {}) {
     return this.#putRequest(`/tracking/read/me`, {
       channel_id: channelId,
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
index 66463af524..b272d12bb8 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
@@ -85,8 +85,9 @@ export default class ChatChannelsManager extends Service {
 
   @debounce(300)
   async markAllChannelsRead() {
-    return this.chatApi.updateCurrentUserTracking().then(() => {
-      this.channels.forEach((channel) => {
+    return this.chatApi.updateCurrentUserTracking().then((response) => {
+      response.updated_memberships.forEach((membership) => {
+        let channel = this.channels.findBy("id", membership.chat_channel_id);
         channel.currentUserMembership.unread_count = 0;
         channel.currentUserMembership.unread_mentions = 0;
       });
diff --git a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
new file mode 100644
index 0000000000..ad7d6c498c
--- /dev/null
+++ b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::Api::ChatTrackingController do
+  describe "#update_user_last_read" do
+    before { sign_in(user) }
+
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
+    fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
+
+    it "returns a 404 when the user is not a channel member" do
+      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
+
+      expect(response.status).to eq(404)
+    end
+
+    it "returns a 404 when the user is not following the channel" do
+      Fabricate(
+        :user_chat_channel_membership,
+        chat_channel: chat_channel,
+        user: user,
+        following: false,
+      )
+
+      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
+
+      expect(response.status).to eq(404)
+    end
+
+    describe "when the user is a channel member" do
+      fab!(:membership) do
+        Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: user)
+      end
+
+      context "when message_id param doesn't link to a message of the channel" do
+        it "raises a not found" do
+          put "/chat/#{chat_channel.id}/read/-999.json"
+
+          expect(response.status).to eq(404)
+        end
+      end
+
+      context "when message_id param is inferior to existing last read" do
+        before { membership.update!(last_read_message_id: message_2.id) }
+
+        it "raises an invalid request" do
+          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
+
+          expect(response.status).to eq(400)
+          expect(response.parsed_body["errors"][0]).to match(/message_id/)
+        end
+      end
+
+      context "when message_id refers to deleted message" do
+        before { message_1.trash!(Discourse.system_user) }
+
+        it "works" do
+          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
+
+          expect(response.status).to eq(200)
+        end
+      end
+
+      it "updates timing records" do
+        expect { put "/chat/#{chat_channel.id}/read/#{message_1.id}.json" }.not_to change {
+          UserChatChannelMembership.count
+        }
+
+        membership.reload
+        expect(membership.chat_channel_id).to eq(chat_channel.id)
+        expect(membership.last_read_message_id).to eq(message_1.id)
+        expect(membership.user_id).to eq(user.id)
+      end
+
+      def create_notification_and_mention_for(user, sender, msg)
+        Notification
+          .create!(
+            notification_type: Notification.types[:chat_mention],
+            user: user,
+            high_priority: true,
+            read: false,
+            data: {
+              message: "chat.mention_notification",
+              chat_message_id: msg.id,
+              chat_channel_id: msg.chat_channel_id,
+              chat_channel_title: msg.chat_channel.title(user),
+              chat_channel_slug: msg.chat_channel.slug,
+              mentioned_by_username: sender.username,
+            }.to_json,
+          )
+          .tap do |notification|
+            ChatMention.create!(user: user, chat_message: msg, notification: notification)
+          end
+      end
+
+      it "marks all mention notifications as read for the channel" do
+        notification = create_notification_and_mention_for(user, other_user, message_1)
+
+        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
+        expect(response.status).to eq(200)
+        expect(notification.reload.read).to eq(true)
+      end
+
+      it "doesn't mark notifications of messages that weren't read yet" do
+        message_3 = Fabricate(:chat_message, chat_channel: chat_channel, user: other_user)
+        notification = create_notification_and_mention_for(user, other_user, message_3)
+
+        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
+
+        expect(response.status).to eq(200)
+        expect(notification.reload.read).to eq(false)
+      end
+    end
+  end
+end
diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb
index 2ba8451579..34eb2d7c75 100644
--- a/plugins/chat/spec/requests/chat_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat_controller_spec.rb
@@ -725,117 +725,6 @@ RSpec.describe Chat::ChatController do
     end
   end
 
-  describe "#update_user_last_read" do
-    before { sign_in(user) }
-
-    fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
-    fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
-
-    it "returns a 404 when the user is not a channel member" do
-      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-      expect(response.status).to eq(404)
-    end
-
-    it "returns a 404 when the user is not following the channel" do
-      Fabricate(
-        :user_chat_channel_membership,
-        chat_channel: chat_channel,
-        user: user,
-        following: false,
-      )
-
-      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-      expect(response.status).to eq(404)
-    end
-
-    describe "when the user is a channel member" do
-      fab!(:membership) do
-        Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: user)
-      end
-
-      context "when message_id param doesn't link to a message of the channel" do
-        it "raises a not found" do
-          put "/chat/#{chat_channel.id}/read/-999.json"
-
-          expect(response.status).to eq(404)
-        end
-      end
-
-      context "when message_id param is inferior to existing last read" do
-        before { membership.update!(last_read_message_id: message_2.id) }
-
-        it "raises an invalid request" do
-          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-          expect(response.status).to eq(400)
-          expect(response.parsed_body["errors"][0]).to match(/message_id/)
-        end
-      end
-
-      context "when message_id refers to deleted message" do
-        before { message_1.trash!(Discourse.system_user) }
-
-        it "works" do
-          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-          expect(response.status).to eq(200)
-        end
-      end
-
-      it "updates timing records" do
-        expect { put "/chat/#{chat_channel.id}/read/#{message_1.id}.json" }.not_to change {
-          Chat::UserChatChannelMembership.count
-        }
-
-        membership.reload
-        expect(membership.chat_channel_id).to eq(chat_channel.id)
-        expect(membership.last_read_message_id).to eq(message_1.id)
-        expect(membership.user_id).to eq(user.id)
-      end
-
-      def create_notification_and_mention_for(user, sender, msg)
-        Notification
-          .create!(
-            notification_type: Notification.types[:chat_mention],
-            user: user,
-            high_priority: true,
-            read: false,
-            data: {
-              message: "chat.mention_notification",
-              chat_message_id: msg.id,
-              chat_channel_id: msg.chat_channel_id,
-              chat_channel_title: msg.chat_channel.title(user),
-              chat_channel_slug: msg.chat_channel.slug,
-              mentioned_by_username: sender.username,
-            }.to_json,
-          )
-          .tap do |notification|
-            Chat::Mention.create!(user: user, chat_message: msg, notification: notification)
-          end
-      end
-
-      it "marks all mention notifications as read for the channel" do
-        notification = create_notification_and_mention_for(user, other_user, message_1)
-
-        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
-        expect(response.status).to eq(200)
-        expect(notification.reload.read).to eq(true)
-      end
-
-      it "doesn't mark notifications of messages that weren't read yet" do
-        message_3 = Fabricate(:chat_message, chat_channel: chat_channel, user: other_user)
-        notification = create_notification_and_mention_for(user, other_user, message_3)
-
-        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
-
-        expect(response.status).to eq(200)
-        expect(notification.reload.read).to eq(false)
-      end
-    end
-  end
-
   describe "react" do
     fab!(:chat_channel) { Fabricate(:category_channel) }
     fab!(:chat_message) { Fabricate(:chat_message, chat_channel: chat_channel, user: user) }

From e534a3b8cd833aa2a017ba90cc3f24a538979c50 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 14:11:35 +1000
Subject: [PATCH 3/9] DEV: Update specs

---
 .../chat/api/chat_tracking_controller.rb      |  34 ++-
 .../services/chat-channels-manager.js         |   6 +-
 plugins/chat/lib/service_runner.rb            |   2 +-
 .../chat/api/chat_tracking_controller_spec.rb | 288 ++++++++++++------
 .../chat/update_user_last_read_spec.rb        |  19 +-
 5 files changed, 233 insertions(+), 116 deletions(-)

diff --git a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
index 681b05d5f4..3aea2b0596 100644
--- a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
@@ -8,19 +8,29 @@ class Chat::Api::ChatTrackingController < Chat::Api
     message_id = params[:message_id]
 
     if channel_id.present? && message_id.present?
-      with_service(Chat::Service::UpdateUserLastRead) do
-        on_failed_policy(:ensure_message_id_recency) do
-          raise Discourse::InvalidParameters.new(:message_id)
-        end
-        on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound }
-        on_model_not_found(:active_membership) { raise Discourse::NotFound }
-        on_model_not_found(:channel) { raise Discourse::NotFound }
-      end
+      mark_single_message_read(channel_id, message_id)
     else
-      with_service(Chat::Service::MarkAllUserChannelsRead) do
-        on_success do
-          render(json: success_json.merge(updated_memberships: result.updated_memberships))
-        end
+      mark_all_messages_read
+    end
+  end
+
+  private
+
+  def mark_single_message_read(channel_id, message_id)
+    with_service(Chat::Service::UpdateUserLastRead) do
+      on_failed_policy(:ensure_message_id_recency) do
+        raise Discourse::InvalidParameters.new(:message_id)
+      end
+      on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound }
+      on_model_not_found(:active_membership) { raise Discourse::NotFound }
+      on_model_not_found(:channel) { raise Discourse::NotFound }
+    end
+  end
+
+  def mark_all_messages_read
+    with_service(Chat::Service::MarkAllUserChannelsRead) do
+      on_success do
+        render(json: success_json.merge(updated_memberships: result.updated_memberships))
       end
     end
   end
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
index b272d12bb8..a44591d3e2 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
@@ -88,8 +88,10 @@ export default class ChatChannelsManager extends Service {
     return this.chatApi.updateCurrentUserTracking().then((response) => {
       response.updated_memberships.forEach((membership) => {
         let channel = this.channels.findBy("id", membership.chat_channel_id);
-        channel.currentUserMembership.unread_count = 0;
-        channel.currentUserMembership.unread_mentions = 0;
+        if (channel) {
+          channel.currentUserMembership.unread_count = 0;
+          channel.currentUserMembership.unread_mentions = 0;
+        }
       });
     });
   }
diff --git a/plugins/chat/lib/service_runner.rb b/plugins/chat/lib/service_runner.rb
index ad3eeafdec..62db6ed2fd 100644
--- a/plugins/chat/lib/service_runner.rb
+++ b/plugins/chat/lib/service_runner.rb
@@ -19,7 +19,7 @@
 # * +on_model_not_found(name)+: will execute the provided block if the service
 #   fails and its model is not present
 #
-# Default actions for each of these are defined in [Chat::BaseController#default_actions_for_service]
+# Default actions for each of these are defined in [Chat::ApiController#default_actions_for_service]
 #
 # @example In a controller
 #   def create
diff --git a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
index ad7d6c498c..1b1f7afec7 100644
--- a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
@@ -1,114 +1,224 @@
 # frozen_string_literal: true
 
 RSpec.describe Chat::Api::ChatTrackingController do
-  describe "#update_user_last_read" do
-    before { sign_in(user) }
+  fab!(:current_user) { Fabricate(:user) }
 
-    fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
-    fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
+  before do
+    SiteSetting.chat_enabled = true
+    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+    sign_in(current_user)
+  end
 
-    it "returns a 404 when the user is not a channel member" do
-      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
+  describe "#read" do
+    describe "marking a single message read" do
+      fab!(:chat_channel) { Fabricate(:chat_channel) }
+      fab!(:other_user) { Fabricate(:user) }
+      fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
+      fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel, user: other_user) }
 
-      expect(response.status).to eq(404)
-    end
+      it "returns a 404 when the user is not a channel member" do
+        put "/chat/api/tracking/read/me.json",
+            params: {
+              channel_id: chat_channel.id,
+              message_id: message_1.id,
+            }
 
-    it "returns a 404 when the user is not following the channel" do
-      Fabricate(
-        :user_chat_channel_membership,
-        chat_channel: chat_channel,
-        user: user,
-        following: false,
-      )
-
-      put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-      expect(response.status).to eq(404)
-    end
-
-    describe "when the user is a channel member" do
-      fab!(:membership) do
-        Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: user)
+        expect(response.status).to eq(404)
       end
 
-      context "when message_id param doesn't link to a message of the channel" do
-        it "raises a not found" do
-          put "/chat/#{chat_channel.id}/read/-999.json"
+      it "returns a 404 when the user is not following the channel" do
+        Fabricate(
+          :user_chat_channel_membership,
+          chat_channel: chat_channel,
+          user: current_user,
+          following: false,
+        )
 
-          expect(response.status).to eq(404)
+        put "/chat/api/tracking/read/me.json",
+            params: {
+              channel_id: chat_channel.id,
+              message_id: message_1.id,
+            }
+
+        expect(response.status).to eq(404)
+      end
+
+      describe "when the user is a channel member" do
+        fab!(:membership) do
+          Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: current_user)
         end
-      end
 
-      context "when message_id param is inferior to existing last read" do
-        before { membership.update!(last_read_message_id: message_2.id) }
+        context "when message_id param doesn't link to a message of the channel" do
+          it "raises a not found" do
+            put "/chat/api/tracking/read/me.json",
+                params: {
+                  channel_id: chat_channel.id,
+                  message_id: -999,
+                }
 
-        it "raises an invalid request" do
-          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-          expect(response.status).to eq(400)
-          expect(response.parsed_body["errors"][0]).to match(/message_id/)
-        end
-      end
-
-      context "when message_id refers to deleted message" do
-        before { message_1.trash!(Discourse.system_user) }
-
-        it "works" do
-          put "/chat/#{chat_channel.id}/read/#{message_1.id}.json"
-
-          expect(response.status).to eq(200)
-        end
-      end
-
-      it "updates timing records" do
-        expect { put "/chat/#{chat_channel.id}/read/#{message_1.id}.json" }.not_to change {
-          UserChatChannelMembership.count
-        }
-
-        membership.reload
-        expect(membership.chat_channel_id).to eq(chat_channel.id)
-        expect(membership.last_read_message_id).to eq(message_1.id)
-        expect(membership.user_id).to eq(user.id)
-      end
-
-      def create_notification_and_mention_for(user, sender, msg)
-        Notification
-          .create!(
-            notification_type: Notification.types[:chat_mention],
-            user: user,
-            high_priority: true,
-            read: false,
-            data: {
-              message: "chat.mention_notification",
-              chat_message_id: msg.id,
-              chat_channel_id: msg.chat_channel_id,
-              chat_channel_title: msg.chat_channel.title(user),
-              chat_channel_slug: msg.chat_channel.slug,
-              mentioned_by_username: sender.username,
-            }.to_json,
-          )
-          .tap do |notification|
-            ChatMention.create!(user: user, chat_message: msg, notification: notification)
+            expect(response.status).to eq(404)
           end
+        end
+
+        context "when message_id param is inferior to existing last read" do
+          before { membership.update!(last_read_message_id: message_2.id) }
+
+          it "raises an invalid request" do
+            put "/chat/api/tracking/read/me.json",
+                params: {
+                  channel_id: chat_channel.id,
+                  message_id: message_1.id,
+                }
+
+            expect(response.status).to eq(400)
+            expect(response.parsed_body["errors"][0]).to match(/message_id/)
+          end
+        end
+
+        context "when message_id refers to deleted message" do
+          before { message_1.trash!(Discourse.system_user) }
+
+          it "works" do
+            put "/chat/api/tracking/read/me",
+                params: {
+                  channel_id: chat_channel.id,
+                  message_id: message_1.id,
+                }
+
+            expect(response.status).to eq(200)
+          end
+        end
+
+        it "updates timing records" do
+          expect {
+            put "/chat/api/tracking/read/me.json",
+                params: {
+                  channel_id: chat_channel.id,
+                  message_id: message_1.id,
+                }
+          }.not_to change { UserChatChannelMembership.count }
+
+          membership.reload
+          expect(membership.chat_channel_id).to eq(chat_channel.id)
+          expect(membership.last_read_message_id).to eq(message_1.id)
+          expect(membership.user_id).to eq(current_user.id)
+        end
+
+        it "marks all mention notifications as read for the channel" do
+          notification = create_notification_and_mention_for(current_user, other_user, message_1)
+
+          put "/chat/api/tracking/read/me.json",
+              params: {
+                channel_id: chat_channel.id,
+                message_id: message_2.id,
+              }
+          expect(response.status).to eq(200)
+          expect(notification.reload.read).to eq(true)
+        end
+
+        it "doesn't mark notifications of messages that weren't read yet" do
+          message_3 = Fabricate(:chat_message, chat_channel: chat_channel, user: other_user)
+          notification = create_notification_and_mention_for(current_user, other_user, message_3)
+
+          put "/chat/api/tracking/read/me.json",
+              params: {
+                channel_id: chat_channel.id,
+                message_id: message_2.id,
+              }
+          expect(response.status).to eq(200)
+          expect(notification.reload.read).to eq(false)
+        end
+      end
+    end
+
+    describe "marking all messages read" do
+      fab!(:chat_channel_1) { Fabricate(:chat_channel) }
+      fab!(:chat_channel_2) { Fabricate(:chat_channel) }
+      fab!(:chat_channel_3) { Fabricate(:chat_channel) }
+
+      fab!(:other_user) { Fabricate(:user) }
+
+      fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, user: other_user) }
+      fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel_1, user: other_user) }
+      fab!(:message_3) { Fabricate(:chat_message, chat_channel: chat_channel_2, user: other_user) }
+      fab!(:message_4) { Fabricate(:chat_message, chat_channel: chat_channel_2, user: other_user) }
+      fab!(:message_5) { Fabricate(:chat_message, chat_channel: chat_channel_3, user: other_user) }
+      fab!(:message_6) { Fabricate(:chat_message, chat_channel: chat_channel_3, user: other_user) }
+
+      fab!(:membership_1) do
+        Fabricate(
+          :user_chat_channel_membership,
+          chat_channel: chat_channel_1,
+          user: current_user,
+          following: true,
+        )
+      end
+      fab!(:membership_2) do
+        Fabricate(
+          :user_chat_channel_membership,
+          chat_channel: chat_channel_2,
+          user: current_user,
+          following: true,
+        )
+      end
+      fab!(:membership_3) do
+        Fabricate(
+          :user_chat_channel_membership,
+          chat_channel: chat_channel_3,
+          user: current_user,
+          following: true,
+        )
       end
 
-      it "marks all mention notifications as read for the channel" do
-        notification = create_notification_and_mention_for(user, other_user, message_1)
+      it "marks all messages as read across the user's channel memberships with the correct last_read_message_id" do
+        put "/chat/api/tracking/read/me.json"
 
-        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
-        expect(response.status).to eq(200)
-        expect(notification.reload.read).to eq(true)
+        expect(membership_1.reload.last_read_message_id).to eq(message_2.id)
+        expect(membership_2.reload.last_read_message_id).to eq(message_4.id)
+        expect(membership_3.reload.last_read_message_id).to eq(message_6.id)
       end
 
-      it "doesn't mark notifications of messages that weren't read yet" do
-        message_3 = Fabricate(:chat_message, chat_channel: chat_channel, user: other_user)
-        notification = create_notification_and_mention_for(user, other_user, message_3)
+      it "doesn't mark messages for channels the user is not following as read" do
+        membership_1.update!(following: false)
 
-        put "/chat/#{chat_channel.id}/read/#{message_2.id}.json"
+        put "/chat/api/tracking/read/me.json"
 
-        expect(response.status).to eq(200)
-        expect(notification.reload.read).to eq(false)
+        expect(membership_1.reload.last_read_message_id).to eq(nil)
+        expect(membership_2.reload.last_read_message_id).to eq(message_4.id)
+        expect(membership_3.reload.last_read_message_id).to eq(message_6.id)
+      end
+
+      it "returns the updated memberships, channels, and last message id" do
+        put "/chat/api/tracking/read/me.json"
+        expect(response.parsed_body["updated_memberships"].first).to eq(
+          {
+            "chat_channel_id" => chat_channel_1.id,
+            "last_read_message_id" => message_2.id,
+            "membership_id" => membership_1.id,
+          },
+        )
       end
     end
   end
+
+  def create_notification_and_mention_for(user, sender, msg)
+    Notification
+      .create!(
+        notification_type: Notification.types[:chat_mention],
+        user: user,
+        high_priority: true,
+        read: false,
+        data: {
+          message: "chat.mention_notification",
+          chat_message_id: msg.id,
+          chat_channel_id: msg.chat_channel_id,
+          chat_channel_title: msg.chat_channel.title(user),
+          chat_channel_slug: msg.chat_channel.slug,
+          mentioned_by_username: sender.username,
+        }.to_json,
+      )
+      .tap do |notification|
+        ChatMention.create!(user: user, chat_message: msg, notification: notification)
+      end
+  end
 end
diff --git a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb
index 87051de62a..8cd9e44b71 100644
--- a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb
+++ b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb
@@ -18,17 +18,10 @@ RSpec.describe Chat::UpdateUserLastRead do
     fab!(:message_1) { Fabricate(:chat_message, chat_channel: membership.chat_channel) }
 
     let(:guardian) { Guardian.new(current_user) }
-    let(:params) do
-      {
-        guardian: guardian,
-        user_id: current_user.id,
-        channel_id: channel.id,
-        message_id: message_1.id,
-      }
-    end
+    let(:params) { { guardian: guardian, channel_id: channel.id, message_id: message_1.id } }
 
     context "when params are not valid" do
-      before { params.delete(:user_id) }
+      before { params.delete(:message_id) }
 
       it { is_expected.to fail_a_contract }
     end
@@ -37,7 +30,7 @@ RSpec.describe Chat::UpdateUserLastRead do
       context "when user has no membership" do
         before { membership.destroy! }
 
-        it { is_expected.to fail_to_find_a_model(:membership) }
+        it { is_expected.to fail_to_find_a_model(:active_membership) }
       end
 
       context "when user can’t access the channel" do
@@ -56,8 +49,10 @@ RSpec.describe Chat::UpdateUserLastRead do
 
       context "when message_id is older than membership's last_read_message_id" do
         before do
-          params[:message_id] = -2
-          membership.update!(last_read_message_id: -1)
+          message_old = Fabricate(:chat_message, chat_channel: channel)
+          message_new = Fabricate(:chat_message, chat_channel: channel)
+          params[:message_id] = message_old.id
+          membership.update!(last_read_message_id: message_new.id)
         end
 
         it { is_expected.to fail_a_policy(:ensure_message_id_recency) }

From f7b9eafa6745438893fcfc8ae15ae3526e7f5131 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 14:51:02 +1000
Subject: [PATCH 4/9] DEV: Add specs and JS minor changes

---
 .../services/mark_all_user_channels_read.rb   |  19 +--
 .../initializers/chat-keyboard-shortcuts.js   |   5 +-
 .../services/chat-channels-manager.js         |   2 +-
 .../mark_all_user_channels_read_spec.rb       | 133 ++++++++++++++++++
 .../system/shortcuts/mark_all_read_spec.rb    |  44 ++++++
 5 files changed, 189 insertions(+), 14 deletions(-)
 create mode 100644 plugins/chat/spec/services/mark_all_user_channels_read_spec.rb
 create mode 100644 plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb

diff --git a/plugins/chat/app/services/mark_all_user_channels_read.rb b/plugins/chat/app/services/mark_all_user_channels_read.rb
index a11cebf443..8e5fb3dfbc 100644
--- a/plugins/chat/app/services/mark_all_user_channels_read.rb
+++ b/plugins/chat/app/services/mark_all_user_channels_read.rb
@@ -3,7 +3,7 @@
 module Chat
   module Service
     # Service responsible for marking all the channels that a user is a
-    # member of as read, including mentions.
+    # member of _and following_ as read, including mentions.
     #
     # @example
     #  Chat::Service::MarkAllUserChannelsRead.call(guardian: guardian)
@@ -12,8 +12,6 @@ module Chat
       include Base
 
       # @!method call(guardian:)
-      #   @param [Integer] channel_id
-      #   @param [Integer] message_id
       #   @param [Guardian] guardian
       #   @return [Chat::Service::Base::Context]
 
@@ -22,8 +20,6 @@ module Chat
         step :mark_associated_mentions_as_read
       end
 
-      step :publish_new_last_read_to_clients
-
       private
 
       def update_last_read_message_ids(guardian:, **)
@@ -41,26 +37,25 @@ module Chat
             subquery.newest_message_id > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND
             user_chat_channel_memberships.user_id = :user_id AND
             user_chat_channel_memberships.following
-          RETURNING user_chat_channel_memberships.id AS membership_id, user_chat_channel_memberships.chat_channel_id,
-            user_chat_channel_memberships.last_read_message_id;
+          RETURNING user_chat_channel_memberships.id AS membership_id,
+                    user_chat_channel_memberships.chat_channel_id AS channel_id,
+                    user_chat_channel_memberships.last_read_message_id;
         SQL
         context[:updated_memberships] = updated_memberships
       end
 
       def mark_associated_mentions_as_read(guardian:, updated_memberships:, **)
+        return if updated_memberships.empty?
+
         Notification
           .where(notification_type: Notification.types[:chat_mention])
           .where(user: guardian.user)
           .where(read: false)
           .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
           .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
-          .where("chat_messages.chat_channel_id IN (?)", updated_memberships.map(&:chat_channel_id))
+          .where("chat_messages.chat_channel_id IN (?)", updated_memberships.map(&:channel_id))
           .update_all(read: true)
       end
-
-      def publish_new_last_read_to_clients(guardian:, **)
-        # ChatPublisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id)
-      end
     end
   end
 end
diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
index 34a00c5ccf..f9d564fcf9 100644
--- a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
+++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
@@ -98,7 +98,10 @@ export default {
     const markAllChannelsRead = (event) => {
       event.preventDefault();
       event.stopPropagation();
-      chatChannelsManager.markAllChannelsRead();
+
+      if (chatStateManager.isActive) {
+        chatChannelsManager.markAllChannelsRead();
+      }
     };
 
     withPluginApi("0.12.1", (api) => {
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
index a44591d3e2..08da50452f 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
@@ -87,7 +87,7 @@ export default class ChatChannelsManager extends Service {
   async markAllChannelsRead() {
     return this.chatApi.updateCurrentUserTracking().then((response) => {
       response.updated_memberships.forEach((membership) => {
-        let channel = this.channels.findBy("id", membership.chat_channel_id);
+        let channel = this.channels.findBy("id", membership.channel_id);
         if (channel) {
           channel.currentUserMembership.unread_count = 0;
           channel.currentUserMembership.unread_mentions = 0;
diff --git a/plugins/chat/spec/services/mark_all_user_channels_read_spec.rb b/plugins/chat/spec/services/mark_all_user_channels_read_spec.rb
new file mode 100644
index 0000000000..80b9e05c60
--- /dev/null
+++ b/plugins/chat/spec/services/mark_all_user_channels_read_spec.rb
@@ -0,0 +1,133 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::Service::MarkAllUserChannelsRead do
+  describe ".call" do
+    subject(:result) { described_class.call(params) }
+
+    let(:params) { { guardian: guardian } }
+    let(:guardian) { Guardian.new(current_user) }
+
+    fab!(:current_user) { Fabricate(:user) }
+
+    fab!(:channel_1) { Fabricate(:chat_channel) }
+    fab!(:channel_2) { Fabricate(:chat_channel) }
+    fab!(:channel_3) { Fabricate(:chat_channel) }
+
+    fab!(:other_user) { Fabricate(:user) }
+
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, user: other_user) }
+    fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1, user: other_user) }
+    fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_2, user: other_user) }
+    fab!(:message_4) { Fabricate(:chat_message, chat_channel: channel_2, user: other_user) }
+    fab!(:message_5) { Fabricate(:chat_message, chat_channel: channel_3, user: other_user) }
+    fab!(:message_6) { Fabricate(:chat_message, chat_channel: channel_3, user: other_user) }
+
+    fab!(:membership_1) do
+      Fabricate(
+        :user_chat_channel_membership,
+        chat_channel: channel_1,
+        user: current_user,
+        following: true,
+      )
+    end
+    fab!(:membership_2) do
+      Fabricate(
+        :user_chat_channel_membership,
+        chat_channel: channel_2,
+        user: current_user,
+        following: true,
+      )
+    end
+    fab!(:membership_3) do
+      Fabricate(
+        :user_chat_channel_membership,
+        chat_channel: channel_3,
+        user: current_user,
+        following: true,
+      )
+    end
+
+    context "when the user has no memberships" do
+      let(:guardian) { Guardian.new(Fabricate(:user)) }
+
+      it "sets the service result as successful" do
+        expect(result).to be_a_success
+      end
+
+      it "returns the updated_memberships in context" do
+        expect(result.updated_memberships).to eq([])
+      end
+    end
+
+    context "when everything is fine" do
+      fab!(:notification_1) do
+        Fabricate(
+          :notification,
+          notification_type: Notification.types[:chat_mention],
+          user: current_user,
+        )
+      end
+      fab!(:notification_2) do
+        Fabricate(
+          :notification,
+          notification_type: Notification.types[:chat_mention],
+          user: current_user,
+        )
+      end
+
+      let(:messages) { MessageBus.track_publish { result } }
+
+      before do
+        ChatMention.create!(
+          notification: notification_1,
+          user: current_user,
+          chat_message: message_1,
+        )
+        ChatMention.create!(
+          notification: notification_2,
+          user: current_user,
+          chat_message: message_3,
+        )
+      end
+
+      it "sets the service result as successful" do
+        expect(result).to be_a_success
+      end
+
+      it "updates the last_read_message_ids" do
+        result
+        expect(membership_1.reload.last_read_message_id).to eq(message_2.id)
+        expect(membership_2.reload.last_read_message_id).to eq(message_4.id)
+        expect(membership_3.reload.last_read_message_id).to eq(message_6.id)
+      end
+
+      it "does not update memberships where the user is not following" do
+        membership_1.update!(following: false)
+        result
+        expect(membership_1.reload.last_read_message_id).to eq(nil)
+      end
+
+      it "does not use deleted messages for the last_read_message_id" do
+        message_2.trash!
+        result
+        expect(membership_1.reload.last_read_message_id).to eq(message_1.id)
+      end
+
+      it "returns the updated_memberships in context" do
+        expect(result.updated_memberships.map(&:channel_id)).to match_array(
+          [channel_1.id, channel_2.id, channel_3.id],
+        )
+      end
+
+      it "marks existing notifications for all affected channels as read" do
+        expect { result }.to change {
+          Notification.where(
+            notification_type: Notification.types[:chat_mention],
+            user: current_user,
+            read: false,
+          ).count
+        }.by(-2)
+      end
+    end
+  end
+end
diff --git a/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb b/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb
new file mode 100644
index 0000000000..70493993fa
--- /dev/null
+++ b/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+RSpec.describe "Shortcuts | mark all read", type: :system, js: true do
+  fab!(:user_1) { Fabricate(:admin) }
+  fab!(:channel_1) { Fabricate(:chat_channel) }
+  fab!(:channel_2) { Fabricate(:chat_channel) }
+  fab!(:channel_3) { Fabricate(:chat_channel) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:drawer) { PageObjects::Pages::ChatDrawer.new }
+
+  before do
+    SiteSetting.navigation_menu = "sidebar"
+    chat_system_bootstrap(user_1, [channel_1, channel_2, channel_3])
+    sign_in(user_1)
+    Fabricate(:chat_message, chat_channel: channel_1)
+    Fabricate(:chat_message, chat_channel: channel_1)
+    Fabricate(:chat_message, chat_channel: channel_2)
+    Fabricate(:chat_message, chat_channel: channel_2)
+  end
+
+  context "when chat is open" do
+    before { visit(channel_3.url) }
+
+    context "when pressing shift+esc" do
+      it "marks all channels read" do
+        pause_test
+        expect(page).to have_css(
+          ".sidebar-section-link.channel-#{channel_1.id} .sidebar-section-link-suffix.unread",
+        )
+        expect(page).to have_css(
+          ".sidebar-section-link.channel-#{channel_2.id} .sidebar-section-link-suffix.unread",
+        )
+        find("body").send_keys(%i[shift escape])
+        expect(page).not_to have_css(
+          ".sidebar-section-link.channel-#{channel_1.id} .sidebar-section-link-suffix.unread",
+        )
+        expect(page).not_to have_css(
+          ".sidebar-section-link.channel-#{channel_2.id} .sidebar-section-link-suffix.unread",
+        )
+      end
+    end
+  end
+end

From 6c39fbbe047403057fba1dbcc7d2b3ddd09b50eb Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 15:01:40 +1000
Subject: [PATCH 5/9] DEV: Test fix

---
 .../chat/api/chat_tracking_controller_spec.rb | 24 ++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
index 1b1f7afec7..9bb84e9674 100644
--- a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
@@ -190,12 +190,24 @@ RSpec.describe Chat::Api::ChatTrackingController do
 
       it "returns the updated memberships, channels, and last message id" do
         put "/chat/api/tracking/read/me.json"
-        expect(response.parsed_body["updated_memberships"].first).to eq(
-          {
-            "chat_channel_id" => chat_channel_1.id,
-            "last_read_message_id" => message_2.id,
-            "membership_id" => membership_1.id,
-          },
+        expect(response.parsed_body["updated_memberships"]).to match_array(
+          [
+            {
+              "channel_id" => chat_channel_1.id,
+              "last_read_message_id" => message_2.id,
+              "membership_id" => membership_1.id,
+            },
+            {
+              "channel_id" => chat_channel_2.id,
+              "last_read_message_id" => message_4.id,
+              "membership_id" => membership_2.id,
+            },
+            {
+              "channel_id" => chat_channel_3.id,
+              "last_read_message_id" => message_6.id,
+              "membership_id" => membership_3.id,
+            },
+          ],
         )
       end
     end

From 6e13a9a5d094385f4804d9531021dca5b25f34f4 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 10 Mar 2023 15:19:54 +1000
Subject: [PATCH 6/9] DEV: Test fix

---
 plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb b/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb
index 70493993fa..760525d11c 100644
--- a/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb
+++ b/plugins/chat/spec/system/shortcuts/mark_all_read_spec.rb
@@ -24,7 +24,6 @@ RSpec.describe "Shortcuts | mark all read", type: :system, js: true do
 
     context "when pressing shift+esc" do
       it "marks all channels read" do
-        pause_test
         expect(page).to have_css(
           ".sidebar-section-link.channel-#{channel_1.id} .sidebar-section-link-suffix.unread",
         )

From 4e3dec65b027bcef564b8a1fa1d17ed716d14177 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Sat, 18 Mar 2023 18:59:38 +0100
Subject: [PATCH 7/9] manu conflicts fix

---
 .../chat/app/controllers/api_controller.rb    |  29 --
 ...g_controller.rb => tracking_controller.rb} |   6 +-
 .../app/controllers/chat_base_controller.rb   |  20 -
 .../chat/app/controllers/chat_controller.rb   | 436 ------------------
 .../chat/mark_all_user_channels_read.rb       |  59 +++
 .../services/chat/update_user_last_read.rb    |  60 +--
 .../services/mark_all_user_channels_read.rb   |  61 ---
 .../app/services/update_user_last_read.rb     |  84 ----
 plugins/chat/config/routes.rb                 |   1 +
 9 files changed, 93 insertions(+), 663 deletions(-)
 delete mode 100644 plugins/chat/app/controllers/api_controller.rb
 rename plugins/chat/app/controllers/chat/api/{chat_tracking_controller.rb => tracking_controller.rb} (84%)
 delete mode 100644 plugins/chat/app/controllers/chat_base_controller.rb
 delete mode 100644 plugins/chat/app/controllers/chat_controller.rb
 create mode 100644 plugins/chat/app/services/chat/mark_all_user_channels_read.rb
 delete mode 100644 plugins/chat/app/services/mark_all_user_channels_read.rb
 delete mode 100644 plugins/chat/app/services/update_user_last_read.rb

diff --git a/plugins/chat/app/controllers/api_controller.rb b/plugins/chat/app/controllers/api_controller.rb
deleted file mode 100644
index 70bf35dc60..0000000000
--- a/plugins/chat/app/controllers/api_controller.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-class Chat::Api < Chat::ChatBaseController
-  before_action :ensure_logged_in
-  before_action :ensure_can_chat
-
-  include Chat::WithServiceHelper
-
-  private
-
-  def ensure_can_chat
-    raise Discourse::NotFound unless SiteSetting.chat_enabled
-    guardian.ensure_can_chat!
-  end
-
-  def default_actions_for_service
-    proc do
-      on_success { render(json: success_json) }
-      on_failure { render(json: failed_json, status: 422) }
-      on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
-      on_failed_contract do
-        render(
-          json: failed_json.merge(errors: result[:"result.contract.default"].errors.full_messages),
-          status: 400,
-        )
-      end
-    end
-  end
-end
diff --git a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb b/plugins/chat/app/controllers/chat/api/tracking_controller.rb
similarity index 84%
rename from plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
rename to plugins/chat/app/controllers/chat/api/tracking_controller.rb
index 3aea2b0596..c3b631dc78 100644
--- a/plugins/chat/app/controllers/chat/api/chat_tracking_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/tracking_controller.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class Chat::Api::ChatTrackingController < Chat::Api
+class Chat::Api::TrackingController < Chat::Api
   def read
     params.permit(:channel_id, :message_id)
 
@@ -17,7 +17,7 @@ class Chat::Api::ChatTrackingController < Chat::Api
   private
 
   def mark_single_message_read(channel_id, message_id)
-    with_service(Chat::Service::UpdateUserLastRead) do
+    with_service(Chat::UpdateUserLastRead) do
       on_failed_policy(:ensure_message_id_recency) do
         raise Discourse::InvalidParameters.new(:message_id)
       end
@@ -28,7 +28,7 @@ class Chat::Api::ChatTrackingController < Chat::Api
   end
 
   def mark_all_messages_read
-    with_service(Chat::Service::MarkAllUserChannelsRead) do
+    with_service(Chat::MarkAllUserChannelsRead) do
       on_success do
         render(json: success_json.merge(updated_memberships: result.updated_memberships))
       end
diff --git a/plugins/chat/app/controllers/chat_base_controller.rb b/plugins/chat/app/controllers/chat_base_controller.rb
deleted file mode 100644
index 6e014502dd..0000000000
--- a/plugins/chat/app/controllers/chat_base_controller.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-class Chat::ChatBaseController < ::ApplicationController
-  before_action :ensure_logged_in
-  before_action :ensure_can_chat
-
-  private
-
-  def ensure_can_chat
-    raise Discourse::NotFound unless SiteSetting.chat_enabled
-    guardian.ensure_can_chat!
-  end
-
-  def set_channel_and_chatable_with_access_check(chat_channel_id: nil)
-    params.require(:chat_channel_id) if chat_channel_id.blank?
-    id_or_name = chat_channel_id || params[:chat_channel_id]
-    @chat_channel = Chat::ChatChannelFetcher.find_with_access_check(id_or_name, guardian)
-    @chatable = @chat_channel.chatable
-  end
-end
diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb
deleted file mode 100644
index a4e30f7ccf..0000000000
--- a/plugins/chat/app/controllers/chat_controller.rb
+++ /dev/null
@@ -1,436 +0,0 @@
-# frozen_string_literal: true
-
-class Chat::ChatController < Chat::ChatBaseController
-  PAST_MESSAGE_LIMIT = 40
-  FUTURE_MESSAGE_LIMIT = 40
-  PAST = "past"
-  FUTURE = "future"
-  CHAT_DIRECTIONS = [PAST, FUTURE]
-
-  # Other endpoints use set_channel_and_chatable_with_access_check, but
-  # these endpoints require a standalone find because they need to be
-  # able to get deleted channels and recover them.
-  before_action :find_chatable, only: %i[enable_chat disable_chat]
-  before_action :find_chat_message,
-                only: %i[delete restore lookup_message edit_message rebake message_link]
-  before_action :set_channel_and_chatable_with_access_check,
-                except: %i[
-                  respond
-                  enable_chat
-                  disable_chat
-                  message_link
-                  lookup_message
-                  set_user_chat_status
-                  dismiss_retention_reminder
-                  flag
-                ]
-
-  def respond
-    render
-  end
-
-  def enable_chat
-    chat_channel = ChatChannel.with_deleted.find_by(chatable: @chatable)
-
-    guardian.ensure_can_join_chat_channel!(chat_channel) if chat_channel
-
-    if chat_channel && chat_channel.trashed?
-      chat_channel.recover!
-    elsif chat_channel
-      return render_json_error I18n.t("chat.already_enabled")
-    else
-      chat_channel = @chatable.chat_channel
-      guardian.ensure_can_join_chat_channel!(chat_channel)
-    end
-
-    success = chat_channel.save
-    if success && chat_channel.chatable_has_custom_fields?
-      @chatable.custom_fields[Chat::HAS_CHAT_ENABLED] = true
-      @chatable.save!
-    end
-
-    if success
-      membership = Chat::ChatChannelMembershipManager.new(channel).follow(user)
-      render_serialized(chat_channel, ChatChannelSerializer, membership: membership)
-    else
-      render_json_error(chat_channel)
-    end
-
-    Chat::ChatChannelMembershipManager.new(channel).follow(user)
-  end
-
-  def disable_chat
-    chat_channel = ChatChannel.with_deleted.find_by(chatable: @chatable)
-    guardian.ensure_can_join_chat_channel!(chat_channel)
-    return render json: success_json if chat_channel.trashed?
-    chat_channel.trash!(current_user)
-
-    success = chat_channel.save
-    if success
-      if chat_channel.chatable_has_custom_fields?
-        @chatable.custom_fields.delete(Chat::HAS_CHAT_ENABLED)
-        @chatable.save!
-      end
-
-      render json: success_json
-    else
-      render_json_error(chat_channel)
-    end
-  end
-
-  def create_message
-    raise Discourse::InvalidAccess if current_user.silenced?
-
-    Chat::ChatMessageRateLimiter.run!(current_user)
-
-    @user_chat_channel_membership =
-      Chat::ChatChannelMembershipManager.new(@chat_channel).find_for_user(
-        current_user,
-        following: true,
-      )
-    raise Discourse::InvalidAccess unless @user_chat_channel_membership
-
-    reply_to_msg_id = params[:in_reply_to_id]
-    if reply_to_msg_id
-      rm = ChatMessage.find(reply_to_msg_id)
-      raise Discourse::NotFound if rm.chat_channel_id != @chat_channel.id
-    end
-
-    content = params[:message]
-
-    chat_message_creator =
-      Chat::ChatMessageCreator.create(
-        chat_channel: @chat_channel,
-        user: current_user,
-        in_reply_to_id: reply_to_msg_id,
-        content: content,
-        staged_id: params[:staged_id],
-        upload_ids: params[:upload_ids],
-      )
-
-    return render_json_error(chat_message_creator.error) if chat_message_creator.failed?
-
-    @user_chat_channel_membership.update!(
-      last_read_message_id: chat_message_creator.chat_message.id,
-    )
-
-    if @chat_channel.direct_message_channel?
-      # If any of the channel users is ignoring, muting, or preventing DMs from
-      # the current user then we shold not auto-follow the channel once again or
-      # publish the new channel.
-      user_ids_allowing_communication =
-        UserCommScreener.new(
-          acting_user: current_user,
-          target_user_ids: @chat_channel.user_chat_channel_memberships.pluck(:user_id),
-        ).allowing_actor_communication
-
-      if user_ids_allowing_communication.any?
-        ChatPublisher.publish_new_channel(
-          @chat_channel,
-          @chat_channel.chatable.users.where(id: user_ids_allowing_communication),
-        )
-
-        @chat_channel
-          .user_chat_channel_memberships
-          .where(user_id: user_ids_allowing_communication)
-          .update_all(following: true)
-      end
-    end
-
-    ChatPublisher.publish_user_tracking_state(
-      current_user,
-      @chat_channel.id,
-      chat_message_creator.chat_message.id,
-    )
-    render json: success_json
-  end
-
-  def edit_message
-    chat_message_updater =
-      Chat::ChatMessageUpdater.update(
-        guardian: guardian,
-        chat_message: @message,
-        new_content: params[:new_message],
-        upload_ids: params[:upload_ids] || [],
-      )
-
-    return render_json_error(chat_message_updater.error) if chat_message_updater.failed?
-
-    render json: success_json
-  end
-
-  def messages
-    page_size = params[:page_size]&.to_i || 1000
-    direction = params[:direction].to_s
-    message_id = params[:message_id]
-    if page_size > 50 ||
-         (
-           message_id.blank? ^ direction.blank? &&
-             (direction.present? && !CHAT_DIRECTIONS.include?(direction))
-         )
-      raise Discourse::InvalidParameters
-    end
-
-    messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
-    messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
-
-    if message_id.present?
-      condition = direction == PAST ? "<" : ">"
-      messages = messages.where("id #{condition} ?", message_id.to_i)
-    end
-
-    # NOTE: This order is reversed when we return the ChatView below if the direction
-    # is not FUTURE.
-    order = direction == FUTURE ? "ASC" : "DESC"
-    messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a
-
-    can_load_more_past = nil
-    can_load_more_future = nil
-
-    if direction == FUTURE
-      can_load_more_future = messages.size == page_size
-    elsif direction == PAST
-      can_load_more_past = messages.size == page_size
-    else
-      # When direction is blank, we'll return the latest messages.
-      can_load_more_future = false
-      can_load_more_past = messages.size == page_size
-    end
-
-    chat_view =
-      ChatView.new(
-        chat_channel: @chat_channel,
-        chat_messages: direction == FUTURE ? messages : messages.reverse,
-        user: current_user,
-        can_load_more_past: can_load_more_past,
-        can_load_more_future: can_load_more_future,
-      )
-    render_serialized(chat_view, ChatViewSerializer, root: false)
-  end
-
-  def react
-    params.require(%i[message_id emoji react_action])
-    guardian.ensure_can_react!
-
-    Chat::ChatMessageReactor.new(current_user, @chat_channel).react!(
-      message_id: params[:message_id],
-      react_action: params[:react_action].to_sym,
-      emoji: params[:emoji],
-    )
-
-    render json: success_json
-  end
-
-  def delete
-    guardian.ensure_can_delete_chat!(@message, @chatable)
-
-    ChatMessageDestroyer.new.trash_message(@message, current_user)
-
-    head :ok
-  end
-
-  def restore
-    chat_channel = @message.chat_channel
-    guardian.ensure_can_restore_chat!(@message, chat_channel.chatable)
-    updated = @message.recover!
-    if updated
-      ChatPublisher.publish_restore!(chat_channel, @message)
-      render json: success_json
-    else
-      render_json_error(@message)
-    end
-  end
-
-  def rebake
-    guardian.ensure_can_rebake_chat_message!(@message)
-    @message.rebake!(invalidate_oneboxes: true)
-    render json: success_json
-  end
-
-  def message_link
-    raise Discourse::NotFound if @message.blank? || @message.deleted_at.present?
-    raise Discourse::NotFound if @message.chat_channel.blank?
-    set_channel_and_chatable_with_access_check(chat_channel_id: @message.chat_channel_id)
-    render json:
-             success_json.merge(
-               chat_channel_id: @chat_channel.id,
-               chat_channel_title: @chat_channel.title(current_user),
-             )
-  end
-
-  def lookup_message
-    set_channel_and_chatable_with_access_check(chat_channel_id: @message.chat_channel_id)
-
-    messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
-    messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
-
-    past_messages =
-      messages
-        .where("created_at < ?", @message.created_at)
-        .order(created_at: :desc)
-        .limit(PAST_MESSAGE_LIMIT)
-
-    future_messages =
-      messages
-        .where("created_at > ?", @message.created_at)
-        .order(created_at: :asc)
-        .limit(FUTURE_MESSAGE_LIMIT)
-
-    can_load_more_past = past_messages.count == PAST_MESSAGE_LIMIT
-    can_load_more_future = future_messages.count == FUTURE_MESSAGE_LIMIT
-    messages = [past_messages.reverse, [@message], future_messages].reduce([], :concat)
-    chat_view =
-      ChatView.new(
-        chat_channel: @chat_channel,
-        chat_messages: messages,
-        user: current_user,
-        can_load_more_past: can_load_more_past,
-        can_load_more_future: can_load_more_future,
-      )
-    render_serialized(chat_view, ChatViewSerializer, root: false)
-  end
-
-  def set_user_chat_status
-    params.require(:chat_enabled)
-
-    current_user.user_option.update(chat_enabled: params[:chat_enabled])
-    render json: { chat_enabled: current_user.user_option.chat_enabled }
-  end
-
-  def invite_users
-    params.require(:user_ids)
-
-    users =
-      User
-        .includes(:groups)
-        .joins(:user_option)
-        .where(user_options: { chat_enabled: true })
-        .not_suspended
-        .where(id: params[:user_ids])
-    users.each do |user|
-      guardian = Guardian.new(user)
-      if guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
-        data = {
-          message: "chat.invitation_notification",
-          chat_channel_id: @chat_channel.id,
-          chat_channel_title: @chat_channel.title(user),
-          chat_channel_slug: @chat_channel.slug,
-          invited_by_username: current_user.username,
-        }
-        data[:chat_message_id] = params[:chat_message_id] if params[:chat_message_id]
-        user.notifications.create(
-          notification_type: Notification.types[:chat_invitation],
-          high_priority: true,
-          data: data.to_json,
-        )
-      end
-    end
-
-    render json: success_json
-  end
-
-  def dismiss_retention_reminder
-    params.require(:chatable_type)
-    guardian.ensure_can_chat!
-    unless ChatChannel.chatable_types.include?(params[:chatable_type])
-      raise Discourse::InvalidParameters
-    end
-
-    field =
-      (
-        if ChatChannel.public_channel_chatable_types.include?(params[:chatable_type])
-          :dismissed_channel_retention_reminder
-        else
-          :dismissed_dm_retention_reminder
-        end
-      )
-    current_user.user_option.update(field => true)
-    render json: success_json
-  end
-
-  def quote_messages
-    params.require(:message_ids)
-
-    message_ids = params[:message_ids].map(&:to_i)
-    markdown =
-      ChatTranscriptService.new(
-        @chat_channel,
-        current_user,
-        messages_or_ids: message_ids,
-      ).generate_markdown
-    render json: success_json.merge(markdown: markdown)
-  end
-
-  def flag
-    RateLimiter.new(current_user, "flag_chat_message", 4, 1.minutes).performed!
-
-    permitted_params =
-      params.permit(
-        %i[chat_message_id flag_type_id message is_warning take_action queue_for_review],
-      )
-
-    chat_message =
-      ChatMessage.includes(:chat_channel, :revisions).find(permitted_params[:chat_message_id])
-
-    flag_type_id = permitted_params[:flag_type_id].to_i
-
-    if !ReviewableScore.types.values.include?(flag_type_id)
-      raise Discourse::InvalidParameters.new(:flag_type_id)
-    end
-
-    set_channel_and_chatable_with_access_check(chat_channel_id: chat_message.chat_channel_id)
-
-    result =
-      Chat::ChatReviewQueue.new.flag_message(chat_message, guardian, flag_type_id, permitted_params)
-
-    if result[:success]
-      render json: success_json
-    else
-      render_json_error(result[:errors])
-    end
-  end
-
-  def set_draft
-    if params[:data].present?
-      ChatDraft.find_or_initialize_by(
-        user: current_user,
-        chat_channel_id: @chat_channel.id,
-      ).update!(data: params[:data])
-    else
-      ChatDraft.where(user: current_user, chat_channel_id: @chat_channel.id).destroy_all
-    end
-
-    render json: success_json
-  end
-
-  private
-
-  def preloaded_chat_message_query
-    query =
-      ChatMessage
-        .includes(in_reply_to: [:user, chat_webhook_event: [:incoming_chat_webhook]])
-        .includes(:revisions)
-        .includes(user: :primary_group)
-        .includes(chat_webhook_event: :incoming_chat_webhook)
-        .includes(reactions: :user)
-        .includes(:bookmarks)
-        .includes(:uploads)
-        .includes(chat_channel: :chatable)
-
-    query = query.includes(user: :user_status) if SiteSetting.enable_user_status
-
-    query
-  end
-
-  def find_chatable
-    @chatable = Category.find_by(id: params[:chatable_id])
-    guardian.ensure_can_moderate_chat!(@chatable)
-  end
-
-  def find_chat_message
-    @message = preloaded_chat_message_query.with_deleted
-    @message = @message.where(chat_channel_id: params[:chat_channel_id]) if params[:chat_channel_id]
-    @message = @message.find_by(id: params[:message_id])
-    raise Discourse::NotFound unless @message
-  end
-end
diff --git a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
new file mode 100644
index 0000000000..44943c2e04
--- /dev/null
+++ b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+module Chat
+  # Service responsible for marking all the channels that a user is a
+  # member of _and following_ as read, including mentions.
+  #
+  # @example
+  #  Chat::Service::MarkAllUserChannelsRead.call(guardian: guardian)
+  #
+  class MarkAllUserChannelsRead
+    include ::Service::Base
+
+    # @!method call(guardian:)
+    #   @param [Guardian] guardian
+    #   @return [Chat::Service::Base::Context]
+
+    transaction do
+      step :update_last_read_message_ids
+      step :mark_associated_mentions_as_read
+    end
+
+    private
+
+    def update_last_read_message_ids(guardian:, **)
+      updated_memberships = DB.query(<<~SQL, user_id: guardian.user.id)
+          UPDATE user_chat_channel_memberships
+          SET last_read_message_id = subquery.newest_message_id
+          FROM
+          (
+            SELECT chat_messages.chat_channel_id, MAX(chat_messages.id) AS newest_message_id
+            FROM chat_messages
+            WHERE chat_messages.deleted_at IS NULL
+            GROUP BY chat_messages.chat_channel_id
+          ) AS subquery
+          WHERE user_chat_channel_memberships.chat_channel_id = subquery.chat_channel_id AND
+            subquery.newest_message_id > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND
+            user_chat_channel_memberships.user_id = :user_id AND
+            user_chat_channel_memberships.following
+          RETURNING user_chat_channel_memberships.id AS membership_id,
+                    user_chat_channel_memberships.chat_channel_id AS channel_id,
+                    user_chat_channel_memberships.last_read_message_id;
+        SQL
+      context[:updated_memberships] = updated_memberships
+    end
+
+    def mark_associated_mentions_as_read(guardian:, updated_memberships:, **)
+      return if updated_memberships.empty?
+
+      Notification
+        .where(notification_type: ::Notification.types[:chat_mention])
+        .where(user: guardian.user)
+        .where(read: false)
+        .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
+        .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
+        .where("chat_messages.chat_channel_id IN (?)", updated_memberships.map(&:channel_id))
+        .update_all(read: true)
+    end
+  end
+end
diff --git a/plugins/chat/app/services/chat/update_user_last_read.rb b/plugins/chat/app/services/chat/update_user_last_read.rb
index 0eb01159b2..3be52cdd98 100644
--- a/plugins/chat/app/services/chat/update_user_last_read.rb
+++ b/plugins/chat/app/services/chat/update_user_last_read.rb
@@ -4,23 +4,23 @@ module Chat
   # Service responsible for updating the last read message id of a membership.
   #
   # @example
-  #  Chat::UpdateUserLastRead.call(user_id: 1, channel_id: 2, message_id: 3, guardian: guardian)
+  #  Chat::UpdateUserLastRead.call(channel_id: 2, message_id: 3, guardian: guardian)
   #
   class UpdateUserLastRead
-    include Service::Base
+    include ::Service::Base
 
-    # @!method call(user_id:, channel_id:, message_id:, guardian:)
-    #   @param [Integer] user_id
+    # @!method call(channel_id:, message_id:, guardian:)
     #   @param [Integer] channel_id
     #   @param [Integer] message_id
     #   @param [Guardian] guardian
-    #   @return [Service::Base::Context]
+    #   @return [Chat::Service::Base::Context]
 
     contract
-    model :membership, :fetch_active_membership
+    model :channel
+    model :active_membership
     policy :invalid_access
-    policy :ensure_message_id_recency
     policy :ensure_message_exists
+    policy :ensure_message_id_recency
     step :update_last_read_message_id
     step :mark_associated_mentions_as_read
     step :publish_new_last_read_to_clients
@@ -28,52 +28,52 @@ module Chat
     # @!visibility private
     class Contract
       attribute :message_id, :integer
-      attribute :user_id, :integer
       attribute :channel_id, :integer
 
-      validates :message_id, :user_id, :channel_id, presence: true
+      validates :message_id, :channel_id, presence: true
     end
 
     private
 
-    def fetch_active_membership(user_id:, channel_id:, **)
-      Chat::UserChatChannelMembership.includes(:user, :chat_channel).find_by(
-        user_id: user_id,
-        chat_channel_id: channel_id,
-        following: true,
-      )
+    def fetch_channel(contract:, **)
+      ::Chat::Channel.find_by(id: contract.channel_id)
     end
 
-    def invalid_access(guardian:, membership:, **)
-      guardian.can_join_chat_channel?(membership.chat_channel)
+    def fetch_active_membership(guardian:, channel:, **)
+      ::Chat::ChannelMembershipManager.new(channel).find_for_user(guardian.user, following: true)
     end
 
-    def ensure_message_id_recency(message_id:, membership:, **)
-      !membership.last_read_message_id || message_id >= membership.last_read_message_id
+    def invalid_access(guardian:, active_membership:, **)
+      guardian.can_join_chat_channel?(active_membership.chat_channel)
     end
 
-    def ensure_message_exists(channel_id:, message_id:, **)
-      Chat::Message.with_deleted.exists?(chat_channel_id: channel_id, id: message_id)
+    def ensure_message_exists(channel:, contract:, **)
+      ::Chat::Message.with_deleted.exists?(chat_channel_id: channel.id, id: contract.message_id)
     end
 
-    def update_last_read_message_id(message_id:, membership:, **)
-      membership.update!(last_read_message_id: message_id)
+    def ensure_message_id_recency(contract:, active_membership:, **)
+      !active_membership.last_read_message_id ||
+        contract.message_id >= active_membership.last_read_message_id
     end
 
-    def mark_associated_mentions_as_read(membership:, message_id:, **)
-      Notification
+    def update_last_read_message_id(contract:, active_membership:, **)
+      active_membership.update!(last_read_message_id: contract.message_id)
+    end
+
+    def mark_associated_mentions_as_read(active_membership:, contract:, **)
+      ::Notification
         .where(notification_type: Notification.types[:chat_mention])
-        .where(user: membership.user)
+        .where(user: active_membership.user)
         .where(read: false)
         .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
         .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
-        .where("chat_messages.id <= ?", message_id)
-        .where("chat_messages.chat_channel_id = ?", membership.chat_channel.id)
+        .where("chat_messages.id <= ?", contract.message_id)
+        .where("chat_messages.chat_channel_id = ?", active_membership.chat_channel.id)
         .update_all(read: true)
     end
 
-    def publish_new_last_read_to_clients(guardian:, channel_id:, message_id:, **)
-      Chat::Publisher.publish_user_tracking_state(guardian.user, channel_id, message_id)
+    def publish_new_last_read_to_clients(guardian:, channel:, contract:, **)
+      ::Chat::Publisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id)
     end
   end
 end
diff --git a/plugins/chat/app/services/mark_all_user_channels_read.rb b/plugins/chat/app/services/mark_all_user_channels_read.rb
deleted file mode 100644
index 8e5fb3dfbc..0000000000
--- a/plugins/chat/app/services/mark_all_user_channels_read.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-# frozen_string_literal: true
-
-module Chat
-  module Service
-    # Service responsible for marking all the channels that a user is a
-    # member of _and following_ as read, including mentions.
-    #
-    # @example
-    #  Chat::Service::MarkAllUserChannelsRead.call(guardian: guardian)
-    #
-    class MarkAllUserChannelsRead
-      include Base
-
-      # @!method call(guardian:)
-      #   @param [Guardian] guardian
-      #   @return [Chat::Service::Base::Context]
-
-      transaction do
-        step :update_last_read_message_ids
-        step :mark_associated_mentions_as_read
-      end
-
-      private
-
-      def update_last_read_message_ids(guardian:, **)
-        updated_memberships = DB.query(<<~SQL, user_id: guardian.user.id)
-          UPDATE user_chat_channel_memberships
-          SET last_read_message_id = subquery.newest_message_id
-          FROM
-          (
-            SELECT chat_messages.chat_channel_id, MAX(chat_messages.id) AS newest_message_id
-            FROM chat_messages
-            WHERE chat_messages.deleted_at IS NULL
-            GROUP BY chat_messages.chat_channel_id
-          ) AS subquery
-          WHERE user_chat_channel_memberships.chat_channel_id = subquery.chat_channel_id AND
-            subquery.newest_message_id > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND
-            user_chat_channel_memberships.user_id = :user_id AND
-            user_chat_channel_memberships.following
-          RETURNING user_chat_channel_memberships.id AS membership_id,
-                    user_chat_channel_memberships.chat_channel_id AS channel_id,
-                    user_chat_channel_memberships.last_read_message_id;
-        SQL
-        context[:updated_memberships] = updated_memberships
-      end
-
-      def mark_associated_mentions_as_read(guardian:, updated_memberships:, **)
-        return if updated_memberships.empty?
-
-        Notification
-          .where(notification_type: Notification.types[:chat_mention])
-          .where(user: guardian.user)
-          .where(read: false)
-          .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
-          .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
-          .where("chat_messages.chat_channel_id IN (?)", updated_memberships.map(&:channel_id))
-          .update_all(read: true)
-      end
-    end
-  end
-end
diff --git a/plugins/chat/app/services/update_user_last_read.rb b/plugins/chat/app/services/update_user_last_read.rb
deleted file mode 100644
index ac889d2ba6..0000000000
--- a/plugins/chat/app/services/update_user_last_read.rb
+++ /dev/null
@@ -1,84 +0,0 @@
-# frozen_string_literal: true
-
-module Chat
-  module Service
-    # Service responsible for updating the last read message id of a membership.
-    #
-    # @example
-    #  Chat::Service::UpdateUserLastRead.call(channel_id: 2, message_id: 3, guardian: guardian)
-    #
-    class UpdateUserLastRead
-      include Base
-
-      # @!method call(channel_id:, message_id:, guardian:)
-      #   @param [Integer] channel_id
-      #   @param [Integer] message_id
-      #   @param [Guardian] guardian
-      #   @return [Chat::Service::Base::Context]
-
-      contract
-      model :channel
-      model :active_membership
-      policy :invalid_access
-      policy :ensure_message_exists
-      policy :ensure_message_id_recency
-      step :update_last_read_message_id
-      step :mark_associated_mentions_as_read
-      step :publish_new_last_read_to_clients
-
-      # @!visibility private
-      class Contract
-        attribute :message_id, :integer
-        attribute :channel_id, :integer
-
-        validates :message_id, :channel_id, presence: true
-      end
-
-      private
-
-      def fetch_channel(contract:, **)
-        ChatChannel.find_by(id: contract.channel_id)
-      end
-
-      def fetch_active_membership(guardian:, channel:, **)
-        Chat::ChatChannelMembershipManager.new(channel).find_for_user(
-          guardian.user,
-          following: true,
-        )
-      end
-
-      def invalid_access(guardian:, active_membership:, **)
-        guardian.can_join_chat_channel?(active_membership.chat_channel)
-      end
-
-      def ensure_message_exists(channel:, contract:, **)
-        ChatMessage.with_deleted.exists?(chat_channel_id: channel.id, id: contract.message_id)
-      end
-
-      def ensure_message_id_recency(contract:, active_membership:, **)
-        !active_membership.last_read_message_id ||
-          contract.message_id >= active_membership.last_read_message_id
-      end
-
-      def update_last_read_message_id(contract:, active_membership:, **)
-        active_membership.update!(last_read_message_id: contract.message_id)
-      end
-
-      def mark_associated_mentions_as_read(active_membership:, contract:, **)
-        Notification
-          .where(notification_type: Notification.types[:chat_mention])
-          .where(user: active_membership.user)
-          .where(read: false)
-          .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
-          .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
-          .where("chat_messages.id <= ?", contract.message_id)
-          .where("chat_messages.chat_channel_id = ?", active_membership.chat_channel.id)
-          .update_all(read: true)
-      end
-
-      def publish_new_last_read_to_clients(guardian:, channel:, contract:, **)
-        ChatPublisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id)
-      end
-    end
-  end
-end
diff --git a/plugins/chat/config/routes.rb b/plugins/chat/config/routes.rb
index 0c66fac78e..a51a5da3dc 100644
--- a/plugins/chat/config/routes.rb
+++ b/plugins/chat/config/routes.rb
@@ -17,6 +17,7 @@ Chat::Engine.routes.draw do
     post "/channels/:channel_id/memberships/me" => "channels_current_user_membership#create"
     put "/channels/:channel_id/notifications-settings/me" =>
           "channels_current_user_notifications_settings#update"
+    put "/tracking/read/me" => "tracking#read"
 
     # Category chatables controller hints. Only used by staff members, we don't want to leak category permissions.
     get "/category-chatables/:id/permissions" => "category_chatables#permissions",

From 5417dc7ab9c7a4f46676981954f09f37f1f0a307 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Sat, 18 Mar 2023 19:03:11 +0100
Subject: [PATCH 8/9] more fixes

---
 .../app/controllers/chat/chat_controller.rb   | 41 -------------------
 .../chat/mark_all_user_channels_read.rb       |  2 +-
 ...er_spec.rb => tracking_controller_spec.rb} |  6 +--
 3 files changed, 4 insertions(+), 45 deletions(-)
 rename plugins/chat/spec/requests/chat/api/{chat_tracking_controller_spec.rb => tracking_controller_spec.rb} (97%)

diff --git a/plugins/chat/app/controllers/chat/chat_controller.rb b/plugins/chat/app/controllers/chat/chat_controller.rb
index fcb918f1e4..8a797bd53a 100644
--- a/plugins/chat/app/controllers/chat/chat_controller.rb
+++ b/plugins/chat/app/controllers/chat/chat_controller.rb
@@ -160,47 +160,6 @@ module Chat
       render json: success_json
     end
 
-    def update_user_last_read
-      membership =
-        Chat::ChannelMembershipManager.new(@chat_channel).find_for_user(
-          current_user,
-          following: true,
-        )
-      raise Discourse::NotFound if membership.nil?
-
-      if membership.last_read_message_id &&
-           params[:message_id].to_i < membership.last_read_message_id
-        raise Discourse::InvalidParameters.new(:message_id)
-      end
-
-      unless Chat::Message.with_deleted.exists?(
-               chat_channel_id: @chat_channel.id,
-               id: params[:message_id],
-             )
-        raise Discourse::NotFound
-      end
-
-      membership.update!(last_read_message_id: params[:message_id])
-
-      Notification
-        .where(notification_type: Notification.types[:chat_mention])
-        .where(user: current_user)
-        .where(read: false)
-        .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id")
-        .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
-        .where("chat_messages.id <= ?", params[:message_id].to_i)
-        .where("chat_messages.chat_channel_id = ?", @chat_channel.id)
-        .update_all(read: true)
-
-      Chat::Publisher.publish_user_tracking_state(
-        current_user,
-        @chat_channel.id,
-        params[:message_id],
-      )
-
-      render json: success_json
-    end
-
     def messages
       page_size = params[:page_size]&.to_i || 1000
       direction = params[:direction].to_s
diff --git a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
index 44943c2e04..3327821ef7 100644
--- a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
+++ b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
@@ -5,7 +5,7 @@ module Chat
   # member of _and following_ as read, including mentions.
   #
   # @example
-  #  Chat::Service::MarkAllUserChannelsRead.call(guardian: guardian)
+  #  Chat::MarkAllUserChannelsRead.call(guardian: guardian)
   #
   class MarkAllUserChannelsRead
     include ::Service::Base
diff --git a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb b/plugins/chat/spec/requests/chat/api/tracking_controller_spec.rb
similarity index 97%
rename from plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
rename to plugins/chat/spec/requests/chat/api/tracking_controller_spec.rb
index 9bb84e9674..be40bdc5c5 100644
--- a/plugins/chat/spec/requests/chat/api/chat_tracking_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/tracking_controller_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe Chat::Api::ChatTrackingController do
+RSpec.describe Chat::Api::TrackingController do
   fab!(:current_user) { Fabricate(:user) }
 
   before do
@@ -96,7 +96,7 @@ RSpec.describe Chat::Api::ChatTrackingController do
                   channel_id: chat_channel.id,
                   message_id: message_1.id,
                 }
-          }.not_to change { UserChatChannelMembership.count }
+          }.not_to change { Chat::UserChatChannelMembership.count }
 
           membership.reload
           expect(membership.chat_channel_id).to eq(chat_channel.id)
@@ -230,7 +230,7 @@ RSpec.describe Chat::Api::ChatTrackingController do
         }.to_json,
       )
       .tap do |notification|
-        ChatMention.create!(user: user, chat_message: msg, notification: notification)
+        Chat::Mention.create!(user: user, chat_message: msg, notification: notification)
       end
   end
 end

From 08fbcfc386cc4b540f34d9f04c4c5cbcda8d2a31 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Sat, 18 Mar 2023 19:05:42 +0100
Subject: [PATCH 9/9] remaining conflicts

---
 .../chat/app/controllers/chat/api/tracking_controller.rb    | 2 +-
 .../services/{ => chat}/mark_all_user_channels_read_spec.rb | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename plugins/chat/spec/services/{ => chat}/mark_all_user_channels_read_spec.rb (97%)

diff --git a/plugins/chat/app/controllers/chat/api/tracking_controller.rb b/plugins/chat/app/controllers/chat/api/tracking_controller.rb
index c3b631dc78..1ae06665fe 100644
--- a/plugins/chat/app/controllers/chat/api/tracking_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/tracking_controller.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class Chat::Api::TrackingController < Chat::Api
+class Chat::Api::TrackingController < Chat::ApiController
   def read
     params.permit(:channel_id, :message_id)
 
diff --git a/plugins/chat/spec/services/mark_all_user_channels_read_spec.rb b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
similarity index 97%
rename from plugins/chat/spec/services/mark_all_user_channels_read_spec.rb
rename to plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
index 80b9e05c60..31adc20f29 100644
--- a/plugins/chat/spec/services/mark_all_user_channels_read_spec.rb
+++ b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe Chat::Service::MarkAllUserChannelsRead do
+RSpec.describe Chat::MarkAllUserChannelsRead do
   describe ".call" do
     subject(:result) { described_class.call(params) }
 
@@ -78,12 +78,12 @@ RSpec.describe Chat::Service::MarkAllUserChannelsRead do
       let(:messages) { MessageBus.track_publish { result } }
 
       before do
-        ChatMention.create!(
+        Chat::Mention.create!(
           notification: notification_1,
           user: current_user,
           chat_message: message_1,
         )
-        ChatMention.create!(
+        Chat::Mention.create!(
           notification: notification_2,
           user: current_user,
           chat_message: message_3,