diff --git a/app/controllers/base_metric_configurations_controller.rb b/app/controllers/base_metric_configurations_controller.rb index 77d18a2..6563b08 100644 --- a/app/controllers/base_metric_configurations_controller.rb +++ b/app/controllers/base_metric_configurations_controller.rb @@ -2,16 +2,16 @@ class BaseMetricConfigurationsController < ApplicationController include OwnershipAuthentication include MetricConfigurationsConcern - before_action :authenticate_user!, except: [:show, :index] + before_action :authenticate_user!, except: [:show] before_action :metric_configuration_owner?, only: [:edit, :update, :destroy] before_action :kalibro_configuration_owner?, only: [:new, :create, :choose_metric] 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] + before_action :set_reading_group!, only: [:show, :edit, :create, :update] def show - @reading_group = ReadingGroup.find(@metric_configuration.reading_group_id) @kalibro_ranges = @metric_configuration.kalibro_ranges end @@ -19,14 +19,16 @@ class BaseMetricConfigurationsController < ApplicationController @metric_configuration = MetricConfiguration.new end + def edit + end + def create - respond_to { |format| save_and_redir format } + 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 @@ -58,14 +60,28 @@ class BaseMetricConfigurationsController < ApplicationController end format.json { render json: @metric_configuration, status: new_record ? :created : :ok } else - failed_action(format, new_record ? :new : :edit) + failed_action(format) end end - # 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 } + def failed_action(format, error = nil) + errors = @metric_configuration.kalibro_errors + errors << error unless error.nil? + + flash[:notice] = errors.join(', ') + + format.json { render json: { errors: errors }, status: :unprocessable_entity } + render_failure_html(format) + end + + def render_failure_html(format) + if action_name == 'create' + format.html { render 'new' } + elsif action_name == 'update' + format.html { render 'edit' } + else + format.html { redirect_to kalibro_configuration_path(@kalibro_configuration.id) } + end end def clear_caches @@ -84,7 +100,6 @@ class BaseMetricConfigurationsController < ApplicationController 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! @@ -111,9 +126,19 @@ class BaseMetricConfigurationsController < ApplicationController 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 } + failed_action(format, 'Invalid combination of metric collector, name/code and type') + end + end + + def set_reading_group! + begin + @reading_group = ReadingGroup.find(@metric_configuration.reading_group_id) + rescue KalibroClient::Errors::RecordNotFound + respond_to do |format| + failed_action(format, 'Invalid reading group') + end end end end diff --git a/app/controllers/hotspot_metric_configurations_controller.rb b/app/controllers/hotspot_metric_configurations_controller.rb index 3e15e61..0bc3b90 100644 --- a/app/controllers/hotspot_metric_configurations_controller.rb +++ b/app/controllers/hotspot_metric_configurations_controller.rb @@ -1,15 +1,15 @@ class HotspotMetricConfigurationsController < BaseMetricConfigurationsController METRIC_TYPE = 'HotspotMetricSnapshot' + skip_before_action :set_reading_group! + def metric_configuration_params # 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 - # 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 } + def render_failure_html(format) + format.html { redirect_to kalibro_configuration_path(@kalibro_configuration.id) } end end -- libgit2 0.21.2