From a22a5af711fb11779aa2a03153c91e2d8f5fd614 Mon Sep 17 00:00:00 2001 From: Hauke Klement Date: Thu, 5 Feb 2015 12:28:09 +0100 Subject: [PATCH] extracted common controller behavior in order to reduce code duplication --- .../code_ocean/files_controller.rb | 17 ++----- app/controllers/concerns/common_behavior.rb | 51 +++++++++++++++++++ app/controllers/consumers_controller.rb | 28 ++-------- .../execution_environments_controller.rb | 28 ++-------- app/controllers/exercises_controller.rb | 27 ++-------- app/controllers/file_types_controller.rb | 28 ++-------- app/controllers/hints_controller.rb | 28 ++-------- app/controllers/internal_users_controller.rb | 29 ++--------- app/controllers/sessions_controller.rb | 2 +- app/controllers/submissions_controller.rb | 11 +--- app/controllers/teams_controller.rb | 28 ++-------- .../internal_users_controller_spec.rb | 5 ++ 12 files changed, 96 insertions(+), 186 deletions(-) create mode 100644 app/controllers/concerns/common_behavior.rb diff --git a/app/controllers/code_ocean/files_controller.rb b/app/controllers/code_ocean/files_controller.rb index 69dd2652..c41712bb 100644 --- a/app/controllers/code_ocean/files_controller.rb +++ b/app/controllers/code_ocean/files_controller.rb @@ -1,5 +1,6 @@ module CodeOcean class FilesController < ApplicationController + include CommonBehavior include FileParameters def authorize! @@ -10,25 +11,13 @@ module CodeOcean def create @file = CodeOcean::File.new(file_params) authorize! - respond_to do |format| - if @file.save - format.html { redirect_to(implement_exercise_path(@file.context.exercise, tab: 2), notice: t('shared.object_created', model: File.model_name.human)) } - format.json { render(:show, location: @file, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @file.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @file, path: implement_exercise_path(@file.context.exercise, tab: 2)) end def destroy @file = CodeOcean::File.find(params[:id]) authorize! - @file.destroy - respond_to do |format| - format.html { redirect_to(@file.context, notice: t('shared.object_destroyed', model: File.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @file, path: @file.context) end def file_params diff --git a/app/controllers/concerns/common_behavior.rb b/app/controllers/concerns/common_behavior.rb new file mode 100644 index 00000000..e84fb7c9 --- /dev/null +++ b/app/controllers/concerns/common_behavior.rb @@ -0,0 +1,51 @@ +module CommonBehavior + def create_and_respond(options = {}, &block) + @object = options[:object] + respond_to do |format| + if @object.save + yield if block_given? + path = options[:path].try(:call) || @object + respond_with_valid_object(format, notice: t('shared.object_created', model: @object.class.model_name.human), path: path, status: :created) + else + respond_with_invalid_object(format, template: :new) + end + end + end + private :create_and_respond + + def destroy_and_respond(options = {}) + @object = options[:object] + @object.destroy + respond_to do |format| + path = options[:path] || send(:"#{@object.class.model_name.collection}_path") + format.html { redirect_to(path, notice: t('shared.object_destroyed', model: @object.class.model_name.human)) } + format.json { head(:no_content) } + end + end + private :destroy_and_respond + + def respond_with_invalid_object(format, options = {}) + format.html { render(options[:template]) } + format.json { render(json: @object.errors, status: :unprocessable_entity) } + end + private :respond_with_invalid_object + + def respond_with_valid_object(format, options = {}) + format.html { redirect_to(options[:path], notice: options[:notice]) } + format.json { render(:show, location: @object, status: options[:status]) } + end + private :respond_with_valid_object + + def update_and_respond(options = {}) + @object = options[:object] + respond_to do |format| + if @object.update(options[:params]) + path = options[:path] || @object + respond_with_valid_object(format, notice: t('shared.object_updated', model: @object.class.model_name.human), path: path, status: :ok) + else + respond_with_invalid_object(format, template: :edit) + end + end + end + private :update_and_respond +end diff --git a/app/controllers/consumers_controller.rb b/app/controllers/consumers_controller.rb index 183fe435..e22d4b3e 100644 --- a/app/controllers/consumers_controller.rb +++ b/app/controllers/consumers_controller.rb @@ -1,4 +1,6 @@ class ConsumersController < ApplicationController + include CommonBehavior + before_action :set_consumer, only: MEMBER_ACTIONS def authorize! @@ -9,23 +11,11 @@ class ConsumersController < ApplicationController def create @consumer = Consumer.new(consumer_params) authorize! - respond_to do |format| - if @consumer.save - format.html { redirect_to(@consumer, notice: t('shared.object_created', model: Consumer.model_name.human)) } - format.json { render(:show, location: @consumer, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @consumer.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @consumer) end def destroy - @consumer.destroy - respond_to do |format| - format.html { redirect_to(consumers_url, notice: t('shared.object_destroyed', model: Consumer.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @consumer) end def edit @@ -56,14 +46,6 @@ class ConsumersController < ApplicationController end def update - respond_to do |format| - if @consumer.update(consumer_params) - format.html { redirect_to(@consumer, notice: t('shared.object_updated', model: Consumer.model_name.human)) } - format.json { render(:show, location: @consumer, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @consumer.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @consumer, params: consumer_params) end end diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 17e32a4e..8cc58670 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -1,4 +1,6 @@ class ExecutionEnvironmentsController < ApplicationController + include CommonBehavior + before_action :set_docker_images, only: [:create, :edit, :new, :update] before_action :set_execution_environment, only: MEMBER_ACTIONS + [:execute_command, :shell] before_action :set_testing_framework_adapters, only: [:create, :edit, :new, :update] @@ -11,23 +13,11 @@ class ExecutionEnvironmentsController < ApplicationController def create @execution_environment = ExecutionEnvironment.new(execution_environment_params) authorize! - respond_to do |format| - if @execution_environment.save - format.html { redirect_to(@execution_environment, notice: t('shared.object_created', model: ExecutionEnvironment.model_name.human)) } - format.json { render(:show, location: @execution_environment, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @execution_environment.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @execution_environment) end def destroy - @execution_environment.destroy - respond_to do |format| - format.html { redirect_to(execution_environments_url, notice: t('shared.object_destroyed', model: ExecutionEnvironment.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @execution_environment) end def edit @@ -85,14 +75,6 @@ class ExecutionEnvironmentsController < ApplicationController end def update - respond_to do |format| - if @execution_environment.update(execution_environment_params) - format.html { redirect_to(@execution_environment, notice: t('shared.object_updated', model: ExecutionEnvironment.model_name.human)) } - format.json { render(:show, location: @execution_environment, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @execution_environment.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @execution_environment, params: execution_environment_params) end end diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 8bf43a03..c717a1b1 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -1,4 +1,5 @@ class ExercisesController < ApplicationController + include CommonBehavior include Lti include SubmissionParameters include SubmissionScoring @@ -27,23 +28,11 @@ class ExercisesController < ApplicationController def create @exercise = Exercise.new(exercise_params) authorize! - respond_to do |format| - if @exercise.save - format.html { redirect_to(@exercise, notice: t('shared.object_created', model: Exercise.model_name.human)) } - format.json { render(:show, location: @exercise, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @exercise.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @exercise) end def destroy - @exercise.destroy - respond_to do |format| - format.html { redirect_to(exercises_url, notice: t('shared.object_destroyed', model: Exercise.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @exercise) end def edit @@ -145,14 +134,6 @@ class ExercisesController < ApplicationController end def update - respond_to do |format| - if @exercise.update(exercise_params) - format.html { redirect_to(@exercise, notice: t('shared.object_updated', model: Exercise.model_name.human)) } - format.json { render(:show, location: @exercise, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @exercise.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @exercise, params: exercise_params) end end diff --git a/app/controllers/file_types_controller.rb b/app/controllers/file_types_controller.rb index 8eb01568..d15de715 100644 --- a/app/controllers/file_types_controller.rb +++ b/app/controllers/file_types_controller.rb @@ -1,4 +1,6 @@ class FileTypesController < ApplicationController + include CommonBehavior + before_action :set_editor_modes, only: [:create, :edit, :new, :update] before_action :set_file_type, only: MEMBER_ACTIONS @@ -10,23 +12,11 @@ class FileTypesController < ApplicationController def create @file_type = FileType.new(file_type_params) authorize! - respond_to do |format| - if @file_type.save - format.html { redirect_to(@file_type, notice: t('shared.object_created', model: FileType.model_name.human)) } - format.json { render(:show, location: @file_type, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @file_type.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @file_type) end def destroy - @file_type.destroy - respond_to do |format| - format.html { redirect_to(file_types_url, notice: t('shared.object_destroyed', model: FileType.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @file_type) end def edit @@ -65,14 +55,6 @@ class FileTypesController < ApplicationController end def update - respond_to do |format| - if @file_type.update(file_type_params) - format.html { redirect_to(@file_type, notice: t('shared.object_updated', model: FileType.model_name.human)) } - format.json { render(:show, location: @file_type, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @file_type.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @file_type, params: file_type_params) end end diff --git a/app/controllers/hints_controller.rb b/app/controllers/hints_controller.rb index 7933a57f..90182b0d 100644 --- a/app/controllers/hints_controller.rb +++ b/app/controllers/hints_controller.rb @@ -1,4 +1,6 @@ class HintsController < ApplicationController + include CommonBehavior + before_action :set_execution_environment before_action :set_hint, only: MEMBER_ACTIONS @@ -10,23 +12,11 @@ class HintsController < ApplicationController def create @hint = Hint.new(hint_params) authorize! - respond_to do |format| - if @hint.save - format.html { redirect_to(execution_environment_hint_path(@execution_environment, @hint.id), notice: t('shared.object_created', model: Hint.model_name.human)) } - format.json { render(:show, location: @hint, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @hint.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @hint, path: Proc.new { execution_environment_hint_path(@execution_environment, @hint) }) end def destroy - @hint.destroy - respond_to do |format| - format.html { redirect_to(execution_environment_hints_path(@execution_environment), notice: t('shared.object_destroyed', model: Hint.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @hint, path: execution_environment_hints_path(@execution_environment)) end def edit @@ -62,14 +52,6 @@ class HintsController < ApplicationController end def update - respond_to do |format| - if @hint.update(hint_params) - format.html { redirect_to(execution_environment_hint_path(params[:execution_environment_id], @hint.id), notice: t('shared.object_updated', model: Hint.model_name.human)) } - format.json { render(:show, location: @hint, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @hint.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @hint, params: hint_params, path: execution_environment_hint_path(@execution_environment, @hint)) end end diff --git a/app/controllers/internal_users_controller.rb b/app/controllers/internal_users_controller.rb index 44ac7ae0..7e53dec0 100644 --- a/app/controllers/internal_users_controller.rb +++ b/app/controllers/internal_users_controller.rb @@ -1,4 +1,6 @@ class InternalUsersController < ApplicationController + include CommonBehavior + before_action :require_activation_token, only: :activate before_action :require_reset_password_token, only: :reset_password before_action :set_user, only: MEMBER_ACTIONS @@ -29,24 +31,11 @@ class InternalUsersController < ApplicationController @user = InternalUser.new(internal_user_params) authorize! @user.send(:setup_activation) - respond_to do |format| - if @user.save - @user.send(:send_activation_needed_email!) - format.html { redirect_to(@user, notice: t('shared.object_created', model: InternalUser.model_name.human)) } - format.json { render(:show, location: @user, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @user.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @user) { @user.send(:send_activation_needed_email!) } end def destroy - @user.destroy - respond_to do |format| - format.html { redirect_to(internal_users_url, notice: t('shared.object_destroyed', model: InternalUser.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @user) end def edit @@ -118,14 +107,6 @@ class InternalUsersController < ApplicationController end def update - respond_to do |format| - if @user.update(internal_user_params) - format.html { redirect_to(@user, notice: t('shared.object_updated', model: InternalUser.model_name.human)) } - format.json { render(:show, location: @user, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @user.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @user, params: internal_user_params) end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6f46ee06..6bf9ce4b 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -22,7 +22,7 @@ class SessionsController < ApplicationController store_lti_session_data(consumer: @consumer, parameters: params) store_nonce(params[:oauth_nonce]) flash[:notice] = I18n.t("sessions.create_through_lti.session_#{lti_outcome_service? ? 'with' : 'without'}_outcome", consumer: @consumer) - redirect_to(implement_exercise_path(@exercise.id)) + redirect_to(implement_exercise_path(@exercise)) end def destroy diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 80658a9e..df8b8696 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -1,5 +1,6 @@ class SubmissionsController < ApplicationController include ActionController::Live + include CommonBehavior include Lti include SubmissionParameters include SubmissionScoring @@ -20,15 +21,7 @@ class SubmissionsController < ApplicationController def create @submission = Submission.new(submission_params) authorize! - respond_to do |format| - format.json do - if @submission.save - render(:show, location: @submission, status: :created) - else - render(nothing: true, status: :unprocessable_entity) - end - end - end + create_and_respond(object: @submission) end def download_file diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index f94ca9dc..167f5899 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -1,4 +1,6 @@ class TeamsController < ApplicationController + include CommonBehavior + before_action :set_team, only: MEMBER_ACTIONS def authorize! @@ -9,23 +11,11 @@ class TeamsController < ApplicationController def create @team = Team.new(team_params) authorize! - respond_to do |format| - if @team.save - format.html { redirect_to(team_path(@team.id), notice: t('shared.object_created', model: Team.model_name.human)) } - format.json { render(:show, location: @team, status: :created) } - else - format.html { render(:new) } - format.json { render(json: @team.errors, status: :unprocessable_entity) } - end - end + create_and_respond(object: @team) end def destroy - @team.destroy - respond_to do |format| - format.html { redirect_to(teams_path, notice: t('shared.object_destroyed', model: Team.model_name.human)) } - format.json { head(:no_content) } - end + destroy_and_respond(object: @team) end def edit @@ -56,14 +46,6 @@ class TeamsController < ApplicationController private :team_params def update - respond_to do |format| - if @team.update(team_params) - format.html { redirect_to(team_path(@team.id), notice: t('shared.object_updated', model: Team.model_name.human)) } - format.json { render(:show, location: @team, status: :ok) } - else - format.html { render(:edit) } - format.json { render(json: @team.errors, status: :unprocessable_entity) } - end - end + update_and_respond(object: @team, params: team_params) end end diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index de562c94..ed0cc0c3 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -120,6 +120,11 @@ describe InternalUsersController do expect(InternalUser.last.activation_token).to be_present end + it 'sends an activation email' do + expect_any_instance_of(InternalUser).to receive(:send_activation_needed_email!) + request.call + end + expect_redirect end