From 285280827a2c7b2e11546bd1e5616af8cc204a90 Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Thu, 28 May 2020 12:18:46 +0200 Subject: [PATCH] Refactor repository column base controller --- .../asset_columns_controller.rb | 2 +- .../base_columns_controller.rb | 28 ------- .../checklist_columns_controller.rb | 2 +- .../date_time_columns_controller.rb | 2 +- .../list_columns_controller.rb | 2 +- .../number_columns_controller.rb | 2 +- .../status_columns_controller.rb | 2 +- .../text_columns_controller.rb | 2 +- .../repository_columns_controller.rb | 41 ++++++----- app/views/my_modules/repository.html.erb | 2 +- .../repositories/_repository_table.html.erb | 2 +- app/views/repositories/show.html.erb | 2 +- .../_delete_column_modal_body.html.erb | 4 +- .../_manage_column_modal_content.html.erb | 2 +- .../_manage_column_modal_index.html.erb | 2 +- config/routes.rb | 13 +--- .../delete_column_service_spec.rb | 73 ------------------- 17 files changed, 36 insertions(+), 147 deletions(-) delete mode 100644 app/controllers/repository_columns/base_columns_controller.rb delete mode 100644 spec/services/repository_columns/delete_column_service_spec.rb diff --git a/app/controllers/repository_columns/asset_columns_controller.rb b/app/controllers/repository_columns/asset_columns_controller.rb index 38fa56fce..0944921d0 100644 --- a/app/controllers/repository_columns/asset_columns_controller.rb +++ b/app/controllers/repository_columns/asset_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class AssetColumnsController < BaseColumnsController + class AssetColumnsController < RepositoryColumnsController include InputSanitizeHelper before_action :load_column, only: :update before_action :check_create_permissions, only: :create diff --git a/app/controllers/repository_columns/base_columns_controller.rb b/app/controllers/repository_columns/base_columns_controller.rb deleted file mode 100644 index 29b46d3d1..000000000 --- a/app/controllers/repository_columns/base_columns_controller.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module RepositoryColumns - class BaseColumnsController < ApplicationController - include InputSanitizeHelper - before_action :load_repository - - private - - def load_repository - @repository = Repository.accessible_by_teams(current_team).find_by(id: params[:repository_id]) - render_404 unless @repository - end - - def load_column - @repository_column = @repository.repository_columns.find_by(id: params[:id]) - render_404 unless @repository_column - end - - def check_create_permissions - render_403 unless can_create_repository_columns?(@repository) - end - - def check_manage_permissions - render_403 unless can_manage_repository_column?(@repository_column) - end - end -end diff --git a/app/controllers/repository_columns/checklist_columns_controller.rb b/app/controllers/repository_columns/checklist_columns_controller.rb index 3978ee446..12e3c43e2 100644 --- a/app/controllers/repository_columns/checklist_columns_controller.rb +++ b/app/controllers/repository_columns/checklist_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class ChecklistColumnsController < BaseColumnsController + class ChecklistColumnsController < RepositoryColumnsController before_action :load_column, only: %i(update items) before_action :check_create_permissions, only: :create before_action :check_manage_permissions, only: :update diff --git a/app/controllers/repository_columns/date_time_columns_controller.rb b/app/controllers/repository_columns/date_time_columns_controller.rb index 21f43e7a7..c1a9ecbc1 100644 --- a/app/controllers/repository_columns/date_time_columns_controller.rb +++ b/app/controllers/repository_columns/date_time_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class DateTimeColumnsController < BaseColumnsController + class DateTimeColumnsController < RepositoryColumnsController include InputSanitizeHelper before_action :load_column, only: :update before_action :check_create_permissions, only: :create diff --git a/app/controllers/repository_columns/list_columns_controller.rb b/app/controllers/repository_columns/list_columns_controller.rb index cb605a33a..3bae9466d 100644 --- a/app/controllers/repository_columns/list_columns_controller.rb +++ b/app/controllers/repository_columns/list_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class ListColumnsController < BaseColumnsController + class ListColumnsController < RepositoryColumnsController before_action :load_column, only: %i(update items) before_action :check_create_permissions, only: :create before_action :check_manage_permissions, only: :update diff --git a/app/controllers/repository_columns/number_columns_controller.rb b/app/controllers/repository_columns/number_columns_controller.rb index 516bad56e..ba3dc3469 100644 --- a/app/controllers/repository_columns/number_columns_controller.rb +++ b/app/controllers/repository_columns/number_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class NumberColumnsController < BaseColumnsController + class NumberColumnsController < RepositoryColumnsController include InputSanitizeHelper before_action :load_column, only: :update before_action :check_create_permissions, only: :create diff --git a/app/controllers/repository_columns/status_columns_controller.rb b/app/controllers/repository_columns/status_columns_controller.rb index b174f5c3c..940c228e5 100644 --- a/app/controllers/repository_columns/status_columns_controller.rb +++ b/app/controllers/repository_columns/status_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class StatusColumnsController < BaseColumnsController + class StatusColumnsController < RepositoryColumnsController include InputSanitizeHelper before_action :load_column, only: %i(update items) before_action :check_create_permissions, only: :create diff --git a/app/controllers/repository_columns/text_columns_controller.rb b/app/controllers/repository_columns/text_columns_controller.rb index 2908059f3..efb35da93 100644 --- a/app/controllers/repository_columns/text_columns_controller.rb +++ b/app/controllers/repository_columns/text_columns_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryColumns - class TextColumnsController < BaseColumnsController + class TextColumnsController < RepositoryColumnsController include InputSanitizeHelper before_action :load_column, only: :update before_action :check_create_permissions, only: :create diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index 1a95cff1b..1b1f94036 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -2,18 +2,13 @@ class RepositoryColumnsController < ApplicationController include InputSanitizeHelper include RepositoryColumnsHelper - ACTIONS = %i( - create index_html create_html available_asset_type_columns available_columns - ).freeze - before_action :load_vars, - except: ACTIONS - before_action :load_vars_nested, - only: ACTIONS - before_action :check_manage_permissions, - except: ACTIONS + before_action :load_repository + before_action :load_column, only: %i(edit destroy_html destroy) + before_action :check_create_permissions, only: :new + before_action :check_manage_permissions, only: %i(edit destroy_html destroy) before_action :load_asset_type_columns, only: :available_asset_type_columns - def index_html + def index render json: { id: @repository.id, html: render_to_string( @@ -22,7 +17,7 @@ class RepositoryColumnsController < ApplicationController } end - def create_html + def new @repository_column = RepositoryColumn.new respond_to do |format| format.json do @@ -35,8 +30,12 @@ class RepositoryColumnsController < ApplicationController end end - def edit_html - render json: { html: render_to_string(partial: 'repository_columns/manage_column_modal_content.html.erb') } + def edit + render json: { + html: render_to_string( + partial: 'repository_columns/manage_column_modal_content.html.erb' + ) + } end def destroy_html @@ -96,16 +95,14 @@ class RepositoryColumnsController < ApplicationController include StringUtility AvailableRepositoryColumn = Struct.new(:id, :name) - def load_vars - @repository = Repository.accessible_by_teams(current_team).find_by_id(params[:repository_id]) + def load_repository + @repository = Repository.accessible_by_teams(current_team).find_by(id: params[:repository_id]) render_404 unless @repository - @repository_column = @repository.repository_columns.find_by_id(params[:id]) - render_404 unless @repository_column end - def load_vars_nested - @repository = Repository.accessible_by_teams(current_team).find_by_id(params[:repository_id]) - render_404 unless @repository + def load_column + @repository_column = @repository.repository_columns.find_by(id: params[:id]) + render_404 unless @repository_column end def load_asset_type_columns @@ -113,6 +110,10 @@ class RepositoryColumnsController < ApplicationController @asset_columns = load_asset_columns(search_params[:q]) end + def check_create_permissions + render_403 unless can_create_repository_columns?(@repository) + end + def check_manage_permissions render_403 unless can_manage_repository_column?(@repository_column) end diff --git a/app/views/my_modules/repository.html.erb b/app/views/my_modules/repository.html.erb index f1bfdb29c..03f7a17b8 100644 --- a/app/views/my_modules/repository.html.erb +++ b/app/views/my_modules/repository.html.erb @@ -19,7 +19,7 @@
diff --git a/app/views/repositories/_repository_table.html.erb b/app/views/repositories/_repository_table.html.erb index 049291b3e..03bc2379d 100644 --- a/app/views/repositories/_repository_table.html.erb +++ b/app/views/repositories/_repository_table.html.erb @@ -39,7 +39,7 @@ class="repository-column" id="<%= column.id %>" data-type="<%= column.data_type %>" - data-edit-column-url="<%= repository_columns_edit_html_path(repository, column) %>" + data-edit-column-url="<%= edit_repository_repository_column_path(repository, column) %>" data-destroy-column-url="<%= repository_columns_destroy_html_path(repository, column) %>" data-editable-row="<%= can_manage_repository_column?(column) %>" <% column.metadata.each do |k, v| %> diff --git a/app/views/repositories/show.html.erb b/app/views/repositories/show.html.erb index fe1e3b6ca..25528e039 100644 --- a/app/views/repositories/show.html.erb +++ b/app/views/repositories/show.html.erb @@ -45,7 +45,7 @@
diff --git a/app/views/repository_columns/_delete_column_modal_body.html.erb b/app/views/repository_columns/_delete_column_modal_body.html.erb index cbea4cade..b84a5b088 100644 --- a/app/views/repository_columns/_delete_column_modal_body.html.erb +++ b/app/views/repository_columns/_delete_column_modal_body.html.erb @@ -1,7 +1,7 @@
@@ -23,7 +23,7 @@
diff --git a/app/views/repository_columns/_manage_column_modal_content.html.erb b/app/views/repository_columns/_manage_column_modal_content.html.erb index 9a4a59735..6256c545e 100644 --- a/app/views/repository_columns/_manage_column_modal_content.html.erb +++ b/app/views/repository_columns/_manage_column_modal_content.html.erb @@ -1,7 +1,7 @@
diff --git a/app/views/repository_columns/_manage_column_modal_index.html.erb b/app/views/repository_columns/_manage_column_modal_index.html.erb index aaae94aa7..71514e772 100644 --- a/app/views/repository_columns/_manage_column_modal_index.html.erb +++ b/app/views/repository_columns/_manage_column_modal_index.html.erb @@ -15,7 +15,7 @@ <% if can_create_repository_columns?(@repository)%> diff --git a/config/routes.rb b/config/routes.rb index 0893e58a3..2f222d8eb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -617,20 +617,9 @@ Rails.application.routes.draw do post 'copy_records', to: 'repository_rows#copy_records', defaults: { format: 'json' } - get 'repository_columns/:id/edit_html', - to: 'repository_columns#edit_html', - as: 'columns_edit_html' get 'repository_columns/:id/destroy_html', to: 'repository_columns#destroy_html', as: 'columns_destroy_html' - get 'index_html', - to: 'repository_columns#index_html', - as: 'columns_index_html', - defaults: { format: 'json' } - get 'create_html', - to: 'repository_columns#create_html', - as: 'columns_create_html', - defaults: { format: 'json' } get 'available_columns', to: 'repository_columns#available_columns', as: 'available_columns', @@ -638,7 +627,7 @@ Rails.application.routes.draw do get :table_toolbar get :status - resources :repository_columns, only: :destroy + resources :repository_columns, only: %i(index new edit destroy) resources :repository_rows, only: %i(create show update) do member do get :assigned_task_list diff --git a/spec/services/repository_columns/delete_column_service_spec.rb b/spec/services/repository_columns/delete_column_service_spec.rb deleted file mode 100644 index 5465f8698..000000000 --- a/spec/services/repository_columns/delete_column_service_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe RepositoryColumns::DeleteColumnService do - let(:user) { create :user } - let!(:user_team) { create :user_team, :admin, user: user, team: team } - let(:team) { create :team } - let(:repository) { create :repository, team: team } - let(:repository_column) { create :repository_column, :list_type } - - context 'when deletes column' do - let(:service_call) do - RepositoryColumns::DeleteColumnService.call(user: user, team: team, column: repository_column) - end - - it 'removes RepositoryColumn record' do - repository_column - - expect { service_call }.to change { RepositoryColumn.count }.by(-1) - end - - it 'adds Activity record' do - expect { service_call }.to(change { Activity.count }.by(1)) - end - - context 'when RepositoryColumn has RepositoryListItems' do - before do - 3.times do - create(:repository_list_item, repository_column: repository_column) - end - end - - it 'removes RepositoryListItem records as well' do - expect { service_call }.to(change { RepositoryListItem.count }.by(-3)) - end - end - - context 'when RepositoryColumn has RepositoryStatusItems' do - before do - 3.times do - create(:repository_status_item, repository_column: repository_column) - end - end - - it 'removes RepositoryStatusItem records as well' do - expect { service_call }.to(change { RepositoryStatusItem.count }.by(-3)) - end - end - end - - context 'when column cannot be deleted' do - let(:service_call) do - RepositoryColumns::DeleteColumnService.call(user: user, team: team, column: repository_column) - end - - before do - allow_any_instance_of(RepositoryColumn).to receive(:destroy!).and_raise(ActiveRecord::RecordNotDestroyed) - end - - it 'returns errors' do - expect(service_call.errors).to have_key(:repository_column) - end - - it 'returns succeed false' do - expect(service_call.succeed?).to be_falsey - end - - it 'does not add Activity record' do - expect { service_call }.not_to(change { Activity.count }) - end - end -end