Skip to content

Commit

Permalink
[refactor] Standardize on flash[:alert] instead of flash[:error]
Browse files Browse the repository at this point in the history
Notice & alert are the two standard ones: https://api.rubyonrails.org/classes/ActionDispatch/Flash.html
  • Loading branch information
sman591 committed May 21, 2019
1 parent 978d6d3 commit 6231f6b
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 45 deletions.
2 changes: 1 addition & 1 deletion app/controllers/manage/bus_lists_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def toggle_bus_captain

def send_update_email
if Message.for_trigger("bus_list.notes_update").empty?
flash[:error] = 'Error: No automated message is configured for bus note updates!'
flash[:alert] = 'Error: No automated message is configured for bus note updates!'
redirect_to [:manage, @bus_list]
return
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/manage/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ def destroy

def deliver
if @message.automated?
flash[:error] = "Automated messages cannot be manually delivered. Only bulk messages can."
flash[:alert] = "Automated messages cannot be manually delivered. Only bulk messages can."
return redirect_to manage_message_path(@message)
end
if @message.status != "drafted"
flash[:error] = "Message cannot be re-delivered"
flash[:alert] = "Message cannot be re-delivered"
return redirect_to manage_messages_path
end
@message.update_attribute(:queued_at, Time.now)
Expand Down Expand Up @@ -96,7 +96,7 @@ def set_message
def check_message_access
return if @message.can_edit?

flash[:error] = "Message can no longer be modified"
flash[:alert] = "Message can no longer be modified"
redirect_to manage_message_path(@message)
end
end
10 changes: 5 additions & 5 deletions app/controllers/manage/questionnaires_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def create
if user.save
@questionnaire.save
else
flash[:error] = user.errors.full_messages.join(", ")
flash[:alert] = user.errors.full_messages.join(", ")
if user.errors.include?(:email)
@questionnaire.errors.add(:email, user.errors[:email].join(", "))
end
Expand Down Expand Up @@ -70,7 +70,7 @@ def check_in
@questionnaire.user.update_attributes(email: email)
end
unless @questionnaire.valid?
flash[:error] = @questionnaire.errors.full_messages.join(", ")
flash[:alert] = @questionnaire.errors.full_messages.join(", ")
redirect_to show_redirect_path
return
end
Expand All @@ -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[:error] = "No check-in action provided!"
flash[:alert] = "No check-in action provided!"
redirect_to show_redirect_path
return
end
Expand All @@ -106,7 +106,7 @@ def destroy
def update_acc_status
new_status = params[:questionnaire][:acc_status]
if new_status.blank?
flash[:error] = "No status provided"
flash[:alert] = "No status provided"
redirect_to(manage_questionnaire_path(@questionnaire))
return
end
Expand All @@ -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[:error] = "Failed to update acceptance status"
flash[:alert] = "Failed to update acceptance status"
end

redirect_to manage_questionnaire_path(@questionnaire)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/manage/schools_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ def merge
def perform_merge
new_school_name = params[:school][:id]
if new_school_name.blank?
flash[:error] = "Error: Must include the new school to merge into!"
flash[:alert] = "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[:error] = "Error: School doesn't exist: #{new_school_name}"
flash[:alert] = "Error: School doesn't exist: #{new_school_name}"
render :merge
return
end
Expand All @@ -68,7 +68,7 @@ def perform_merge
if @school.questionnaire_count < 1
@school.destroy
else
flash[:error] = "*** Old school NOT deleted: #{@school.questionnaire_count} questionnaires still associated!\n"
flash[:alert] = "*** Old school NOT deleted: #{@school.questionnaire_count} questionnaires still associated!\n"
end

redirect_to manage_school_path(new_school)
Expand Down
16 changes: 8 additions & 8 deletions app/controllers/rsvps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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[:error] = rsvp_error_notice
flash[:alert] = rsvp_error_notice
end
redirect_to rsvp_path
end
Expand All @@ -34,7 +34,7 @@ def deny
if @questionnaire.save
flash[:notice] = "Your RSVP has been updated."
else
flash[:error] = rsvp_error_notice
flash[:alert] = rsvp_error_notice
end
redirect_to rsvp_path
end
Expand All @@ -44,13 +44,13 @@ def deny
# rubocop:disable PerceivedComplexity
def update
unless @questionnaire.update_attributes(params.require(:questionnaire).permit(:agreement_accepted, :phone))
flash[:error] = @questionnaire.errors.full_messages.join(", ")
flash[:alert] = @questionnaire.errors.full_messages.join(", ")
redirect_to rsvp_path
return
end

unless ["rsvp_confirmed", "rsvp_denied"].include? params[:questionnaire][:acc_status]
flash[:error] = "Please select a RSVP status."
flash[:alert] = "Please select a RSVP status."
redirect_to rsvp_path
return
end
Expand All @@ -64,22 +64,22 @@ 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[:error] = "Sorry, that bus is full. You are still signed up for the '#{@questionnaire.bus_list.name}' bus."
flash[:alert] = "Sorry, that bus is full. You are still signed up for the '#{@questionnaire.bus_list.name}' bus."
else
flash[:error] = "Sorry, that bus is full. You may need to arrange other plans for transportation."
flash[:alert] = "Sorry, that bus is full. You may need to arrange other plans for transportation."
end
else
@questionnaire.bus_list = new_bus_list
@questionnaire.bus_captain_interest = params[:questionnaire][:bus_captain_interest]
end

unless @questionnaire.save
flash[:error] = @questionnaire.errors.full_message.join(", ")
flash[:alert] = @questionnaire.errors.full_message.join(", ")
redirect_to rsvp_path
return
end

if flash[:notice].blank? && flash[:error].blank?
if flash[:notice].blank? && flash[:alert].blank?
flash[:notice] = "Your RSVP has been updated."
flash[:notice] += " See below for additional bus information!" if @questionnaire.bus_list_id?
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def mlh
end

def failure
flash[:error] = "External authentication failed - try again?"
flash[:alert] = "External authentication failed - try again?"
redirect_to new_user_session_url
end
end
7 changes: 0 additions & 7 deletions app/views/layouts/_flashes.html.haml
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
- if flash[:error].present?
.flashes.flashes--error
.container
.form-container
%span.fa.fa-info-circle.flashes__icon
= flash[:error].html_safe

- if flash[:alert].present?
.flashes.flashes--error
.container
Expand Down
4 changes: 2 additions & 2 deletions app/views/layouts/manage/_flashes.html.haml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
- if flash[:error].present?
- if flash[:alert].present?
.alert.alert-danger.mt-3
%span.fa.fa-exclamation-triangle.icon-space-r-half
= flash[:error].html_safe
= flash[:alert].html_safe

- if flash[:notice].present?
.alert.alert-info.mt-3
Expand Down
6 changes: 3 additions & 3 deletions test/controllers/manage/messages_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,22 +305,22 @@ class Manage::MessagesControllerTest < ActionController::TestCase
assert_difference("enqueued_jobs.size", 0) do
patch :deliver, params: { id: @message }
end
assert_match /cannot be manually delivered/, flash[:error]
assert_match /cannot be manually delivered/, flash[:alert]
assert_redirected_to manage_message_path(assigns(:message))
end

should "not allow multiple deliveries" do
patch :deliver, params: { id: @message }
assert_match /queued/, flash[:notice]
patch :deliver, params: { id: @message }
assert_match /cannot/, flash[:error]
assert_match /cannot/, flash[:alert]
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[:error]
assert_match /can no longer/, flash[:alert]
assert_equal old_message_name, @message.reload.name
end

Expand Down
2 changes: 1 addition & 1 deletion test/controllers/manage/questionnaires_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,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[:error]
assert_match /No check-in action provided/, flash[:alert]
assert_response :redirect
assert_redirected_to manage_questionnaire_path(@questionnaire)
end
Expand Down
22 changes: 11 additions & 11 deletions test/controllers/rsvps_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class RsvpsControllerTest < ActionController::TestCase
context "attempting #{status}" do
should "include error message" do
get status
assert_match /was an error/, flash[:error]
assert_match /was an error/, flash[:alert]
end

should "not change acceptance status" do
Expand All @@ -121,7 +121,7 @@ class RsvpsControllerTest < ActionController::TestCase
should "include hackathon name in notice" do
HackathonConfig["name"] = "Foo Bar"
get status
assert_match /Foo Bar Agreement/, flash[:error]
assert_match /Foo Bar Agreement/, flash[:alert]
end
end
end
Expand Down Expand Up @@ -154,8 +154,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[:error]
assert_no_match /still signed up/, flash[:error]
assert_match /full/, flash[:alert]
assert_no_match /still signed up/, flash[:alert]
assert_redirected_to rsvp_path
end

Expand All @@ -181,19 +181,19 @@ 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[:error]
assert_match /still signed up/, flash[:error]
assert_match /full/, flash[:alert]
assert_match /still signed up/, flash[:alert]
assert_redirected_to rsvp_path
end

should "not error if submitting while already on a full bus" do
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[:error], "should not complain about bus being full"
assert_no_match /full/, flash[:alert], "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[:error], "should not complain about bus being full"
assert_no_match /full/, flash[:alert], "should not complain about bus being full"
end

should "not send email if updating info after confirming" do
Expand Down Expand Up @@ -223,7 +223,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[:error]
assert_not_nil flash[:alert]
assert_equal "1111111111", @questionnaire.reload.phone
assert_redirected_to rsvp_path
end
Expand All @@ -232,15 +232,15 @@ class RsvpsControllerTest < ActionController::TestCase
@questionnaire.update_attribute(:phone, "1111111111")
@questionnaire.update_attribute(:first_name, "")
patch :update, params: { questionnaire: { phone: "1234567890" } }
assert_not_nil flash[:error]
assert_not_nil flash[:alert]
assert_equal "1111111111", @questionnaire.reload.phone
assert_redirected_to rsvp_path
end

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[:error]
assert_match /select a RSVP status/, flash[:alert]
assert_redirected_to rsvp_path
end
end
Expand Down

0 comments on commit 6231f6b

Please sign in to comment.