From d5215fea93f854f56e3dab47397970bd23dff7c5 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Mon, 17 May 2021 10:32:59 +0530 Subject: [PATCH] feat: APIs for Integration Hooks (#2250) - Introduces JSON Schema validations via JSONSchemer - Add CRUD APIs for integration hooks --- Gemfile | 2 + Gemfile.lock | 9 ++ .../accounts/integrations/hooks_controller.rb | 31 +++++ app/models/integrations/hook.rb | 17 +++ app/policies/hook_policy.rb | 13 ++ .../integrations/apps/index.json.jbuilder | 7 +- .../integrations/apps/show.json.jbuilder | 8 +- .../integrations/hooks/create.json.jbuilder | 1 + .../integrations/hooks/update.json.jbuilder | 1 + app/views/api/v1/models/_app.json.jbuilder | 6 + app/views/api/v1/models/_hook.json.jbuilder | 4 + config/integration/apps.yml | 38 ++++++ config/routes.rb | 1 + lib/integrations/slack/hook_builder.rb | 1 - .../integrations/hooks_controller_spec.rb | 111 ++++++++++++++++++ spec/factories/integrations/hooks.rb | 10 +- spec/jobs/hook_job_spec.rb | 2 +- .../dialogflow/processor_service_spec.rb | 2 +- spec/models/conversation_spec.rb | 2 +- spec/models/integrations/hook_spec.rb | 18 +++ .../integrations/slack_request_spec.rb | 2 +- 21 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 app/controllers/api/v1/accounts/integrations/hooks_controller.rb create mode 100644 app/policies/hook_policy.rb create mode 100644 app/views/api/v1/accounts/integrations/hooks/create.json.jbuilder create mode 100644 app/views/api/v1/accounts/integrations/hooks/update.json.jbuilder create mode 100644 app/views/api/v1/models/_app.json.jbuilder create mode 100644 app/views/api/v1/models/_hook.json.jbuilder create mode 100644 spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb diff --git a/Gemfile b/Gemfile index f321b40d0..4e430a7c8 100644 --- a/Gemfile +++ b/Gemfile @@ -31,6 +31,8 @@ gem 'haikunator' gem 'liquid' # Parse Markdown to HTML gem 'commonmarker' +# Validate Data against JSON Schema +gem 'json_schemer' ##-- for active storage --## gem 'aws-sdk-s3', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 0fd91f389..e3061c705 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -181,6 +181,8 @@ GEM dotenv-rails (2.7.6) dotenv (= 2.7.6) railties (>= 3.2) + ecma-re-validator (0.2.1) + regexp_parser (~> 1.2) equalizer (0.0.11) erubi (1.10.0) et-orbi (1.2.4) @@ -292,6 +294,11 @@ GEM railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (2.3.1) + json_schemer (0.2.16) + ecma-re-validator (~> 0.2) + hana (~> 1.3) + regexp_parser (~> 1.5) + uri_template (~> 0.7) jwt (2.2.3) kaminari (1.2.1) activesupport (>= 4.1.0) @@ -568,6 +575,7 @@ GEM unf_ext (0.0.7.7) unicode-display_width (1.7.0) uniform_notifier (1.13.0) + uri_template (0.7.0) valid_email2 (3.3.1) activemodel (>= 3.2) mail (~> 2.5) @@ -641,6 +649,7 @@ DEPENDENCIES hashie jbuilder json_refs! + json_schemer jwt kaminari koala diff --git a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb new file mode 100644 index 000000000..18a16a30d --- /dev/null +++ b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb @@ -0,0 +1,31 @@ +class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::BaseController + before_action :fetch_hook, only: [:update, :destroy] + before_action :check_authorization + + def create + @hook = Current.account.hooks.create!(permitted_params) + end + + def update + @hook.update!(permitted_params.slice(:status, :settings)) + end + + def destroy + @hook.destroy + head :ok + end + + private + + def fetch_hook + @hook = Current.account.hooks.find(params[:id]) + end + + def check_authorization + authorize(:hook) + end + + def permitted_params + params.require(:hook).permit(:app_id, :inbox_id, :status, settings: {}) + end +end diff --git a/app/models/integrations/hook.rb b/app/models/integrations/hook.rb index ad3184484..55e64410c 100644 --- a/app/models/integrations/hook.rb +++ b/app/models/integrations/hook.rb @@ -17,8 +17,13 @@ class Integrations::Hook < ApplicationRecord include Reauthorizable + attr_readonly :app_id, :account_id, :inbox_id, :hook_type + before_validation :ensure_hook_type validates :account_id, presence: true validates :app_id, presence: true + validates :inbox_id, presence: true, if: -> { hook_type == 'inbox' } + validate :validate_settings_json_schema + validates :app_id, uniqueness: { scope: [:account_id], unless: -> { app.present? && app.params[:allow_multiple_hooks].present? } } enum status: { disabled: 0, enabled: 1 } @@ -39,4 +44,16 @@ class Integrations::Hook < ApplicationRecord def disable update(status: 'disabled') end + + private + + def ensure_hook_type + self.hook_type = app.params[:hook_type] if app.present? + end + + def validate_settings_json_schema + return if app.blank? || app.params[:settings_json_schema].blank? + + errors.add(:settings, ': Invalid settings data') unless JSONSchemer.schema(app.params[:settings_json_schema]).valid?(settings) + end end diff --git a/app/policies/hook_policy.rb b/app/policies/hook_policy.rb new file mode 100644 index 000000000..3f25cf8e4 --- /dev/null +++ b/app/policies/hook_policy.rb @@ -0,0 +1,13 @@ +class HookPolicy < ApplicationPolicy + def create? + @account_user.administrator? + end + + def update? + @account_user.administrator? + end + + def destroy? + @account_user.administrator? + end +end diff --git a/app/views/api/v1/accounts/integrations/apps/index.json.jbuilder b/app/views/api/v1/accounts/integrations/apps/index.json.jbuilder index dca7a4fdc..09acda0cb 100644 --- a/app/views/api/v1/accounts/integrations/apps/index.json.jbuilder +++ b/app/views/api/v1/accounts/integrations/apps/index.json.jbuilder @@ -1,10 +1,5 @@ json.payload do json.array! @apps do |app| - json.id app.id - json.name app.name - json.description app.description - json.logo app.logo - json.enabled app.enabled?(@current_account) - json.action app.action + json.partial! 'api/v1/models/app.json.jbuilder', resource: app end end diff --git a/app/views/api/v1/accounts/integrations/apps/show.json.jbuilder b/app/views/api/v1/accounts/integrations/apps/show.json.jbuilder index 93d139e9a..442b50a49 100644 --- a/app/views/api/v1/accounts/integrations/apps/show.json.jbuilder +++ b/app/views/api/v1/accounts/integrations/apps/show.json.jbuilder @@ -1,7 +1 @@ -json.id @app.id -json.name @app.name -json.logo @app.logo -json.description @app.description -json.fields @app.fields -json.enabled @app.enabled?(@current_account) -json.button @app.action +json.partial! 'api/v1/models/app.json.jbuilder', resource: @app diff --git a/app/views/api/v1/accounts/integrations/hooks/create.json.jbuilder b/app/views/api/v1/accounts/integrations/hooks/create.json.jbuilder new file mode 100644 index 000000000..6362fabd4 --- /dev/null +++ b/app/views/api/v1/accounts/integrations/hooks/create.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/hook.json.jbuilder', resource: @hook diff --git a/app/views/api/v1/accounts/integrations/hooks/update.json.jbuilder b/app/views/api/v1/accounts/integrations/hooks/update.json.jbuilder new file mode 100644 index 000000000..6362fabd4 --- /dev/null +++ b/app/views/api/v1/accounts/integrations/hooks/update.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/hook.json.jbuilder', resource: @hook diff --git a/app/views/api/v1/models/_app.json.jbuilder b/app/views/api/v1/models/_app.json.jbuilder new file mode 100644 index 000000000..6fce9fee2 --- /dev/null +++ b/app/views/api/v1/models/_app.json.jbuilder @@ -0,0 +1,6 @@ +json.call(resource.params, *resource.params.keys) +json.name resource.name +json.description resource.description +json.enabled resource.enabled?(@current_account) +json.button resource.action +json.hooks @current_account.hooks.where(app_id: resource.id) diff --git a/app/views/api/v1/models/_hook.json.jbuilder b/app/views/api/v1/models/_hook.json.jbuilder new file mode 100644 index 000000000..d9860b8a2 --- /dev/null +++ b/app/views/api/v1/models/_hook.json.jbuilder @@ -0,0 +1,4 @@ +json.id resource.id +json.app resource.app.params.to_h +json.enabled resource.enabled? +json.inbox_id resource.inbox_id diff --git a/config/integration/apps.yml b/config/integration/apps.yml index dd8f1be4c..1c6276894 100644 --- a/config/integration/apps.yml +++ b/config/integration/apps.yml @@ -1,15 +1,53 @@ +###### Attributes Supported by Integration Apps ####### +# id: Internal Id for the integrations, used by the hooks +# logo: place the image in /public/dashboard/images/integrations and reference here +# i18n_key: the key under which translations for the integration is placed in en.yml +# action: if integration requires external redirect url +# hook_type: ( account / inbox ) +# allow_multiple_hooks: whether multiple hooks can be created for the integration +# settings_json_schema: the json schema used to validate the settings hash (https://json-schema.org/) +# settings_form_schema: the formulate schema used in frontend to render settings form (https://vueformulate.com/) +######################################################## + slack: id: slack logo: slack.png i18n_key: slack action: https://slack.com/oauth/v2/authorize?scope=commands,chat:write,channels:read,channels:manage,channels:join,groups:write,im:write,mpim:write,users:read,users:read.email,chat:write.customize,channels:history,groups:history,mpim:history,im:history + hook_type: account + allow_multiple_hooks: false webhooks: id: webhook logo: cable.svg i18n_key: webhooks action: /webhook + hook_type: account + allow_multiple_hooks: true dialogflow: id: dialogflow logo: dialogflow.svg i18n_key: dialogflow action: /dialogflow + hook_type: inbox + allow_multiple_hooks: true + settings_json_schema: { + "type": "object", + "properties": { + "project_id": { "type": "string" }, + "credentials": { "type": "object" } + }, + "required": ["project_id", "credentials"], + "additionalProperties": false + } + settings_form_schema: [ + { + "label": "Dialogflow Project ID", + "type": "text", + "name": "project_id" + }, + { + "label": "Dialogflow Project Key File", + "type": "textarea", + "name": "credentials", + } + ] diff --git a/config/routes.rb b/config/routes.rb index e558f0905..140e19400 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -121,6 +121,7 @@ Rails.application.routes.draw do resources :webhooks, except: [:show] namespace :integrations do resources :apps, only: [:index, :show] + resources :hooks, only: [:create, :update, :destroy] resource :slack, only: [:create, :update, :destroy], controller: 'slack' end resources :working_hours, only: [:update] diff --git a/lib/integrations/slack/hook_builder.rb b/lib/integrations/slack/hook_builder.rb index 93bff9656..f898905fa 100644 --- a/lib/integrations/slack/hook_builder.rb +++ b/lib/integrations/slack/hook_builder.rb @@ -12,7 +12,6 @@ class Integrations::Slack::HookBuilder access_token: token, status: 'enabled', inbox_id: params[:inbox_id], - hook_type: hook_type, app_id: 'slack' ) diff --git a/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb new file mode 100644 index 000000000..3c92f9026 --- /dev/null +++ b/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb @@ -0,0 +1,111 @@ +require 'rails_helper' + +RSpec.describe 'Integration Hooks API', type: :request do + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:inbox) { create(:inbox, account: account) } + let(:params) { { app_id: 'dialogflow', inbox_id: inbox.id, settings: { project_id: 'xx', credentials: { test: 'test' } } } } + + describe 'POST /api/v1/accounts/{account.id}/integrations/hooks' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + post api_v1_account_integrations_hooks_url(account_id: account.id), + params: params, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'return unauthorized if agent' do + post api_v1_account_integrations_hooks_url(account_id: account.id), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'creates hooks if admin' do + post api_v1_account_integrations_hooks_url(account_id: account.id), + params: params, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + data = JSON.parse(response.body) + expect(data['app']['id']).to eq params[:app_id] + end + end + end + + describe 'PATCH /api/v1/accounts/{account.id}/integrations/hooks/{hook_id}' do + let(:hook) { create(:integrations_hook, account: account) } + + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + patch api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + params: params, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'return unauthorized if agent' do + patch api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'updates hook if admin' do + patch api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + params: params, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + data = JSON.parse(response.body) + expect(data['app']['id']).to eq 'slack' + end + end + end + + describe 'DELETE /api/v1/accounts/{account.id}/integrations/hooks/{hook_id}' do + let(:hook) { create(:integrations_hook, account: account) } + + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + delete api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'return unauthorized if agent' do + delete api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'updates hook if admin' do + delete api_v1_account_integrations_hook_url(account_id: account.id, id: hook.id), + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(::Integrations::Hook.exists?(hook.id)).to eq false + end + end + end +end diff --git a/spec/factories/integrations/hooks.rb b/spec/factories/integrations/hooks.rb index bd142246f..ac7c36fbc 100644 --- a/spec/factories/integrations/hooks.rb +++ b/spec/factories/integrations/hooks.rb @@ -1,12 +1,16 @@ FactoryBot.define do factory :integrations_hook, class: 'Integrations::Hook' do - status { Integrations::Hook.statuses['enabled'] } + app_id { 'slack' } inbox account - app_id { 'slack' } settings { { 'test': 'test' } } - hook_type { Integrations::Hook.statuses['account'] } + status { Integrations::Hook.statuses['enabled'] } access_token { SecureRandom.hex } reference_id { SecureRandom.hex } + + trait :dialogflow do + app_id { 'dialogflow' } + settings { { project_id: 'test', credentials: {} } } + end end end diff --git a/spec/jobs/hook_job_spec.rb b/spec/jobs/hook_job_spec.rb index c48d862d0..225a9c183 100644 --- a/spec/jobs/hook_job_spec.rb +++ b/spec/jobs/hook_job_spec.rb @@ -29,7 +29,7 @@ RSpec.describe HookJob, type: :job do end it 'calls Integrations::Dialogflow::ProcessorService when its a dialogflow intergation' do - hook = create(:integrations_hook, app_id: 'dialogflow', account: account) + hook = create(:integrations_hook, :dialogflow, account: account) allow(Integrations::Dialogflow::ProcessorService).to receive(:new).and_return(process_service) expect(Integrations::Dialogflow::ProcessorService).to receive(:new) described_class.perform_now(hook, event_name, event_data) diff --git a/spec/lib/integrations/dialogflow/processor_service_spec.rb b/spec/lib/integrations/dialogflow/processor_service_spec.rb index 7f70e315e..ca852a943 100644 --- a/spec/lib/integrations/dialogflow/processor_service_spec.rb +++ b/spec/lib/integrations/dialogflow/processor_service_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe Integrations::Dialogflow::ProcessorService do let(:account) { create(:account) } - let(:hook) { create(:integrations_hook, app_id: 'dialogflow', account: account) } + let(:hook) { create(:integrations_hook, :dialogflow, account: account) } let(:conversation) { create(:conversation, account: account, status: :bot) } let(:message) { create(:message, account: account, conversation: conversation) } let(:event_name) { 'message.created' } diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 6ae1337ee..bcfc91a70 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -351,7 +351,7 @@ RSpec.describe Conversation, type: :model do end describe '#botintegration: when conversation created in inbox with dialogflow integration' do - let(:hook) { create(:integrations_hook, app_id: 'dialogflow') } + let(:hook) { create(:integrations_hook, :dialogflow) } let(:conversation) { create(:conversation, inbox: hook.inbox) } it 'returns conversation status as bot' do diff --git a/spec/models/integrations/hook_spec.rb b/spec/models/integrations/hook_spec.rb index da5f1ff20..87822a0dc 100644 --- a/spec/models/integrations/hook_spec.rb +++ b/spec/models/integrations/hook_spec.rb @@ -9,4 +9,22 @@ RSpec.describe Integrations::Hook, type: :model do describe 'associations' do it { is_expected.to belong_to(:account) } end + + describe 'when trying to create multiple hooks for an app' do + let(:account) { create(:account) } + + context 'when app allows multiple hooks' do + it 'allows to create succesfully' do + create(:integrations_hook, account: account, app_id: 'webhook') + expect(build(:integrations_hook, account: account, app_id: 'webhook').valid?).to eq true + end + end + + context 'when app doesnot allow multiple hooks' do + it 'throws invalid error' do + create(:integrations_hook, account: account, app_id: 'slack') + expect(build(:integrations_hook, account: account, app_id: 'slack').valid?).to eq false + end + end + end end diff --git a/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb b/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb index e68eabf73..c61c8d376 100644 --- a/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb +++ b/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'Api::V1::Accounts::Integrations::Slacks', type: :request do context 'when it is an authenticated user' do it 'creates hook' do hook_builder = Integrations::Slack::HookBuilder.new(account: account, code: SecureRandom.hex) - expect(hook_builder).to receive(:fetch_access_token).and_return(SecureRandom.hex) + expect(hook_builder).to receive(:perform).and_return(hook) expect(Integrations::Slack::HookBuilder).to receive(:new).and_return(hook_builder) channel_builder = Integrations::Slack::ChannelBuilder.new(hook: hook, channel: 'channel')