diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb index 0f27d8d2c..e2e572b27 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb @@ -2,7 +2,7 @@ class Api::V1::Accounts::Captain::AssistantResponsesController < Api::V1::Accoun before_action -> { check_authorization(Captain::Assistant) } before_action :set_current_page, only: [:index] - before_action :set_assistant, only: [:create] + before_action :set_assistant, only: [:create, :update] before_action :set_responses, except: [:create] before_action :set_response, only: [:show, :update, :destroy] @@ -17,13 +17,15 @@ class Api::V1::Accounts::Captain::AssistantResponsesController < Api::V1::Accoun def show; end def create - @response = Current.account.captain_assistant_responses.new(response_params) + @response = Current.account.captain_assistant_responses.new(response_params.except(:assistant_id)) + @response.assistant = @assistant @response.documentable = Current.user @response.save! end def update - @response.update!(response_params) + @response.assistant = @assistant if response_params.key?(:assistant_id) + @response.update!(response_params.except(:assistant_id)) end def destroy @@ -55,7 +57,7 @@ class Api::V1::Accounts::Captain::AssistantResponsesController < Api::V1::Accoun end def set_assistant - @assistant = Current.account.captain_assistants.find_by(id: params[:assistant_id]) + @assistant = Current.account.captain_assistants.find_by(id: response_params[:assistant_id]) end def set_responses diff --git a/enterprise/app/models/captain/assistant_response.rb b/enterprise/app/models/captain/assistant_response.rb index 758bc4203..0635ff43a 100644 --- a/enterprise/app/models/captain/assistant_response.rb +++ b/enterprise/app/models/captain/assistant_response.rb @@ -33,6 +33,7 @@ class Captain::AssistantResponse < ApplicationRecord validates :question, presence: true validates :answer, presence: true + validate :assistant_belongs_to_account before_validation :ensure_account before_validation :ensure_status @@ -66,7 +67,13 @@ class Captain::AssistantResponse < ApplicationRecord end def ensure_account - self.account = assistant&.account + self.account ||= assistant&.account + end + + def assistant_belongs_to_account + return if assistant.blank? || assistant.account_id == account_id + + errors.add(:assistant, :invalid) end def update_response_embedding diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb index e4086314b..556b870f2 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb @@ -192,6 +192,20 @@ RSpec.describe 'Api::V1::Accounts::Captain::AssistantResponses', type: :request expect(Captain::AssistantResponse.last).to be_approved end + it 'does not create a response for an assistant in another account' do + other_assistant = create(:captain_assistant) + params = valid_params.deep_merge(assistant_response: { assistant_id: other_assistant.id }) + + expect do + post "/api/v1/accounts/#{account.id}/captain/assistant_responses", + params: params, + headers: admin.create_new_auth_token, + as: :json + end.not_to change(Captain::AssistantResponse, :count) + + expect(response).to have_http_status(:unprocessable_entity) + end + context 'with invalid params' do let(:invalid_params) do { @@ -236,6 +250,20 @@ RSpec.describe 'Api::V1::Accounts::Captain::AssistantResponses', type: :request expect(json_response[:answer]).to eq('Updated answer') end + it 'does not move a response to an assistant in another account' do + other_assistant = create(:captain_assistant) + params = update_params.deep_merge(assistant_response: { assistant_id: other_assistant.id }) + + patch "/api/v1/accounts/#{account.id}/captain/assistant_responses/#{response_record.id}", + params: params, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response_record.reload.assistant).to eq(assistant) + expect(response_record.question).not_to eq('Updated question?') + end + context 'with invalid params' do let(:invalid_params) do { diff --git a/spec/enterprise/models/captain/assistant_response_spec.rb b/spec/enterprise/models/captain/assistant_response_spec.rb new file mode 100644 index 000000000..c16306583 --- /dev/null +++ b/spec/enterprise/models/captain/assistant_response_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Captain::AssistantResponse, type: :model do + describe 'account validation' do + it 'uses the assistant account when the account is not set' do + assistant = create(:captain_assistant) + assistant_response = build(:captain_assistant_response, assistant: assistant, account: nil) + + expect(assistant_response).to be_valid + expect(assistant_response.account).to eq(assistant.account) + end + + it 'rejects an assistant from another account' do + account = create(:account) + assistant_response = build(:captain_assistant_response, account: account) + + expect(assistant_response).not_to be_valid + expect(assistant_response.errors[:assistant]).to include('is invalid') + end + end +end diff --git a/spec/factories/captain/assistant_response.rb b/spec/factories/captain/assistant_response.rb index b257c39ab..fe5c16d06 100644 --- a/spec/factories/captain/assistant_response.rb +++ b/spec/factories/captain/assistant_response.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :captain_assistant_response, class: 'Captain::AssistantResponse' do association :assistant, factory: :captain_assistant - association :account + account { assistant.account } sequence(:question) { |n| "Test question #{n}?" } sequence(:answer) { |n| "Test answer #{n}" } embedding { Array.new(1536) { rand(-1.0..1.0) } }