Chore: Code Cleanup in API controllers (#932)

* Chore: Code Cleanup in API controllers

* Remove unnecessary scoping for accounts controller
This commit is contained in:
Sojan Jose 2020-06-07 13:58:05 +05:30 committed by GitHub
parent 3d84568a37
commit 051871a3cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
35 changed files with 176 additions and 182 deletions

View file

@ -11,10 +11,6 @@ class Api::BaseController < ApplicationController
request.headers[:api_access_token].present? || request.headers[:HTTP_API_ACCESS_TOKEN].present?
end
def set_conversation
@conversation ||= current_account.conversations.find_by(display_id: params[:conversation_id])
end
def check_billing_enabled
raise ActionController::RoutingError, 'Not Found' unless ENV['BILLING_ENABLED']
end

View file

@ -1,10 +1,10 @@
class Api::V1::Accounts::Actions::ContactMergesController < Api::BaseController
class Api::V1::Accounts::Actions::ContactMergesController < Api::V1::Accounts::BaseController
before_action :set_base_contact, only: [:create]
before_action :set_mergee_contact, only: [:create]
def create
contact_merge_action = ContactMergeAction.new(
account: current_account,
account: Current.account,
base_contact: @base_contact,
mergee_contact: @mergee_contact
)
@ -23,6 +23,6 @@ class Api::V1::Accounts::Actions::ContactMergesController < Api::BaseController
end
def contacts
@contacts ||= current_account.contacts
@contacts ||= Current.account.contacts
end
end

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::AgentsController < Api::BaseController
class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController
before_action :fetch_agent, except: [:create, :index]
before_action :check_authorization
before_action :find_user, only: [:create]
@ -46,7 +46,7 @@ class Api::V1::Accounts::AgentsController < Api::BaseController
def save_account_user
AccountUser.create!(
account_id: current_account.id,
account_id: Current.account.id,
user_id: @user.id,
role: new_agent_params[:role],
inviter_id: current_user.id
@ -64,6 +64,6 @@ class Api::V1::Accounts::AgentsController < Api::BaseController
end
def agents
@agents ||= current_account.users
@agents ||= Current.account.users
end
end

View file

@ -0,0 +1,31 @@
class Api::V1::Accounts::BaseController < Api::BaseController
before_action :current_account
private
def current_account
@current_account ||= ensure_current_account
Current.account = @current_account
end
def ensure_current_account
account = Account.find(params[:account_id])
if current_user
account_accessible_for_user?(account)
elsif @resource&.is_a?(AgentBot)
account_accessible_for_bot?(account)
end
switch_locale account
account
end
def account_accessible_for_user?(account)
@current_account_user = account.account_users.find_by(user_id: current_user.id)
Current.account_user = @current_account_user
render_unauthorized('You are not authorized to access this account') unless @current_account_user
end
def account_accessible_for_bot?(account)
render_unauthorized('You are not authorized to access this account') unless @resource.agent_bot_inboxes.find_by(account_id: account.id)
end
end

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::CallbacksController < Api::BaseController
class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController
before_action :inbox, only: [:reauthorize_page]
def register_facebook_page
@ -7,11 +7,11 @@ class Api::V1::Accounts::CallbacksController < Api::BaseController
page_id = params[:page_id]
inbox_name = params[:inbox_name]
ActiveRecord::Base.transaction do
facebook_channel = current_account.facebook_pages.create!(
facebook_channel = Current.account.facebook_pages.create!(
page_id: page_id, user_access_token: user_access_token,
page_access_token: page_access_token
)
@facebook_inbox = current_account.inboxes.create!(name: inbox_name, channel: facebook_channel)
@facebook_inbox = Current.account.inboxes.create!(name: inbox_name, channel: facebook_channel)
set_avatar(@facebook_inbox, page_id)
rescue StandardError => e
Rails.logger.info e
@ -22,7 +22,7 @@ class Api::V1::Accounts::CallbacksController < Api::BaseController
@page_details = mark_already_existing_facebook_pages(fb_object.get_connections('me', 'accounts'))
end
# get params[:inbox_id], current_account, params[:omniauth_token]
# get params[:inbox_id], current_account. params[:omniauth_token]
def reauthorize_page
if @inbox&.facebook?
fb_page_id = @inbox.channel.page_id
@ -40,7 +40,7 @@ class Api::V1::Accounts::CallbacksController < Api::BaseController
private
def inbox
@inbox = current_account.inboxes.find_by(id: params[:inbox_id])
@inbox = Current.account.inboxes.find_by(id: params[:inbox_id])
end
def update_fb_page(fb_page_id, access_token)
@ -50,7 +50,7 @@ class Api::V1::Accounts::CallbacksController < Api::BaseController
end
def get_fb_page(fb_page_id)
current_account.facebook_pages.find_by(page_id: fb_page_id)
Current.account.facebook_pages.find_by(page_id: fb_page_id)
end
def fb_object
@ -69,7 +69,7 @@ class Api::V1::Accounts::CallbacksController < Api::BaseController
return [] if data.empty?
data.inject([]) do |result, page_detail|
page_detail[:exists] = current_account.facebook_pages.exists?(page_id: page_detail['id']) ? true : false
page_detail[:exists] = Current.account.facebook_pages.exists?(page_id: page_detail['id']) ? true : false
result << page_detail
end
end

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::CannedResponsesController < Api::BaseController
class Api::V1::Accounts::CannedResponsesController < Api::V1::Accounts::BaseController
before_action :fetch_canned_response, only: [:update, :destroy]
def index
@ -6,7 +6,7 @@ class Api::V1::Accounts::CannedResponsesController < Api::BaseController
end
def create
@canned_response = current_account.canned_responses.new(canned_response_params)
@canned_response = Current.account.canned_responses.new(canned_response_params)
@canned_response.save!
render json: @canned_response
end
@ -24,7 +24,7 @@ class Api::V1::Accounts::CannedResponsesController < Api::BaseController
private
def fetch_canned_response
@canned_response = current_account.canned_responses.find(params[:id])
@canned_response = Current.account.canned_responses.find(params[:id])
end
def canned_response_params
@ -33,9 +33,9 @@ class Api::V1::Accounts::CannedResponsesController < Api::BaseController
def canned_responses
if params[:search]
current_account.canned_responses.where('short_code ILIKE ?', "#{params[:search]}%")
Current.account.canned_responses.where('short_code ILIKE ?', "#{params[:search]}%")
else
current_account.canned_responses
Current.account.canned_responses
end
end
end

View file

@ -1,5 +1,4 @@
class Api::V1::Accounts::Channels::TwilioChannelsController < Api::BaseController
before_action :current_account
class Api::V1::Accounts::Channels::TwilioChannelsController < Api::V1::Accounts::BaseController
before_action :authorize_request
def create
@ -38,13 +37,13 @@ class Api::V1::Accounts::Channels::TwilioChannelsController < Api::BaseControlle
end
def build_inbox
@twilio_channel = current_account.twilio_sms.create!(
@twilio_channel = Current.account.twilio_sms.create!(
account_sid: permitted_params[:account_sid],
auth_token: permitted_params[:auth_token],
phone_number: phone_number,
medium: medium
)
@inbox = current_account.inboxes.create(
@inbox = Current.account.inboxes.create(
name: permitted_params[:name],
channel: @twilio_channel
)

View file

@ -1,6 +1,6 @@
class Api::V1::Accounts::Contacts::ConversationsController < Api::BaseController
class Api::V1::Accounts::Contacts::ConversationsController < Api::V1::Accounts::BaseController
def index
@conversations = current_account.conversations.includes(
@conversations = Current.account.conversations.includes(
:assignee, :contact, :inbox
).where(inbox_id: inbox_ids, contact_id: permitted_params[:contact_id])
end
@ -9,7 +9,7 @@ class Api::V1::Accounts::Contacts::ConversationsController < Api::BaseController
def inbox_ids
if current_user.administrator?
current_account.inboxes.pluck(:id)
Current.account.inboxes.pluck(:id)
elsif current_user.agent?
current_user.assigned_inboxes.pluck(:id)
else

View file

@ -1,17 +1,17 @@
class Api::V1::Accounts::ContactsController < Api::BaseController
class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
protect_from_forgery with: :null_session
before_action :check_authorization
before_action :fetch_contact, only: [:show, :update]
def index
@contacts = current_account.contacts
@contacts = Current.account.contacts
end
def show; end
def create
@contact = Contact.new(contact_create_params)
@contact = Current.account.contacts.new(contact_create_params)
@contact.save!
render json: @contact
end
@ -31,10 +31,10 @@ class Api::V1::Accounts::ContactsController < Api::BaseController
end
def fetch_contact
@contact = current_account.contacts.find(params[:id])
@contact = Current.account.contacts.find(params[:id])
end
def contact_create_params
params.require(:contact).permit(:account_id, :inbox_id).merge!(name: SecureRandom.hex)
params.require(:contact).permit(:name, :email, :phone_number)
end
end

View file

@ -1,10 +1,8 @@
class Api::V1::Accounts::Conversations::AssignmentsController < Api::BaseController
before_action :set_conversation, only: [:create]
class Api::V1::Accounts::Conversations::AssignmentsController < Api::V1::Accounts::Conversations::BaseController
# assign agent to a conversation
def create
# if params[:assignee_id] is not a valid id, it will set to nil, hence unassigning the conversation
assignee = current_account.users.find_by(id: params[:assignee_id])
assignee = Current.account.users.find_by(id: params[:assignee_id])
@conversation.update_assignee(assignee)
render json: assignee
end

View file

@ -0,0 +1,9 @@
class Api::V1::Accounts::Conversations::BaseController < Api::V1::Accounts::BaseController
before_action :conversation
private
def conversation
@conversation ||= Current.account.conversations.find_by(display_id: params[:conversation_id])
end
end

View file

@ -1,6 +1,4 @@
class Api::V1::Accounts::Conversations::LabelsController < Api::BaseController
before_action :set_conversation, only: [:create, :index]
class Api::V1::Accounts::Conversations::LabelsController < Api::V1::Accounts::Conversations::BaseController
def create
@conversation.update_labels(params[:labels])
@labels = @conversation.label_list

View file

@ -1,6 +1,4 @@
class Api::V1::Accounts::Conversations::MessagesController < Api::BaseController
before_action :set_conversation, only: [:index, :create]
class Api::V1::Accounts::Conversations::MessagesController < Api::V1::Accounts::Conversations::BaseController
def index
@messages = message_finder.perform
end

View file

@ -1,6 +1,5 @@
class Api::V1::Accounts::ConversationsController < Api::BaseController
class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseController
include Events::Types
before_action :current_account
before_action :conversation, except: [:index]
before_action :contact_inbox, only: [:create]
@ -62,7 +61,7 @@ class Api::V1::Accounts::ConversationsController < Api::BaseController
end
def conversation
@conversation ||= current_account.conversations.find_by(display_id: params[:id])
@conversation ||= Current.account.conversations.find_by(display_id: params[:id])
end
def contact_inbox
@ -71,7 +70,7 @@ class Api::V1::Accounts::ConversationsController < Api::BaseController
def conversation_params
{
account_id: current_account.id,
account_id: Current.account.id,
inbox_id: @contact_inbox.inbox_id,
contact_id: @contact_inbox.contact_id,
contact_inbox_id: @contact_inbox.id

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::FacebookIndicatorsController < Api::BaseController
class Api::V1::Accounts::FacebookIndicatorsController < Api::V1::Accounts::BaseController
before_action :set_access_token
around_action :handle_with_exception
@ -38,7 +38,7 @@ class Api::V1::Accounts::FacebookIndicatorsController < Api::BaseController
end
def inbox
@inbox ||= current_account.inboxes.find(permitted_params[:inbox_id])
@inbox ||= Current.account.inboxes.find(permitted_params[:inbox_id])
end
def set_access_token

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::InboxMembersController < Api::BaseController
class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseController
before_action :fetch_inbox, only: [:create, :show]
before_action :current_agents_ids, only: [:create]
@ -12,7 +12,7 @@ class Api::V1::Accounts::InboxMembersController < Api::BaseController
end
def show
@agents = current_account.users.where(id: @inbox.members.pluck(:user_id))
@agents = Current.account.users.where(id: @inbox.members.pluck(:user_id))
end
private
@ -40,6 +40,6 @@ class Api::V1::Accounts::InboxMembersController < Api::BaseController
end
def fetch_inbox
@inbox = current_account.inboxes.find(params[:inbox_id])
@inbox = Current.account.inboxes.find(params[:inbox_id])
end
end

View file

@ -1,17 +1,16 @@
class Api::V1::Accounts::InboxesController < Api::BaseController
before_action :current_account
class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController
before_action :fetch_inbox, except: [:index, :create]
before_action :fetch_agent_bot, only: [:set_agent_bot]
before_action :check_authorization
def index
@inboxes = policy_scope(current_account.inboxes)
@inboxes = policy_scope(Current.account.inboxes)
end
def create
ActiveRecord::Base.transaction do
channel = web_widgets.create!(permitted_params[:channel].except(:type)) if permitted_params[:channel][:type] == 'web_widget'
@inbox = current_account.inboxes.build(name: permitted_params[:name], channel: channel)
@inbox = Current.account.inboxes.build(name: permitted_params[:name], channel: channel)
@inbox.avatar.attach(permitted_params[:avatar])
@inbox.save!
end
@ -41,7 +40,7 @@ class Api::V1::Accounts::InboxesController < Api::BaseController
private
def fetch_inbox
@inbox = current_account.inboxes.find(params[:id])
@inbox = Current.account.inboxes.find(params[:id])
end
def fetch_agent_bot
@ -49,7 +48,7 @@ class Api::V1::Accounts::InboxesController < Api::BaseController
end
def web_widgets
current_account.web_widgets
Current.account.web_widgets
end
def check_authorization

View file

@ -1,16 +1,16 @@
class Api::V1::Accounts::LabelsController < Api::BaseController
class Api::V1::Accounts::LabelsController < Api::V1::Accounts::BaseController
before_action :current_account
before_action :fetch_label, except: [:index, :create]
before_action :check_authorization
def index
@labels = policy_scope(current_account.labels)
@labels = policy_scope(Current.account.labels)
end
def show; end
def create
@label = current_account.labels.create!(permitted_params)
@label = Current.account.labels.create!(permitted_params)
end
def update
@ -25,7 +25,7 @@ class Api::V1::Accounts::LabelsController < Api::BaseController
private
def fetch_label
@label = current_account.labels.find(params[:id])
@label = Current.account.labels.find(params[:id])
end
def check_authorization

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::NotificationSettingsController < Api::BaseController
class Api::V1::Accounts::NotificationSettingsController < Api::V1::Accounts::BaseController
before_action :set_user, :load_notification_setting
def show; end
@ -16,7 +16,7 @@ class Api::V1::Accounts::NotificationSettingsController < Api::BaseController
end
def load_notification_setting
@notification_setting = @user.notification_settings.find_by(account_id: current_account.id)
@notification_setting = @user.notification_settings.find_by(account_id: Current.account.id)
end
def notification_setting_params

View file

@ -1,10 +1,10 @@
class Api::V1::Accounts::NotificationsController < Api::BaseController
class Api::V1::Accounts::NotificationsController < Api::V1::Accounts::BaseController
protect_from_forgery with: :null_session
before_action :fetch_notification, only: [:update]
def index
@notifications = current_user.notifications.where(account_id: current_account.id)
@notifications = current_user.notifications.where(account_id: Current.account.id)
render json: @notifications
end

View file

@ -1,13 +1,13 @@
class Api::V1::Accounts::SubscriptionsController < Api::BaseController
class Api::V1::Accounts::SubscriptionsController < Api::V1::Accounts::BaseController
skip_before_action :check_subscription
before_action :check_billing_enabled
def index
render json: current_account.subscription_data
render json: Current.account.subscription_data
end
def status
render json: current_account.subscription.summary
render json: Current.account.subscription.summary
end
end

View file

@ -1,14 +1,13 @@
class Api::V1::Accounts::WebhooksController < Api::BaseController
before_action :current_account
class Api::V1::Accounts::WebhooksController < Api::V1::Accounts::BaseController
before_action :check_authorization
before_action :fetch_webhook, only: [:update, :destroy]
def index
@webhooks = current_account.webhooks
@webhooks = Current.account.webhooks
end
def create
@webhook = current_account.webhooks.new(webhook_params)
@webhook = Current.account.webhooks.new(webhook_params)
@webhook.save!
end
@ -28,7 +27,7 @@ class Api::V1::Accounts::WebhooksController < Api::BaseController
end
def fetch_webhook
@webhook = current_account.webhooks.find(params[:id])
@webhook = Current.account.webhooks.find(params[:id])
end
def check_authorization

View file

@ -1,4 +1,4 @@
class Api::V1::Accounts::AccountsController < Api::BaseController
class Api::V1::AccountsController < Api::BaseController
include AuthHelper
skip_before_action :verify_authenticity_token, only: [:create]

View file

@ -1,4 +1,7 @@
class Api::V1::Widget::BaseController < ApplicationController
before_action :set_web_widget
before_action :set_contact
private
def conversation

View file

@ -1,7 +1,4 @@
class Api::V1::Widget::ContactsController < Api::V1::Widget::BaseController
before_action :set_web_widget
before_action :set_contact
def update
contact_identify_action = ContactIdentifyAction.new(
contact: @contact,

View file

@ -1,7 +1,5 @@
class Api::V1::Widget::ConversationsController < Api::V1::Widget::BaseController
include Events::Types
before_action :set_web_widget
before_action :set_contact
def index
@conversation = conversation

View file

@ -1,7 +1,5 @@
class Api::V1::Widget::EventsController < Api::V1::Widget::BaseController
include Events::Types
before_action :set_web_widget
before_action :set_contact
def create
Rails.configuration.dispatcher.dispatch(permitted_params[:name], Time.zone.now, contact_inbox: @contact_inbox)

View file

@ -1,5 +1,5 @@
class Api::V1::Widget::InboxMembersController < Api::V1::Widget::BaseController
before_action :set_web_widget
skip_before_action :set_contact
def index
@inbox_members = @web_widget.inbox.inbox_members.includes(:user)

View file

@ -1,7 +1,4 @@
class Api::V1::Widget::LabelsController < Api::V1::Widget::BaseController
before_action :set_web_widget
before_action :set_contact
def create
conversation.label_list.add(permitted_params[:label])
conversation.save!

View file

@ -1,6 +1,4 @@
class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
before_action :set_web_widget
before_action :set_contact
before_action :set_conversation, only: [:create]
before_action :set_message, only: [:update]

View file

@ -1,6 +1,6 @@
class Api::V2::Accounts::ReportsController < Api::BaseController
class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController
def account
builder = V2::ReportBuilder.new(current_account, account_report_params)
builder = V2::ReportBuilder.new(Current.account, account_report_params)
data = builder.build
render json: data
end
@ -29,7 +29,7 @@ class Api::V2::Accounts::ReportsController < Api::BaseController
end
def account_summary_metrics
builder = V2::ReportBuilder.new(current_account, account_summary_params)
builder = V2::ReportBuilder.new(Current.account, account_summary_params)
builder.summary
end
end

View file

@ -13,40 +13,6 @@ class ApplicationController < ActionController::Base
private
def current_account
@current_account ||= find_current_account
Current.account = @current_account
end
def find_current_account
account = Account.find(params[:account_id])
if current_user
account_accessible_for_user?(account)
elsif @resource&.is_a?(AgentBot)
account_accessible_for_bot?(account)
end
switch_locale account
account
end
def switch_locale(account)
# priority is for locale set in query string (mostly for widget/from js sdk)
locale ||= (I18n.available_locales.map(&:to_s).include?(params[:locale]) ? params[:locale] : nil)
# if local is not set in param, lets try account
locale ||= (I18n.available_locales.map(&:to_s).include?(account.locale) ? account.locale : nil)
I18n.locale = locale || I18n.default_locale
end
def account_accessible_for_user?(account)
@current_account_user = account.account_users.find_by(user_id: current_user.id)
Current.account_user = @current_account_user
render_unauthorized('You are not authorized to access this account') unless @current_account_user
end
def account_accessible_for_bot?(account)
render_unauthorized('You are not authorized to access this account') unless @resource.agent_bot_inboxes.find_by(account_id: account.id)
end
def handle_with_exception
yield
rescue ActiveRecord::RecordNotFound => e
@ -65,7 +31,7 @@ class ApplicationController < ActionController::Base
end
def current_subscription
@subscription ||= current_account.subscription
@subscription ||= Current.account.subscription
end
def render_unauthorized(message)
@ -94,6 +60,14 @@ class ApplicationController < ActionController::Base
render json: exception.to_hash, status: exception.http_status
end
def switch_locale(account)
# priority is for locale set in query string (mostly for widget/from js sdk)
locale ||= (I18n.available_locales.map(&:to_s).include?(params[:locale]) ? params[:locale] : nil)
# if local is not set in param, lets try account
locale ||= (I18n.available_locales.map(&:to_s).include?(account.locale) ? account.locale : nil)
I18n.locale = locale || I18n.default_locale
end
def check_subscription
# This block is left over from the initial version of chatwoot
# We might reuse this later in the hosted version of chatwoot.

View file

@ -22,10 +22,12 @@ Rails.application.routes.draw do
namespace :v1 do
# ----------------------------------
# start of account scoped api routes
resources :accounts, only: [:create, :show, :update], module: :accounts do
resources :accounts, only: [:create, :show, :update] do
member do
post :update_active_at
end
scope module: :accounts do
namespace :actions do
resource :contact_merge, only: [:create]
end
@ -76,8 +78,8 @@ Rails.application.routes.draw do
post :set_agent_bot, on: :member
end
resources :inbox_members, only: [:create, :show], param: :inbox_id
resources :labels, only: [:index, :show, :create, :update, :destroy]
resources :notifications, only: [:index, :update]
resource :notification_settings, only: [:show, :update]
@ -90,6 +92,7 @@ Rails.application.routes.draw do
resources :webhooks, except: [:show]
end
end
# end of account scoped api routes
# ----------------------------------

View file

@ -53,7 +53,7 @@ RSpec.describe 'Contacts API', type: :request do
end
describe 'POST /api/v1/accounts/{account.id}/contacts' do
let(:valid_params) { { contact: { account_id: account.id } } }
let(:valid_params) { { contact: { name: 'test' } } }
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do