From 4eef3d70d58a6cf6a1dddb9cd8b1c9fdb5eb1363 Mon Sep 17 00:00:00 2001 From: Hauke Klement Date: Mon, 23 Mar 2015 16:42:57 +0100 Subject: [PATCH] minor refactoring of flash messages --- app/controllers/application_controller.rb | 3 +-- app/controllers/internal_users_controller.rb | 8 ++----- app/controllers/sessions_controller.rb | 8 ++----- app/helpers/application_helper.rb | 7 +----- app/views/application/_breadcrumbs.html.slim | 2 +- config/initializers/monkey_patches.rb | 6 +++++ .../application_controller_spec.rb | 15 ++++-------- .../execution_environments_controller_spec.rb | 5 +--- spec/controllers/exercises_controller_spec.rb | 3 +-- .../internal_users_controller_spec.rb | 1 + spec/controllers/sessions_controller_spec.rb | 13 +++++----- spec/support/controllers.rb | 24 ++++++++++++++----- 12 files changed, 46 insertions(+), 49 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c7533db8..617bab02 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,8 +17,7 @@ class ApplicationController < ActionController::Base end def render_not_authorized - flash[:danger] = t('application.not_authorized') - redirect_to(:root) + redirect_to(:root, alert: t('application.not_authorized')) end private :render_not_authorized diff --git a/app/controllers/internal_users_controller.rb b/app/controllers/internal_users_controller.rb index d2153a7c..b9efb276 100644 --- a/app/controllers/internal_users_controller.rb +++ b/app/controllers/internal_users_controller.rb @@ -39,8 +39,7 @@ class InternalUsersController < ApplicationController def deliver_reset_password_instructions if params[:email].present? InternalUser.find_by(email: params[:email]).try(:deliver_reset_password_instructions!) - flash[:notice] = t('.success') - redirect_to(:root) + redirect_to(:root, notice: t('.success')) end end private :deliver_reset_password_instructions @@ -77,10 +76,7 @@ class InternalUsersController < ApplicationController end def render_forgot_password_form - if current_user - flash[:warning] = t('shared.already_signed_in') - redirect_to(:root) - end + redirect_to(:root, alert: t('shared.already_signed_in')) if current_user end private :render_forgot_password_form diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f605d782..ce1418bb 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -21,8 +21,7 @@ class SessionsController < ApplicationController set_current_user 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)) + redirect_to(implement_exercise_path(@exercise), notice: t("sessions.create_through_lti.session_#{lti_outcome_service? ? 'with' : 'without'}_outcome", consumer: @consumer)) end def destroy @@ -41,9 +40,6 @@ class SessionsController < ApplicationController end def new - if current_user - flash[:warning] = t('shared.already_signed_in') - redirect_to(:root) - end + redirect_to(:root, alert: t('shared.already_signed_in')) if current_user end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 122967ac..f11da84b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -22,7 +22,7 @@ module ApplicationHelper def label_column(label) content_tag(:div, class: 'col-sm-3') do content_tag(:strong) do - translation_present?("activerecord.attributes.#{label}") ? t("activerecord.attributes.#{label}") : t(label) + I18n.translation_present?("activerecord.attributes.#{label}") ? t("activerecord.attributes.#{label}") : t(label) end end end @@ -60,11 +60,6 @@ module ApplicationHelper end end - def translation_present?(key) - I18n.t(key, default: '').present? - end - private :translation_present? - def value_column(value) content_tag(:div, class: 'col-sm-9') do block_given? ? yield : symbol_for(value) diff --git a/app/views/application/_breadcrumbs.html.slim b/app/views/application/_breadcrumbs.html.slim index 2a1f33a8..216dcc7f 100644 --- a/app/views/application/_breadcrumbs.html.slim +++ b/app/views/application/_breadcrumbs.html.slim @@ -11,7 +11,7 @@ - if object li = link_to(object, send(:"#{model.model_name.singular}_path", object)) li.active - - if translation_present?("shared.#{params[:action]}") + - if I18n.translation_present?("shared.#{params[:action]}") = t("shared.#{params[:action]}") - else = t("#{controller_name}.index.#{params[:action]}") diff --git a/config/initializers/monkey_patches.rb b/config/initializers/monkey_patches.rb index b03b9668..a910b23a 100644 --- a/config/initializers/monkey_patches.rb +++ b/config/initializers/monkey_patches.rb @@ -13,3 +13,9 @@ unless Array.respond_to?(:to_h) end end end + +module I18n + def self.translation_present?(key) + t(key, default: '').present? + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 695eb74c..0ad4b20d 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -22,18 +22,13 @@ describe ApplicationController do end describe '#render_not_authorized' do - let(:render_not_authorized) { controller.send(:render_not_authorized) } - - it 'displays a flash message' do - expect(controller).to receive(:redirect_to) - render_not_authorized - expect(flash[:danger]).to eq(I18n.t('application.not_authorized')) + before(:each) do + expect(controller).to receive(:welcome) { controller.send(:render_not_authorized) } + get :welcome end - it 'redirects to the root URL' do - expect(controller).to receive(:redirect_to).with(:root) - render_not_authorized - end + expect_flash_message(:alert, I18n.t('application.not_authorized')) + expect_redirect(:root) end describe '#set_locale' do diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 8166ccbb..331c3ec7 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -118,10 +118,7 @@ describe ExecutionEnvironmentsController do end expect_assigns(docker_images: Array) - - it 'displays a flash message' do - expect(flash[:warning]).to eq(error_message) - end + expect_flash_message(:warning, :error_message) end end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index b311eb55..6082004e 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -46,8 +46,7 @@ describe ExercisesController do end expect_assigns(exercise: Exercise) - - expect_flash_message(:danger) + expect_flash_message(:danger, :'shared.message_failure') expect_redirect(:exercise) end end diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index ed7647da..e0efb339 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -180,6 +180,7 @@ describe InternalUsersController do get :forgot_password end + expect_flash_message(:alert, :'shared.already_signed_in') expect_redirect(:root) end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index d8c12eb5..aa76dcca 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -103,25 +103,25 @@ describe SessionsController do end context 'when LTI outcomes are supported' do + let(:message) { I18n.t('sessions.create_through_lti.session_with_outcome', consumer: consumer) } + before(:each) do expect(controller).to receive(:lti_outcome_service?).and_return(true) request end - it 'displays a flash message' do - expect(flash[:notice]).to eq(I18n.t('sessions.create_through_lti.session_with_outcome', consumer: consumer)) - end + expect_flash_message(:notice, :message) end context 'when LTI outcomes are not supported' do + let(:message) { I18n.t('sessions.create_through_lti.session_without_outcome', consumer: consumer) } + before(:each) do expect(controller).to receive(:lti_outcome_service?).and_return(false) request end - it 'displays a flash message' do - expect(flash[:notice]).to eq(I18n.t('sessions.create_through_lti.session_without_outcome', consumer: consumer)) - end + expect_flash_message(:notice, :message) end it 'redirects to the requested exercise' do @@ -206,6 +206,7 @@ describe SessionsController do get :new end + expect_flash_message(:alert, :'shared.already_signed_in') expect_redirect(:root) end end diff --git a/spec/support/controllers.rb b/spec/support/controllers.rb index 75444fea..d075216a 100644 --- a/spec/support/controllers.rb +++ b/spec/support/controllers.rb @@ -24,7 +24,7 @@ end def expect_flash_message(type, message = nil) it 'displays a flash message' do if message - expect(flash[type]).to eq(message.is_a?(String) ? message : I18n.t(message)) + expect(flash[type]).to eq(obtain_message(message)) else expect(flash[type]).to be_present end @@ -59,13 +59,25 @@ def expect_template(template) end end -def obtain_object(value) - case value +def obtain_object(object) + case object when Proc - value.call + object.call when Symbol - send(value) + send(object) else - value + object end end +private :obtain_object + +def obtain_message(object) + if object.is_a?(String) + object + elsif I18n.translation_present?(object) + I18n.t(object) + else + send(object) + end +end +private :obtain_message