diff --git a/app/assets/stylesheets/general/_flashes.sass b/app/assets/stylesheets/general/_flashes.sass index e3086d02f..05c7529db 100644 --- a/app/assets/stylesheets/general/_flashes.sass +++ b/app/assets/stylesheets/general/_flashes.sass @@ -9,6 +9,10 @@ animation: flashesFadeIn 400ms 100ms ease-out forwards animation-delay: 0.4s +.flashes--error + background: $flashes--error--background + color: $flashes--error--text + .flashes__icon margin-right: 0.4em diff --git a/app/assets/stylesheets/general/_variables.sass b/app/assets/stylesheets/general/_variables.sass index 16a2e3a8a..df5839ffe 100644 --- a/app/assets/stylesheets/general/_variables.sass +++ b/app/assets/stylesheets/general/_variables.sass @@ -121,6 +121,9 @@ $alert--text--error: $white_pure !default $flashes--background: hsl(51, 87%, 94%) !default $flashes--text: hsl(40, 68%, 20%) !default +$flashes--error--background: hsl(11, 87%, 94%) !default +$flashes--error--text: hsl(0, 68%, 20%) !default + /* * Account header */ diff --git a/app/controllers/manage/messages_controller.rb b/app/controllers/manage/messages_controller.rb index 8ea3c7de2..ab4e0cf42 100644 --- a/app/controllers/manage/messages_controller.rb +++ b/app/controllers/manage/messages_controller.rb @@ -44,11 +44,11 @@ def destroy def deliver if @message.automated? - flash[:notice] = "Automated messages cannot be manually delivered. Only bulk messages can." + flash[:error] = "Automated messages cannot be manually delivered. Only bulk messages can." return redirect_to manage_message_path(@message) end if @message.status != "drafted" - flash[:notice] = "Message cannot be re-delivered" + flash[:error] = "Message cannot be re-delivered" return redirect_to manage_messages_path end @message.update_attribute(:queued_at, Time.now) @@ -96,7 +96,7 @@ def set_message def check_message_access return if @message.can_edit? - flash[:notice] = "Message can no longer be modified" + flash[:error] = "Message can no longer be modified" redirect_to manage_message_path(@message) end end diff --git a/app/controllers/manage/questionnaires_controller.rb b/app/controllers/manage/questionnaires_controller.rb index e97451778..6918459f4 100644 --- a/app/controllers/manage/questionnaires_controller.rb +++ b/app/controllers/manage/questionnaires_controller.rb @@ -38,7 +38,7 @@ def create if user.save @questionnaire.save else - flash[:notice] = user.errors.full_messages.join(", ") + flash[:error] = user.errors.full_messages.join(", ") if user.errors.include?(:email) @questionnaire.errors.add(:email, user.errors[:email].join(", ")) end @@ -70,7 +70,7 @@ def check_in @questionnaire.user.update_attributes(email: email) end unless @questionnaire.valid? - flash[:notice] = @questionnaire.errors.full_messages.join(", ") + flash[:error] = @questionnaire.errors.full_messages.join(", ") redirect_to show_redirect_path return end @@ -82,7 +82,7 @@ def check_in @questionnaire.update_attribute(:checked_in_by_id, current_user.id) flash[:notice] = "#{@questionnaire.full_name} no longer checked in." else - flash[:notice] = "No check-in action provided!" + flash[:error] = "No check-in action provided!" redirect_to show_redirect_path return end @@ -106,7 +106,7 @@ def destroy def update_acc_status new_status = params[:questionnaire][:acc_status] if new_status.blank? - flash[:notice] = "No status provided" + flash[:error] = "No status provided" redirect_to(manage_questionnaire_path(@questionnaire)) return end @@ -118,7 +118,7 @@ def update_acc_status if @questionnaire.save(validate: false) flash[:notice] = "Updated acceptance status to \"#{Questionnaire::POSSIBLE_ACC_STATUS[new_status]}\"" else - flash[:notice] = "Failed to update acceptance status" + flash[:error] = "Failed to update acceptance status" end redirect_to manage_questionnaire_path(@questionnaire) diff --git a/app/controllers/manage/schools_controller.rb b/app/controllers/manage/schools_controller.rb index 17fe5d4f0..b91344239 100644 --- a/app/controllers/manage/schools_controller.rb +++ b/app/controllers/manage/schools_controller.rb @@ -45,14 +45,14 @@ def merge def perform_merge new_school_name = params[:school][:id] if new_school_name.blank? - flash[:notice] = "Error: Must include the new school to merge into!" + flash[:error] = "Error: Must include the new school to merge into!" render :merge return end new_school = School.where(name: new_school_name).first if new_school.blank? - flash[:notice] = "Error: School doesn't exist: #{new_school_name}" + flash[:error] = "Error: School doesn't exist: #{new_school_name}" render :merge return end @@ -68,7 +68,7 @@ def perform_merge if @school.questionnaire_count < 1 @school.destroy else - flash[:notice] = "*** Old school NOT deleted: #{@school.questionnaire_count} questionnaires still associated!\n" + flash[:error] = "*** Old school NOT deleted: #{@school.questionnaire_count} questionnaires still associated!\n" end redirect_to manage_school_path(new_school) diff --git a/app/controllers/rsvps_controller.rb b/app/controllers/rsvps_controller.rb index 2a28f22c4..7c84b90f7 100644 --- a/app/controllers/rsvps_controller.rb +++ b/app/controllers/rsvps_controller.rb @@ -21,7 +21,7 @@ def accept flash[:notice] = "Thank you for confirming your attendance! You're all set to attend." flash[:notice] += " See below for additional bus information." if BusList.any? else - flash[:notice] = rsvp_error_notice + flash[:error] = rsvp_error_notice end redirect_to rsvp_path end @@ -31,10 +31,11 @@ def deny @questionnaire.acc_status = "rsvp_denied" @questionnaire.acc_status_author_id = current_user.id @questionnaire.acc_status_date = Time.now - unless @questionnaire.save - flash[:notice] = rsvp_error_notice + if @questionnaire.save + flash[:notice] = "Your RSVP has been updated." + else + flash[:error] = rsvp_error_notice end - flash[:notice] = "Your RSVP has been updated." if flash[:notice].blank? redirect_to rsvp_path end @@ -43,13 +44,13 @@ def deny # rubocop:disable PerceivedComplexity def update unless @questionnaire.update_attributes(params.require(:questionnaire).permit(:agreement_accepted, :phone)) - flash[:notice] = @questionnaire.errors.full_messages.join(", ") + flash[:error] = @questionnaire.errors.full_messages.join(", ") redirect_to rsvp_path return end unless ["rsvp_confirmed", "rsvp_denied"].include? params[:questionnaire][:acc_status] - flash[:notice] = "Please select a RSVP status." + flash[:error] = "Please select a RSVP status." redirect_to rsvp_path return end @@ -63,9 +64,9 @@ def update is_joining_bus = new_bus_list.present? && @questionnaire.bus_list != new_bus_list if is_joining_bus && new_bus_list.full? if @questionnaire.bus_list_id? - flash[:notice] = "Sorry, that bus is full. You are still signed up for the '#{@questionnaire.bus_list.name}' bus." + flash[:error] = "Sorry, that bus is full. You are still signed up for the '#{@questionnaire.bus_list.name}' bus." else - flash[:notice] = "Sorry, that bus is full. You may need to arrange other plans for transportation." + flash[:error] = "Sorry, that bus is full. You may need to arrange other plans for transportation." end else @questionnaire.bus_list = new_bus_list @@ -73,13 +74,15 @@ def update end unless @questionnaire.save - flash[:notice] = @questionnaire.errors.full_message.join(", ") + flash[:error] = @questionnaire.errors.full_message.join(", ") redirect_to rsvp_path return end - flash[:notice] = "Your RSVP has been updated." if flash[:notice].blank? - flash[:notice] += " See below for additional bus information!" if @questionnaire.bus_list_id? + if flash[:notice].blank? && flash[:error].blank? + flash[:notice] = "Your RSVP has been updated." + flash[:notice] += " See below for additional bus information!" if @questionnaire.bus_list_id? + end redirect_to rsvp_path end @@ -103,6 +106,7 @@ def check_user_has_questionnaire def require_accepted_questionnaire return if @questionnaire.can_rsvp? || @questionnaire.checked_in? + redirect_to new_questionnaires_path end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 1c420af87..ddf0acb40 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -11,7 +11,7 @@ def mlh end def failure - flash[:notice] = "External authentication failed - try again?" + flash[:error] = "External authentication failed - try again?" redirect_to new_user_session_url end end diff --git a/app/views/layouts/_flashes.html.haml b/app/views/layouts/_flashes.html.haml index 1630e5f19..70986fc13 100644 --- a/app/views/layouts/_flashes.html.haml +++ b/app/views/layouts/_flashes.html.haml @@ -1,3 +1,10 @@ +- if flash[:error].present? + .flashes.flashes--error + .container + .form-container + %span.fa.fa-info-circle.flashes__icon + = flash[:error].html_safe + - if flash[:notice].present? .flashes .container diff --git a/app/views/layouts/manage/_flashes.html.haml b/app/views/layouts/manage/_flashes.html.haml index c86e1fc48..f32b1f3e0 100644 --- a/app/views/layouts/manage/_flashes.html.haml +++ b/app/views/layouts/manage/_flashes.html.haml @@ -1,3 +1,11 @@ +- flash[:error] = 'This is a special error flash' +- flash[:notice] = 'This is a regular notice flash' + +- if flash[:error].present? + .alert.alert-danger.mt-3 + %span.fa.fa-exclamation-triangle.icon-space-r-half + = flash[:error].html_safe + - if flash[:notice].present? .alert.alert-info.mt-3 %span.fa.fa-info-circle.icon-space-r-half diff --git a/test/controllers/manage/messages_controller_test.rb b/test/controllers/manage/messages_controller_test.rb index 51244d767..6d2a88e3c 100644 --- a/test/controllers/manage/messages_controller_test.rb +++ b/test/controllers/manage/messages_controller_test.rb @@ -303,7 +303,7 @@ class Manage::MessagesControllerTest < ActionController::TestCase assert_difference('BulkMessageWorker.jobs.size', 0) do patch :deliver, params: { id: @message } end - assert_match /Automated/, flash[:notice] + assert_match /cannot be manually delivered/, flash[:error] assert_redirected_to manage_message_path(assigns(:message)) end @@ -311,14 +311,14 @@ class Manage::MessagesControllerTest < ActionController::TestCase patch :deliver, params: { id: @message } assert_match /queued/, flash[:notice] patch :deliver, params: { id: @message } - assert_match /cannot/, flash[:notice] + assert_match /cannot/, flash[:error] end should "not be able to modify message after delivery" do @message.update_attribute(:delivered_at, 1.minute.ago) old_message_name = @message.name patch :update, params: { id: @message, message: { name: "New Message Name" } } - assert_match /can no longer/, flash[:notice] + assert_match /can no longer/, flash[:error] assert_equal old_message_name, @message.reload.name end diff --git a/test/controllers/manage/questionnaires_controller_test.rb b/test/controllers/manage/questionnaires_controller_test.rb index 81c6f92de..7f111724d 100644 --- a/test/controllers/manage/questionnaires_controller_test.rb +++ b/test/controllers/manage/questionnaires_controller_test.rb @@ -375,7 +375,7 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase assert_equal false, @questionnaire.can_share_info assert_equal "", @questionnaire.phone assert_equal "old_email@example.com", @questionnaire.email - assert_match /No check-in action provided/, flash[:notice] + assert_match /No check-in action provided/, flash[:error] assert_response :redirect assert_redirected_to manage_questionnaire_path(@questionnaire) end diff --git a/test/controllers/rsvps_controller_test.rb b/test/controllers/rsvps_controller_test.rb index 1db0ec688..c5b99bdc4 100644 --- a/test/controllers/rsvps_controller_test.rb +++ b/test/controllers/rsvps_controller_test.rb @@ -109,7 +109,7 @@ class RsvpsControllerTest < ActionController::TestCase context "attempting #{status}" do should "include error message" do get status - assert_match /was an error/, flash[:notice] + assert_match /was an error/, flash[:error] end should "not change acceptance status" do @@ -120,7 +120,7 @@ class RsvpsControllerTest < ActionController::TestCase should "include hackathon name in notice" do HackathonConfig['name'] = 'Foo Bar' get status - assert_match /Foo Bar Agreement/, flash[:notice] + assert_match /Foo Bar Agreement/, flash[:error] end end end @@ -153,8 +153,8 @@ class RsvpsControllerTest < ActionController::TestCase assert_equal "rsvp_confirmed", @questionnaire.reload.acc_status assert_equal false, @questionnaire.reload.bus_list_id? assert_equal 0, bus_list.passengers.count - assert_match /full/, flash[:notice] - assert_no_match /still signed up/, flash[:notice] + assert_match /full/, flash[:error] + assert_no_match /still signed up/, flash[:error] assert_redirected_to rsvp_path end @@ -180,8 +180,8 @@ class RsvpsControllerTest < ActionController::TestCase assert_equal "rsvp_confirmed", @questionnaire.reload.acc_status assert_equal 0, bus_list2.passengers.count, 'passenger should not be assigned to bus that is full' assert_equal 1, bus_list1.passengers.count, 'passenger should stay on original bus' - assert_match /full/, flash[:notice] - assert_match /still signed up/, flash[:notice] + assert_match /full/, flash[:error] + assert_match /still signed up/, flash[:error] assert_redirected_to rsvp_path end @@ -189,10 +189,10 @@ class RsvpsControllerTest < ActionController::TestCase bus_list = create(:bus_list, capacity: 1) # Initial sign up patch :update, params: { questionnaire: { acc_status: "rsvp_confirmed", bus_list_id: bus_list.id } } - assert_no_match /full/, flash[:notice], 'should not complain about bus being full' + assert_no_match /full/, flash[:error], 'should not complain about bus being full' # Submit again patch :update, params: { questionnaire: { acc_status: "rsvp_confirmed", bus_list_id: bus_list.id } } - assert_no_match /full/, flash[:notice], 'should not complain about bus being full' + assert_no_match /full/, flash[:error], 'should not complain about bus being full' end should "not send email if updating info after confirming" do @@ -222,7 +222,7 @@ class RsvpsControllerTest < ActionController::TestCase @questionnaire.update_attribute(:phone, "1111111111") @questionnaire.update_attribute(:agreement_accepted, false) patch :update, params: { questionnaire: { phone: "1234567890" } } - assert_not_nil flash[:notice] + assert_not_nil flash[:error] assert_equal "1111111111", @questionnaire.reload.phone assert_redirected_to rsvp_path end @@ -231,7 +231,7 @@ class RsvpsControllerTest < ActionController::TestCase @questionnaire.update_attribute(:phone, "1111111111") @questionnaire.update_attribute(:first_name, "") patch :update, params: { questionnaire: { phone: "1234567890" } } - assert_not_nil flash[:notice] + assert_not_nil flash[:error] assert_equal "1111111111", @questionnaire.reload.phone assert_redirected_to rsvp_path end @@ -239,7 +239,7 @@ class RsvpsControllerTest < ActionController::TestCase should "not allow forbidden status update to questionnaire" do patch :update, params: { questionnaire: { acc_status: "pending" } } assert_equal "accepted", @questionnaire.reload.acc_status - assert_match /select a RSVP status/, flash[:notice] + assert_match /select a RSVP status/, flash[:error] assert_redirected_to rsvp_path end end