Chore: Inbox Members API improvements (#3008)

- New Inbox Member APIs
- Return JSON errors for Platform APIs

app/controllers/api/v1/accounts/inbox_members_controller.rb

@@ -1,26 +1,40 @@
 class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseController
-  before_action :fetch_inbox, only: [:create, :show]
-  before_action :current_agents_ids, only: [:create]
+  before_action :fetch_inbox
+  before_action :current_agents_ids, only: [:update]
 
   def create
     authorize @inbox, :create?
-    begin
-      # update also done via same action
-      update_agents_list
-      head :ok
-    rescue StandardError => e
-      Rails.logger.debug { "Rescued: #{e.inspect}" }
-      render_could_not_create_error('Could not add agents to inbox')
+    ActiveRecord::Base.transaction do
+      params[:user_ids].map { |user_id| @inbox.add_member(user_id) }
     end
+    fetch_updated_agents
   end
 
   def show
     authorize @inbox, :show?
-    @agents = Current.account.users.where(id: @inbox.members.select(:user_id))
+    fetch_updated_agents
+  end
+
+  def update
+    authorize @inbox, :update?
+    update_agents_list
+    fetch_updated_agents
+  end
+
+  def destroy
+    authorize @inbox, :destroy?
+    ActiveRecord::Base.transaction do
+      params[:user_ids].map { |user_id| @inbox.remove_member(user_id) }
+    end
+    head :ok
   end
 
   private
 
+  def fetch_updated_agents
+    @agents = Current.account.users.where(id: @inbox.members.select(:user_id))
+  end
+
   def update_agents_list
     # get all the user_ids which the inbox currently has as members.
     # get the list of  user_ids from params

app/controllers/api/v1/accounts/inboxes_controller.rb

@@ -43,7 +43,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController
     @inbox.update_working_hours(params.permit(working_hours: Inbox::OFFISABLE_ATTRS)[:working_hours]) if params[:working_hours]
 
     channel_attributes = get_channel_attributes(@inbox.channel_type)
-    @inbox.channel.update!(permitted_params(channel_attributes)[:channel])
+    @inbox.channel.update!(permitted_params(channel_attributes)[:channel]) if permitted_params(channel_attributes)[:channel].present?
     update_channel_feature_flags
   end
 

app/controllers/application_controller.rb

@@ -1,5 +1,6 @@
 class ApplicationController < ActionController::Base
   include DeviseTokenAuth::Concerns::SetUserByToken
+  include RequestExceptionHandler
   include Pundit
   include SwitchLocale
 
@@ -9,22 +10,8 @@ class ApplicationController < ActionController::Base
   around_action :switch_locale
   around_action :handle_with_exception, unless: :devise_controller?
 
-  rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid
-
   private
 
-  def handle_with_exception
-    yield
-  rescue ActiveRecord::RecordNotFound => e
-    Sentry.capture_exception(e)
-    render_not_found_error('Resource could not be found')
-  rescue Pundit::NotAuthorizedError
-    render_unauthorized('You are not authorized to do this action')
-  ensure
-    # to address the thread variable leak issues in Puma/Thin webserver
-    Current.reset
-  end
-
   def set_current_user
     @user ||= current_user
     Current.user = @user
@@ -34,32 +21,6 @@ class ApplicationController < ActionController::Base
     @subscription ||= Current.account.subscription
   end
 
-  def render_unauthorized(message)
-    render json: { error: message }, status: :unauthorized
-  end
-
-  def render_not_found_error(message)
-    render json: { error: message }, status: :not_found
-  end
-
-  def render_could_not_create_error(message)
-    render json: { error: message }, status: :unprocessable_entity
-  end
-
-  def render_internal_server_error(message)
-    render json: { error: message }, status: :internal_server_error
-  end
-
-  def render_record_invalid(exception)
-    render json: {
-      message: exception.record.errors.full_messages.join(', ')
-    }, status: :unprocessable_entity
-  end
-
-  def render_error_response(exception)
-    render json: exception.to_hash, status: exception.http_status
-  end
-
   def pundit_user
     {
       user: Current.user,

app/controllers/concerns/request_exception_handler.rb

@@ -0,0 +1,47 @@
+module RequestExceptionHandler
+  extend ActiveSupport::Concern
+
+  included do
+    rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid
+  end
+
+  private
+
+  def handle_with_exception
+    yield
+  rescue ActiveRecord::RecordNotFound => e
+    Sentry.capture_exception(e)
+    render_not_found_error('Resource could not be found')
+  rescue Pundit::NotAuthorizedError
+    render_unauthorized('You are not authorized to do this action')
+  ensure
+    # to address the thread variable leak issues in Puma/Thin webserver
+    Current.reset
+  end
+
+  def render_unauthorized(message)
+    render json: { error: message }, status: :unauthorized
+  end
+
+  def render_not_found_error(message)
+    render json: { error: message }, status: :not_found
+  end
+
+  def render_could_not_create_error(message)
+    render json: { error: message }, status: :unprocessable_entity
+  end
+
+  def render_internal_server_error(message)
+    render json: { error: message }, status: :internal_server_error
+  end
+
+  def render_record_invalid(exception)
+    render json: {
+      message: exception.record.errors.full_messages.join(', ')
+    }, status: :unprocessable_entity
+  end
+
+  def render_error_response(exception)
+    render json: exception.to_hash, status: exception.http_status
+  end
+end

app/controllers/platform/api/v1/users_controller.rb

@@ -7,8 +7,8 @@ class Platform::Api::V1::UsersController < PlatformController
 
   def create
     @resource = (User.find_by(email: user_params[:email]) || User.new(user_params))
-    @resource.confirm
     @resource.save!
+    @resource.confirm
     @platform_app.platform_app_permissibles.find_or_create_by(permissible: @resource)
   end
 

app/controllers/platform_controller.rb

@@ -1,4 +1,6 @@
 class PlatformController < ActionController::API
+  include RequestExceptionHandler
+
   before_action :ensure_access_token
   before_action :set_platform_app
   before_action :set_resource, only: [:update, :show, :destroy]

app/controllers/public_controller.rb

@@ -1,5 +1,6 @@
 # TODO: we should switch to ActionController::API for the base classes
 # One of the specs is failing when I tried doing that, lets revisit in future
 class PublicController < ActionController::Base
+  include RequestExceptionHandler
   skip_before_action :verify_authenticity_token
 end

app/javascript/dashboard/api/inboxMembers.js

@@ -6,8 +6,8 @@ class InboxMembers extends ApiClient {
     super('inbox_members', { accountScoped: true });
   }
 
-  create({ inboxId, agentList }) {
-    return axios.post(this.url, {
+  update({ inboxId, agentList }) {
+    return axios.patch(this.url, {
       inbox_id: inboxId,
       user_ids: agentList,
     });

app/javascript/dashboard/store/modules/inboxMembers.js

@@ -5,7 +5,7 @@ export const actions = {
     return InboxMembersAPI.show(inboxId);
   },
   create(_, { inboxId, agentList }) {
-    return InboxMembersAPI.create({ inboxId, agentList });
+    return InboxMembersAPI.update({ inboxId, agentList });
   },
 };
 

app/models/inbox.rb

@@ -62,7 +62,7 @@ class Inbox < ApplicationRecord
   end
 
   def remove_member(user_id)
-    member = inbox_members.find_by(user_id: user_id)
+    member = inbox_members.find_by!(user_id: user_id)
     member.try(:destroy)
   end
 

app/views/api/v1/accounts/inbox_members/create.json.jbuilder

@@ -0,0 +1,5 @@
+json.payload do
+  json.array! @agents do |agent|
+    json.partial! 'api/v1/models/agent.json.jbuilder', resource: agent
+  end
+end

app/views/api/v1/accounts/inbox_members/update.json.jbuilder

@@ -0,0 +1,5 @@
+json.payload do
+  json.array! @agents do |agent|
+    json.partial! 'api/v1/models/agent.json.jbuilder', resource: agent
+  end
+end