diff --git a/app/controllers/object_manager_attributes_controller.rb b/app/controllers/object_manager_attributes_controller.rb index d377d0cfb..51d6b0e39 100644 --- a/app/controllers/object_manager_attributes_controller.rb +++ b/app/controllers/object_manager_attributes_controller.rb @@ -29,44 +29,20 @@ class ObjectManagerAttributesController < ApplicationController ) raise Exceptions::UnprocessableEntity, 'already exists' if exists - begin - object_manager_attribute = ObjectManager::Attribute.add( - object: permitted_params[:object], - name: permitted_params[:name], - display: permitted_params[:display], - data_type: permitted_params[:data_type], - data_option: permitted_params[:data_option], - active: permitted_params[:active], - screens: permitted_params[:screens], - position: 1550, - editable: true, - ) - render json: object_manager_attribute.attributes_with_association_ids, status: :created - rescue => e - logger.error e - raise Exceptions::UnprocessableEntity, e - end + add_attribute_using_params(permitted_params.merge(position: 1550), status: :created) end # PUT /object_manager_attributes/1 def update - - object_manager_attribute = ObjectManager::Attribute.add( - object: permitted_params[:object], - name: permitted_params[:name], - display: permitted_params[:display], - data_type: permitted_params[:data_type], - data_option: permitted_params[:data_option], - active: permitted_params[:active], - screens: permitted_params[:screens], - position: 1550, - editable: true, + # check if attribute already exists + exists = ObjectManager::Attribute.get( + object: permitted_params[:object], + name: permitted_params[:name], ) - render json: object_manager_attribute.attributes_with_association_ids, status: :ok - rescue => e - logger.error e - raise Exceptions::UnprocessableEntity, e + raise Exceptions::UnprocessableEntity, 'does not exist' if !exists + + add_attribute_using_params(permitted_params, status: :ok) end # DELETE /object_manager_attributes/1 @@ -150,4 +126,16 @@ class ObjectManagerAttributesController < ApplicationController permitted end end + + def add_attribute_using_params(given_params, status:) + attributes = given_params.slice(:object, :name, :display, :data_type, :data_option, :active, :screens, :position) + attributes[:editable] = true + + object_manager_attribute = ObjectManager::Attribute.add(attributes) + + render json: object_manager_attribute.attributes_with_association_ids, status: status + rescue => e + logger.error e + raise Exceptions::UnprocessableEntity, e + end end diff --git a/spec/requests/integration/monitoring_spec.rb b/spec/requests/integration/monitoring_spec.rb index 5a68ea634..f393c9291 100644 --- a/spec/requests/integration/monitoring_spec.rb +++ b/spec/requests/integration/monitoring_spec.rb @@ -527,7 +527,7 @@ RSpec.describe 'Monitoring', type: :request do Delayed::Job.destroy_all # add a new object - object = create(:object_manager_attribute_text) + object = create(:object_manager_attribute_text, name: 'test4') migration = ObjectManager::Attribute.migration_execute expect(true).to eq(migration) diff --git a/spec/requests/integration/object_manager_attributes_spec.rb b/spec/requests/integration/object_manager_attributes_spec.rb index cd94a41f7..cd9e51c1f 100644 --- a/spec/requests/integration/object_manager_attributes_spec.rb +++ b/spec/requests/integration/object_manager_attributes_spec.rb @@ -114,7 +114,7 @@ RSpec.describe 'ObjectManager Attributes', type: :request do # parameters for updating params = { - 'name': 'test4', + 'name': object.name, 'object': 'Ticket', 'display': 'Test 4', 'active': true, @@ -152,7 +152,7 @@ RSpec.describe 'ObjectManager Attributes', type: :request do expect(response).to have_http_status(:ok) expect(json_response).to be_truthy expect(json_response['data_option']['null']).to be_truthy - expect(json_response['name']).to eq('test4') + expect(json_response['name']).to eq(object.name) expect(json_response['display']).to eq('Test 4') end @@ -263,10 +263,9 @@ RSpec.describe 'ObjectManager Attributes', type: :request do expect(json_response['name']).to eq('test5') end - it 'does update user select object', db_strategy: :reset do - + it 'does update user select object', authenticated_as: -> { admin_user }, db_strategy: :reset do # add a new object - object = create(:object_manager_attribute_text) + object = create(:object_manager_attribute_text, object_name: 'User') migration = ObjectManager::Attribute.migration_execute expect(migration).to eq(true) @@ -285,7 +284,7 @@ RSpec.describe 'ObjectManager Attributes', type: :request do data_type: 'select', display: 'Test 7', id: 'c-204', - name: 'test7', + name: object.name, object: 'User', screens: { create: { @@ -322,12 +321,11 @@ RSpec.describe 'ObjectManager Attributes', type: :request do } # update the object - authenticated_as(admin_user) put "/api/v1/object_manager_attributes/#{object.id}", params: params, as: :json expect(response).to have_http_status(:ok) expect(json_response).to be_truthy expect(json_response['data_option']['options']).to be_truthy - expect(json_response['name']).to eq('test7') + expect(json_response['name']).to eq(object.name) expect(json_response['display']).to eq('Test 7') end @@ -1019,5 +1017,75 @@ RSpec.describe 'ObjectManager Attributes', type: :request do expect(json_response['data_option_new']['default']).to eq('fuu') expect(json_response['data_type']).to eq('select') end + + it "doesn't let to update item that doesn't exist", authenticated_as: -> { admin_user } do + params = { + active: true, + data_option: { + type: 'text', + maxlength: 200 + }, + data_type: 'input', + display: 'Test 7', + name: 'attribute_that_doesnt_exist', + object: 'User', + } + + # update the object + put '/api/v1/object_manager_attributes/abc', params: params, as: :json + expect(response).to have_http_status(:unprocessable_entity) + end + + context 'position handling', authenticated_as: -> { admin_user } do + let(:params) do + { + 'name': "customerdescription_#{rand(999_999_999)}", + 'object': 'Ticket', + 'display': "custom description #{rand(999_999_999)}", + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': 'test', + 'type': 'text', + 'maxlength': 120, + }, + } + end + + let(:new_attribute_id) { json_response['id'] } + let(:new_attribute_object) { ObjectManager::Attribute.find new_attribute_id } + + before { post '/api/v1/object_manager_attributes', params: params, as: :json } + + context 'when creating a new attribute' do + it 'defaults to 1550' do + expect(new_attribute_object.position).to eq 1550 + end + end + + context 'when updating an existing attribute' do + let(:alternative_position) { 123 } + let(:alternative_display) { 'another description' } + let(:alternative_params) { params.deep_dup.update(display: alternative_display) } + + before do + new_attribute_object.update! position: alternative_position + + put "/api/v1/object_manager_attributes/#{new_attribute_id}", params: alternative_params, as: :json + + new_attribute_object.reload + end + + # confirm that test build up was correct + it 'request succeeds' do + expect(new_attribute_object.display).to eq alternative_display + end + + # https://github.com/zammad/zammad/issues/3044 + it 'position did not reset' do + expect(new_attribute_object.position).to eq alternative_position + end + end + end end end