From f9a1b952b661b18d83a7c3d137de61dac18cbc3f Mon Sep 17 00:00:00 2001 From: Alex Kriuchykhin Date: Wed, 24 Jan 2024 10:04:38 +0100 Subject: [PATCH] Split inventory item relationships API V2 endpoint into separate endpoints for parents and children [SCI-10078] (#6996) --- app/controllers/api/v1/base_controller.rb | 17 -- ...ory_item_child_relationships_controller.rb | 58 +++++ ...ry_item_parent_relationships_controller.rb | 60 +++++ ...inventory_item_relationships_controller.rb | 65 ------ config/routes/api_v2.rb | 7 +- ...em_child_relationships_controller_spec.rb} | 108 +++++---- ...em_parent_relationships_controller_spec.rb | 208 ++++++++++++++++++ 7 files changed, 392 insertions(+), 131 deletions(-) create mode 100644 app/controllers/api/v2/inventory_item_child_relationships_controller.rb create mode 100644 app/controllers/api/v2/inventory_item_parent_relationships_controller.rb delete mode 100644 app/controllers/api/v2/inventory_item_relationships_controller.rb rename spec/requests/api/v2/{inventory_item_relationships_controller_spec.rb => inventory_item_child_relationships_controller_spec.rb} (63%) create mode 100644 spec/requests/api/v2/inventory_item_parent_relationships_controller_spec.rb diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 457a67380..f1fdc5d96 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -11,15 +11,6 @@ module Api class FilterParamError < StandardError; end - class MutuallyExclusiveParamsError < StandardError - attr_reader :first_param, :second_param - - def initialize(first_param, second_param) - @first_param = first_param - @second_param = second_param - end - end - class PermissionError < StandardError attr_reader :klass, :mode @@ -37,14 +28,6 @@ module Api :bad_request) end - rescue_from MutuallyExclusiveParamsError do |e| - render_error(I18n.t('api.core.errors.mutually_exclusive_params_error.title'), - I18n.t('api.core.errors.mutually_exclusive_params_error.detail', - first_param: e.first_param, - second_param: e.second_param), - :bad_request) - end - rescue_from FilterParamError do |e| logger.error e.message logger.error e.backtrace.join("\n") diff --git a/app/controllers/api/v2/inventory_item_child_relationships_controller.rb b/app/controllers/api/v2/inventory_item_child_relationships_controller.rb new file mode 100644 index 000000000..8d6043753 --- /dev/null +++ b/app/controllers/api/v2/inventory_item_child_relationships_controller.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Api + module V2 + class InventoryItemChildRelationshipsController < BaseController + before_action :load_team, :load_inventory, :load_inventory_item + before_action :load_child_connection, only: %w(show destroy) + before_action :check_manage_permission, only: %w(create destroy) + + def index + child_connections = timestamps_filter(@inventory_item.child_connections).page(params.dig(:page, :number)) + .per(params.dig(:page, :size)) + render jsonapi: child_connections, each_serializer: InventoryItemRelationshipSerializer, include: include_params + end + + def show + render jsonapi: @child_connection, serializer: InventoryItemRelationshipSerializer, include: include_params + end + + def create + inventory_item_to_link = RepositoryRow.where(repository: Repository.accessible_by_teams(@team)) + .find(connection_params[:child_id]) + child_connection = @inventory_item.child_connections.create!( + child: inventory_item_to_link, + created_by: current_user, + last_modified_by: current_user + ) + + render jsonapi: child_connection, serializer: InventoryItemRelationshipSerializer, status: :created + end + + def destroy + @child_connection.destroy! + render body: nil + end + + private + + def load_child_connection + @child_connection = @inventory_item.child_connections.find(params.require(:id)) + end + + def check_manage_permission + raise PermissionError.new(Repository, :manage) unless can_connect_repository_rows?(@inventory) + end + + def connection_params + raise TypeError unless params.require(:data).require(:type) == 'inventory_item_relationships' + + params.require(:data).require(:attributes).permit(:child_id) + end + + def permitted_includes + %w(child) + end + end + end +end diff --git a/app/controllers/api/v2/inventory_item_parent_relationships_controller.rb b/app/controllers/api/v2/inventory_item_parent_relationships_controller.rb new file mode 100644 index 000000000..dec716a7d --- /dev/null +++ b/app/controllers/api/v2/inventory_item_parent_relationships_controller.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Api + module V2 + class InventoryItemParentRelationshipsController < BaseController + before_action :load_team, :load_inventory, :load_inventory_item + before_action :load_parent_connection, only: %w(show destroy) + before_action :check_manage_permission, only: %w(create destroy) + + def index + parent_connections = timestamps_filter(@inventory_item.parent_connections).page(params.dig(:page, :number)) + .per(params.dig(:page, :size)) + render jsonapi: parent_connections, + each_serializer: InventoryItemRelationshipSerializer, + include: include_params + end + + def show + render jsonapi: @parent_connection, serializer: InventoryItemRelationshipSerializer, include: include_params + end + + def create + inventory_item_to_link = RepositoryRow.where(repository: Repository.accessible_by_teams(@team)) + .find(connection_params[:parent_id]) + parent_connection = @inventory_item.parent_connections.create!( + parent: inventory_item_to_link, + created_by: current_user, + last_modified_by: current_user + ) + + render jsonapi: parent_connection, serializer: InventoryItemRelationshipSerializer, status: :created + end + + def destroy + @parent_connection.destroy! + render body: nil + end + + private + + def load_parent_connection + @parent_connection = @inventory_item.parent_connections.find(params.require(:id)) + end + + def check_manage_permission + raise PermissionError.new(Repository, :manage) unless can_connect_repository_rows?(@inventory) + end + + def connection_params + raise TypeError unless params.require(:data).require(:type) == 'inventory_item_relationships' + + params.require(:data).require(:attributes).permit(:parent_id) + end + + def permitted_includes + %w(parent) + end + end + end +end diff --git a/app/controllers/api/v2/inventory_item_relationships_controller.rb b/app/controllers/api/v2/inventory_item_relationships_controller.rb deleted file mode 100644 index 5bab0b3c8..000000000 --- a/app/controllers/api/v2/inventory_item_relationships_controller.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -module Api - module V2 - class InventoryItemRelationshipsController < BaseController - before_action :load_team, :load_inventory, :load_inventory_item - before_action :check_manage_permission, only: %w(create destroy) - before_action :load_create_params, only: :create - - def create - parent = @relation == :parent ? @inventory_item : @inventory_item_to_link - child = @relation == :child ? @inventory_item : @inventory_item_to_link - - @connection = RepositoryRowConnection.create!( - parent: parent, - child: child, - created_by: current_user, - last_modified_by: current_user - ) - - render jsonapi: @connection, serializer: InventoryItemRelationshipSerializer, status: :created - end - - def destroy - @connection = @inventory_item.parent_connections - .or(@inventory_item.child_connections) - .find(params.require(:id)) - @connection.destroy! - render body: nil - end - - private - - def check_manage_permission - raise PermissionError.new(Repository, :manage) unless can_manage_repository?(@inventory) - end - - def load_create_params - if connection_params[:parent_id].present? && connection_params[:child_id].present? - raise MutuallyExclusiveParamsError.new(:parent_id, :child_id) - end - - if connection_params[:parent_id].present? - @relation = :parent - @inventory_item_to_link = RepositoryRow.find(connection_params[:parent_id]) - elsif connection_params[:child_id].present? - @relation = :child - @inventory_item_to_link = RepositoryRow.find(connection_params[:child_id]) - end - - raise ActiveRecord::RecordNotFound unless @inventory_item_to_link - end - - def connection_params - raise TypeError unless params.require(:data).require(:type) == 'inventory_item_relationships' - - params.require(:data).require(:attributes).permit(%i(parent_id child_id)) - end - - def permitted_includes - %w(parent child) - end - end - end -end diff --git a/config/routes/api_v2.rb b/config/routes/api_v2.rb index f1155d051..8e99ca45b 100644 --- a/config/routes/api_v2.rb +++ b/config/routes/api_v2.rb @@ -36,7 +36,12 @@ namespace :v2 do path: 'items', only: [], as: :items do - resources :inventory_item_relationships, only: %i(create destroy) + resources :child_relationships, + only: %i(index show create destroy), + controller: :inventory_item_child_relationships + resources :parent_relationships, + only: %i(index show create destroy), + controller: :inventory_item_parent_relationships end end end diff --git a/spec/requests/api/v2/inventory_item_relationships_controller_spec.rb b/spec/requests/api/v2/inventory_item_child_relationships_controller_spec.rb similarity index 63% rename from spec/requests/api/v2/inventory_item_relationships_controller_spec.rb rename to spec/requests/api/v2/inventory_item_child_relationships_controller_spec.rb index dacd87a98..249ce14fc 100644 --- a/spec/requests/api/v2/inventory_item_relationships_controller_spec.rb +++ b/spec/requests/api/v2/inventory_item_child_relationships_controller_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request do +RSpec.describe 'Api::V2::InventoryItemChildRelationshipsController', type: :request do before :all do @user = create(:user) @normal_user = create(:user, full_name: 'Normal User') @@ -8,6 +8,8 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d @inventory = create(:repository, team: @team, created_by: @user) @other_inventory = create(:repository, team: @team, created_by: @user) @inventory_item = create(:repository_row, repository: @inventory, created_by: @user) + @child_inventory_item = create(:repository_row, repository: @other_inventory, created_by: @user) + @child_connection = create(:repository_row_connection, parent: @inventory_item, child: @child_inventory_item, created_by: @user) @other_inventory_item = create(:repository_row, repository: @other_inventory, created_by: @user) @valid_headers = { @@ -16,9 +18,56 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d } end + describe 'GET #index' do + it 'Response with correct inventory child relationship items' do + hash_body = nil + get api_v2_team_inventory_item_child_relationships_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + include: :child + ), headers: @valid_headers + expect { hash_body = json }.not_to raise_exception + expect(hash_body[:data]).to match_array( + JSON.parse( + ActiveModelSerializers::SerializableResource + .new(@inventory_item.child_connections.order(:id), + each_serializer: Api::V2::InventoryItemRelationshipSerializer, + include: :child) + .to_json + )['data'] + ) + expect(hash_body).to include('included') + end + end + + describe 'GET #show' do + it 'Response with correct inventory child relationship item' do + hash_body = nil + get api_v2_team_inventory_item_child_relationship_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + id: @child_connection.id, + include: :child + ), headers: @valid_headers + expect { hash_body = json }.not_to raise_exception + expect(hash_body[:data]).to match_array( + JSON.parse( + ActiveModelSerializers::SerializableResource + .new(@child_connection, + serializer: Api::V2::InventoryItemRelationshipSerializer, + include: :child) + .to_json + )['data'] + ) + expect(hash_body).to include('included') + end + end + describe 'POST #create' do let(:create_action) do - post api_v2_team_inventory_item_inventory_item_relationships_path( + post api_v2_team_inventory_item_child_relationships_path( team_id: @team.id, inventory_id: @inventory.id, item_id: @inventory_item.id @@ -26,31 +75,13 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d end context 'with valid parameters' do - context 'using parent_id' do - let(:request_body) do - { - data: { - type: 'inventory_item_relationships', - attributes: { - parent_id: @other_inventory_item.id - } - } - } - end - - it 'creates a new relationship' do - expect { create_action }.to change { RepositoryRowConnection.count }.by(1) - expect(response).to have_http_status(201) - end - end - context 'using child_id' do let(:request_body) do { data: { type: 'inventory_item_relationships', attributes: { - parent_id: @other_inventory_item.id + child_id: @other_inventory_item.id } } } @@ -63,26 +94,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d end end - context 'with non valid parameters' do - context 'with mutually exclusive parameters' do - let(:request_body) do - { - data: { - type: 'inventory_item_relationships', - attributes: { - parent_id: @inventory_item.id, - child_id: @other_inventory_item.id - } - } - } - end - - it 'raises a mutually exclusive parameters error' do - create_action - expect(response).to have_http_status(400) - end - end - + context 'with non valid parameters' do context 'with missing type' do let(:request_body) { { data: { type: '', attributes: { parent_id: @other_inventory_item.id } } } } @@ -92,13 +104,13 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d end end - context 'with missing item with parent_id or child_id' do + context 'with child_id for missing item' do let(:request_body) do { data: { type: 'inventory_item_relationships', attributes: { - parent_id: -1 + child_id: -1 } } } @@ -112,7 +124,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d context 'without manage permission' do let(:another_create_action) do - post api_v2_team_inventory_item_inventory_item_relationships_path( + post api_v2_team_inventory_item_child_relationships_path( team_id: @team.id, inventory_id: @inventory.id, item_id: @inventory_item.id @@ -121,7 +133,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d data: { type: 'inventory_item_relationships', attributes: { - parent_id: @other_inventory_item.id + child_id: @other_inventory_item.id } } }.to_json, @@ -143,7 +155,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d let(:relationship) { create(:repository_row_connection, parent: @inventory_item, child: @other_inventory_item, created_by: @user) } let(:destroy_action) do - delete api_v2_team_inventory_item_inventory_item_relationship_path( + delete api_v2_team_inventory_item_child_relationship_path( team_id: @team.id, inventory_id: @inventory.id, item_id: @inventory_item.id, @@ -161,7 +173,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d context 'without manage permission' do let(:another_destroy_action) do - delete api_v2_team_inventory_item_inventory_item_relationship_path( + delete api_v2_team_inventory_item_child_relationship_path( team_id: @team.id, inventory_id: @inventory.id, item_id: @inventory_item.id, @@ -182,7 +194,7 @@ RSpec.describe 'Api::V2::InventoryItemRelationshipsController', type: :request d context 'with invalid ID' do it 'returns 404 not found' do - delete api_v2_team_inventory_item_inventory_item_relationship_path( + delete api_v2_team_inventory_item_child_relationship_path( team_id: @team.id, inventory_id: @inventory.id, item_id: @inventory_item.id, diff --git a/spec/requests/api/v2/inventory_item_parent_relationships_controller_spec.rb b/spec/requests/api/v2/inventory_item_parent_relationships_controller_spec.rb new file mode 100644 index 000000000..4eeba7046 --- /dev/null +++ b/spec/requests/api/v2/inventory_item_parent_relationships_controller_spec.rb @@ -0,0 +1,208 @@ +require 'rails_helper' + +RSpec.describe 'Api::V2::InventoryItemParentRelationshipsController', type: :request do + before :all do + @user = create(:user) + @normal_user = create(:user, full_name: 'Normal User') + @team = create(:team, created_by: @user) + @inventory = create(:repository, team: @team, created_by: @user) + @other_inventory = create(:repository, team: @team, created_by: @user) + @inventory_item = create(:repository_row, repository: @inventory, created_by: @user) + @parent_inventory_item = create(:repository_row, repository: @other_inventory, created_by: @user) + @parent_connection = create(:repository_row_connection, child: @inventory_item, parent: @parent_inventory_item, created_by: @user) + @other_inventory_item = create(:repository_row, repository: @other_inventory, created_by: @user) + + @valid_headers = { + 'Authorization': 'Bearer ' + generate_token(@user.id), + 'Content-Type': 'application/json' + } + end + + describe 'GET #index' do + it 'Response with correct inventory parent relationship items' do + hash_body = nil + get api_v2_team_inventory_item_parent_relationships_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + include: :parent + ), headers: @valid_headers + expect { hash_body = json }.not_to raise_exception + expect(hash_body[:data]).to match_array( + JSON.parse( + ActiveModelSerializers::SerializableResource + .new(@inventory_item.parent_connections.order(:id), + each_serializer: Api::V2::InventoryItemRelationshipSerializer, + include: :parent) + .to_json + )['data'] + ) + expect(hash_body).to include('included') + end + end + + describe 'GET #show' do + it 'Response with correct inventory parent relationship item' do + hash_body = nil + get api_v2_team_inventory_item_parent_relationship_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + id: @parent_connection.id, + include: :parent + ), headers: @valid_headers + expect { hash_body = json }.not_to raise_exception + expect(hash_body[:data]).to match_array( + JSON.parse( + ActiveModelSerializers::SerializableResource + .new(@parent_connection, + serializer: Api::V2::InventoryItemRelationshipSerializer, + include: :parent) + .to_json + )['data'] + ) + expect(hash_body).to include('included') + end + end + + describe 'POST #create' do + let(:create_action) do + post api_v2_team_inventory_item_parent_relationships_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id + ), params: request_body.to_json, headers: @valid_headers + end + + context 'with valid parameters' do + context 'using parent_id' do + let(:request_body) do + { + data: { + type: 'inventory_item_relationships', + attributes: { + parent_id: @other_inventory_item.id + } + } + } + end + + it 'creates a new relationship' do + expect { create_action }.to change { RepositoryRowConnection.count }.by(1) + expect(response).to have_http_status(201) + end + end + end + + context 'with non valid parameters' do + context 'with missing type' do + let(:request_body) { { data: { type: '', attributes: { parent_id: @other_inventory_item.id } } } } + + it 'renders 400 bad request' do + create_action + expect(response).to have_http_status(400) + end + end + + context 'with parent_id for missing item' do + let(:request_body) do + { + data: { + type: 'inventory_item_relationships', + attributes: { + parent_id: -1 + } + } + } + end + + it 'renders 404 not found' do + create_action + expect(response).to have_http_status(404) + end + end + + context 'without manage permission' do + let(:another_create_action) do + post api_v2_team_inventory_item_parent_relationships_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id + ), + params: { + data: { + type: 'inventory_item_relationships', + attributes: { + parent_id: @other_inventory_item.id + } + } + }.to_json, + headers: { + 'Authorization': 'Bearer ' + generate_token(@normal_user.id), + 'Content-Type': 'application/json' + } + end + + it 'renders 403 forbidden' do + another_create_action + expect(response).to have_http_status(403) + end + end + end + end + + describe 'DELETE #destroy' do + let(:relationship) { create(:repository_row_connection, child: @inventory_item, parent: @other_inventory_item, created_by: @user) } + + let(:destroy_action) do + delete api_v2_team_inventory_item_parent_relationship_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + id: relationship.id + ), headers: @valid_headers + end + + context 'with valid ID' do + it 'deletes the relationship' do + destroy_action + expect(response).to have_http_status(200) + expect(RepositoryRowConnection.where(id: relationship.id)).to_not exist + end + end + + context 'without manage permission' do + let(:another_destroy_action) do + delete api_v2_team_inventory_item_parent_relationship_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + id: relationship.id + ), + headers: { + 'Authorization': 'Bearer ' + generate_token(@normal_user.id), + 'Content-Type': 'application/json' + } + end + + it 'renders 403 forbidden' do + another_destroy_action + expect(response).to have_http_status(403) + expect(RepositoryRowConnection.where(id: relationship.id)).to exist + end + end + + context 'with invalid ID' do + it 'returns 404 not found' do + delete api_v2_team_inventory_item_parent_relationship_path( + team_id: @team.id, + inventory_id: @inventory.id, + item_id: @inventory_item.id, + id: -1 + ), headers: @valid_headers + + expect(response).to have_http_status(404) + end + end + end +end