unify restful controller logic for projects and commands

... later add more piece by piece

app/controllers/commands_controller.rb

@@ -1,89 +1,38 @@
 # frozen_string_literal: true
-class CommandsController < ApplicationController
+class CommandsController < ResourceController
   include CurrentProject
 
   PUBLIC = [:index, :show, :new].freeze
 
-  before_action :find_command, only: [:update, :show, :destroy]
   before_action :authorize_custom_project_admin!, except: PUBLIC
 
-  def index
-    @commands = Command.order(:project_id)
-    if search = params[:search]
-      if query = search[:query].presence
-        query = ActiveRecord::Base.send(:sanitize_sql_like, query)
-        @commands = @commands.where(Command.arel_table[:command].matches("%#{query}%"))
-      end
-
-      if project_id = search[:project_id].presence
-        project_id = nil if project_id == 'global'
-        @commands = @commands.where(project_id: project_id)
-      end
-    end
-    @pagy, @commands = pagy(@commands, page: page, items: 15)
-  end
-
-  def new
-    @command = Command.new
-    render :show
-  end
-
-  def create
-    @command = Command.create(command_params)
-
-    if @command.persisted?
-      successful_response 'Command created.'
-    else
-      render :show
-    end
-  end
-
-  def show
-  end
-
-  def update
-    if @command.update_attributes(command_params)
-      successful_response('Command updated.')
-    else
-      respond_to do |format|
-        format.html { render :show }
-        format.json { render json: {}, status: :unprocessable_entity }
-      end
-    end
-  end
-
   def destroy
     # Destroy specific stage command usage if `stage_id` is passed in to allow for inline deletion
     remove_stage_usage_if_exists
 
-    if @command.destroy
-      successful_response('Command removed.')
-    else
-      respond_to do |format|
-        format.html { render :show }
-        format.json { render json: {}, status: :unprocessable_entity }
-      end
-    end
+    super
   end
 
   private
 
-  def command_params
-    params.require(:command).permit(:command, :project_id)
-  end
+  def search_resources
+    commands = Command.order(:project_id)
+    if search = params[:search]
+      if query = search[:query].presence
+        query = ActiveRecord::Base.send(:sanitize_sql_like, query)
+        commands = commands.where(Command.arel_table[:command].matches("%#{query}%"))
+      end
 
-  def successful_response(notice)
-    respond_to do |format|
-      format.html do
-        flash[:notice] = notice
-        redirect_to @command
+      if project_id = search[:project_id].presence
+        project_id = nil if project_id == 'global'
+        commands = commands.where(project_id: project_id)
       end
-      format.json { render json: {} }
     end
+    commands
   end
 
-  def find_command
-    @command = Command.find(params[:id])
+  def resource_params
+    super.permit(:command, :project_id)
   end
 
   def authorize_custom_project_admin!
@@ -92,7 +41,7 @@ def authorize_custom_project_admin!
         [project_from_params]
       elsif action_name == 'update'
         projects = [@command.project]
-        projects << project_from_params if command_params.key?(:project_id) # when moving, need to be able to write both
+        projects << project_from_params if resource_params.key?(:project_id) # moving: need to be able to write both
         projects
       else
         [@command.project]
@@ -104,7 +53,7 @@ def authorize_custom_project_admin!
   end
 
   def project_from_params
-    if id = command_params[:project_id].presence
+    if id = resource_params[:project_id].presence
       Project.find(id)
     end
   end

app/controllers/concerns/current_project.rb

@@ -7,12 +7,12 @@ module CurrentProject
     helper_method :current_project
   end
 
+  protected
+
   def current_project
     @project
   end
 
-  protected
-
   def require_project
     @project = (Project.find_by_param!(params[:project_id]) if params[:project_id])
   end

app/controllers/concerns/current_user.rb

@@ -68,7 +68,7 @@ def resource_action
   def can?(action, resource_namespace, scope = nil)
     scope ||= if resource_namespace == :locks && respond_to?(:current_stage)
       current_stage
-    elsif respond_to?(:current_project)
+    elsif respond_to?(:current_project, true)
       current_project
     end
     AccessControl.can?(current_user, action, resource_namespace, scope)

app/controllers/projects_controller.rb

@@ -1,13 +1,11 @@
 # frozen_string_literal: true
 require 'csv'
 
-class ProjectsController < ApplicationController
-  include CurrentProject
-
-  skip_before_action :require_project, only: [:index, :new, :create]
-
+class ProjectsController < ResourceController
+  before_action :find_resource, only: [:show, :edit, :update, :destroy, :deploy_group_versions]
   before_action :authorize_resource!, except: [:deploy_group_versions, :edit]
 
+  # TODO: make this behave more like resource_controller
   def index
     projects = projects_for_user
 
@@ -38,70 +36,46 @@ def index
   end
 
   def new
-    @project = Project.new
-    @project.current_user = current_user
+    super
     @project.stages.build(name: "Production")
   end
 
-  def create
-    @project = Project.new(project_params)
-    @project.current_user = current_user
-
-    if @project.save
-      if Rails.application.config.samson.project_created_email
-        ProjectMailer.created_email(@current_user, @project).deliver_now
-      end
-      redirect_to @project
-      Rails.logger.info("#{@current_user.name_and_email} created a new project #{@project.to_param}")
-    else
-      render :new
+  def deploy_group_versions
+    before = params[:before] ? Time.parse(params[:before]) : Time.now
+    deploy_group_versions = @project.last_deploy_by_group(before).each_with_object({}) do |(id, deploy), hash|
+      hash[id] = deploy.as_json
     end
+    render json: deploy_group_versions
   end
 
-  def show
-    respond_to do |format|
-      format.html { @stages = @project.stages }
-      format.json do
-        render_as_json :project, @project, allowed_includes: [
-          :environment_variable_groups,
-          :environment_variables_with_scope,
-        ]
-      end
-    end
-  end
+  protected
 
-  def edit
+  # compatibility with authorize_resource!
+  def current_project
+    @project
   end
 
-  def update
-    if @project.update_attributes(project_params)
-      redirect_to @project
-    else
-      render :edit
+  def create_callback
+    if Rails.application.config.samson.project_created_email
+      ProjectMailer.created_email(current_user, @project).deliver_now
     end
   end
 
-  def destroy
-    @project.soft_delete(validate: false)
-
+  def destroy_callback
     if Rails.application.config.samson.project_deleted_email
-      ProjectMailer.deleted_email(@current_user, @project).deliver_now
+      ProjectMailer.deleted_email(current_user, @project).deliver_now
     end
-    redirect_to projects_path, notice: "Project removed."
   end
 
-  def deploy_group_versions
-    before = params[:before] ? Time.parse(params[:before]) : Time.now
-    deploy_group_versions = @project.last_deploy_by_group(before).each_with_object({}) do |(id, deploy), hash|
-      hash[id] = deploy.as_json
-    end
-    render json: deploy_group_versions
+  def allowed_includes
+    [
+      :environment_variable_groups,
+      :environment_variables_with_scope,
+    ]
   end
 
-  protected
-
-  def project_params
-    params.require(:project).permit(
+  def resource_params
+    super.permit(
       *[
         :name,
         :repository_url,
@@ -116,7 +90,7 @@ def project_params
         :include_new_deploy_groups,
         :dashboard,
       ] + Samson::Hooks.fire(:project_permitted_params)
-    )
+    ).merge(current_user: current_user)
   end
 
   # TODO: rename ... not user anymore
@@ -147,11 +121,6 @@ def projects_for_user
     scope.alphabetical.order(id: :desc)
   end
 
-  # Overriding require_project from CurrentProject
-  def require_project
-    @project = (Project.find_by_param!(params[:id]) if params[:id])
-  end
-
   # Avoiding N+1 queries on project index
   def last_deploy_for(project)
     @projects_last_deployed_at ||= Deploy.successful.