diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 05aa0ea0..391c9ce7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -51,6 +51,6 @@ def check_teacher_admin end def set_sentry_user - Sentry.set_user(id: session[:user_id], email: current_user&.email) + Sentry.set_user(id: session[:user_id], email: current_user&.primary_email) end end diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index d64b2430..58d25ddb 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -42,22 +42,23 @@ def new @school = @teacher.school # maybe delegate this @readonly = false if omniauth_data.present? + email_data = { email: omniauth_data.delete("email"), primary: true } @teacher.assign_attributes(omniauth_data) + @teacher.email_addresses.build(email_data) end end # TODO: This needs to be re-written. # If you are logged in and not an admin, this should fail. def create - # Find by email, but allow updating other info. - @teacher = Teacher.find_by(email: teacher_params[:email]) + @teacher = EmailAddress.find_by(email: params.dig(:email, :primary))&.teacher if @teacher && defined?(current_user.id) && (current_user.id == @teacher.id) params[:id] = current_user.id update return elsif @teacher redirect_to login_path, - notice: "You already have signed up with '#{@teacher.email}'. Please log in." + notice: "You already have signed up with '#{@teacher.primary_email}'. Please log in." return end @@ -71,12 +72,14 @@ def create end @teacher = Teacher.new(teacher_params) + @teacher.email_addresses.build(email: params[:email][:primary], primary: true) + @teacher.try_append_ip(request.remote_ip) @teacher.session_count += 1 @teacher.school = @school if @teacher.save @teacher.not_reviewed! - flash[:success] = "Thanks for signing up for BJC, #{@teacher.first_name}! You'll hear from us shortly. Your email address is: #{@teacher.email}." + flash[:success] = "Thanks for signing up for BJC, #{@teacher.first_name}! You'll hear from us shortly. Your email address is: #{@teacher.primary_email}." TeacherMailer.form_submission(@teacher).deliver_now TeacherMailer.teacher_form_submission(@teacher).deliver_now redirect_to root_path @@ -95,7 +98,15 @@ def edit def update load_school ordered_schools + + primary_email = params.dig(:email, :primary) + personal_emails = params[:email]&.select { |key, value| key.start_with?("personal_") }&.values + + # Now, `params[:teacher]` does not contain primary_email or any personal_emailX fields @teacher.assign_attributes(teacher_params) + + update_primary_email(primary_email) + update_personal_emails(personal_emails) if teacher_params[:school_id].present? @teacher.school = @school else @@ -111,13 +122,14 @@ def update redirect_to root_path, alert: "Failed to update your information. You have already been denied. If you have questions, please email contact@bjc.berkeley.edu." return end - if (@teacher.email_changed? || @teacher.snap_changed?) && !is_admin? - redirect_to edit_teacher_path(current_user.id), alert: "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu." + if (@teacher.email_changed_flag || @teacher.snap_changed?) && !is_admin? + @teacher.email_changed_flag = false + redirect_to edit_teacher_path(params[:id]), alert: "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu." return end - if !@teacher.save - redirect_to edit_teacher_path(current_user.id), - alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}" + unless @teacher.save + redirect_to edit_teacher_path(params[:id]), + alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}" return end if !@teacher.validated? && !current_user.admin? @@ -125,12 +137,12 @@ def update TeacherMailer.teacher_form_submission(@teacher).deliver_now end if is_admin? - redirect_to edit_teacher_path(current_user.id), notice: "Saved #{@teacher.full_name}" + redirect_to edit_teacher_path(params[:id]), notice: "Saved #{@teacher.full_name}" return else @teacher.try_append_ip(request.remote_ip) end - redirect_to edit_teacher_path(current_user.id), notice: "Successfully updated your information" + redirect_to edit_teacher_path(params[:id]), notice: "Successfully updated your information" end def send_email_if_application_status_changed_and_email_resend_enabled @@ -209,13 +221,12 @@ def load_school end def teacher_params - teacher_attributes = [:first_name, :last_name, :school, :email, :status, :snap, - :more_info, :personal_website, :education_level, :school_id] - if is_admin? - teacher_attributes << [:personal_email, :application_status, - :request_reason, :skip_email] - end - params.require(:teacher).permit(*teacher_attributes, languages: []) + teacher_attributes = [:first_name, :last_name, :school, :status, :snap, + :more_info, :personal_website, :education_level, :school_id, languages: []] + admin_attributes = [:application_status, :request_reason, :skip_email] + teacher_attributes.push(*admin_attributes) if is_admin? + + params.require(:teacher).permit(*teacher_attributes) end def school_params @@ -230,7 +241,7 @@ def omniauth_data def ordered_schools if params[:id].present? load_teacher - @ordered_schools ||= [ @teacher.school ] + + @ordered_schools ||= [@teacher.school] + School.all.order(:name).reject { |s| s.id == @teacher.school_id } else @ordered_schools ||= School.all.order(:name) @@ -260,4 +271,30 @@ def sanitize_params def load_pages @pages ||= Page.where(viewer_permissions: Page.viewable_pages(current_user)) end + + def update_primary_email(primary_email) + return unless primary_email.present? + + # First, ensure the current primary email is marked as not primary if it's not the same as the new one + @teacher.email_addresses.find_by(primary: true)&.update(primary: false) + + primary_email_record = @teacher.email_addresses.find_or_initialize_by(email: primary_email) + primary_email_record.primary = true + + primary_email_record.save if primary_email_record.changed? + end + + def update_personal_emails(personal_emails) + return unless personal_emails.present? + personal_emails = personal_emails.reject(&:empty?) + return if personal_emails.empty? + + current_emails = @teacher.email_addresses.pluck(:email) + + new_emails = personal_emails - current_emails + + new_emails.each do |email| + @teacher.email_addresses.build(email:, primary: false) + end + end end diff --git a/app/models/email_address.rb b/app/models/email_address.rb new file mode 100644 index 00000000..7bf8dc0d --- /dev/null +++ b/app/models/email_address.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: email_addresses +# +# id :bigint not null, primary key +# email :string not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null +# +# Indexes +# +# index_email_addresses_on_email (email) UNIQUE +# index_email_addresses_on_teacher_id (teacher_id) +# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true) +# +# Foreign Keys +# +# fk_rails_... (teacher_id => teachers.id) +# +class EmailAddress < ApplicationRecord + belongs_to :teacher + + # Rail's bulit-in validation for email format regex + validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP } + validate :only_one_primary_email_per_teacher + + before_save :normalize_email + before_save :flag_teacher_if_email_changed + + private + def only_one_primary_email_per_teacher + if primary? && EmailAddress.where(teacher_id:, primary: true).where.not(id:).exists? + errors.add(:primary, "There can only be one primary email per teacher.") + end + end + + def normalize_email + self.email = email.strip.downcase + end + + def flag_teacher_if_email_changed + if self.email_changed? && !self.new_record? + teacher.email_changed_flag = true + teacher.handle_relevant_changes + end + end +end diff --git a/app/models/teacher.rb b/app/models/teacher.rb index 8a023449..09203aa8 100644 --- a/app/models/teacher.rb +++ b/app/models/teacher.rb @@ -40,13 +40,16 @@ class Teacher < ApplicationRecord WORLD_LANGUAGES = [ "Afrikaans", "Albanian", "Arabic", "Armenian", "Basque", "Bengali", "Bulgarian", "Catalan", "Cambodian", "Chinese (Mandarin)", "Croatian", "Czech", "Danish", "Dutch", "English", "Estonian", "Fiji", "Finnish", "French", "Georgian", "German", "Greek", "Gujarati", "Hebrew", "Hindi", "Hungarian", "Icelandic", "Indonesian", "Irish", "Italian", "Japanese", "Javanese", "Korean", "Latin", "Latvian", "Lithuanian", "Macedonian", "Malay", "Malayalam", "Maltese", "Maori", "Marathi", "Mongolian", "Nepali", "Norwegian", "Persian", "Polish", "Portuguese", "Punjabi", "Quechua", "Romanian", "Russian", "Samoan", "Serbian", "Slovak", "Slovenian", "Spanish", "Swahili", "Swedish ", "Tamil", "Tatar", "Telugu", "Thai", "Tibetan", "Tonga", "Turkish", "Ukrainian", "Urdu", "Uzbek", "Vietnamese", "Welsh", "Xhosa" ].freeze - validates :first_name, :last_name, :email, :status, presence: true - validates :email, uniqueness: true - validates :personal_email, uniqueness: true, if: -> { personal_email.present? } - validate :ensure_unique_personal_email, if: -> { email_changed? || personal_email_changed? } + has_many :email_addresses, dependent: :destroy + accepts_nested_attributes_for :email_addresses, allow_destroy: true + + validates :first_name, :last_name, :status, presence: true validate :valid_languages before_validation :sort_and_clean_languages + # Custom attribute for tracking email changes. This is not persisted in the database. + attribute :email_changed_flag, :boolean, default: false + enum application_status: { validated: "Validated", denied: "Denied", @@ -127,7 +130,7 @@ def full_name def email_name # We need to normalize names for emails. - "#{full_name} <#{email}>".delete(",") + "#{full_name} <#{primary_email}>".delete(",") end def snap_username @@ -218,15 +221,8 @@ def short_application_status end def self.user_from_omniauth(omniauth) - teachers = Teacher.where("LOWER(email) = :email or LOWER(personal_email) = :email", - email: omniauth.email.downcase) - if teachers.length > 1 - raise "Too Many Teachers Found" - elsif teachers.length == 1 - teachers.first - else - nil - end + teacher = EmailAddress.find_by(email: omniauth.email.downcase)&.teacher + teacher end def try_append_ip(ip) @@ -241,8 +237,9 @@ def email_attributes teacher_first_name: self.first_name, teacher_last_name: self.last_name, teacher_full_name: self.full_name, - teacher_email: self.email, - teacher_personal_email: self.personal_email, + teacher_email: self.primary_email, + # TODO: change to personal_emails + teacher_personal_email: self.non_primary_emails, teacher_more_info: self.more_info, teacher_snap: self.snap_username, teacher_snap_username: self.snap_username, @@ -292,21 +289,25 @@ def self.csv_export end end - private - def ensure_unique_personal_email - # We want to ensure that both email and personal_email are unique across - # BOTH columns (i.e. the email only appears once in the combined set) - # However, it's perfectly valid for personal_email to be nil - # TODO: This really suggests email needs to be its own table... - teacher_with_email = Teacher.where.not(id: self.id).exists?(["email = :value OR personal_email = :value", { value: self.email }]) - if teacher_with_email - errors.add(:email, "must be unique across both email and personal_email columns") - end - return unless self.personal_email.present? + def email + # Default return primary email + primary_email + end - teacher_personal_email = Teacher.where.not(id: self.id).exists?(["email = :value OR personal_email = :value", { value: self.personal_email }]) - if teacher_personal_email - errors.add(:personal_email, "must be unique across both email and personal_email columns") - end + def primary_email + # ||:email this code is temporary for this PR: https://github.com/cs169/BJC-Teacher-Tracker-App/pull/49 + # to make sure at least original data in db still work and passed the existing tests + self[:email] || email_addresses.find_by(primary: true)&.email + end + + def personal_emails + non_primary_emails + end + + private + def non_primary_emails + # email_addresses.where(primary: false)&.pluck(:email) + # below code is temporary for current PR, to make sure the frontend same as before (only one personal email) + personal_email || email_addresses.where(primary: false)&.pluck(:email)&.first end end diff --git a/app/views/schools/show.html.erb b/app/views/schools/show.html.erb index 5a579716..092554f0 100644 --- a/app/views/schools/show.html.erb +++ b/app/views/schools/show.html.erb @@ -66,7 +66,7 @@ <% @school.teachers.each do |teacher| %> <%= link_to(teacher.full_name, teacher_path(teacher)) %> - <%= teacher.email %> + <%= teacher.primary_email %> <%= teacher.display_status %> <%= teacher.display_application_status %> <%= teacher.created_at.strftime("%b %d, %Y") %> diff --git a/app/views/teachers/_form.html.erb b/app/views/teachers/_form.html.erb index 20903a9a..0d8e1889 100644 --- a/app/views/teachers/_form.html.erb +++ b/app/views/teachers/_form.html.erb @@ -57,9 +57,9 @@ status ONLY IF the person viewing this page is an admin. %>
- <%= f.label :email, "School Email", class: "label-required" %> - <%= f.text_field :email, placeholder: 'alonzo@snap.berkeley.edu', class: 'form-control', - required: true, type: :email, readonly: @readonly %> + <%= label_tag 'email[primary]', "School Email", class: "label-required" %> + <%= text_field_tag 'email[primary]', @teacher.primary_email, placeholder: 'alonzo@snap.berkeley.edu', + class: 'form-control', required: true, type: :email, readonly: @readonly %>
@@ -81,13 +81,14 @@ status ONLY IF the person viewing this page is an admin. %>
<%# For now... only admins can enter/edit personal emails. %> - <%- if current_user&.admin? || @teacher.personal_email.present? %> + + <%- if current_user&.admin? || @teacher.personal_emails.present? %>
- <%= f.label :personal_email do %> + <%= label_tag "email[personal_1]" do %> Personal Email <% end %> - <%= f.text_field :personal_email, placeholder: 'alonzo@gmail.com', class: 'form-control', + <%= text_field_tag "email[personal_1]", @teacher.personal_emails, placeholder: 'alonzo@gmail.com', class: 'form-control', required: false, type: :email, readonly: @readonly %>
diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index beed2b95..aae3c6af 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -3,10 +3,11 @@ <%= link_to('(Web)', teacher.personal_website, target: '_blank') if teacher.personal_website.present? %> - <%= teacher.email %> - <%- if teacher.personal_email.present? %> -
<%= teacher.personal_email %> - <%- end %> + <%= teacher.primary_email %> + + <%#- if teacher.personal_emails.present? %> + + <%#- end %> <%= teacher.display_education_level %>
Email:
- > <%= teacher.email %> + > <%= teacher.primary_email %>
- +
Personal Email:
- - ><%= teacher.personal_email %> + + ><%= teacher.personal_emails %>
-
diff --git a/db/migrate/20240407190126_create_new_email_addresses.rb b/db/migrate/20240407190126_create_new_email_addresses.rb new file mode 100644 index 00000000..09fdc0f7 --- /dev/null +++ b/db/migrate/20240407190126_create_new_email_addresses.rb @@ -0,0 +1,15 @@ +class CreateNewEmailAddresses < ActiveRecord::Migration[6.1] + def change + create_table :email_addresses do |t| + t.references :teacher, null: false, foreign_key: true + t.string :email, null: false + t.boolean :primary, default: false, null: false + + t.timestamps + end + + add_index :email_addresses, :email, unique: true + # quickly find primary emails for teachers + add_index :email_addresses, [:teacher_id, :primary], unique: true, where: '"primary" = TRUE' + end +end diff --git a/db/schema.rb b/db/schema.rb index 39c506c2..fc5c4fcd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_03_14_160959) do +ActiveRecord::Schema.define(version: 2024_04_07_190126) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -53,6 +53,17 @@ t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end + create_table "email_addresses", force: :cascade do |t| + t.bigint "teacher_id", null: false + t.string "email", null: false + t.boolean "primary", default: false, null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["email"], name: "index_email_addresses_on_email", unique: true + t.index ["teacher_id", "primary"], name: "index_email_addresses_on_teacher_id_and_primary", unique: true, where: "(\"primary\" = true)" + t.index ["teacher_id"], name: "index_email_addresses_on_teacher_id" + end + create_table "email_templates", force: :cascade do |t| t.text "body" t.string "path" @@ -129,6 +140,7 @@ add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" + add_foreign_key "email_addresses", "teachers" add_foreign_key "pages", "teachers", column: "creator_id" add_foreign_key "pages", "teachers", column: "last_editor_id" add_foreign_key "teachers", "schools" diff --git a/db/seed_data.rb b/db/seed_data.rb index f5fd7128..ce084203 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -153,8 +153,9 @@ def self.create_schools School.find_or_create_by( name: "UC Berkeley", city: "Berkeley", + country: "US", + website: "https://bjc.berkeley.edu", state: "CA", - website: "https://bjc.berkeley.edu" ) end @@ -163,21 +164,57 @@ def self.teachers { first_name: "Michael", last_name: "Ball", - email: "ball@berkeley.edu", admin: true, status: 0, application_status: "Validated", - school: School.find_by(name: "UC Berkeley") + school: School.find_by(name: "UC Berkeley"), + + # Note: email field does not exist in the new schema of the Teacher model + # Include it in the seed data is to simulate the behavior of creating a new teacher, + # because we need to use it to compared with the EmailAddress model, + # to determine the existence of the teacher + email: "ball@berkeley.edu", }, { first_name: "Lauren", last_name: "Mock", - email: "lmock@berkeley.edu", admin: true, status: 0, application_status: "Validated", - school: School.find_by(name: "UC Berkeley") + school: School.find_by(name: "UC Berkeley"), + + email: "lmock@berkeley.edu", } ] end + + def self.email_addresses + [ + { + email: "ball@berkeley.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Michael"), + }, + { + email: "lmock@berkeley.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Lauren"), + }, + { + email: "ball2@berkeley.edu", + primary: false, + teacher: Teacher.find_by(first_name: "Michael"), + }, + { + email: "ball3@berkeley.edu", + primary: false, + teacher: Teacher.find_by(first_name: "Michael"), + }, + { + email: "lmock2@berkeley.edu", + primary: false, + teacher: Teacher.find_by(first_name: "Lauren"), + }, + ] + end end diff --git a/db/seeds.rb b/db/seeds.rb index 2f4a8fc3..9497fb63 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -9,6 +9,31 @@ SeedData.create_schools -SeedData.teachers.each do |teacher| - Teacher.find_or_create_by(email: teacher[:email]).update(teacher) +SeedData.teachers.each do |teacher_attr| + email_address = EmailAddress.find_or_initialize_by(email: teacher_attr[:email]) + puts "teacher_attr: #{teacher_attr}" + + if email_address.new_record? + puts "email_address.new_record?: #{email_address.new_record?}" + + teacher = Teacher.create(teacher_attr) + if teacher.persisted? + email_address.teacher_id = teacher.id + email_address_saved = email_address.save + puts "New teacher created and email_address saved: #{email_address_saved}" + else + puts "Failed to create teacher. Errors: #{teacher.errors.full_messages.join(", ")}" + end + else + teacher = Teacher.find_by(id: email_address.teacher_id) + if teacher&.update(teacher_attr) + puts "Teacher updated successfully." + else + puts "Failed to update teacher. Errors: #{teacher.errors.full_messages.join(", ")}" + end + end +end + +SeedData.email_addresses.each do |email_address| + EmailAddress.find_or_create_by(email: email_address[:email]).update(email_address) end diff --git a/features/access.feature b/features/access.feature index 3b37e633..5d6df362 100644 --- a/features/access.feature +++ b/features/access.feature @@ -5,7 +5,7 @@ Feature: access control for new users or non-admin users Background: Has an Admin and a teacher in DB Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Alice | Admin | true | testadminuser@berkeley.edu | | Todd | Teacher | false | testteacher@berkeley.edu | diff --git a/features/admin.feature b/features/admin.feature index fcec4b12..65780ce3 100644 --- a/features/admin.feature +++ b/features/admin.feature @@ -6,7 +6,7 @@ Feature: basic admin functionality Background: Has an Admin in DB Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Admin | User | true | testadminuser@berkeley.edu | Scenario: Logging in as an admin @@ -79,7 +79,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | + | first_name | last_name | admin | primary_email | school | snap | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo | Given I am on the BJC home page Given I have an admin email @@ -99,7 +99,7 @@ Feature: basic admin functionality | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | languages | + | first_name | last_name | admin | primary_email | school | languages | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- French\n- Spanish | Given I am on the BJC home page And I have an admin email @@ -113,7 +113,7 @@ Feature: basic admin functionality | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | languages | + | first_name | last_name | admin | primary_email | school | languages | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- Hindi | Given I am on the BJC home page And I have an admin email @@ -134,7 +134,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | application_status | + | first_name | last_name | admin | primary_email | school | snap | application_status | | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied | Given I am on the BJC home page And I have an admin email @@ -169,7 +169,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | application_status | + | first_name | last_name | admin | primary_email | school | snap | application_status | | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied | Given I am on the BJC home page And I have an admin email @@ -187,7 +187,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | application_status | + | first_name | last_name | admin | primary_email | school | snap | application_status | | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied | Given I am on the BJC home page And I have an admin email @@ -207,7 +207,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | Given I am on the BJC home page Given I have an admin email @@ -229,7 +229,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | When I go to the edit page for Joseph Mamoa Then should see "You need to log in to access this." @@ -239,7 +239,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | application_status | + | first_name | last_name | admin | primary_email | school | application_status | | Victor | Validateme | false | testteacher1@berkeley.edu | UC Berkeley | Validated | | Danny | Denyme | false | testteacher2@berkeley.edu | UC Berkeley | Denied | | Peter | Pendme | false | testteacher3@berkeley.edu | UC Berkeley | Not Reviewed | @@ -263,7 +263,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | + | first_name | last_name | admin | primary_email | school | snap | | Joseph | Test | false | testteacher@berkeley.edu | UC Berkeley | alonzo | Given I am on the BJC home page Given I have an admin email @@ -284,7 +284,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | + | first_name | last_name | admin | primary_email | school | snap | | Joseph | Mamoa New | false | testteacher@berkeley.edu | UC Berkeley | alonzo | Given I am on the BJC home page Given I have an admin email @@ -308,7 +308,7 @@ Feature: basic admin functionality | name | country | city | state | website | grade_level | school_type | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | application_status | + | first_name | last_name | admin | primary_email | school | snap | application_status | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo | validated | Given I am on the BJC home page Given I have an admin email @@ -345,7 +345,7 @@ Feature: basic admin functionality | 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 | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | And I am on the BJC home page And I have an admin email @@ -368,7 +368,7 @@ Feature: basic admin functionality | 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 | + | first_name | last_name | admin | primary_email | school | | Jane | Doe | false | janedoe@berkeley.edu | UC Berkeley | Given I am on the BJC home page And I have an admin email @@ -376,7 +376,7 @@ Feature: basic admin functionality 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 fill in "School Email" with "" And I press "Update" Then I should be on the edit page for Jane Doe diff --git a/features/email_template.feature b/features/email_template.feature index e527204c..0d561f1a 100644 --- a/features/email_template.feature +++ b/features/email_template.feature @@ -4,7 +4,7 @@ Feature: email template features Background: I am logged in as an admin, and email templates are generated Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Given I am on the BJC home page Given I have an admin email diff --git a/features/form_submission.feature b/features/form_submission.feature index 574e26d6..092e9a58 100644 --- a/features/form_submission.feature +++ b/features/form_submission.feature @@ -27,7 +27,7 @@ Feature: submit a form as a teacher And I select "Public" from "School Type" And I press "Submit" Then I see a confirmation "Thanks for signing up for BJC" - And I send a form submission email to both admin and teacher with email "TESTkpzhu@berkeley.edu" + And I send a form submission email to both admin and teacher with email "testkpzhu@berkeley.edu" Scenario: Not Correctly filling out and unsuccessful form submission Given I am on the BJC home page @@ -125,7 +125,7 @@ Feature: submit a form as a teacher Scenario: Filling out new form with existing email should not update information Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Alice | Adams | false | alice@berkeley.edu | And I am on the BJC home page And I enter my "First Name" as "Mallory" @@ -148,7 +148,7 @@ Feature: submit a form as a teacher Scenario: Filling out new form with existing Snap should not create new teacher Given the following teachers exist: - | first_name | last_name | admin | email | snap | + | first_name | last_name | admin | primary_email | snap | | Alice | Adams | false | alice@berkeley.edu | aliceadams | And I am on the BJC home page And I enter my "First Name" as "Mallory" @@ -235,7 +235,7 @@ Feature: submit a form as a teacher | 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 | application_status | + | first_name | last_name | admin | primary_email | school | application_status | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | info_needed | And I am on the BJC home page And I have a teacher Google email diff --git a/features/pages_admin.feature b/features/pages_admin.feature index 3189dbea..822d2d8e 100644 --- a/features/pages_admin.feature +++ b/features/pages_admin.feature @@ -4,7 +4,7 @@ Feature: pages features as an admin Background: I am logged in as an admin Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Given I am on the BJC home page Given I have an admin email diff --git a/features/pages_permissions.feature b/features/pages_permissions.feature index 21537976..09991457 100644 --- a/features/pages_permissions.feature +++ b/features/pages_permissions.feature @@ -4,7 +4,7 @@ Feature: pages features a verified teacher Background: Has admin and teacher in DB along with pages of each permission type Given the following teachers exist: - | first_name | last_name | admin | email | application_status | + | first_name | last_name | admin | primary_email | application_status | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Not Reviewed | | Todd | Teacher | false | testteacher@berkeley.edu | Validated | Given the following pages exist: diff --git a/features/school_form_submission.feature b/features/school_form_submission.feature index 0710f69a..ad9175e9 100644 --- a/features/school_form_submission.feature +++ b/features/school_form_submission.feature @@ -9,7 +9,7 @@ Scenario: Viewing the schools page should show the all current schools | UC Irvine | US | Irvine | CA | https://www.uci.edu | | UC Scam Diego | US | La Jolla | CA | https://www.ucsd.edu | And the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Admin | User | true | testadminuser@berkeley.edu | UC Berkeley | | Joseph | Mamoa | false | jmomoa@berkeley.edu | UC Berkeley | | Dwayne | Johnson | false | djohnson@berkeley.edu | UC Berkeley | @@ -26,7 +26,7 @@ Scenario: Viewing the schools page should show the all current schools Scenario: Admins can see Tags and NCES ID Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Given I am on the BJC home page Given I have an admin email @@ -39,7 +39,7 @@ Scenario: Admins can see Tags and NCES ID Scenario: Admins can create new schools Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Given I am on the BJC home page Given I have an admin email diff --git a/features/step_definitions/admin_steps.rb b/features/step_definitions/admin_steps.rb index 40e85d69..33aa424a 100644 --- a/features/step_definitions/admin_steps.rb +++ b/features/step_definitions/admin_steps.rb @@ -79,6 +79,9 @@ # sent first, as this is how it is implemented in the code admin_fs_email = ActionMailer::Base.deliveries[-2] teacher_fs_email = ActionMailer::Base.deliveries.last + + expect(admin_fs_email).not_to eq nil + expect(teacher_fs_email).not_to eq nil admin_fs_email.subject.should eq "Form Submission" admin_fs_email.to[0].should eq "lmock@berkeley.edu" admin_fs_email.to[1].should eq "contact@bjc.berkeley.edu" diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb index b1dbc9b7..eed1d6a0 100644 --- a/features/step_definitions/form_steps.rb +++ b/features/step_definitions/form_steps.rb @@ -9,7 +9,7 @@ end And(/^"(.*)" is not in the database$/) do |email| - expect(Teacher.exists?(email:)).to be false + expect(EmailAddress.exists?(email:)).to be false end Given(/^I enter (?:my)? "(.*)" as "(.*)"$/) do |field_name, input| @@ -91,11 +91,14 @@ end Then(/the "(.*)" of the user with email "(.*)" should be "(.*)"/) do |field, email, expected| - expect(Teacher.find_by(email:)[field]).to eq(expected) + email = EmailAddress.find_by(email:) + expect(email.teacher[field]).to eq(expected) end Then(/^I should find a teacher with email "([^"]*)" and school country "([^"]*)" in the database$/) do |email, country| - teacher = Teacher.includes(:school).where(email:, 'schools.country': country).first + teacher = Teacher.joins(:email_addresses, :school) + .where(email_addresses: { email: }, schools: { country: }) + .first expect(teacher).not_to be_nil, "No teacher found with email #{email} and country #{country}" end diff --git a/features/step_definitions/teacher_steps.rb b/features/step_definitions/teacher_steps.rb index 07798cee..b67f6529 100644 --- a/features/step_definitions/teacher_steps.rb +++ b/features/step_definitions/teacher_steps.rb @@ -45,7 +45,6 @@ teachers_default = { first_name: "Alonzo", last_name: "Church", - email: "alonzo@snap.berkeley.edu", snap: "", status: "Other - Please specify below.", education_level: 1, @@ -53,7 +52,13 @@ admin: false, personal_website: "https://snap.berkeley.edu", application_status: "Not Reviewed", - languages: ["English"] + languages: ["English"], + + # Note: primary email field does not exist in the new schema of the Teacher model + # Include it in the seed data is to simulate the behavior of creating a new teacher, + # because we need to use it to compared with the EmailAddress model, + # to determine the existence of the teacher + primary_email: "alonzo@snap.berkeley.edu", } teachers_table.symbolic_hashes.each do |teacher| @@ -68,10 +73,13 @@ end end + email = teacher.delete(:primary_email) + school_name = teacher.delete(:school) school = School.find_by(name: school_name || "UC Berkeley") teacher[:school_id] = school.id - Teacher.create!(teacher) + teacher = Teacher.create!(teacher) + EmailAddress.create!(email:, teacher:, primary: true) School.reset_counters(school.id, :teachers) end end diff --git a/features/teacher.feature b/features/teacher.feature index 8edbb36c..da7bab66 100644 --- a/features/teacher.feature +++ b/features/teacher.feature @@ -27,7 +27,7 @@ Scenario: Logging in as a teacher with Google account should be able to edit the | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | | Cupertino High School | US | Cupertino | CA | https://chs.fuhsd.org | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | Given I have a teacher Google email Given I am on the BJC home page @@ -54,7 +54,7 @@ Scenario: Logging in as a teacher with Microsoft account should be able to edit | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | Given I have a teacher Microsoft email Given I am on the BJC home page @@ -68,7 +68,7 @@ Scenario: Logging in as a teacher with Snap account should be able to edit their | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | Given I have a teacher Snap email Given I am on the BJC home page @@ -82,7 +82,7 @@ Scenario: Logging in as a teacher with Snap account should be able to edit their | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | Given I have a teacher Clever email Given I am on the BJC home page @@ -96,7 +96,7 @@ Scenario: Logged in teacher with Not_Reviewed application status can update thei | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | + | first_name | last_name | admin | primary_email | school | snap | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo | Given I have a teacher Google email Given I am on the BJC home page @@ -123,7 +123,7 @@ Scenario: Logged in teacher with Not_Reviewed application status can update thei | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | school | snap | + | first_name | last_name | admin | primary_email | school | snap | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo | Given I have a teacher Google email Given I am on the BJC home page @@ -149,7 +149,7 @@ Scenario: Logged in teacher can only edit their own information | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | false | testteacher@berkeley.edu | | Jane | Austin | false | testteacher2@berkeley.edu | Given I have a teacher Google email @@ -164,7 +164,7 @@ Scenario: Logging in as a teacher with not_reviewed status should see "Update" i | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | false | testteacher@berkeley.edu | Given I have a teacher Google email Given I am on the BJC home page @@ -178,7 +178,7 @@ Scenario: Frontend should not allow Teacher to edit their email | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | + | first_name | last_name | admin | primary_email | snap | | Jane | Austin | false | testteacher@berkeley.edu | Jane | Given I have a teacher Google email Given I am on the BJC home page @@ -196,7 +196,7 @@ Scenario: Validated teacher should see resend button | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | validated | Given I have a teacher Google email Given I am on the BJC home page @@ -210,7 +210,7 @@ Scenario: teacher with not_reviewed status should not see resend button | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | Not Reviewed | Given I have a teacher Google email Given I am on the BJC home page @@ -224,7 +224,7 @@ Scenario: Denied teacher should not see resend button | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | denied | Given I have a teacher Google email Given I am on the BJC home page @@ -238,7 +238,7 @@ Scenario: Denied teacher cannot edit their information | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | more_info | + | first_name | last_name | admin | primary_email | snap | application_status | more_info | | Jane | Austin | false | testteacher@berkeley.edu | Jane | denied | Original Information | Given I have a teacher Google email Given I am on the BJC home page @@ -254,7 +254,7 @@ Scenario: Validated teacher should not see Tags or NCES ID | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | validated | Given I have a teacher Google email Given I am on the BJC home page @@ -269,7 +269,7 @@ Scenario: Teacher with not_reviewed status should not see Tags or NCES ID | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | Not Reviewed | Given I have a teacher Google email Given I am on the BJC home page @@ -284,7 +284,7 @@ Scenario: Denied teacher should not see Tags or NCES ID | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | email | snap | application_status | + | first_name | last_name | admin | primary_email | snap | application_status | | Jane | Austin | false | testteacher@berkeley.edu | Jane | denied | Given I have a teacher Google email Given I am on the BJC home page diff --git a/features/teacher_info_request.feature b/features/teacher_info_request.feature index 367d60a4..a78bfd99 100644 --- a/features/teacher_info_request.feature +++ b/features/teacher_info_request.feature @@ -6,7 +6,7 @@ Feature: Request additional information for teacher application Background: Has an Admin in DB Given the following teachers exist: - | first_name | last_name | admin | email | + | first_name | last_name | admin | primary_email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Scenario: Admin requests more information and teacher sees notification upon login @@ -14,7 +14,7 @@ Feature: Request additional information for teacher application | 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 | + | first_name | last_name | admin | primary_email | school | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | And I am on the BJC home page And I have an admin email @@ -25,7 +25,7 @@ Feature: Request additional information for teacher application And I should see "Request Info from Joseph Mamoa" And I fill in "request_reason" with "Please provide more details on your teaching experience" And I press "Submit" - Then I can send a request info email + Then I send a request info email And I follow "Logout" Given I am on the BJC home page @@ -39,7 +39,7 @@ Feature: Request additional information for teacher application | 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 | application_status | + | first_name | last_name | admin | primary_email | school | application_status | | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | info_needed | And I am on the BJC home page And I have a teacher Google email "testteacher@berkeley.edu" diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index 6127f54b..1f038ee5 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -14,7 +14,10 @@ ApplicationController.any_instance.stub(:is_admin?).and_return(false) short_app = Teacher.find_by(first_name: "Short") post :create, params: { teacher: { first_name: "First", last_name: "Last", status: 0, education_level: 0, - email: "new@user.com", password: "pa33word!", more_info: "info", school_id: short_app.school_id } } + password: "pa33word!", more_info: "info", + school_id: short_app.school_id }, + email: { primary: "new@user.com" } + } user = Teacher.find_by(first_name: "First") expect(user).not_to be_nil expect(user.session_count).to eq 1 @@ -24,7 +27,10 @@ ApplicationController.any_instance.stub(:is_admin?).and_return(false) short_app = Teacher.find_by(first_name: "Short") post :create, params: { teacher: { first_name: "First", last_name: "Last", status: 0, education_level: 0, - email: "new@user.com", password: "pa33word!", more_info: "info", school_id: short_app.school_id } } + password: "pa33word!", more_info: "info", + school_id: short_app.school_id }, + email: { primary: "new@user.com" } + } user = Teacher.find_by(first_name: "First") expect(user).not_to be_nil expect(user.ip_history).to include(request.remote_ip) @@ -35,7 +41,10 @@ short_app = Teacher.find_by(first_name: "Short") session_count_orig = short_app.session_count post :create, params: { teacher: { first_name: "Short", last_name: "Last", status: 0, education_level: 0, - email: short_app.email, password: "pa33word!", more_info: "info", school_id: short_app.school_id } } + password: "pa33word!", more_info: "info", + school_id: short_app.school_id }, + email: { primary: short_app.primary_email } + } expect(Teacher.find_by(first_name: "Short").session_count).to eq session_count_orig end @@ -112,7 +121,7 @@ post :update, params: { id: short_app.id, teacher: { id: short_app.id, email: "wrong@berkeley.edu", school_id: short_app.school_id } } post :update, params: { id: short_app.id, teacher: { id: short_app.id, snap: "wrong", school_id: short_app.school_id } } short_app = Teacher.find_by(first_name: "Short") - expect(short_app.email).to eq "short@long.com" + expect(short_app.primary_email).to eq "short@long.com" expect(short_app.snap).to eq "song" end @@ -158,14 +167,20 @@ teacher: { first_name: "Valid", last_name: "Example", - email: "valid_example@valid_example.edu", status: 0, snap: "valid_example", admin: true, school_id: School.first.id + }, + email: { + primary: "valid_example@validexample.edu", } } - expect(Teacher.find_by(email: "valid_example@valid_example.edu").admin).to be(false) + + email_address = EmailAddress.find_by(email: "valid_example@validexample.edu") + expect(email_address).not_to be_nil + teacher = email_address.teacher + expect(teacher.admin).to be(false) end it "rejects malicious auto-approve signup attempt" do @@ -173,14 +188,19 @@ teacher: { first_name: "Valid", last_name: "Example", - email: "valid_example@valid_example.edu", status: 0, application_status: "validated", snap: "valid_example", school_id: School.first.id, + }, + email: { + primary: "valid_example@validexample.edu", } } - expect(Teacher.find_by(email: "valid_example@valid_example.edu").not_reviewed?).to be true + email_address = EmailAddress.find_by(email: "valid_example@validexample.edu") + expect(email_address).not_to be_nil + teacher = email_address.teacher + expect(teacher.not_reviewed?).to be(true) end end diff --git a/spec/controllers/teachers_signup_spec.rb b/spec/controllers/teachers_signup_spec.rb index b3f6ca1b..13b61a12 100644 --- a/spec/controllers/teachers_signup_spec.rb +++ b/spec/controllers/teachers_signup_spec.rb @@ -60,9 +60,11 @@ teacher: { first_name: "valid_example", last_name: "valid_example", - email: "valid_example@valid_example.edu", status: 0, snap: "valid_example" + }, + email: { + primary: "valid_example@validexample.edu", } } expect(Teacher.count).to eq(previous_count + 1) @@ -74,7 +76,10 @@ school: { id: 1 }, - teacher: Teacher.first.attributes + teacher: Teacher.first.attributes, + email: { + primary: EmailAddress.find_by(teacher_id: Teacher.first.id).email + } } expect(response).to redirect_to(login_path) expect(flash[:notice]).to match(/Please log in/) diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb new file mode 100644 index 00000000..57e34dc9 --- /dev/null +++ b/spec/factories/email_addresses.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: email_addresses +# +# id :bigint not null, primary key +# email :string not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null +# +# Indexes +# +# index_email_addresses_on_email (email) UNIQUE +# index_email_addresses_on_teacher_id (teacher_id) +# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true) +# +# Foreign Keys +# +# fk_rails_... (teacher_id => teachers.id) +# +FactoryBot.define do + factory :email_address do + association :teacher + sequence(:email) { |n| "teacher-#{n}@example.edu" } + primary { false } + end +end diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb index 6db86bc2..36ce3284 100644 --- a/spec/factories/teachers.rb +++ b/spec/factories/teachers.rb @@ -42,10 +42,13 @@ first_name { "Teacher" } last_name { "User" } snap { "teacher" } - sequence(:email) { |n| "teacher-#{n}@example.edu" } status { 0 } application_status { "Validated" } personal_website { "https://www.school.edu/teacher" } admin { false } + + after(:create) do |teacher| + create(:email_address, teacher:, primary: true) + end end end diff --git a/spec/fixtures/email_addresses.yml b/spec/fixtures/email_addresses.yml new file mode 100644 index 00000000..e84403dd --- /dev/null +++ b/spec/fixtures/email_addresses.yml @@ -0,0 +1,62 @@ +# == Schema Information +# +# Table name: email_addresses +# +# id :bigint not null, primary key +# email :string not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null +# +# Indexes +# +# index_email_addresses_on_email (email) UNIQUE +# index_email_addresses_on_teacher_id (teacher_id) +# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true) +# +# Foreign Keys +# +# fk_rails_... (teacher_id => teachers.id) +# +admin_email: +# teacher: admin + teacher_id: 1 + email: 'ye@berkeley.edu' + primary: true + +validated_teacher_email: +# teacher: validated_teacher + teacher_id: 2 + email: 'validated@teacher.edu' + primary: true + +bob_email: +# teacher: bob + teacher_id: 3 + email: 'bob@gmail.com' + primary: true + +long_email: +# teacher: long + teacher_id: 4 + email: 'short@long.com' + primary: true + +reimu_email: +# teacher: reimu + teacher_id: 5 + email: 'reimu@touhou.com' + primary: true + +barney_email: +# teacher: barney + teacher_id: 6 + email: 'barneydinosaur@gmail.com' + primary: true + +barney_personal_email1: +# teacher: barney + teacher_id: 6 + email: 'bigpurpletrex@gmail.com' + primary: false diff --git a/spec/fixtures/teachers.yml b/spec/fixtures/teachers.yml index befd9393..0c7ae2ae 100644 --- a/spec/fixtures/teachers.yml +++ b/spec/fixtures/teachers.yml @@ -43,7 +43,6 @@ admin: last_name: Wang status: 4 more_info: A CS169 Student - email: 'ye@berkeley.edu' snap: ye application_status: Validated education_level: -1 @@ -56,7 +55,6 @@ validated_teacher: last_name: Teacher status: 4 more_info: A CS169 Student - email: validated@teacher.edu snap: validated application_status: Validated education_level: -1 @@ -67,7 +65,6 @@ bob: first_name: Bob last_name: Johnson snap: BobJohnson - email: 'bob@gmail.com' status: 1 more_info: '' application_status: Denied @@ -79,7 +76,6 @@ long: first_name: Short last_name: Long snap: song - email: 'short@long.com' status: 3 school: berkeley more_info: '' @@ -91,7 +87,6 @@ reimu: first_name: Reimu last_name: Hakurei snap: reimu - email: 'reimu@touhou.com' status: 4 more_info: Best Touhou Character school: berkeley @@ -103,11 +98,8 @@ barney: first_name: Barney last_name: Dinosaur snap: barney - email: 'barneydinosaur@gmail.com' - personal_email: 'bigpurpletrex@gmail.com' status: 1 more_info: '' application_status: Denied school: berkeley education_level: -1 - diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index e0e8c626..4f7f90bc 100644 --- a/spec/models/teacher_spec.rb +++ b/spec/models/teacher_spec.rb @@ -78,15 +78,15 @@ it "does not change a info_need status to not_reviewed if submitted without changes" do expect do - # Same email, which means no any change compared to last time - teacher.update(email: "reimu@touhou.com") + # Same primary email, which means no any change compared to last time + teacher.email_addresses.find_by(email: "reimu@touhou.com").update(primary: true) end.not_to change(teacher, :application_status) end it "changes a info_needed status to not_reviewed" do expect do # Changes the status to not_reviewed only when there are meaningful changes - teacher.update(email: "reimu_different_email@touhou.com") + teacher.email_addresses.find_by(email: "reimu@touhou.com").update(email: "reimu_different_email@touhou.com") end.to change(teacher, :application_status) .from("info_needed").to("not_reviewed") end diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb index a5906f02..f4e35723 100644 --- a/spec/support/sessions_helper.rb +++ b/spec/support/sessions_helper.rb @@ -9,7 +9,7 @@ def log_in(user, provider: "google_oauth2") uid: "123456789", info: { name: user.full_name, - email: user.email + email: user.primary_email } })