diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index 0ee2bcc1..8fb28361 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -18,7 +18,7 @@ def update flash[:success] = "Updated #{@email_template.title} template successfully." redirect_to email_templates_path else - flash.now[:alert] = "Failed to save #{@email_template.title} template: #{@email_template.errors.full_messages.join(", ")}" + flash.now[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(', ')}" render "edit" end end @@ -36,7 +36,7 @@ def create flash[:success] = "Created #{@email_template.title} successfully." redirect_to email_templates_path else - flash.now[:alert] = "Failed to submit information: #{@email_template.errors.full_messages.join(", ")}" + flash[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(', ')}" render "new" end end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 0a913198..e82d6109 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -25,7 +25,7 @@ def create flash[:success] = "Created #{@page.title} page successfully." redirect_to action: "show", url_slug: @page.url_slug else - flash.now[:alert] = "An error occurred! #{@page.errors.full_messages}" + flash.now[:alert] = "An error occurred: #{@page.errors.full_messages.join(", ")}" render "edit" end end @@ -58,7 +58,7 @@ def update flash[:success] = "Updated #{@page.title} page successfully." redirect_to pages_path else - flash.now[:alert] = "An error occurred! #{@page.errors.full_messages.to_sentence}" + flash.now[:alert] = "An error occurred: #{@page.errors.full_messages.join(', ')}" render "edit" end end diff --git a/app/controllers/schools_controller.rb b/app/controllers/schools_controller.rb index 0ea311f1..500f6ca0 100644 --- a/app/controllers/schools_controller.rb +++ b/app/controllers/schools_controller.rb @@ -27,7 +27,7 @@ def create flash[:success] = "Created #{@school.name} successfully." redirect_to schools_path else - flash[:alert] = "Failed to submit information :(" + flash[:alert] = "An error occurred: #{@school.errors.full_messages.join(', ')}" render "new" end end @@ -45,10 +45,11 @@ def update @school = School.find(params[:id]) @school.assign_attributes(school_params) if @school.save - flash[:success] = "Update #{@school.name} successfully." + flash[:success] = "Updated #{@school.name} successfully." redirect_to school_path(@school) else - render "edit", alert: "Failed to submit information :(" + flash[:alert] = "An error occurred: #{@school.errors.full_messages.join(', ')}" + render "edit" end end diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 6d66226c..d64b2430 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -65,7 +65,7 @@ def create if @school.new_record? @school = School.new(school_params) unless @school.save - flash[:alert] = "An error occurred! #{@school.errors.full_messages}" + flash[:alert] = "An error occurred! #{@school.errors.full_messages.join(', ')}" render "new" && return end end @@ -81,7 +81,7 @@ def create TeacherMailer.teacher_form_submission(@teacher).deliver_now redirect_to root_path else - redirect_to new_teacher_path, alert: "An error occurred while trying to save. #{@teacher.errors.full_messages}" + redirect_to new_teacher_path, alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}" end end @@ -100,7 +100,10 @@ def update @teacher.school = @school else @school.update(school_params) if school_params - @school.save! + unless @school.save + flash[:alert] = "An error occurred: #{@school.errors.full_messages.join(', ')}" + render "new" && return + end @teacher.school = @school end send_email_if_application_status_changed_and_email_resend_enabled @@ -114,7 +117,7 @@ def update end if !@teacher.save redirect_to edit_teacher_path(current_user.id), - alert: "Failed to update data. #{@teacher.errors.full_messages.to_sentence}" + alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}" return end if !@teacher.validated? && !current_user.admin? @@ -122,7 +125,7 @@ def update TeacherMailer.teacher_form_submission(@teacher).deliver_now end if is_admin? - redirect_to teachers_path, notice: "Saved #{@teacher.full_name}" + redirect_to edit_teacher_path(current_user.id), notice: "Saved #{@teacher.full_name}" return else @teacher.try_append_ip(request.remote_ip) diff --git a/app/models/email_template.rb b/app/models/email_template.rb index f8d516c2..a3fb339e 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -22,8 +22,8 @@ class EmailTemplate < ApplicationRecord validates :title, inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase }, if: -> { self.required? } - validates :body, presence: { message: "cannot be blank" } - validates :to, presence: { message: "cannot be blank" } + validates :body, presence: true + validates :to, presence: true before_destroy :prevent_deleting_required_emails diff --git a/app/models/school.rb b/app/models/school.rb index 8bb950c2..f33f4c8b 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -29,7 +29,7 @@ class School < ApplicationRecord VALID_STATES = [ "AL", "AK", "AS", "AZ", "AR", "CA", "CO", "CT", "DE", "DC", "FM", "FL", "GA", "GU", "HI", "ID", "IL", "IN", "IA", "KS", "KY", "LA", "ME", "MH", "MD", "MA", "MI", "MN", "MS", "MO", "MT", "NE", "NV", "NH", "NJ", "NM", "NY", "NC", "ND", "MP", "OH", "OK", "OR", "PW", "PA", "PR", "RI", "SC", "SD", "TN", "TX", "UT", "VT", "VI", "VA", "WA", "WV", "WI", "WY"].freeze validates :name, :city, :website, :country, presence: true - validates :country, inclusion: { in: ISO3166::Country.all.map(&:alpha2), message: "%{value} is not a valid country" }, presence: true + validates :country, inclusion: { in: ISO3166::Country.all.map(&:alpha2), message: "%{value} is not a valid country" } validates :state, presence: true, if: -> { country == "US" } validates :state, inclusion: { in: VALID_STATES }, if: -> { country == "US" } validates_format_of :website, with: /.+\..+/, on: :create diff --git a/app/views/schools/_form.html.erb b/app/views/schools/_form.html.erb index 07ebc9bd..453b53e0 100644 --- a/app/views/schools/_form.html.erb +++ b/app/views/schools/_form.html.erb @@ -2,32 +2,32 @@
<%= f.label :name, "School Name", class: "label-required" %> - <%= f.text_field :name, placeholder: 'UC Berkeley', class: 'form-control', + <%= f.text_field :name, placeholder: 'Name of School', class: 'form-control', required: false, id: 'school_name' %>
<%= f.label :city, class: "label-required", for: "school_city" %> - <%= f.text_field :city, placeholder: 'Berkeley', class: 'form-control', + <%= f.text_field :city, placeholder: 'Name of City', class: 'form-control', required: false, id: 'school_city' %>
<%= f.label :state, class: "label-required", for: "school_state" %> - <%= f.select :state, School::VALID_STATES, { include_blank: "State" }, { id: "state_select", class: 'form-control' } %> + <%= f.select :state, School::VALID_STATES, { include_blank: "Select a state" }, { id: "state_select", class: 'form-control' } %>
<%= f.label :state, for: "school_state" %> - <%= f.text_field :state, placeholder: "State", class: 'form-control', id: "state_textfield" %> + <%= f.text_field :state, placeholder: "Name of State", class: 'form-control', id: "state_textfield" %>
<%= f.label :country, "Country", class: "label-required", for: "school_country" %> <%= f.country_select( :country, - { priority_countries: ['United States'] }, + { priority_countries: ['United States'], include_blank: "Select a country" }, { class: 'form-control', required: false, id: 'school_country', format: :with_full_country_name, selected: 'United States'} ) %>
@@ -110,14 +110,18 @@ function handleCountryChange() { if (countrySelected.val() === 'US') { - stateSelectContainer.show().attr('required', true); + stateSelectContainer.show(); + stateSelect.attr('required', '') stateTextfieldContainer.hide(); stateTextfield.removeAttr('name'); stateSelect.attr('name', 'school[state]'); + } else { - stateTextfieldContainer.show().attr('required', false); + stateTextfieldContainer.show(); + stateSelect.removeAttr('required') //else make state select not required stateSelectContainer.hide(); + stateSelect.removeAttr('name'); stateTextfield.attr('name', 'school[state]'); diff --git a/features/admin.feature b/features/admin.feature index 3d1ba0e4..fcec4b12 100644 --- a/features/admin.feature +++ b/features/admin.feature @@ -198,7 +198,8 @@ Feature: basic admin functionality And I set my application status as "Validated" And I press "Update" Then I see a confirmation "Saved" - When I check "Validated" + When I go to the teachers page + And I check "Validated" Then I should see "Bobby John" Scenario: Deny teacher as an admin @@ -362,6 +363,24 @@ Feature: basic admin functionality Then I send a request info email + Scenario: Admin update info without mandatory field shows error + Given the following schools exist: + | name | country | city | state | website | grade_level | school_type | + | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | + And the following teachers exist: + | first_name | last_name | admin | email | school | + | Jane | Doe | false | janedoe@berkeley.edu | UC Berkeley | + Given I am on the BJC home page + And I have an admin email + And I follow "Log In" + Then I can log in with Google + When I go to the teachers page + And I go to the edit page for Jane Doe + And I fill in "teacher_email" with "" + And I press "Update" + Then I should be on the edit page for Jane Doe + + # Scenario: Admin can import csv file. The loader should filter invalid record and create associate school. # Given the following schools exist: # | name | country | city | state | website | diff --git a/features/email_template.feature b/features/email_template.feature index 36e96d97..e527204c 100644 --- a/features/email_template.feature +++ b/features/email_template.feature @@ -38,7 +38,7 @@ Scenario: Creating new email template with blank fields displays flash error Given I am on the email templates index And I press "New Email Templates" When I press "Submit" - Then I should see "Failed to submit information: Body cannot be blank, To cannot be blank" + Then I should see "An error occurred: Body can't be blank, To can't be blank" Scenario: Creating and deleting new email template with valid fields succeeds Given I am on the email templates index @@ -59,7 +59,7 @@ Scenario: Editing email template to have blank body or to field displays flash e And I follow "Welcome Email" And I fill in "email_template_to" with "" And I press "Submit" - Then I should see "Failed to save Welcome Email template: To cannot be blank" + Then I should see "An error occurred: To can't be blank" When I fill in TinyMCE email form with "" And I press "Submit" - Then I should see "Failed to save Welcome Email template: Body cannot be blank" + Then I should see "An error occurred: Body can't be blank" diff --git a/features/school_form_submission.feature b/features/school_form_submission.feature index 618aae17..0710f69a 100644 --- a/features/school_form_submission.feature +++ b/features/school_form_submission.feature @@ -70,7 +70,7 @@ Scenario: User can create an international school And I fill in the school name selectize box with "Bucharest International School" and choose to add a new school And I select "Romania" from "Country" And I enter my "City" as "Bucharest" - And I enter my "State" as "Bucharest, Sector 1" + And I fill in state with "Bucharest, Sector 1" And I enter my "School Website" as "https://chs.fuhsd.org" And I select "University" from "Grade Level" And I select "Public" from "School Type" @@ -90,7 +90,7 @@ Scenario: Admin can see international schools in the submission And I fill in the school name selectize box with "Bucharest International School" and choose to add a new school And I select "Romania" from "Country" And I enter my "City" as "Bucharest" - And I enter my "State" as "Bucharest, Sector 1" + And I fill in state with "Bucharest, Sector 1" And I enter my "School Website" as "https://chs.fuhsd.org" And I select "University" from "Grade Level" And I select "Public" from "School Type" diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb index f5d4621c..b1dbc9b7 100644 --- a/features/step_definitions/form_steps.rb +++ b/features/step_definitions/form_steps.rb @@ -110,6 +110,10 @@ find("#school_selectize").click.set(value) end +When(/^(?:|I )fill in state with "([^"]*)"$/) do |text| + fill_in("state_textfield", with: text) +end + Then(/^the new teacher form should not be submitted$/) do expect(current_path).to eq(new_teacher_path) expect(page).to have_content("Your Information") diff --git a/spec/controllers/schools_controller_spec.rb b/spec/controllers/schools_controller_spec.rb index ee826722..791c5331 100644 --- a/spec/controllers/schools_controller_spec.rb +++ b/spec/controllers/schools_controller_spec.rb @@ -9,7 +9,7 @@ before(:all) do @create_school_name = "University of California, Berkeley" - @fail_flash_alert = /Failed to submit information :\(/ + @fail_flash_error_text = "An error occurred: " @success_flash_alert = Regexp.new("Created #{@create_school_name} successfully.") end @@ -72,7 +72,8 @@ } } expect(School.find_by(name: @create_school_name)).to be_nil - expect(flash[:alert]).to match @fail_flash_alert + error = @fail_flash_error_text + "City can't be blank" + expect(flash[:alert]).to match error post schools_path, params: { school: { @@ -88,7 +89,8 @@ } } expect(School.find_by(name: @create_school_name)).to be_nil - expect(flash[:alert]).to match @fail_flash_alert + error = @fail_flash_error_text + "Website can't be blank, Website is invalid" + expect(flash[:alert]).to match error post schools_path, params: { school: { @@ -104,7 +106,8 @@ } } expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] + error = @fail_flash_error_text + "Name can't be blank" + expect(flash[:alert]).to match error end it "requires proper inputs for fields" do @@ -124,7 +127,8 @@ } } expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] + error = @fail_flash_error_text + "State is not included in the list" + expect(flash[:alert]).to match error post schools_path, params: { school: { @@ -140,7 +144,8 @@ } } expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] + error = @fail_flash_error_text + "Website is invalid" + expect(flash[:alert]).to match error # Incorrect school type expect { post schools_path, params: { @@ -179,6 +184,43 @@ expect(School.find_by(name: @create_school_name)).to be_nil end + it "does not allow invalid country" do + post schools_path, params: { + school: { + name: @create_school_name, + city: "Test City", + country: "XX", # invalid country + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 + } + } + expect(School.find_by(name: @create_school_name)).to be_nil + error = @fail_flash_error_text + "Country XX is not a valid country" + expect(flash[:alert]).to match error + end + + it "allows any state when country is not US" do + post schools_path, params: { + school: { + name: @create_school_name, + city: "Ottawa", + country: "CA", # Canada + state: "Ontario", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 + } + } + expect(School.find_by(name: @create_school_name)).not_to be_nil + expect(@success_flash_alert).to match flash[:success] + end + it "does not create duplicate schools in the same city" do expect(School.where(name: @create_school_name).count).to eq 0