From eaa33fc40d8149e9a271e3f7675a2276c2c83014 Mon Sep 17 00:00:00 2001 From: Heitor Reis Date: Fri, 15 Jan 2016 16:14:42 -0200 Subject: [PATCH] Refactor MetricConfiguration controllers --- app/controllers/base_metric_configurations_controller.rb | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------- app/controllers/compound_metric_configurations_controller.rb | 67 +++++++------------------------------------------------------------ app/controllers/hotspot_metric_configurations_controller.rb | 40 +++++++--------------------------------- app/controllers/metric_configurations_controller.rb | 86 ++++++++++++-------------------------------------------------------------------------- 4 files changed, 123 insertions(+), 187 deletions(-) diff --git a/app/controllers/base_metric_configurations_controller.rb b/app/controllers/base_metric_configurations_controller.rb index ec81d1f..77d18a2 100644 --- a/app/controllers/base_metric_configurations_controller.rb +++ b/app/controllers/base_metric_configurations_controller.rb @@ -1,42 +1,119 @@ -include OwnershipAuthentication -include MetricConfigurationsConcern - class BaseMetricConfigurationsController < ApplicationController + include OwnershipAuthentication + include MetricConfigurationsConcern + before_action :authenticate_user!, except: [:show, :index] before_action :metric_configuration_owner?, only: [:edit, :update, :destroy] before_action :kalibro_configuration_owner?, only: [:new, :create, :choose_metric] - before_action :set_metric_configuration, only: [:show, :edit, :update, :destroy] + before_action :set_kalibro_configuration! + before_action :find_metric_configuration!, only: [:show, :edit, :update, :destroy] + before_action :new_metric_configuration!, only: [:create] + before_action :set_metric!, only: [:create, :update] + + def show + @reading_group = ReadingGroup.find(@metric_configuration.reading_group_id) + @kalibro_ranges = @metric_configuration.kalibro_ranges + end def new - update_metric_configuration(MetricConfiguration.new) + @metric_configuration = MetricConfiguration.new end - def show - if metric_configuration - @reading_group = ReadingGroup.find(metric_configuration.reading_group_id) - @kalibro_ranges = metric_configuration.kalibro_ranges - else - raise NotImplementedError + def create + respond_to { |format| save_and_redir format } + end + + def update + respond_to do |format| + save_and_redir(format) do |metric_configuration| + metric_configuration.reading_group_id = params[:reading_group_id].to_i + metric_configuration.update(metric_configuration_params) + end end end - def create - update_metric_configuration(MetricConfiguration.new(metric_configuration_params)) + def destroy + @metric_configuration.destroy + clear_caches + + respond_to do |format| + format.html { redirect_to kalibro_configuration_path(@kalibro_configuration.id) } + format.json { head :no_content } + end end protected - def metric_configuration - raise NotImplementedError + def save_and_redir(format) + new_record = @metric_configuration.id.nil? + result = block_given? ? (yield @metric_configuration) : @metric_configuration.save + + if result + clear_caches + + format.html do + redirect_to kalibro_configuration_path(@kalibro_configuration.id), + notice: t(new_record ? 'successfully_created' : 'successfully_updated', + record: t(@metric_configuration.class)) + end + format.json { render json: @metric_configuration, status: new_record ? :created : :ok } + else + failed_action(format, new_record ? :new : :edit) + end end - def update_metric_configuration (new_metric_configuration) - raise NotImplementedError + # FIXME: This action should render with an error message on rendering the html + def failed_action(format, action) + format.html { render action } + format.json { render json: @metric_configuration.kalibro_errors, status: :unprocessable_entity } + end + + def clear_caches + Rails.cache.delete("#{@kalibro_configuration.id}_tree_metric_configurations") + Rails.cache.delete("#{@kalibro_configuration.id}_hotspot_metric_configurations") end - # Never trust parameters from the scary internet, only allow the white list through. - # TODO: this should be refactored to the concern metric configuration def metric_configuration_params - params[:metric_configuration] + raise NotImplementedError + end + + def set_kalibro_configuration! + @kalibro_configuration = KalibroConfiguration.find params[:kalibro_configuration_id].to_i + end + + def new_metric_configuration! + @metric_configuration = MetricConfiguration.new metric_configuration_params + @metric_configuration.kalibro_configuration_id = @kalibro_configuration.id + @metric_configuration.reading_group_id = params[:reading_group_id].to_i + end + + def find_metric_configuration! + @metric_configuration = MetricConfiguration.find params[:id].to_i + + # Make sure the metric configuration is really from the kalibro configuration we're being told it is + if @metric_configuration.kalibro_configuration_id != @kalibro_configuration.id + raise KalibroClient::Errors::RecordNotFound + end + end + + def set_metric! + collector = KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(params[:metric_collector_name]) + unless collector.nil? + # FIXME: Some view pass metric code as a parameter instead of metric name + if params.key?(:metric_code) + metric = collector.find_metric_by_code(params[:metric_code]) + else + metric = collector.find_metric_by_name(params[:metric_name]) + end + + if !metric.nil? && metric.type == self.class::METRIC_TYPE + @metric_configuration.metric = metric + return + end + end + respond_to do |format| + format.html { redirect_to kalibro_configurations_path(@kalibro_configuration.id) } + format.json { render json: { errors: 'Invalid metric type' }, status: :unprocessable_entity } + end end end diff --git a/app/controllers/compound_metric_configurations_controller.rb b/app/controllers/compound_metric_configurations_controller.rb index 3bacd4c..6fd2869 100644 --- a/app/controllers/compound_metric_configurations_controller.rb +++ b/app/controllers/compound_metric_configurations_controller.rb @@ -1,72 +1,19 @@ class CompoundMetricConfigurationsController < BaseMetricConfigurationsController - before_action :set_metric_configurations, only: [:new, :edit] - - def create - super - @compound_metric_configuration.metric.type = "CompoundMetricSnapshot" - respond_to do |format| - create_and_redir(format) - end - Rails.cache.delete("#{params[:kalibro_configuration_id].to_i}_tree_metric_configurations") - end - - def show - update_metric_configuration(@metric_configuration) - super - end - - def edit - @compound_metric_configuration = @metric_configuration - @kalibro_configuration_id = params[:kalibro_configuration_id].to_i - @compound_metric_configuration.kalibro_configuration_id = @kalibro_configuration_id - end + METRIC_TYPE = 'CompoundMetricSnapshot' - def update - respond_to do |format| - edit - if @compound_metric_configuration.update(metric_configuration_params) - format.html { redirect_to kalibro_configuration_path(@compound_metric_configuration.kalibro_configuration_id), notice: t('successfully_updated', :record => t('compound') + " " + @compound_metric_configuration.class.model_name.human) } - format.json { head :no_content } - else - failed_action(format, 'edit') - end - Rails.cache.delete("#{@compound_metric_configuration.kalibro_configuration_id}_tree_metric_configurations") - end - end + before_action :set_metric_configurations, only: [:new, :edit] protected - def metric_configuration - @compound_metric_configuration - end - - def update_metric_configuration (new_metric_configuration) - @kalibro_configuration_id = params[:kalibro_configuration_id] - @compound_metric_configuration = new_metric_configuration - end - - private - - # Duplicated code on create and update actions extracted here - def failed_action(format, destiny_action) - @kalibro_configuration_id = params[:kalibro_configuration_id] - - set_metric_configurations - format.html { render action: destiny_action } - format.json { render json: @compound_metric_configuration.kalibro_errors, status: :unprocessable_entity } + def set_metric! + @metric_configuration.metric.type = 'CompoundMetricSnapshot' end - #Code extracted from create action - def create_and_redir(format) - if @compound_metric_configuration.save - format.html { redirect_to kalibro_configuration_path(@compound_metric_configuration.kalibro_configuration_id), notice: t('successfully_created', :record => t('compound') + " " + @compound_metric_configuration.class.model_name.human) } - else - failed_action(format, 'new') - end + def metric_configuration_params + params.require(:metric_configuration).permit(:reading_group_id, :weight, :metric => [:name, :description, :script, :scope, :code]) end def set_metric_configurations - @metric_configurations = MetricConfiguration.metric_configurations_of(params[:kalibro_configuration_id].to_i) + @metric_configurations = MetricConfiguration.metric_configurations_of(@kalibro_configuration.id) end - end diff --git a/app/controllers/hotspot_metric_configurations_controller.rb b/app/controllers/hotspot_metric_configurations_controller.rb index 121439a..3e15e61 100644 --- a/app/controllers/hotspot_metric_configurations_controller.rb +++ b/app/controllers/hotspot_metric_configurations_controller.rb @@ -1,41 +1,15 @@ class HotspotMetricConfigurationsController < BaseMetricConfigurationsController - def create - super - @metric_configuration.metric = KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(params[:metric_collector_name]).find_metric_by_code params[:metric_code] - respond_to do |format| - if @metric_configuration.save - format.html do - redirect_to kalibro_configuration_path(@metric_configuration.kalibro_configuration_id), - notice: t('successfully_created', record: t(@metric_configuration.class)) - end - else - failed_action(format) - end - end - clear_caches - end - - protected - - def update_metric_configuration(new_metric_configuration) - @kalibro_configuration_id = params[:kalibro_configuration_id] - @metric_configuration = new_metric_configuration - end + METRIC_TYPE = 'HotspotMetricSnapshot' def metric_configuration_params - { kalibro_configuration_id: params[:kalibro_configuration_id] } + # Hotspot metric configurations don't accept any parameters on creation + # But we must make that explicit as the method isn't implemented in the parent class + params.permit end - private - - def failed_action(format) - @kalibro_configuration_id = params[:kalibro_configuration_id] - format.html { redirect_to kalibro_configuration_choose_metric_path } + # FIXME: This action should render with an error message on rendering the html + def failed_action(format, _) + format.html { redirect_to kalibro_configurations_path(@kalibro_configuration.id) } format.json { render json: @metric_configuration.kalibro_errors, status: :unprocessable_entity } end - - def clear_caches - Rails.cache.delete("#{params[:kalibro_configuration_id]}_tree_metric_configurations") - Rails.cache.delete("#{params[:kalibro_configuration_id]}_hotspot_metric_configurations") - end end diff --git a/app/controllers/metric_configurations_controller.rb b/app/controllers/metric_configurations_controller.rb index 951d320..d80e48b 100644 --- a/app/controllers/metric_configurations_controller.rb +++ b/app/controllers/metric_configurations_controller.rb @@ -1,91 +1,29 @@ class MetricConfigurationsController < BaseMetricConfigurationsController - def choose_metric - @kalibro_configuration = KalibroConfiguration.find(params[:kalibro_configuration_id].to_i) - @metric_configuration_id = params[:metric_configuration_id].to_i - @metric_collectors_names = KalibroClient::Entities::Processor::MetricCollectorDetails.all_names - end + METRIC_TYPE = 'NativeMetricSnapshot' - def new - super - # FIXME: find_by_name throws an exception instead of returning nil, unlike ActiveRecord's API - metric_configuration.metric = KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(params[:metric_collector_name]).find_metric_by_code params[:metric_code] - @reading_groups = ReadingGroup.public_or_owned_by_user(current_user).map { |reading_group| - [reading_group.name, reading_group.id] - } - end + before_action :set_reading_groups!, only: [:new, :edit] - def create + def new super - @metric_configuration.metric = KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(params[:metric_collector_name]).find_metric_by_name params[:metric_name] - respond_to do |format| - create_and_redir(format) - end - clear_caches - end - - def edit - # FIXME: set the configuration id just once! - @kalibro_configuration_id = params[:kalibro_configuration_id] - @metric_configuration.kalibro_configuration_id = @kalibro_configuration_id - @reading_groups = ReadingGroup.public_or_owned_by_user(current_user).map { |reading_group| - [reading_group.name, reading_group.id] - } - end - - def update - respond_to do |format| - @metric_configuration.kalibro_configuration_id = params[:kalibro_configuration_id] - if @metric_configuration.update(metric_configuration_params) - format.html { redirect_to(kalibro_configuration_path(@metric_configuration.kalibro_configuration_id), notice: t('successfully_updated', :record => t(metric_configuration.class))) } - format.json { head :no_content } - clear_caches - else - failed_action(format, 'edit') - end - end + # The metric parameter comes from the choose metric form + set_metric! end - def destroy - @metric_configuration.destroy - respond_to do |format| - format.html { redirect_to kalibro_configuration_path(params[:kalibro_configuration_id]) } - format.json { head :no_content } - end - clear_caches + def choose_metric + @metric_collectors_names = KalibroClient::Entities::Processor::MetricCollectorDetails.all_names end protected - def metric_configuration - @metric_configuration - end - - def update_metric_configuration (new_metric_configuration) - @kalibro_configuration_id = params[:kalibro_configuration_id] - @metric_configuration = new_metric_configuration + def metric_configuration_params + params.require(:metric_configuration).permit(:reading_group_id, :weight, :aggregation_form) end private - def clear_caches - Rails.cache.delete("#{params[:kalibro_configuration_id]}_tree_metric_configurations") - Rails.cache.delete("#{params[:kalibro_configuration_id]}_hotspot_metric_configurations") - end - - # FIXME: Duplicated code on create and update actions extracted here - def failed_action(format, destiny_action) - @kalibro_configuration_id = params[:kalibro_configuration_id] - - format.html { render action: destiny_action } - format.json { render json: @metric_configuration.kalibro_errors, status: :unprocessable_entity } - end - - # Code extracted from create action - def create_and_redir(format) - if @metric_configuration.save - format.html { redirect_to kalibro_configuration_path(@metric_configuration.kalibro_configuration_id), notice: t('successfully_created', :record => t(metric_configuration.class)) } - else - failed_action(format, 'new') + def set_reading_groups! + @reading_groups = ReadingGroup.public_or_owned_by_user(current_user).map do |reading_group| + [reading_group.name, reading_group.id] end end end -- libgit2 0.21.2