From 5d07d1b099efbbebae92ea8c85c9a113da09d624 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 14:53:58 +0200 Subject: [PATCH 01/19] Fix(Gemfile): Remove jbuilder gem and associated files --- Gemfile | 2 -- Gemfile.lock | 5 ----- app/views/app_settings/_app_setting.json.jbuilder | 4 ---- app/views/operating_systems/_operating_system.json.jbuilder | 4 ---- app/views/operating_systems/index.json.jbuilder | 3 --- app/views/request_templates/_request_template.json.jbuilder | 4 ---- app/views/request_templates/index.json.jbuilder | 3 --- app/views/request_templates/show.json.jbuilder | 3 --- app/views/requests/_request.json.jbuilder | 4 ---- app/views/requests/index.json.jbuilder | 3 --- app/views/requests/show.json.jbuilder | 3 --- app/views/servers/_server.json.jbuilder | 4 ---- app/views/servers/index.json.jbuilder | 3 --- app/views/servers/show.json.jbuilder | 3 --- 14 files changed, 48 deletions(-) delete mode 100644 app/views/app_settings/_app_setting.json.jbuilder delete mode 100644 app/views/operating_systems/_operating_system.json.jbuilder delete mode 100644 app/views/operating_systems/index.json.jbuilder delete mode 100644 app/views/request_templates/_request_template.json.jbuilder delete mode 100644 app/views/request_templates/index.json.jbuilder delete mode 100644 app/views/request_templates/show.json.jbuilder delete mode 100644 app/views/requests/_request.json.jbuilder delete mode 100644 app/views/requests/index.json.jbuilder delete mode 100644 app/views/requests/show.json.jbuilder delete mode 100644 app/views/servers/_server.json.jbuilder delete mode 100644 app/views/servers/index.json.jbuilder delete mode 100644 app/views/servers/show.json.jbuilder diff --git a/Gemfile b/Gemfile index bc4f20f8..91dffc6f 100644 --- a/Gemfile +++ b/Gemfile @@ -36,8 +36,6 @@ gem 'pundit' # https://github.com/varvet/pundit gem 'bootsnap', '>= 1.1.0', require: false # https://github.com/Shopify/bootsnap # Use CoffeeScript for .coffee assets and views gem 'coffee-rails', '~> 4.2' # https://github.com/rails/coffee-rails -# Build JSON APIs with ease. -gem 'jbuilder', '~> 2.5' # https://github.com/rails/jbuilder # Use Redis adapter to run Action Cable in production # gem 'redis', '~> 4.0' # Use ActiveModel has_secure_password diff --git a/Gemfile.lock b/Gemfile.lock index c4b86545..ec53bb80 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -158,9 +158,6 @@ GEM i18n (1.5.3) concurrent-ruby (~> 1.0) jaro_winkler (1.5.2) - jbuilder (2.8.0) - activesupport (>= 4.2.0) - multi_json (>= 1.2) jquery-datatables (1.10.19.1) jquery-rails (4.3.3) rails-dom-testing (>= 1, < 3) @@ -199,7 +196,6 @@ GEM libv8 (>= 6.3) minitest (5.11.3) msgpack (1.2.6) - multi_json (1.13.1) nenv (0.3.0) net-ssh (5.1.0) nio4r (2.3.1) @@ -420,7 +416,6 @@ DEPENDENCIES font-awesome-rails git guard - jbuilder (~> 2.5) jquery-datatables jquery-rails listen (>= 3.0.5, < 3.2) diff --git a/app/views/app_settings/_app_setting.json.jbuilder b/app/views/app_settings/_app_setting.json.jbuilder deleted file mode 100644 index ba33ffe8..00000000 --- a/app/views/app_settings/_app_setting.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -json.extract! app_setting, :id, :singleton_guard, :git_repository_url, :git_repository_name, :github_user_name, :github_user_email, :vsphere_server_ip, :vsphere_server_user, :vsphere_server_password, :email_notification_smtp_address, :email_notification_smtp_port, :email_notification_smtp_domain, :email_notification_smtp_user, :email_notification_smtp_password, :vm_archivation_timeout, :created_at, :updated_at -json.url app_setting_url(app_setting, format: :json) diff --git a/app/views/operating_systems/_operating_system.json.jbuilder b/app/views/operating_systems/_operating_system.json.jbuilder deleted file mode 100644 index 8e57783b..00000000 --- a/app/views/operating_systems/_operating_system.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -json.extract! operating_system, :id, :name, :created_at, :updated_at -json.url operating_system_url(operating_system, format: :json) diff --git a/app/views/operating_systems/index.json.jbuilder b/app/views/operating_systems/index.json.jbuilder deleted file mode 100644 index 7c8ae5c3..00000000 --- a/app/views/operating_systems/index.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.array! @operating_systems, partial: 'operating_systems/operating_system', as: :operating_system diff --git a/app/views/request_templates/_request_template.json.jbuilder b/app/views/request_templates/_request_template.json.jbuilder deleted file mode 100644 index ae45e02c..00000000 --- a/app/views/request_templates/_request_template.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -json.extract! request_template, :id, :cpu_count, :ram_gb, :storage_mgb, :operating_system, :created_at, :updated_at -json.url request_template_url(request_template, format: :json) diff --git a/app/views/request_templates/index.json.jbuilder b/app/views/request_templates/index.json.jbuilder deleted file mode 100644 index 9df055bc..00000000 --- a/app/views/request_templates/index.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.array! @request_templates, partial: 'request_templates/request_template', as: :request_template diff --git a/app/views/request_templates/show.json.jbuilder b/app/views/request_templates/show.json.jbuilder deleted file mode 100644 index abe3a7e1..00000000 --- a/app/views/request_templates/show.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.partial! 'request_templates/request_template', request_template: @request_template diff --git a/app/views/requests/_request.json.jbuilder b/app/views/requests/_request.json.jbuilder deleted file mode 100644 index 2508d87e..00000000 --- a/app/views/requests/_request.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -json.extract! request, :id, :name, :cpu_cores, :ram_gb, :storage_gb, :operating_system, :comment, :rejection_information, :status, :created_at, :updated_at -json.url request_url(request, format: :json) diff --git a/app/views/requests/index.json.jbuilder b/app/views/requests/index.json.jbuilder deleted file mode 100644 index 28a377dc..00000000 --- a/app/views/requests/index.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.array! @requests, partial: 'requests/request', as: :request diff --git a/app/views/requests/show.json.jbuilder b/app/views/requests/show.json.jbuilder deleted file mode 100644 index 0cb56862..00000000 --- a/app/views/requests/show.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.partial! 'requests/request', request: @request diff --git a/app/views/servers/_server.json.jbuilder b/app/views/servers/_server.json.jbuilder deleted file mode 100644 index ae501685..00000000 --- a/app/views/servers/_server.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -json.extract! server, :id, :created_at, :updated_at -json.url server_url(server, format: :json) diff --git a/app/views/servers/index.json.jbuilder b/app/views/servers/index.json.jbuilder deleted file mode 100644 index 127a398e..00000000 --- a/app/views/servers/index.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.array! @servers, partial: 'servers/server', as: :server diff --git a/app/views/servers/show.json.jbuilder b/app/views/servers/show.json.jbuilder deleted file mode 100644 index 9f2cff97..00000000 --- a/app/views/servers/show.json.jbuilder +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -json.partial! 'servers/server', server: @server From f86ffc9bbf3f9cbab9d277be5ff66da8acee8da0 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 14:56:51 +0200 Subject: [PATCH 02/19] Fix(Requests controller): Remove extracted methods --- app/controllers/requests_controller.rb | 31 ++++++++++---------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index efef2b21..8d2057cd 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -6,14 +6,12 @@ class RequestsController < ApplicationController before_action :authenticate_state_change, only: %i[request_change_state] # GET /requests - # GET /requests.json def index requests = current_user.admin? ? Request.all : Request.select { |r| r.user == current_user } split_requests(requests) end # GET /requests/1 - # GET /requests/1.json def show redirect_to edit_request_path(@request) if current_user.admin? && @request.pending? @request_templates = RequestTemplate.all @@ -38,7 +36,6 @@ def notify_users(title, message, link) end # POST /requests - # POST /requests.json def create prepare_params @@ -48,9 +45,13 @@ def create # check for validity first, before checking enough_resources? # this is neccessary to ensure that the request contains all information needed for enough_resources? if @request.valid? && enough_resources? && @request.save - successful_save(format) + notify_users('New VM request', @request.description_text, @request.description_url(host_url)) + format.html { redirect_to requests_path, notice: 'Request was successfully created.' } + format.json { render :show, status: :created, location: @request } else - unsuccessful_action(format, :new) + @request_templates = RequestTemplate.all + format.html { render :new } + format.json { render json: @request.errors, status: :unprocessable_entity } end end end @@ -66,7 +67,9 @@ def update notify_request_update safe_create_vm_for format, @request else - unsuccessful_action format, :edit + @request_templates = RequestTemplate.all + format.html { render :edit } + format.json { render json: @request.errors, status: :unprocessable_entity } end end end @@ -80,7 +83,9 @@ def reject format.html { redirect_to requests_path, notice: "Request '#{@request.name}' rejected!" } format.json { render status: :ok, action: :index } else - unsuccessful_action format, :edit + @request_templates = RequestTemplate.all + format.html { render :edit } + format.json { render json: @request.errors, status: :unprocessable_entity } end end end @@ -158,18 +163,6 @@ def notify_request_update end end - def successful_save(format) - notify_users('New VM request', @request.description_text, @request.description_url(host_url)) - format.html { redirect_to requests_path, notice: 'Request was successfully created.' } - format.json { render :show, status: :created, location: @request } - end - - def unsuccessful_action(format, method) - @request_templates = RequestTemplate.all - format.html { render method } - format.json { render json: @request.errors, status: :unprocessable_entity } - end - # Storage and RAM are displayed in GB but internally stored in MB. # sudo_user_ids always contain one empty element which must be removed def prepare_params From f703813c56fa5913f5bc3bd1dcf92ed7e904eada Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:11:20 +0200 Subject: [PATCH 03/19] Fix(Requests Controller): Remove JSON from 'reject' method --- app/controllers/requests_controller.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 8d2057cd..edc5a03d 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -76,17 +76,13 @@ def update def reject @request = Request.find params[:id] - respond_to do |format| - @request.reject! - if @request&.update(rejection_params) - notify_request_update - format.html { redirect_to requests_path, notice: "Request '#{@request.name}' rejected!" } - format.json { render status: :ok, action: :index } - else - @request_templates = RequestTemplate.all - format.html { render :edit } - format.json { render json: @request.errors, status: :unprocessable_entity } - end + @request.reject! + if @request.update(rejection_params) + notify_request_update + redirect_to requests_path, notice: "Request '#{@request.name}' rejected!" + else + @request_templates = RequestTemplate.all + render :edit end end From 1d12014807a9a806be7518df2128295582605883 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:16:31 +0200 Subject: [PATCH 04/19] Refacor(Requests Controller): 'destroy' method --- app/controllers/requests_controller.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index edc5a03d..dda76174 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class RequestsController < ApplicationController + @@resource_name = Request.model_name.human.titlecase + before_action :set_request, only: %i[show edit update push_to_git destroy request_state_change] before_action :authenticate_employee before_action :authenticate_state_change, only: %i[request_change_state] @@ -87,13 +89,13 @@ def reject end # DELETE /requests/1 - # DELETE /requests/1.json def destroy - @request.destroy - respond_to do |format| - format.html { redirect_to requests_url, notice: 'Request was successfully destroyed.' } - format.json { head :no_content } + if @request.destroy + notice = t('flash.destroy.notice', resource: @@resource_name, model: @request.name) + else + alert = t('flash.destroy.alert', resource: @@resource_name, model: @request.name) end + redirect_to requests_url, notice: notice, alert: alert end # Creates puppet files for request and pushes the created files into a git repository From ff5105d1fd3e8765edd98362d4fe6ccbed4f107d Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:23:23 +0200 Subject: [PATCH 05/19] Refactor(Requests Controller): 'create' method --- app/controllers/requests_controller.rb | 20 ++++++++------------ spec/end_to_end/end_to_end_spec.rb | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index dda76174..ae5ec28c 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -43,18 +43,14 @@ def create @request = Request.new(request_params.merge(user: current_user)) @request.assign_sudo_users(request_params[:sudo_user_ids]) - respond_to do |format| - # check for validity first, before checking enough_resources? - # this is neccessary to ensure that the request contains all information needed for enough_resources? - if @request.valid? && enough_resources? && @request.save - notify_users('New VM request', @request.description_text, @request.description_url(host_url)) - format.html { redirect_to requests_path, notice: 'Request was successfully created.' } - format.json { render :show, status: :created, location: @request } - else - @request_templates = RequestTemplate.all - format.html { render :new } - format.json { render json: @request.errors, status: :unprocessable_entity } - end + # check for validity first, before checking enough_resources? + # this is neccessary to ensure that the request contains all information needed for enough_resources? + if @request.valid? && enough_resources? && @request.save + notify_users('New VM request', @request.description_text, @request.description_url(host_url)) + redirect_to requests_path, notice: t('flash.create.notice', resource: @@resource_name, model: @request.name) + else + @request_templates = RequestTemplate.all + render :new end end diff --git a/spec/end_to_end/end_to_end_spec.rb b/spec/end_to_end/end_to_end_spec.rb index 626fe220..dbcf5d50 100644 --- a/spec/end_to_end/end_to_end_spec.rb +++ b/spec/end_to_end/end_to_end_spec.rb @@ -40,7 +40,7 @@ select(@project.name, from: 'request_project_id') fill_in('Description', with: 'test') click_on 'Create Request' - expect(page).to have_text('Request was successfully created.') + expect(page).to have_css '.alert-success' expect(page).to have_current_path('/vms/requests') click_on @requestname expect(page.body).to have_text('Request Information') From 0ee964de6480cb8b580bca039ed8192bc35c4017 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:33:43 +0200 Subject: [PATCH 06/19] Refactor(Requests Controller): introduce 'set_request_templates' --- app/controllers/requests_controller.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index ae5ec28c..2a7add13 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -4,6 +4,7 @@ class RequestsController < ApplicationController @@resource_name = Request.model_name.human.titlecase before_action :set_request, only: %i[show edit update push_to_git destroy request_state_change] + before_action :set_request_templates, only: %i[show new edit update create reject] before_action :authenticate_employee before_action :authenticate_state_change, only: %i[request_change_state] @@ -16,19 +17,16 @@ def index # GET /requests/1 def show redirect_to edit_request_path(@request) if current_user.admin? && @request.pending? - @request_templates = RequestTemplate.all end # GET /requests/new def new @request = Request.new - @request_templates = RequestTemplate.all end # GET /requests/1/edit def edit redirect_to @request unless @request.pending? - @request_templates = RequestTemplate.all end def notify_users(title, message, link) @@ -49,7 +47,6 @@ def create notify_users('New VM request', @request.description_text, @request.description_url(host_url)) redirect_to requests_path, notice: t('flash.create.notice', resource: @@resource_name, model: @request.name) else - @request_templates = RequestTemplate.all render :new end end @@ -65,7 +62,6 @@ def update notify_request_update safe_create_vm_for format, @request else - @request_templates = RequestTemplate.all format.html { render :edit } format.json { render json: @request.errors, status: :unprocessable_entity } end @@ -79,7 +75,6 @@ def reject notify_request_update redirect_to requests_path, notice: "Request '#{@request.name}' rejected!" else - @request_templates = RequestTemplate.all render :edit end end @@ -133,6 +128,10 @@ def set_request @request = Request.find(params[:id]) end + def set_request_templates + @request_templates = RequestTemplate.all + end + def authenticate_state_change @request = Request.find(params[:id]) authenticate_admin From b2f8a0ad4767e21ebad5af0535a3fc9f37b7e496 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:36:01 +0200 Subject: [PATCH 07/19] Refactor(Requests Controller): Remove 'split requests' --- app/controllers/requests_controller.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 2a7add13..457c76e7 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -11,7 +11,8 @@ class RequestsController < ApplicationController # GET /requests def index requests = current_user.admin? ? Request.all : Request.select { |r| r.user == current_user } - split_requests(requests) + @pending_requests = requests.select(&:pending?) + @resolved_requests = requests.reject(&:pending?) end # GET /requests/1 @@ -192,11 +193,6 @@ def puppet_name_script(request) helper_method :puppet_name_script - def split_requests(requests) - @pending_requests = requests.select(&:pending?) - @resolved_requests = requests.reject(&:pending?) - end - def enough_resources? hosts = VSphere::Host.all if hosts.empty? From 0581a76d2f836a5028d3d6e62ea337351a1d9d4f Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:47:19 +0200 Subject: [PATCH 08/19] Fix(Requests Form): Display base errors --- app/views/requests/_form.html.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/requests/_form.html.erb b/app/views/requests/_form.html.erb index c0559aa4..b279957c 100644 --- a/app/views/requests/_form.html.erb +++ b/app/views/requests/_form.html.erb @@ -2,6 +2,8 @@ <%= bootstrap_form_with(model: request, local: true) do |form| %> <%# https://github.com/bootstrap-ruby/bootstrap_form#alert-messages %> <%= form.alert_message "Please fix the errors below.", error_summary: false %> + <%# Displays errors set in controllers on model.errors[:base] %> + <%= form.errors_on :base %>
From 92e05ec1c0fe3240c5e4dffde75761cf0a06ebff Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:49:58 +0200 Subject: [PATCH 09/19] Refactor(Requests Controller): Make 'notify users' private --- app/controllers/requests_controller.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 457c76e7..325447c6 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -30,12 +30,6 @@ def edit redirect_to @request unless @request.pending? end - def notify_users(title, message, link) - ([@request.user] + @request.users + User.admin).uniq.each do |each| - each.notify(title, message, link) - end - end - # POST /requests def create prepare_params @@ -133,6 +127,12 @@ def set_request_templates @request_templates = RequestTemplate.all end + def notify_users(title, message, link) + ([@request.user] + @request.users + User.admin).uniq.each do |each| + each.notify(title, message, link) + end + end + def authenticate_state_change @request = Request.find(params[:id]) authenticate_admin From aee6cb57351d40fee30c8bce373d37d6706b8d5a Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 15:55:51 +0200 Subject: [PATCH 10/19] Refactor(Requests Controller): Remove 'authenticate_state_change' --- app/controllers/requests_controller.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 325447c6..e1315401 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -6,7 +6,6 @@ class RequestsController < ApplicationController before_action :set_request, only: %i[show edit update push_to_git destroy request_state_change] before_action :set_request_templates, only: %i[show new edit update create reject] before_action :authenticate_employee - before_action :authenticate_state_change, only: %i[request_change_state] # GET /requests def index @@ -133,12 +132,6 @@ def notify_users(title, message, link) end end - def authenticate_state_change - @request = Request.find(params[:id]) - authenticate_admin - redirect_to @request, alert: I18n.t('request.unauthorized_state_change') if @request.user == current_user && !current_user.admin? - end - def notify_request_update return if @request.pending? From e17a37ccf3973265b902532b3a3e9dd2fc81791d Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:09:55 +0200 Subject: [PATCH 11/19] Refactor(Requests Controller): Remove ref to 'authenticate_state_change' --- app/controllers/requests_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index e1315401..28eb063b 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -3,7 +3,7 @@ class RequestsController < ApplicationController @@resource_name = Request.model_name.human.titlecase - before_action :set_request, only: %i[show edit update push_to_git destroy request_state_change] + before_action :set_request, only: %i[show edit update push_to_git destroy] before_action :set_request_templates, only: %i[show new edit update create reject] before_action :authenticate_employee @@ -62,6 +62,7 @@ def update end end + # PATCH /requests/reject/1 def reject @request = Request.find params[:id] @request.reject! From ad3f2229c7f5cff01def6bde0dbf69f2dd50929d Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:11:38 +0200 Subject: [PATCH 12/19] Refactor(Requests Controller): Use 'set_request' --- app/controllers/requests_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 28eb063b..3a2191f5 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -3,7 +3,7 @@ class RequestsController < ApplicationController @@resource_name = Request.model_name.human.titlecase - before_action :set_request, only: %i[show edit update push_to_git destroy] + before_action :set_request, only: %i[show edit update push_to_git destroy reject] before_action :set_request_templates, only: %i[show new edit update create reject] before_action :authenticate_employee @@ -64,7 +64,6 @@ def update # PATCH /requests/reject/1 def reject - @request = Request.find params[:id] @request.reject! if @request.update(rejection_params) notify_request_update From 6ef9aa33c2aa77e22d9ce84f319f9f7c2cc04f83 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:23:36 +0200 Subject: [PATCH 13/19] Refactor(Requests Controller): 'update' method --- app/controllers/requests_controller.rb | 28 +++++++++++--------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 3a2191f5..16d65670 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -46,19 +46,15 @@ def create end # PATCH/PUT /requests/1 - # PATCH/PUT /requests/1.json def update - respond_to do |format| - prepare_params - @request.assign_sudo_users(request_params[:sudo_user_ids]) - @request.accept! - if @request.update(request_params) - notify_request_update - safe_create_vm_for format, @request - else - format.html { render :edit } - format.json { render json: @request.errors, status: :unprocessable_entity } - end + prepare_params + @request.assign_sudo_users(request_params[:sudo_user_ids]) + @request.accept! + if @request.update(request_params) + notify_request_update + safe_create_vm_for @request + else + render :edit end end @@ -101,16 +97,16 @@ def notice_for(vm, warning) # rubocop:disable Naming/UncommunicativeMethodParamN end end - def safe_create_vm_for(format, request) + def safe_create_vm_for(request) vm, warning = request.create_vm notices = notice_for vm, warning if vm - format.html { redirect_to({ controller: :vms, action: 'edit_config', id: vm.name }, { method: :get }.merge(notices)) } + redirect_to({ controller: :vms, action: 'edit_config', id: vm.name }, { method: :get }.merge(notices)) else - format.html { redirect_to requests_path, notices } + redirect_to requests_path, notices end rescue RbVmomi::Fault => fault - format.html { redirect_to requests_path, alert: "VM could not be created, error: \"#{fault.message}\"" } + redirect_to requests_path, alert: "VM could not be created, error: \"#{fault.message}\"" end def host_url From 20dae883f0c19fc0ca26b7b36095b59004b3e5f9 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:32:45 +0200 Subject: [PATCH 14/19] Refactor(Requests Controller): 'index' db access --- app/controllers/requests_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 16d65670..54560c6e 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -9,7 +9,11 @@ class RequestsController < ApplicationController # GET /requests def index - requests = current_user.admin? ? Request.all : Request.select { |r| r.user == current_user } + if current_user.admin? + requests = Request.all + else + requests = Request.where(user: current_user) + end @pending_requests = requests.select(&:pending?) @resolved_requests = requests.reject(&:pending?) end From 6716bd07a0e439dca56e8b412664dcf23d33d85d Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:38:12 +0200 Subject: [PATCH 15/19] Refactor(Requests Controller): 'safe_create_vm_for' --- app/controllers/requests_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 54560c6e..314ba0ed 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -105,7 +105,7 @@ def safe_create_vm_for(request) vm, warning = request.create_vm notices = notice_for vm, warning if vm - redirect_to({ controller: :vms, action: 'edit_config', id: vm.name }, { method: :get }.merge(notices)) + redirect_to edit_config_path(vm.name), notices else redirect_to requests_path, notices end From 24fc8a2b05f86be40bda08c77abc34848f8205a0 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 16:55:49 +0200 Subject: [PATCH 16/19] Refactor(Requests Controller): Remove 'notice_for' --- app/controllers/requests_controller.rb | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 314ba0ed..f742ee7c 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -91,26 +91,15 @@ def push_to_git private - def notice_for(vm, warning) # rubocop:disable Naming/UncommunicativeMethodParamName - if warning - { alert: warning } - elsif vm - { notice: 'VM was successfully created!' } - else - { alert: 'VM could not be created due to an unkown error! Please create it manually in vSphere' } - end - end - def safe_create_vm_for(request) vm, warning = request.create_vm - notices = notice_for vm, warning if vm - redirect_to edit_config_path(vm.name), notices + redirect_to edit_config_path(vm.name), notice: 'VM was successfully created!' else - redirect_to requests_path, notices + redirect_to requests_path, alert: "Error while creating VM! #{warning || 'Please create it manually in vSphere'}" end rescue RbVmomi::Fault => fault - redirect_to requests_path, alert: "VM could not be created, error: \"#{fault.message}\"" + redirect_to requests_path, alert: "Error while creating VM! #{fault.message}" end def host_url From 3754542751019a2cb521fa73f8467d145c3d5819 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 17:29:21 +0200 Subject: [PATCH 17/19] Refactor(Requests Controller): Remove 'prepare_params' --- app/controllers/requests_controller.rb | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index f742ee7c..27394ce2 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -35,8 +35,6 @@ def edit # POST /requests def create - prepare_params - @request = Request.new(request_params.merge(user: current_user)) @request.assign_sudo_users(request_params[:sudo_user_ids]) # check for validity first, before checking enough_resources? @@ -51,7 +49,6 @@ def create # PATCH/PUT /requests/1 def update - prepare_params @request.assign_sudo_users(request_params[:sudo_user_ids]) @request.accept! if @request.update(request_params) @@ -139,21 +136,17 @@ def notify_request_update end end - # Storage and RAM are displayed in GB but internally stored in MB. - # sudo_user_ids always contain one empty element which must be removed - def prepare_params - request_parameters = params[:request] - return unless request_parameters - - request_parameters[:sudo_user_ids] &&= request_parameters[:sudo_user_ids][1..-1] - - # the user_ids must contain the ids of ALL users, sudo or not - request_parameters[:user_ids] ||= [] - request_parameters[:user_ids] += request_parameters[:sudo_user_ids] if request_parameters[:sudo_user_ids] - end - # Never trust parameters from the scary internet, only allow the white list through. + # Modify parameters received from the request form def request_params + if params[:request] + # sudo_user_ids always contain one empty element which must be removed + params[:request][:sudo_user_ids].reject!(&:blank?) + # the user_ids must contain the ids of ALL users, sudo or not + params[:request][:user_ids] ||= [] + params[:request][:user_ids] += params[:request][:sudo_user_ids] if params[:request][:sudo_user_ids] + end + params.require(:request).permit(:name, :cpu_cores, :ram_gb, :storage_gb, :operating_system, :port, :application_name, :description, :comment, :project_id, :port_forwarding, :rejection_information, responsible_user_ids: [], From a554364db32372a02807e61316039987b3a32f88 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Wed, 3 Apr 2019 17:39:50 +0200 Subject: [PATCH 18/19] Chore(Requests Controller): Whitespace --- app/controllers/requests_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 27394ce2..bb3afa91 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -146,7 +146,7 @@ def request_params params[:request][:user_ids] ||= [] params[:request][:user_ids] += params[:request][:sudo_user_ids] if params[:request][:sudo_user_ids] end - + params.require(:request).permit(:name, :cpu_cores, :ram_gb, :storage_gb, :operating_system, :port, :application_name, :description, :comment, :project_id, :port_forwarding, :rejection_information, responsible_user_ids: [], From 1966d12d48daa76ef0df2b19e92d85ebac7f41a3 Mon Sep 17 00:00:00 2001 From: Christoph Matthies Date: Thu, 25 Apr 2019 15:16:51 +0200 Subject: [PATCH 19/19] Fix(Request model): Use scopes for status --- app/controllers/requests_controller.rb | 4 ++-- app/models/request.rb | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index bb3afa91..0a7f88fa 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -14,8 +14,8 @@ def index else requests = Request.where(user: current_user) end - @pending_requests = requests.select(&:pending?) - @resolved_requests = requests.reject(&:pending?) + @pending_requests = requests.pending + @resolved_requests = requests.resolved end # GET /requests/1 diff --git a/app/models/request.rb b/app/models/request.rb index 16cab4f7..a2bbbe11 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -16,6 +16,8 @@ class Request < ApplicationRecord MAX_NAME_LENGTH = 20 enum status: %i[pending accepted rejected] + scope :resolved, -> { where.not(status: :pending) } + validates :name, length: { maximum: MAX_NAME_LENGTH, message: 'only allows a maximum of %{count} characters' }, format: { with: /\A[a-z0-9\-]+\z/, message: 'only allows lowercase letters, numbers and "-"' }