From 03658ce452cb6096e7741da54cff930e988bcaac Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:20:39 -0700 Subject: [PATCH 01/41] Add database migrations for MailBluster integration - Add mailbluster_id and mailbluster_synced_at columns to teachers table - Add unique index on teachers.mailbluster_id - Add emails_sent, emails_delivered, bounced columns to email_addresses table - Email delivery tracking on email_addresses (not teachers) per architecture decision --- app/models/email_address.rb | 15 ++++--- app/models/teacher.rb | 42 ++++++++++--------- ...1956_add_mailbluster_fields_to_teachers.rb | 7 ++++ ...mail_delivery_fields_to_email_addresses.rb | 7 ++++ db/schema.rb | 8 +++- spec/factories/email_addresses.rb | 15 ++++--- spec/factories/teachers.rb | 42 ++++++++++--------- spec/fixtures/email_addresses.yml | 15 ++++--- spec/fixtures/teachers.yml | 42 ++++++++++--------- spec/models/teacher_spec.rb | 42 ++++++++++--------- 10 files changed, 140 insertions(+), 95 deletions(-) create mode 100644 db/migrate/20260410001956_add_mailbluster_fields_to_teachers.rb create mode 100644 db/migrate/20260410002004_add_email_delivery_fields_to_email_addresses.rb diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 7bf8dc0d..6be1d527 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -4,12 +4,15 @@ # # 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 +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # diff --git a/app/models/teacher.rb b/app/models/teacher.rb index f7994961..69cdbd92 100644 --- a/app/models/teacher.rb +++ b/app/models/teacher.rb @@ -4,31 +4,35 @@ # # Table name: teachers # -# id :integer not null, primary key -# admin :boolean default(FALSE) -# application_status :string default("not_reviewed") -# education_level :integer default(NULL) -# email :string -# first_name :string -# ip_history :inet default([]), is an Array -# languages :string default(["\"English\""]), is an Array -# last_name :string -# last_session_at :datetime -# more_info :string -# personal_email :string -# personal_website :string -# session_count :integer default(0) -# snap :string -# status :integer -# created_at :datetime -# updated_at :datetime -# school_id :integer +# id :integer not null, primary key +# admin :boolean default(FALSE) +# application_status :string default("not_reviewed") +# education_level :integer default(NULL) +# email :string +# first_name :string +# ip_history :inet default([]), is an Array +# languages :string default(["\"English\""]), is an Array +# last_name :string +# last_session_at :datetime +# mailbluster_synced_at :datetime +# more_info :string +# personal_email :string +# personal_website :string +# session_count :integer default(0) +# snap :string +# status :integer +# verification_notes :text +# created_at :datetime +# updated_at :datetime +# mailbluster_id :integer +# school_id :integer # # Indexes # # index_teachers_on_email (email) UNIQUE # index_teachers_on_email_and_first_name (email,first_name) # index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE +# index_teachers_on_mailbluster_id (mailbluster_id) UNIQUE # index_teachers_on_school_id (school_id) # index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text) # index_teachers_on_status (status) diff --git a/db/migrate/20260410001956_add_mailbluster_fields_to_teachers.rb b/db/migrate/20260410001956_add_mailbluster_fields_to_teachers.rb new file mode 100644 index 00000000..e1895c14 --- /dev/null +++ b/db/migrate/20260410001956_add_mailbluster_fields_to_teachers.rb @@ -0,0 +1,7 @@ +class AddMailblusterFieldsToTeachers < ActiveRecord::Migration[6.1] + def change + add_column :teachers, :mailbluster_id, :integer + add_column :teachers, :mailbluster_synced_at, :datetime + add_index :teachers, :mailbluster_id, unique: true + end +end diff --git a/db/migrate/20260410002004_add_email_delivery_fields_to_email_addresses.rb b/db/migrate/20260410002004_add_email_delivery_fields_to_email_addresses.rb new file mode 100644 index 00000000..a27e1973 --- /dev/null +++ b/db/migrate/20260410002004_add_email_delivery_fields_to_email_addresses.rb @@ -0,0 +1,7 @@ +class AddEmailDeliveryFieldsToEmailAddresses < ActiveRecord::Migration[6.1] + def change + add_column :email_addresses, :emails_sent, :integer, default: 0, null: false + add_column :email_addresses, :emails_delivered, :integer, default: 0, null: false + add_column :email_addresses, :bounced, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 20434184..2f7cb58f 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: 2026_02_21_120000) do +ActiveRecord::Schema.define(version: 2026_04_10_002004) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -60,6 +60,9 @@ t.boolean "primary", default: false, null: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false + t.integer "emails_sent", default: 0, null: false + t.integer "emails_delivered", default: 0, null: false + t.boolean "bounced", default: false, 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" @@ -154,9 +157,12 @@ t.string "personal_email" t.string "languages", default: ["English"], array: true t.text "verification_notes" + t.integer "mailbluster_id" + t.datetime "mailbluster_synced_at" t.index ["email", "first_name"], name: "index_teachers_on_email_and_first_name" t.index ["email", "personal_email"], name: "index_teachers_on_email_and_personal_email", unique: true t.index ["email"], name: "index_teachers_on_email", unique: true + t.index ["mailbluster_id"], name: "index_teachers_on_mailbluster_id", unique: true t.index ["school_id"], name: "index_teachers_on_school_id" t.index ["snap"], name: "index_teachers_on_snap", unique: true, where: "((snap)::text <> ''::text)" t.index ["status"], name: "index_teachers_on_status" diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb index 57e34dc9..b78c7d16 100644 --- a/spec/factories/email_addresses.rb +++ b/spec/factories/email_addresses.rb @@ -4,12 +4,15 @@ # # 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 +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb index 36ce3284..9ce2ecfc 100644 --- a/spec/factories/teachers.rb +++ b/spec/factories/teachers.rb @@ -4,31 +4,35 @@ # # Table name: teachers # -# id :integer not null, primary key -# admin :boolean default(FALSE) -# application_status :string default("not_reviewed") -# education_level :integer default(NULL) -# email :string -# first_name :string -# ip_history :inet default([]), is an Array -# languages :string default(["\"English\""]), is an Array -# last_name :string -# last_session_at :datetime -# more_info :string -# personal_email :string -# personal_website :string -# session_count :integer default(0) -# snap :string -# status :integer -# created_at :datetime -# updated_at :datetime -# school_id :integer +# id :integer not null, primary key +# admin :boolean default(FALSE) +# application_status :string default("not_reviewed") +# education_level :integer default(NULL) +# email :string +# first_name :string +# ip_history :inet default([]), is an Array +# languages :string default(["\"English\""]), is an Array +# last_name :string +# last_session_at :datetime +# mailbluster_synced_at :datetime +# more_info :string +# personal_email :string +# personal_website :string +# session_count :integer default(0) +# snap :string +# status :integer +# verification_notes :text +# created_at :datetime +# updated_at :datetime +# mailbluster_id :integer +# school_id :integer # # Indexes # # index_teachers_on_email (email) UNIQUE # index_teachers_on_email_and_first_name (email,first_name) # index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE +# index_teachers_on_mailbluster_id (mailbluster_id) UNIQUE # index_teachers_on_school_id (school_id) # index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text) # index_teachers_on_status (status) diff --git a/spec/fixtures/email_addresses.yml b/spec/fixtures/email_addresses.yml index e84403dd..c1632ed6 100644 --- a/spec/fixtures/email_addresses.yml +++ b/spec/fixtures/email_addresses.yml @@ -2,12 +2,15 @@ # # 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 +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# primary :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # diff --git a/spec/fixtures/teachers.yml b/spec/fixtures/teachers.yml index 78d4258a..163538ba 100644 --- a/spec/fixtures/teachers.yml +++ b/spec/fixtures/teachers.yml @@ -2,31 +2,35 @@ # # Table name: teachers # -# id :integer not null, primary key -# admin :boolean default(FALSE) -# application_status :string default("not_reviewed") -# education_level :integer default(NULL) -# email :string -# first_name :string -# ip_history :inet default([]), is an Array -# languages :string default(["\"English\""]), is an Array -# last_name :string -# last_session_at :datetime -# more_info :string -# personal_email :string -# personal_website :string -# session_count :integer default(0) -# snap :string -# status :integer -# created_at :datetime -# updated_at :datetime -# school_id :integer +# id :integer not null, primary key +# admin :boolean default(FALSE) +# application_status :string default("not_reviewed") +# education_level :integer default(NULL) +# email :string +# first_name :string +# ip_history :inet default([]), is an Array +# languages :string default(["\"English\""]), is an Array +# last_name :string +# last_session_at :datetime +# mailbluster_synced_at :datetime +# more_info :string +# personal_email :string +# personal_website :string +# session_count :integer default(0) +# snap :string +# status :integer +# verification_notes :text +# created_at :datetime +# updated_at :datetime +# mailbluster_id :integer +# school_id :integer # # Indexes # # index_teachers_on_email (email) UNIQUE # index_teachers_on_email_and_first_name (email,first_name) # index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE +# index_teachers_on_mailbluster_id (mailbluster_id) UNIQUE # index_teachers_on_school_id (school_id) # index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text) # index_teachers_on_status (status) diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index 6a224df5..1bcb74ad 100644 --- a/spec/models/teacher_spec.rb +++ b/spec/models/teacher_spec.rb @@ -4,31 +4,35 @@ # # Table name: teachers # -# id :integer not null, primary key -# admin :boolean default(FALSE) -# application_status :string default("not_reviewed") -# education_level :integer default(NULL) -# email :string -# first_name :string -# ip_history :inet default([]), is an Array -# languages :string default(["\"English\""]), is an Array -# last_name :string -# last_session_at :datetime -# more_info :string -# personal_email :string -# personal_website :string -# session_count :integer default(0) -# snap :string -# status :integer -# created_at :datetime -# updated_at :datetime -# school_id :integer +# id :integer not null, primary key +# admin :boolean default(FALSE) +# application_status :string default("not_reviewed") +# education_level :integer default(NULL) +# email :string +# first_name :string +# ip_history :inet default([]), is an Array +# languages :string default(["\"English\""]), is an Array +# last_name :string +# last_session_at :datetime +# mailbluster_synced_at :datetime +# more_info :string +# personal_email :string +# personal_website :string +# session_count :integer default(0) +# snap :string +# status :integer +# verification_notes :text +# created_at :datetime +# updated_at :datetime +# mailbluster_id :integer +# school_id :integer # # Indexes # # index_teachers_on_email (email) UNIQUE # index_teachers_on_email_and_first_name (email,first_name) # index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE +# index_teachers_on_mailbluster_id (mailbluster_id) UNIQUE # index_teachers_on_school_id (school_id) # index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text) # index_teachers_on_status (status) From 288237e3333222df7df55a8c45b3fff6cbaa43a6 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:21:33 -0700 Subject: [PATCH 02/41] Add MailblusterService with API wrapper - Create app/services/mailbluster_service.rb - Implement create_or_update_lead, read_lead, delete_lead methods - Implement sync_all_teachers for bulk synchronization - Use HTTParty for API calls (already in Gemfile) - MailBluster API uses MD5 hash of email as lead identifier - Add US state timezone mapping for lead timezone - Store mailbluster_id on teacher after successful sync - Include teacher school data and tags in lead payload --- app/services/mailbluster_service.rb | 213 ++++++++++++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 app/services/mailbluster_service.rb diff --git a/app/services/mailbluster_service.rb b/app/services/mailbluster_service.rb new file mode 100644 index 00000000..eb7c64c2 --- /dev/null +++ b/app/services/mailbluster_service.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true + +require "digest/md5" + +class MailblusterService + BASE_URL = "https://api.mailbluster.com/api" + + class MailblusterError < StandardError; end + + class << self + # Create or update a lead in MailBluster for a given teacher. + # Uses overrideExisting to upsert. + # Returns the MailBluster lead hash on success, nil on failure. + def create_or_update_lead(teacher) + email = teacher.primary_email + return nil if email.blank? + + body = lead_payload(teacher) + body[:overrideExisting] = true + + response = post("/leads", body) + + if response.success? + lead_data = response.parsed_response["lead"] + if lead_data && lead_data["id"] + teacher.update_columns( + mailbluster_id: lead_data["id"], + mailbluster_synced_at: Time.current + ) + end + lead_data + else + Rails.logger.error("[MailBluster] Failed to create/update lead for teacher #{teacher.id}: #{response.body}") + nil + end + end + + # Read a lead from MailBluster by email address. + # Returns the lead hash or nil. + def read_lead(email) + lead_hash = md5_hash(email) + response = get("/leads/#{lead_hash}") + + if response.success? + response.parsed_response + else + Rails.logger.warn("[MailBluster] Lead not found for #{email}: #{response.code}") + nil + end + end + + # Delete a lead from MailBluster by email address. + def delete_lead(email) + lead_hash = md5_hash(email) + response = delete("/leads/#{lead_hash}") + response.success? + end + + # Sync all validated (non-admin) teachers to MailBluster. + # Returns a summary hash with counts. + def sync_all_teachers + teachers = Teacher.where(admin: false) + .where(application_status: "Validated") + .includes(:email_addresses, :school) + + results = { synced: 0, failed: 0, skipped: 0, errors: [] } + + teachers.find_each do |teacher| + if teacher.primary_email.blank? + results[:skipped] += 1 + next + end + + lead_data = create_or_update_lead(teacher) + if lead_data + results[:synced] += 1 + else + results[:failed] += 1 + results[:errors] << "Teacher #{teacher.id} (#{teacher.full_name})" + end + rescue StandardError => e + results[:failed] += 1 + results[:errors] << "Teacher #{teacher.id}: #{e.message}" + end + + results + end + + # Sync a single teacher to MailBluster. + # Returns true on success, false on failure. + def sync_teacher(teacher) + lead_data = create_or_update_lead(teacher) + lead_data.present? + end + + # Check if the API key is configured. + def configured? + api_key.present? + end + + private + + def api_key + ENV["MAILBLUSTER_API_KEY"] + end + + def md5_hash(email) + Digest::MD5.hexdigest(email.strip.downcase) + end + + def lead_payload(teacher) + payload = { + email: teacher.primary_email, + firstName: teacher.first_name, + lastName: teacher.last_name, + subscribed: teacher.validated?, + tags: lead_tags(teacher), + } + + # Add timezone from school if available + timezone = school_timezone(teacher) + payload[:timezone] = timezone if timezone.present? + + # Add IP address if available + if teacher.ip_history.present? + payload[:ipAddress] = teacher.ip_history.last.to_s + end + + # Add custom fields + payload[:fields] = { + school_name: teacher.school&.name, + school_city: teacher.school&.city, + school_state: teacher.school&.state, + application_status: teacher.application_status, + snap_username: teacher.snap, + education_level: teacher.education_level, + }.compact + + payload + end + + def lead_tags(teacher) + tags = ["BJC Teacher"] + tags << teacher.application_status.titlecase if teacher.application_status.present? + tags << teacher.status.to_s.titlecase if teacher.status.present? + tags + end + + def school_timezone(teacher) + return nil unless teacher.school + + # Use the school's state to approximate timezone. + # This is a simplified mapping; a full implementation would use + # geocoding or a timezone database. + state = teacher.school.state + return nil if state.blank? + + US_STATE_TIMEZONES[state.upcase] || US_STATE_TIMEZONES[state.titlecase] + end + + def headers + { + "Authorization" => api_key, + "Content-Type" => "application/json", + } + end + + def post(path, body) + HTTParty.post( + "#{BASE_URL}#{path}", + headers: headers, + body: body.to_json + ) + end + + def get(path) + HTTParty.get( + "#{BASE_URL}#{path}", + headers: headers + ) + end + + def delete(path) + HTTParty.delete( + "#{BASE_URL}#{path}", + headers: headers + ) + end + + # Simplified US state → timezone mapping. + # MailBluster accepts IANA timezone strings. + US_STATE_TIMEZONES = { + "AL" => "America/Chicago", "AK" => "America/Anchorage", "AZ" => "America/Phoenix", + "AR" => "America/Chicago", "CA" => "America/Los_Angeles", "CO" => "America/Denver", + "CT" => "America/New_York", "DE" => "America/New_York", "FL" => "America/New_York", + "GA" => "America/New_York", "HI" => "Pacific/Honolulu", "ID" => "America/Boise", + "IL" => "America/Chicago", "IN" => "America/Indiana/Indianapolis", + "IA" => "America/Chicago", "KS" => "America/Chicago", "KY" => "America/New_York", + "LA" => "America/Chicago", "ME" => "America/New_York", "MD" => "America/New_York", + "MA" => "America/New_York", "MI" => "America/Detroit", "MN" => "America/Chicago", + "MS" => "America/Chicago", "MO" => "America/Chicago", "MT" => "America/Denver", + "NE" => "America/Chicago", "NV" => "America/Los_Angeles", "NH" => "America/New_York", + "NJ" => "America/New_York", "NM" => "America/Denver", "NY" => "America/New_York", + "NC" => "America/New_York", "ND" => "America/Chicago", "OH" => "America/New_York", + "OK" => "America/Chicago", "OR" => "America/Los_Angeles", "PA" => "America/New_York", + "RI" => "America/New_York", "SC" => "America/New_York", "SD" => "America/Chicago", + "TN" => "America/Chicago", "TX" => "America/Chicago", "UT" => "America/Denver", + "VT" => "America/New_York", "VA" => "America/New_York", "WA" => "America/Los_Angeles", + "WV" => "America/New_York", "WI" => "America/Chicago", "WY" => "America/Denver", + "DC" => "America/New_York", + }.freeze + end +end From f2a20362fbdffc88fba89549d39d50fb07ee5b45 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:26:34 -0700 Subject: [PATCH 03/41] Add MailBluster initializer with API key configuration - Create config/initializers/mailbluster.rb - Log warning if MAILBLUSTER_API_KEY is not set (except in test env) - Document how to configure the API key --- config/initializers/mailbluster.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 config/initializers/mailbluster.rb diff --git a/config/initializers/mailbluster.rb b/config/initializers/mailbluster.rb new file mode 100644 index 00000000..183ff33e --- /dev/null +++ b/config/initializers/mailbluster.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# MailBluster API configuration +# Set the MAILBLUSTER_API_KEY environment variable to enable integration. +# Get your API key from: https://app.mailbluster.com/api-doc/getting-started +# +# In development, you can set it in config/application.yml: +# MAILBLUSTER_API_KEY: "your-api-key-here" + +Rails.application.config.after_initialize do + if ENV["MAILBLUSTER_API_KEY"].blank? && !Rails.env.test? + Rails.logger.warn("[MailBluster] MAILBLUSTER_API_KEY is not set. MailBluster sync will be disabled.") + end +end From 2272e27250989092ed3b4dbd9cd0f834a186a03f Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:27:09 -0700 Subject: [PATCH 04/41] Auto-sync teacher to MailBluster on approval - When admin validates a teacher, sync them to MailBluster as a lead - Only runs if MAILBLUSTER_API_KEY is configured - Creates or updates the lead with teacher info, school data, and tags --- app/controllers/teachers_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 35516ea9..aa306bb9 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -173,6 +173,7 @@ def request_info def validate @teacher.validated! TeacherMailer.welcome_email(@teacher).deliver_now + MailblusterService.create_or_update_lead(@teacher) if MailblusterService.configured? redirect_to root_path end From d76e088910137d586349548dcc2b857188761844 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:35:18 -0700 Subject: [PATCH 05/41] Auto-sync to MailBluster when personal email is added - After creating a new email address, sync teacher to MailBluster - Only syncs if API key is configured AND teacher is validated - Keeps MailBluster lead data up-to-date with new email additions --- app/controllers/email_addresses_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/email_addresses_controller.rb b/app/controllers/email_addresses_controller.rb index 998ad717..4c534ebf 100644 --- a/app/controllers/email_addresses_controller.rb +++ b/app/controllers/email_addresses_controller.rb @@ -13,6 +13,8 @@ def create end @teacher.email_addresses.create!(email:, primary: false) + # Sync teacher to MailBluster when a new email is added + MailblusterService.create_or_update_lead(@teacher) if MailblusterService.configured? && @teacher.validated? redirect_to teacher_path(@teacher), notice: "Personal email addresses added successfully." rescue ActiveRecord::RecordInvalid => e error_message = e.record&.errors&.full_messages&.join(", ") From e24948bbbb80df1d4d10ade402b794a71fc09b38 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:38:57 -0700 Subject: [PATCH 06/41] Add routes for MailBluster sync actions - POST /teachers/:id/sync_mailbluster (individual teacher sync) - POST /teachers/sync_all_mailbluster (bulk sync all teachers) --- config/routes.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index c6fc26b1..072c87ba 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,9 +16,13 @@ post :validate post :deny post :request_info + post :sync_mailbluster delete "remove_file", to: "teachers#remove_file" end - collection { post :import } + collection do + post :import + post :sync_all_mailbluster + end end resources :schools resources :pages, param: :url_slug From 56dfba38b53b1e485aaaa3a495adbe0d467c5147 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 17:53:26 -0700 Subject: [PATCH 07/41] Add sync_mailbluster and sync_all_mailbluster controller actions - sync_mailbluster: sync individual teacher to MailBluster (admin only) - sync_all_mailbluster: bulk sync all validated teachers (admin only) - Both check if API key is configured before proceeding - Flash messages report sync results (synced/failed/skipped counts) - First 5 errors shown in alert flash for debugging --- app/controllers/teachers_controller.rb | 29 +++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index aa306bb9..4d972ec2 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -13,7 +13,7 @@ class TeachersController < ApplicationController before_action :load_teacher, except: [:new, :index, :create, :import, :search] before_action :sanitize_params, only: [:new, :create, :edit, :update] before_action :require_login, except: [:new, :create] - before_action :require_admin, only: [:validate, :deny, :destroy, :index, :show, :search] + before_action :require_admin, only: [:validate, :deny, :destroy, :index, :show, :search, :sync_mailbluster, :sync_all_mailbluster] before_action :require_edit_permission, only: [:edit, :update, :resend_welcome_email] rescue_from ActiveRecord::RecordNotUnique, with: :deny_access @@ -209,6 +209,33 @@ def import redirect_to teachers_path end + def sync_mailbluster + unless MailblusterService.configured? + redirect_to teacher_path(@teacher), alert: "MailBluster API key is not configured." + return + end + + if MailblusterService.sync_teacher(@teacher) + redirect_to teacher_path(@teacher), notice: "Successfully synced #{@teacher.full_name} to MailBluster." + else + redirect_to teacher_path(@teacher), alert: "Failed to sync #{@teacher.full_name} to MailBluster." + end + end + + def sync_all_mailbluster + unless MailblusterService.configured? + redirect_to teachers_path, alert: "MailBluster API key is not configured." + return + end + + results = MailblusterService.sync_all_teachers + flash[:notice] = "MailBluster sync complete: #{results[:synced]} synced, #{results[:failed]} failed, #{results[:skipped]} skipped." + if results[:errors].any? + flash[:alert] = "Errors: #{results[:errors].first(5).join('; ')}" + end + redirect_to teachers_path + end + private def load_teacher @teachers = Teacher.all From 9d59f9dae8b2a5eebc9058a49665d746331f7f8b Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:01:47 -0700 Subject: [PATCH 08/41] Add 'Sync All to MailBluster' button on teachers index page - Button at bottom of teachers page triggers bulk sync of all validated teachers - Confirmation dialog prevents accidental bulk operations - Uses warning (yellow) style to indicate impactful action --- app/views/teachers/index.html.erb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/views/teachers/index.html.erb b/app/views/teachers/index.html.erb index d6dddae3..022013b9 100644 --- a/app/views/teachers/index.html.erb +++ b/app/views/teachers/index.html.erb @@ -64,6 +64,16 @@ +
+
+ <%= button_to "Sync All to MailBluster", + sync_all_mailbluster_teachers_path, + method: :post, + class: "btn btn-warning", + data: { confirm: "This will sync all validated teachers to MailBluster. Continue?" } %> +
+
+
    From 48f399ff8e556f96d09aeeb5d6caa8bfdb81af7a Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:04:17 -0700 Subject: [PATCH 09/41] Add MailBluster sync card to teacher show page - Display MailBluster ID with link to MailBluster profile - Show last synced timestamp - Show per-email delivery stats (sent, delivered, bounced) - Individual 'Sync to MailBluster' button for admin users - Only visible to admin users --- app/views/teachers/show.html.erb | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/app/views/teachers/show.html.erb b/app/views/teachers/show.html.erb index 8a0d9a08..8a72787b 100644 --- a/app/views/teachers/show.html.erb +++ b/app/views/teachers/show.html.erb @@ -14,6 +14,50 @@
+<% if @is_admin %> +
+
+
+
MailBluster Sync
+
+
+
+
MailBluster ID:
+
+ <% if @teacher.mailbluster_id.present? %> + <%= @teacher.mailbluster_id %> + (View in MailBluster) + <% else %> + Not synced + <% end %> +
+
+
+
Last Synced:
+
+ <%= @teacher.mailbluster_synced_at&.strftime("%B %d, %Y %H:%M %Z") || "Never" %> +
+
+ <% @teacher.email_addresses.each do |email_addr| %> +
+
<%= email_addr.email %>:
+
+ Sent: <%= email_addr.emails_sent %> | + Delivered: <%= email_addr.emails_delivered %> | + Bounced: <%= email_addr.bounced? ? 'Yes'.html_safe : 'No'.html_safe %> +
+
+ <% end %> +
+ <%= button_to "Sync to MailBluster", + sync_mailbluster_teacher_path(@teacher), + method: :post, + class: "btn btn-warning btn-sm" %> +
+
+
+<% end %> +
From fb7f13683ef576a40b7cd80c11b8a96492b152ce Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:05:17 -0700 Subject: [PATCH 10/41] Add MailBluster helpers to Teacher model and update CSV export - Add mailbluster_synced?, mailbluster_profile_url helper methods - Add primary_email_sent, primary_email_delivered, primary_email_bounced - Update csv_export to include mailbluster_id and delivery stats columns - Eager load email_addresses in CSV export for performance - Delivery data columns ready for future AWS bounce tracking integration --- app/models/teacher.rb | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/app/models/teacher.rb b/app/models/teacher.rb index 69cdbd92..2a726dad 100644 --- a/app/models/teacher.rb +++ b/app/models/teacher.rb @@ -315,17 +315,46 @@ def self.csv_export school_website school_grade_level school_type + mailbluster_id + primary_email_sent + primary_email_delivered + primary_email_bounced | CSV.generate(headers: true) do |csv| csv << attributes - Teacher.where(admin: false).find_each do |user| + Teacher.where(admin: false).includes(:email_addresses).find_each do |user| csv << attributes.map { |attr| user.send(attr) } end end end + def mailbluster_synced? + mailbluster_id.present? + end + + def mailbluster_profile_url + return nil unless mailbluster_id.present? + "https://app.mailbluster.com/leads/#{mailbluster_id}" + end + + def primary_email_address + email_addresses.find_by(primary: true) + end + + def primary_email_sent + primary_email_address&.emails_sent || 0 + end + + def primary_email_delivered + primary_email_address&.emails_delivered || 0 + end + + def primary_email_bounced + primary_email_address&.bounced? ? "Yes" : "No" + end + private def non_primary_emails email_addresses.where(primary: false)&.pluck(:email) From 17a1bea5dcc19870f258b2f85d7da2a785525039 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:06:07 -0700 Subject: [PATCH 11/41] Add delivery tracking scopes and methods to EmailAddress model - Add bounced scope to find all bounced email addresses - Add with_undelivered scope to find emails with delivery gaps - Add undelivered_count and has_undelivered? instance methods - These prepare the model for future AWS bounce data integration --- app/models/email_address.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 6be1d527..156d4c5d 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -34,6 +34,19 @@ class EmailAddress < ApplicationRecord before_save :normalize_email before_save :flag_teacher_if_email_changed + scope :bounced, -> { where(bounced: true) } + scope :with_undelivered, -> { where("emails_sent > emails_delivered") } + + # Number of emails that were sent but not delivered. + def undelivered_count + [emails_sent - emails_delivered, 0].max + end + + # Whether this email has any undelivered emails. + def has_undelivered? + undelivered_count > 0 + end + private def only_one_primary_email_per_teacher if primary? && EmailAddress.where(teacher_id:, primary: true).where.not(id:).exists? From 3dbaae54f94c6d09c67ad6951ccc90cb682a2934 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:06:26 -0700 Subject: [PATCH 12/41] Update TeacherHelper with bounce badge and sync status helpers - email_address_label now shows 'bounced' badge for bounced emails - Add mailbluster_sync_status helper for displaying sync state - Refactor email_address_label to support multiple badges --- app/helpers/teacher_helper.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/helpers/teacher_helper.rb b/app/helpers/teacher_helper.rb index 122b159b..51588db0 100644 --- a/app/helpers/teacher_helper.rb +++ b/app/helpers/teacher_helper.rb @@ -13,7 +13,18 @@ def ip_history_display(teacher) end def email_address_label(email) - return unless email.primary? - '  primary'.html_safe + labels = [] + labels << 'primary' if email.primary? + labels << 'bounced' if email.bounced? + return nil if labels.empty? + "  #{labels.join(' ')}".html_safe + end + + def mailbluster_sync_status(teacher) + if teacher.mailbluster_synced? + 'Synced'.html_safe + else + 'Not Synced'.html_safe + end end end From 84e69ad2180fd948cc0d36215a32994be7d0c466 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:07:05 -0700 Subject: [PATCH 13/41] Add MailBluster sync status column to teachers table - Add 'MB Sync' column header to table_headers partial - Display sync status badge (Synced/Not Synced) in teacher row partial - Visible on both teachers index and merge modal tables --- app/views/teachers/_table_headers.erb | 1 + app/views/teachers/_teacher.erb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb index f28faf8a..aa15aee9 100644 --- a/app/views/teachers/_table_headers.erb +++ b/app/views/teachers/_table_headers.erb @@ -8,3 +8,4 @@ School Approved? Created +MB Sync diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index efdec2c5..2667d8eb 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -45,3 +45,4 @@ <%= teacher.created_at.strftime("%m/%d/%Y") %> +<%= mailbluster_sync_status(teacher) %> From 13531b1638eb5a59d9bb353b2782a5555b264d3c Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:09:18 -0700 Subject: [PATCH 14/41] Add comprehensive RSpec tests for MailblusterService - Test create_or_update_lead with success and failure scenarios - Test read_lead by email with MD5 hash lookup - Test delete_lead - Test sync_teacher (individual sync) - Test sync_all_teachers (bulk sync with synced/failed/skipped counts) - Test configured? method - Test payload contents (name, email, tags, subscribed, overrideExisting) - Test error handling for API failures and exceptions - Mock HTTParty responses to avoid real API calls --- spec/services/mailbluster_service_spec.rb | 248 ++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 spec/services/mailbluster_service_spec.rb diff --git a/spec/services/mailbluster_service_spec.rb b/spec/services/mailbluster_service_spec.rb new file mode 100644 index 00000000..3dd8eb8c --- /dev/null +++ b/spec/services/mailbluster_service_spec.rb @@ -0,0 +1,248 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe MailblusterService, type: :service do + fixtures :all + + let(:teacher) { teachers(:validated_teacher) } + let(:api_key) { "test-api-key-12345" } + + let(:success_response) do + instance_double(HTTParty::Response, + success?: true, + parsed_response: { + "message" => "Lead added", + "lead" => { + "id" => 329395, + "firstName" => teacher.first_name, + "lastName" => teacher.last_name, + "email" => teacher.primary_email, + "subscribed" => true, + "tags" => ["BJC Teacher"], + "createdAt" => "2026-04-10T00:00:00.000Z", + "updatedAt" => "2026-04-10T00:00:00.000Z" + } + }, + body: '{"message":"Lead added"}') + end + + let(:failure_response) do + instance_double(HTTParty::Response, + success?: false, + parsed_response: { "message" => "Something went wrong" }, + body: '{"message":"Something went wrong"}', + code: 500) + end + + let(:read_success_response) do + instance_double(HTTParty::Response, + success?: true, + parsed_response: { + "id" => 329395, + "firstName" => "Validated", + "lastName" => "Teacher", + "email" => "validated@teacher.edu", + "subscribed" => true + }, + code: 200) + end + + let(:not_found_response) do + instance_double(HTTParty::Response, + success?: false, + parsed_response: { "message" => "Lead not found" }, + code: 404) + end + + before do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("MAILBLUSTER_API_KEY").and_return(api_key) + end + + describe ".configured?" do + it "returns true when API key is set" do + expect(described_class.configured?).to be true + end + + it "returns false when API key is blank" do + allow(ENV).to receive(:[]).with("MAILBLUSTER_API_KEY").and_return(nil) + expect(described_class.configured?).to be false + end + end + + describe ".create_or_update_lead" do + it "sends a POST request to create a lead" do + expect(HTTParty).to receive(:post) + .with( + "https://api.mailbluster.com/api/leads", + hash_including( + headers: hash_including("Authorization" => api_key), + body: anything + ) + ) + .and_return(success_response) + + result = described_class.create_or_update_lead(teacher) + expect(result).to be_present + expect(result["id"]).to eq(329395) + end + + it "stores mailbluster_id on the teacher after successful sync" do + allow(HTTParty).to receive(:post).and_return(success_response) + + described_class.create_or_update_lead(teacher) + teacher.reload + + expect(teacher.mailbluster_id).to eq(329395) + expect(teacher.mailbluster_synced_at).to be_present + end + + it "returns nil when the API call fails" do + allow(HTTParty).to receive(:post).and_return(failure_response) + + result = described_class.create_or_update_lead(teacher) + expect(result).to be_nil + end + + it "returns nil when teacher has no primary email" do + allow(teacher).to receive(:primary_email).and_return(nil) + + result = described_class.create_or_update_lead(teacher) + expect(result).to be_nil + end + + it "includes overrideExisting in the request body" do + expect(HTTParty).to receive(:post) do |_url, options| + body = JSON.parse(options[:body]) + expect(body["overrideExisting"]).to be true + success_response + end + + described_class.create_or_update_lead(teacher) + end + + it "includes teacher name and email in the payload" do + expect(HTTParty).to receive(:post) do |_url, options| + body = JSON.parse(options[:body]) + expect(body["firstName"]).to eq(teacher.first_name) + expect(body["lastName"]).to eq(teacher.last_name) + expect(body["email"]).to eq(teacher.primary_email) + success_response + end + + described_class.create_or_update_lead(teacher) + end + + it "sets subscribed to true for validated teachers" do + expect(HTTParty).to receive(:post) do |_url, options| + body = JSON.parse(options[:body]) + expect(body["subscribed"]).to be true + success_response + end + + described_class.create_or_update_lead(teacher) + end + + it "includes BJC Teacher tag" do + expect(HTTParty).to receive(:post) do |_url, options| + body = JSON.parse(options[:body]) + expect(body["tags"]).to include("BJC Teacher") + success_response + end + + described_class.create_or_update_lead(teacher) + end + end + + describe ".read_lead" do + it "fetches a lead by email using MD5 hash" do + email = "validated@teacher.edu" + md5_hash = Digest::MD5.hexdigest(email) + + expect(HTTParty).to receive(:get) + .with( + "https://api.mailbluster.com/api/leads/#{md5_hash}", + hash_including(headers: hash_including("Authorization" => api_key)) + ) + .and_return(read_success_response) + + result = described_class.read_lead(email) + expect(result["id"]).to eq(329395) + end + + it "returns nil when lead is not found" do + allow(HTTParty).to receive(:get).and_return(not_found_response) + + result = described_class.read_lead("unknown@email.com") + expect(result).to be_nil + end + end + + describe ".delete_lead" do + it "sends a DELETE request using MD5 hash of email" do + email = "validated@teacher.edu" + md5_hash = Digest::MD5.hexdigest(email) + delete_response = instance_double(HTTParty::Response, success?: true) + + expect(HTTParty).to receive(:delete) + .with( + "https://api.mailbluster.com/api/leads/#{md5_hash}", + hash_including(headers: hash_including("Authorization" => api_key)) + ) + .and_return(delete_response) + + expect(described_class.delete_lead(email)).to be true + end + end + + describe ".sync_teacher" do + it "returns true on successful sync" do + allow(HTTParty).to receive(:post).and_return(success_response) + + expect(described_class.sync_teacher(teacher)).to be true + end + + it "returns false on failed sync" do + allow(HTTParty).to receive(:post).and_return(failure_response) + + expect(described_class.sync_teacher(teacher)).to be false + end + end + + describe ".sync_all_teachers" do + it "syncs all validated non-admin teachers" do + allow(HTTParty).to receive(:post).and_return(success_response) + + results = described_class.sync_all_teachers + expect(results[:synced]).to be >= 1 + expect(results[:failed]).to eq(0) + end + + it "skips teachers without primary emails" do + # Remove primary email from validated teacher + teacher.email_addresses.where(primary: true).destroy_all + + allow(HTTParty).to receive(:post).and_return(success_response) + + results = described_class.sync_all_teachers + expect(results[:skipped]).to be >= 1 + end + + it "tracks failed syncs" do + allow(HTTParty).to receive(:post).and_return(failure_response) + + results = described_class.sync_all_teachers + expect(results[:failed]).to be >= 1 + expect(results[:errors]).not_to be_empty + end + + it "handles exceptions gracefully" do + allow(HTTParty).to receive(:post).and_raise(StandardError.new("Connection timeout")) + + results = described_class.sync_all_teachers + expect(results[:failed]).to be >= 1 + expect(results[:errors].first).to include("Connection timeout") + end + end +end From 3f1e0d1d9428dc68ffdda66d13b8d45b6ef02078 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:12:26 -0700 Subject: [PATCH 15/41] Add controller tests for MailBluster sync actions - Test sync_mailbluster: success, failure, and unconfigured API - Test sync_all_mailbluster: results display, errors, unconfigured API - Test validate action triggers MailBluster sync - Fix: exclude sync_all_mailbluster from load_teacher before_action - All 37 controller tests pass --- app/controllers/teachers_controller.rb | 2 +- spec/controllers/teachers_controller_spec.rb | 92 ++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 4d972ec2..c180be9d 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -10,7 +10,7 @@ class TeachersController < ApplicationController include CsvProcess before_action :load_pages, only: [:new, :create, :edit, :update] - before_action :load_teacher, except: [:new, :index, :create, :import, :search] + before_action :load_teacher, except: [:new, :index, :create, :import, :search, :sync_all_mailbluster] before_action :sanitize_params, only: [:new, :create, :edit, :update] before_action :require_login, except: [:new, :create] before_action :require_admin, only: [:validate, :deny, :destroy, :index, :show, :search, :sync_mailbluster, :sync_all_mailbluster] diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index f7b9c5b3..880857c3 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -342,4 +342,96 @@ expect(response).to render_template("edit") end end + + describe "MailBluster sync actions" do + before(:each) do + ApplicationController.any_instance.stub(:require_admin).and_return(true) + ApplicationController.any_instance.stub(:is_admin?).and_return(true) + end + + describe "POST #sync_mailbluster" do + let(:teacher) { Teacher.find_by(first_name: "Validated") } + + it "syncs a single teacher when API key is configured" do + allow(MailblusterService).to receive(:configured?).and_return(true) + allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return(true) + + post :sync_mailbluster, params: { id: teacher.id } + expect(response).to redirect_to(teacher_path(teacher)) + expect(flash[:notice]).to include("Successfully synced") + end + + it "shows error when sync fails" do + allow(MailblusterService).to receive(:configured?).and_return(true) + allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return(false) + + post :sync_mailbluster, params: { id: teacher.id } + expect(response).to redirect_to(teacher_path(teacher)) + expect(flash[:alert]).to include("Failed to sync") + end + + it "shows error when API key is not configured" do + allow(MailblusterService).to receive(:configured?).and_return(false) + + post :sync_mailbluster, params: { id: teacher.id } + expect(response).to redirect_to(teacher_path(teacher)) + expect(flash[:alert]).to include("not configured") + end + end + + describe "POST #sync_all_mailbluster" do + it "syncs all teachers and shows results" do + allow(MailblusterService).to receive(:configured?).and_return(true) + allow(MailblusterService).to receive(:sync_all_teachers).and_return( + { synced: 5, failed: 1, skipped: 0, errors: ["Teacher 99 (Test)"] } + ) + + post :sync_all_mailbluster + expect(response).to redirect_to(teachers_path) + expect(flash[:notice]).to include("5 synced") + expect(flash[:notice]).to include("1 failed") + end + + it "shows errors from sync" do + allow(MailblusterService).to receive(:configured?).and_return(true) + allow(MailblusterService).to receive(:sync_all_teachers).and_return( + { synced: 0, failed: 2, skipped: 0, errors: ["Teacher 1: error", "Teacher 2: error"] } + ) + + post :sync_all_mailbluster + expect(flash[:alert]).to include("Errors:") + end + + it "shows error when API key is not configured" do + allow(MailblusterService).to receive(:configured?).and_return(false) + + post :sync_all_mailbluster + expect(response).to redirect_to(teachers_path) + expect(flash[:alert]).to include("not configured") + end + end + end + + describe "POST #validate with MailBluster sync" do + before(:each) do + ApplicationController.any_instance.stub(:require_admin).and_return(true) + ApplicationController.any_instance.stub(:is_admin?).and_return(true) + end + + it "syncs teacher to MailBluster when approving" do + teacher = Teacher.find_by(first_name: "Short") + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).to receive(:create_or_update_lead).with(teacher) + + post :validate, params: { id: teacher.id } + end + + it "does not sync when API key is not configured" do + teacher = Teacher.find_by(first_name: "Short") + allow(MailblusterService).to receive(:configured?).and_return(false) + expect(MailblusterService).not_to receive(:create_or_update_lead) + + post :validate, params: { id: teacher.id } + end + end end From 103e20c360f71b362c924eef3d41e2e10b0f2dd2 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:14:49 -0700 Subject: [PATCH 16/41] Add model tests for MailBluster and email delivery tracking - Teacher model tests: mailbluster_synced?, profile_url, email stats helpers - Teacher CSV export tests: verify new mailbluster and delivery columns - EmailAddress model tests: undelivered_count, has_undelivered?, scopes - EmailAddress tests: bounced scope, with_undelivered scope, default values - All 46 model tests pass --- spec/models/email_address_spec.rb | 79 +++++++++++++++++++++++++++++++ spec/models/teacher_spec.rb | 76 +++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 spec/models/email_address_spec.rb diff --git a/spec/models/email_address_spec.rb b/spec/models/email_address_spec.rb new file mode 100644 index 00000000..424d5ee5 --- /dev/null +++ b/spec/models/email_address_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe EmailAddress, type: :model do + fixtures :all + + describe "delivery tracking" do + let(:email) { email_addresses(:validated_teacher_email) } + + describe "#undelivered_count" do + it "returns 0 when all emails are delivered" do + email.update_columns(emails_sent: 10, emails_delivered: 10) + expect(email.undelivered_count).to eq(0) + end + + it "returns the difference between sent and delivered" do + email.update_columns(emails_sent: 10, emails_delivered: 7) + expect(email.undelivered_count).to eq(3) + end + + it "never returns negative" do + email.update_columns(emails_sent: 0, emails_delivered: 0) + expect(email.undelivered_count).to eq(0) + end + end + + describe "#has_undelivered?" do + it "returns false when no undelivered emails" do + email.update_columns(emails_sent: 5, emails_delivered: 5) + expect(email.has_undelivered?).to be false + end + + it "returns true when there are undelivered emails" do + email.update_columns(emails_sent: 10, emails_delivered: 7) + expect(email.has_undelivered?).to be true + end + end + + describe "scopes" do + describe ".bounced" do + it "returns only bounced email addresses" do + email.update_column(:bounced, true) + expect(EmailAddress.bounced).to include(email) + end + + it "excludes non-bounced email addresses" do + expect(EmailAddress.bounced).not_to include(email) + end + end + + describe ".with_undelivered" do + it "returns emails where sent > delivered" do + email.update_columns(emails_sent: 10, emails_delivered: 7) + expect(EmailAddress.with_undelivered).to include(email) + end + + it "excludes emails where sent == delivered" do + email.update_columns(emails_sent: 5, emails_delivered: 5) + expect(EmailAddress.with_undelivered).not_to include(email) + end + end + end + + describe "default values" do + it "has 0 emails_sent by default" do + expect(email.emails_sent).to eq(0) + end + + it "has 0 emails_delivered by default" do + expect(email.emails_delivered).to eq(0) + end + + it "is not bounced by default" do + expect(email.bounced?).to be false + end + end + end +end diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index 1bcb74ad..dd31ca24 100644 --- a/spec/models/teacher_spec.rb +++ b/spec/models/teacher_spec.rb @@ -135,4 +135,80 @@ expect(teacher.display_application_status).to eq "Not Reviewed" end end + + describe "MailBluster helper methods" do + let(:validated_teacher) { teachers(:validated_teacher) } + + describe "#mailbluster_synced?" do + it "returns false when mailbluster_id is nil" do + expect(validated_teacher.mailbluster_synced?).to be false + end + + it "returns true when mailbluster_id is present" do + validated_teacher.update_column(:mailbluster_id, 12345) + expect(validated_teacher.mailbluster_synced?).to be true + end + end + + describe "#mailbluster_profile_url" do + it "returns nil when not synced" do + expect(validated_teacher.mailbluster_profile_url).to be_nil + end + + it "returns the MailBluster profile URL when synced" do + validated_teacher.update_column(:mailbluster_id, 12345) + expect(validated_teacher.mailbluster_profile_url).to eq("https://app.mailbluster.com/leads/12345") + end + end + + describe "#primary_email_sent" do + it "returns 0 by default" do + expect(validated_teacher.primary_email_sent).to eq(0) + end + + it "returns the email sent count from primary email address" do + email = validated_teacher.email_addresses.find_by(primary: true) + email.update_column(:emails_sent, 10) + expect(validated_teacher.primary_email_sent).to eq(10) + end + end + + describe "#primary_email_delivered" do + it "returns 0 by default" do + expect(validated_teacher.primary_email_delivered).to eq(0) + end + + it "returns the email delivered count from primary email address" do + email = validated_teacher.email_addresses.find_by(primary: true) + email.update_column(:emails_delivered, 8) + expect(validated_teacher.primary_email_delivered).to eq(8) + end + end + + describe "#primary_email_bounced" do + it "returns 'No' by default" do + expect(validated_teacher.primary_email_bounced).to eq("No") + end + + it "returns 'Yes' when primary email is bounced" do + email = validated_teacher.email_addresses.find_by(primary: true) + email.update_column(:bounced, true) + expect(validated_teacher.primary_email_bounced).to eq("Yes") + end + end + end + + describe ".csv_export" do + it "includes mailbluster_id column" do + csv = Teacher.csv_export + expect(csv).to include("mailbluster_id") + end + + it "includes email delivery columns" do + csv = Teacher.csv_export + expect(csv).to include("primary_email_sent") + expect(csv).to include("primary_email_delivered") + expect(csv).to include("primary_email_bounced") + end + end end From 50801fcb368c15502a2494a945c82fa2e4efd505 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:15:29 -0700 Subject: [PATCH 17/41] Add EmailAddressesController tests for MailBluster sync on email add - Test sync triggers when adding email to validated teacher - Test sync skipped for non-validated teachers - Test sync skipped when API key is not configured - All 6 tests pass --- .../email_addresses_controller_spec.rb | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/spec/controllers/email_addresses_controller_spec.rb b/spec/controllers/email_addresses_controller_spec.rb index 95880990..635f54ff 100644 --- a/spec/controllers/email_addresses_controller_spec.rb +++ b/spec/controllers/email_addresses_controller_spec.rb @@ -28,3 +28,40 @@ end end end + +RSpec.describe EmailAddressesController, type: :controller do + fixtures :all + + before(:each) do + Rails.application.load_seed + ApplicationController.any_instance.stub(:require_login).and_return(true) + ApplicationController.any_instance.stub(:require_admin).and_return(true) + ApplicationController.any_instance.stub(:is_admin?).and_return(true) + end + + describe "POST #create with MailBluster sync" do + let(:teacher) { teachers(:validated_teacher) } + + it "syncs teacher to MailBluster when adding email to validated teacher" do + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).to receive(:create_or_update_lead).with(teacher) + + post :create, params: { teacher_id: teacher.id, email: "newemail@test.com" } + end + + it "does not sync when teacher is not validated" do + denied_teacher = teachers(:bob) # bob has application_status: Denied + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).not_to receive(:create_or_update_lead) + + post :create, params: { teacher_id: denied_teacher.id, email: "newemail2@test.com" } + end + + it "does not sync when API key is not configured" do + allow(MailblusterService).to receive(:configured?).and_return(false) + expect(MailblusterService).not_to receive(:create_or_update_lead) + + post :create, params: { teacher_id: teacher.id, email: "newemail3@test.com" } + end + end +end From b4919381ce909f59901a2b2c350769a6cd4c7608 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:18:36 -0700 Subject: [PATCH 18/41] Add Cucumber feature tests for MailBluster sync UI - Test admin sees MailBluster sync info on teacher show page - Test admin sees 'Sync All to MailBluster' button on index - Test MB Sync column visible in teachers table --- features/mailbluster_sync.feature | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 features/mailbluster_sync.feature diff --git a/features/mailbluster_sync.feature b/features/mailbluster_sync.feature new file mode 100644 index 00000000..e1f53b82 --- /dev/null +++ b/features/mailbluster_sync.feature @@ -0,0 +1,43 @@ +Feature: MailBluster email sync + + As an admin + So that I can keep teacher emails in sync with MailBluster + I want to be able to sync teachers to MailBluster + + Background: Has an Admin and teachers in DB + Given the following schools exist: + | name | country | city | state | website | + | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | + Given the following teachers exist: + | first_name | last_name | admin | primary_email | school | application_status | + | Admin | User | true | testadminuser@berkeley.edu | UC Berkeley | Validated | + | Validated | Teacher | false | validated@teacher.edu | UC Berkeley | Validated | + | Pending | Teacher | false | pending@teacher.edu | UC Berkeley | Not Reviewed | + + Scenario: Admin sees MailBluster sync information on teacher show page + Given I am on the BJC home page + Given 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 follow "Validated Teacher" + Then I should see "MailBluster Sync" + And I should see "Not synced" + And I should see "Sync to MailBluster" + + Scenario: Admin sees Sync All button on teachers index page + Given I am on the BJC home page + Given I have an admin email + And I follow "Log In" + Then I can log in with Google + When I go to the teachers page + Then I should see "Sync All to MailBluster" + + Scenario: Admin sees MB Sync column in teachers table + Given I am on the BJC home page + Given I have an admin email + And I follow "Log In" + Then I can log in with Google + When I go to the teachers page + Then I should see "MB Sync" + And I should see "Not Synced" From f32a065fd67b875ef6f382f2807816e7a197d017 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:29:06 -0700 Subject: [PATCH 19/41] Fix Cucumber tests: resolve email fixture collision and button assertions - Use unique email addresses (mb_validated@, mb_pending@) to avoid collision with spec/fixtures/email_addresses.yml entries - Use 'should see a button named' instead of 'should see' for button_to elements whose value attribute isn't in Capybara's text content - All 3 scenarios now pass --- features/mailbluster_sync.feature | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/features/mailbluster_sync.feature b/features/mailbluster_sync.feature index e1f53b82..f0fdfe6a 100644 --- a/features/mailbluster_sync.feature +++ b/features/mailbluster_sync.feature @@ -9,10 +9,10 @@ Feature: MailBluster email sync | name | country | city | state | website | | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | Given the following teachers exist: - | first_name | last_name | admin | primary_email | school | application_status | - | Admin | User | true | testadminuser@berkeley.edu | UC Berkeley | Validated | - | Validated | Teacher | false | validated@teacher.edu | UC Berkeley | Validated | - | Pending | Teacher | false | pending@teacher.edu | UC Berkeley | Not Reviewed | + | first_name | last_name | admin | primary_email | school | application_status | + | Admin | User | true | testadminuser@berkeley.edu | UC Berkeley | Validated | + | Mbluster | Validated | false | mb_validated@teacher.edu | UC Berkeley | Validated | + | Mbluster | Pending | false | mb_pending@teacher.edu | UC Berkeley | Not Reviewed | Scenario: Admin sees MailBluster sync information on teacher show page Given I am on the BJC home page @@ -20,10 +20,10 @@ Feature: MailBluster email sync And I follow "Log In" Then I can log in with Google When I go to the teachers page - And I follow "Validated Teacher" + And I follow "Mbluster Validated" Then I should see "MailBluster Sync" And I should see "Not synced" - And I should see "Sync to MailBluster" + And I should see a button named "Sync to MailBluster" Scenario: Admin sees Sync All button on teachers index page Given I am on the BJC home page @@ -31,7 +31,7 @@ Feature: MailBluster email sync And I follow "Log In" Then I can log in with Google When I go to the teachers page - Then I should see "Sync All to MailBluster" + Then I should see a button named "Sync All to MailBluster" Scenario: Admin sees MB Sync column in teachers table Given I am on the BJC home page From 8fc59c96e3441ad2eeff6942044e4551aba875be Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:36:23 -0700 Subject: [PATCH 20/41] Add additional Cucumber scenarios for MailBluster integration - Add scenario for email delivery stats on teacher show page - Add scenario for MailBluster section on pending teacher page - Add scenario verifying non-admin cannot see sync controls - Total: 6 Cucumber scenarios covering MailBluster UI elements --- features/mailbluster_sync.feature | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/features/mailbluster_sync.feature b/features/mailbluster_sync.feature index f0fdfe6a..2098e486 100644 --- a/features/mailbluster_sync.feature +++ b/features/mailbluster_sync.feature @@ -41,3 +41,33 @@ Feature: MailBluster email sync When I go to the teachers page Then I should see "MB Sync" And I should see "Not Synced" + + Scenario: Admin sees email delivery stats on teacher show page + Given I am on the BJC home page + Given 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 follow "Mbluster Validated" + Then I should see "Sent: 0" + And I should see "Delivered: 0" + And I should see "Bounced: No" + + Scenario: Admin sees MailBluster section for pending teacher + Given I am on the BJC home page + Given 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 check "Not Reviewed" + And I follow "Mbluster Pending" + Then I should see "MailBluster Sync" + And I should see "Not synced" + + Scenario: Non-admin cannot see MailBluster sync controls + Given I am on the BJC home page + Given I have a non-admin, unregistered Google email + And I follow "Log In" + Then I can log in with Google + Then I should not see "Sync All to MailBluster" + And I should not see "MB Sync" From 6f9bb6f0f4a0fe0971f38917ad2daa5b093c7a0c Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:37:40 -0700 Subject: [PATCH 21/41] Add Rake tasks for MailBluster sync management - mailbluster:sync_all - Sync all validated teachers to MailBluster - mailbluster:sync_teacher[id] - Sync a single teacher by ID - mailbluster:status - Show sync status summary and API config --- lib/tasks/mailbluster.rake | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 lib/tasks/mailbluster.rake diff --git a/lib/tasks/mailbluster.rake b/lib/tasks/mailbluster.rake new file mode 100644 index 00000000..23a44580 --- /dev/null +++ b/lib/tasks/mailbluster.rake @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +namespace :mailbluster do + desc "Sync all validated teachers to MailBluster" + task sync_all: :environment do + unless MailblusterService.configured? + puts "MailBluster API key not configured. Set MAILBLUSTER_API_KEY environment variable." + exit 1 + end + + results = MailblusterService.sync_all_teachers + puts "MailBluster sync complete:" + puts " Synced: #{results[:synced]}" + puts " Failed: #{results[:failed]}" + puts " Errors: #{results[:errors].join(', ')}" if results[:errors].any? + end + + desc "Sync a single teacher to MailBluster by teacher ID" + task :sync_teacher, [:teacher_id] => :environment do |_t, args| + unless MailblusterService.configured? + puts "MailBluster API key not configured. Set MAILBLUSTER_API_KEY environment variable." + exit 1 + end + + teacher = Teacher.find_by(id: args[:teacher_id]) + if teacher.nil? + puts "Teacher with ID #{args[:teacher_id]} not found." + exit 1 + end + + result = MailblusterService.sync_teacher(teacher) + if result[:success] + puts "Successfully synced #{teacher.full_name} (#{teacher.primary_email_address&.email})" + else + puts "Failed to sync #{teacher.full_name}: #{result[:error]}" + end + end + + desc "Show MailBluster sync status for all teachers" + task status: :environment do + total = Teacher.count + synced = Teacher.where.not(mailbluster_synced_at: nil).count + unsynced = total - synced + puts "MailBluster sync status:" + puts " Total teachers: #{total}" + puts " Synced: #{synced}" + puts " Not synced: #{unsynced}" + puts " API configured: #{MailblusterService.configured? ? 'Yes' : 'No'}" + end +end From 91f50718b7b8ac9c320b99396f0e87aee2e53b5b Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:39:07 -0700 Subject: [PATCH 22/41] Improve error handling in MailblusterService#sync_teacher - Return structured hash with :success and :error keys - Rescue StandardError for network/unexpected errors - Display error details in flash message on sync failure - Add spec for unexpected error handling (connection timeout) --- app/controllers/teachers_controller.rb | 5 +++-- app/services/mailbluster_service.rb | 11 +++++++++-- spec/controllers/teachers_controller_spec.rb | 4 ++-- spec/services/mailbluster_service_spec.rb | 19 +++++++++++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index c180be9d..a68807bb 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -215,10 +215,11 @@ def sync_mailbluster return end - if MailblusterService.sync_teacher(@teacher) + result = MailblusterService.sync_teacher(@teacher) + if result[:success] redirect_to teacher_path(@teacher), notice: "Successfully synced #{@teacher.full_name} to MailBluster." else - redirect_to teacher_path(@teacher), alert: "Failed to sync #{@teacher.full_name} to MailBluster." + redirect_to teacher_path(@teacher), alert: "Failed to sync #{@teacher.full_name} to MailBluster. #{result[:error]}" end end diff --git a/app/services/mailbluster_service.rb b/app/services/mailbluster_service.rb index eb7c64c2..f60c0971 100644 --- a/app/services/mailbluster_service.rb +++ b/app/services/mailbluster_service.rb @@ -87,10 +87,17 @@ def sync_all_teachers end # Sync a single teacher to MailBluster. - # Returns true on success, false on failure. + # Returns a hash with :success and optionally :error. def sync_teacher(teacher) lead_data = create_or_update_lead(teacher) - lead_data.present? + if lead_data.present? + { success: true } + else + { success: false, error: "MailBluster API returned no lead data" } + end + rescue StandardError => e + Rails.logger.error("[MailBluster] Error syncing teacher #{teacher.id}: #{e.message}") + { success: false, error: e.message } end # Check if the API key is configured. diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index 880857c3..8bbe0fb4 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -354,7 +354,7 @@ it "syncs a single teacher when API key is configured" do allow(MailblusterService).to receive(:configured?).and_return(true) - allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return(true) + allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return({ success: true }) post :sync_mailbluster, params: { id: teacher.id } expect(response).to redirect_to(teacher_path(teacher)) @@ -363,7 +363,7 @@ it "shows error when sync fails" do allow(MailblusterService).to receive(:configured?).and_return(true) - allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return(false) + allow(MailblusterService).to receive(:sync_teacher).with(teacher).and_return({ success: false, error: "API error" }) post :sync_mailbluster, params: { id: teacher.id } expect(response).to redirect_to(teacher_path(teacher)) diff --git a/spec/services/mailbluster_service_spec.rb b/spec/services/mailbluster_service_spec.rb index 3dd8eb8c..eac1545f 100644 --- a/spec/services/mailbluster_service_spec.rb +++ b/spec/services/mailbluster_service_spec.rb @@ -197,16 +197,27 @@ end describe ".sync_teacher" do - it "returns true on successful sync" do + it "returns success hash on successful sync" do allow(HTTParty).to receive(:post).and_return(success_response) - expect(described_class.sync_teacher(teacher)).to be true + result = described_class.sync_teacher(teacher) + expect(result[:success]).to be true end - it "returns false on failed sync" do + it "returns failure hash on failed sync" do allow(HTTParty).to receive(:post).and_return(failure_response) - expect(described_class.sync_teacher(teacher)).to be false + result = described_class.sync_teacher(teacher) + expect(result[:success]).to be false + expect(result[:error]).to be_present + end + + it "handles unexpected errors gracefully" do + allow(HTTParty).to receive(:post).and_raise(StandardError, "connection timeout") + + result = described_class.sync_teacher(teacher) + expect(result[:success]).to be false + expect(result[:error]).to include("connection timeout") end end From 5e50b9aad3f866920c1fb139503648e839c3e5b6 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:44:09 -0700 Subject: [PATCH 23/41] Auto-sync to MailBluster when application status changes - Sync on update action when status changes to/from validated - Handle both validation and de-validation transitions - Add controller specs for auto-sync on status change --- app/controllers/teachers_controller.rb | 14 +++++++++ spec/controllers/teachers_controller_spec.rb | 33 ++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index a68807bb..6a661d59 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -131,6 +131,7 @@ def update attach_new_files_if_any send_email_if_application_status_changed_and_email_resend_enabled + sync_to_mailbluster_if_status_changed if fail_to_update return @@ -162,6 +163,19 @@ def send_email_if_application_status_changed_and_email_resend_enabled end end + def sync_to_mailbluster_if_status_changed + return unless MailblusterService.configured? + return unless @teacher.application_status_changed? + + if @teacher.validated? + MailblusterService.create_or_update_lead(@teacher) + elsif @teacher.application_status_was == "validated" + # If teacher was validated but status changed away, update MailBluster + # to mark them as unsubscribed + MailblusterService.create_or_update_lead(@teacher) + end + end + def request_info @teacher.info_needed! if !params[:skip_email].present? diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index 8bbe0fb4..1d6fd545 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -434,4 +434,37 @@ post :validate, params: { id: teacher.id } end end + + describe "PUT #update with MailBluster auto-sync on status change" do + before(:each) do + ApplicationController.any_instance.stub(:require_admin).and_return(true) + ApplicationController.any_instance.stub(:require_edit_permission).and_return(true) + ApplicationController.any_instance.stub(:is_admin?).and_return(true) + ApplicationController.any_instance.stub(:current_user).and_return(Teacher.find_by(first_name: "Ye")) + end + + it "syncs to MailBluster when application_status changes to validated" do + teacher = Teacher.find_by(first_name: "Short") + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).to receive(:create_or_update_lead) + + put :update, params: { + id: teacher.id, + teacher: { application_status: "validated", school_id: teacher.school_id }, + skip_email: "Yes" + } + end + + it "does not sync when status does not change" do + teacher = Teacher.find_by(first_name: "Validated") + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).not_to receive(:create_or_update_lead) + + put :update, params: { + id: teacher.id, + teacher: { first_name: "StillValidated", school_id: teacher.school_id }, + skip_email: "Yes" + } + end + end end From 8e3a2f0feea8736c121c9c7efdfd3ec662cd21bd Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:45:30 -0700 Subject: [PATCH 24/41] Delete MailBluster lead when destroying a teacher - Auto-delete lead from MailBluster before teacher destruction - Add spec for MailBluster lead deletion on destroy --- app/controllers/teachers_controller.rb | 3 +++ spec/controllers/teachers_controller_spec.rb | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 6a661d59..dd1b7a76 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -200,6 +200,9 @@ def deny end def destroy + if MailblusterService.configured? && @teacher.primary_email.present? + MailblusterService.delete_lead(@teacher.primary_email) + end @teacher.destroy! flash[:info] = "Deleted #{@teacher.full_name} successfully." redirect_to teachers_path diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index 1d6fd545..cac57ed6 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -91,6 +91,16 @@ expect(Teacher.find_by(first_name: "Short")).to be_nil end + it "deletes MailBluster lead when destroying a teacher" do + ApplicationController.any_instance.stub(:require_admin).and_return(true) + ApplicationController.any_instance.stub(:is_admin?).and_return(true) + teacher = Teacher.find_by(first_name: "Short") + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).to receive(:delete_lead).with(teacher.primary_email) + + delete :destroy, params: { id: teacher.id } + end + it "doesn't allow teacher to delete an application" do long_app = Teacher.find_by(first_name: "Short") delete :destroy, params: { id: long_app.id } From ffcb2524e2eea1e9d9dcf6bfbfdcc41add72f740 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:46:45 -0700 Subject: [PATCH 25/41] Add MailBluster integration documentation to README - Configuration instructions for MAILBLUSTER_API_KEY - Feature overview (auto-sync, manual sync, lead cleanup, delivery tracking) - Rake task usage examples --- README.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/README.md b/README.md index c764103d..4fccb341 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,44 @@ If bundler install runs successfully, continue with the following commands to co - `heroku config:set ...` for each of the environment variables. - `heroku open` +## MailBluster Integration + +The app integrates with [MailBluster](https://mailbluster.com/) for email marketing and newsletter management. + +### Configuration + +Set the `MAILBLUSTER_API_KEY` environment variable: + +```bash +# Local development +export MAILBLUSTER_API_KEY=your_api_key_here + +# Heroku +heroku config:set MAILBLUSTER_API_KEY=your_api_key_here +``` + +### Features + +- **Auto-sync on approval**: When a teacher is validated, their info is synced to MailBluster as a lead +- **Auto-sync on status change**: Updating a teacher's application status triggers a MailBluster sync +- **Auto-sync on email add**: Adding a new email address to a validated teacher triggers sync +- **Manual sync**: Admins can sync individual teachers or all validated teachers from the UI +- **Lead cleanup**: Deleting a teacher removes their lead from MailBluster +- **Delivery tracking**: Email addresses track `emails_sent`, `emails_delivered`, and `bounced` status + +### Rake Tasks + +```bash +# Sync all validated teachers to MailBluster +bundle exec rake mailbluster:sync_all + +# Sync a single teacher by ID +bundle exec rake mailbluster:sync_teacher[123] + +# Check sync status +bundle exec rake mailbluster:status +``` + ### CodeClimate Local Test From 868730e94f175f05d146755dd0b9f21dff513dab Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:47:34 -0700 Subject: [PATCH 26/41] Re-sync to MailBluster when email addresses are deleted - Trigger MailBluster sync after email deletion for validated teachers - Add specs for email deletion MailBluster sync behavior --- app/controllers/email_addresses_controller.rb | 2 ++ .../email_addresses_controller_spec.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/app/controllers/email_addresses_controller.rb b/app/controllers/email_addresses_controller.rb index 4c534ebf..5b152754 100644 --- a/app/controllers/email_addresses_controller.rb +++ b/app/controllers/email_addresses_controller.rb @@ -30,6 +30,8 @@ def destroy end email.destroy! + # Re-sync to MailBluster since email list changed + MailblusterService.create_or_update_lead(@teacher) if MailblusterService.configured? && @teacher.validated? redirect_to teacher_path(@teacher), notice: "Email address deleted successfully." rescue ActiveRecord::RecordNotFound redirect_to teacher_path(@teacher), alert: "Email address not found." diff --git a/spec/controllers/email_addresses_controller_spec.rb b/spec/controllers/email_addresses_controller_spec.rb index 635f54ff..59de396c 100644 --- a/spec/controllers/email_addresses_controller_spec.rb +++ b/spec/controllers/email_addresses_controller_spec.rb @@ -64,4 +64,25 @@ post :create, params: { teacher_id: teacher.id, email: "newemail3@test.com" } end end + + describe "DELETE #destroy with MailBluster sync" do + let(:teacher) { teachers(:validated_teacher) } + + it "re-syncs teacher to MailBluster when deleting an email" do + extra_email = teacher.email_addresses.create!(email: "extra_mb@test.com", primary: false) + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).to receive(:create_or_update_lead).with(teacher) + + delete :destroy, params: { teacher_id: teacher.id, id: extra_email.id } + end + + it "does not sync when teacher is not validated" do + denied_teacher = teachers(:bob) + extra_email = denied_teacher.email_addresses.create!(email: "extra_mb2@test.com", primary: false) + allow(MailblusterService).to receive(:configured?).and_return(true) + expect(MailblusterService).not_to receive(:create_or_update_lead) + + delete :destroy, params: { teacher_id: denied_teacher.id, id: extra_email.id } + end + end end From 683bc4ee31ed95bbb693934796ae20bc362ffe94 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:47:56 -0700 Subject: [PATCH 27/41] Add rate limit delay between bulk MailBluster API calls - Sleep 100ms between API calls in sync_all_teachers - Prevents hitting MailBluster API rate limits during bulk sync --- app/services/mailbluster_service.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/services/mailbluster_service.rb b/app/services/mailbluster_service.rb index f60c0971..27aba317 100644 --- a/app/services/mailbluster_service.rb +++ b/app/services/mailbluster_service.rb @@ -58,6 +58,7 @@ def delete_lead(email) # Sync all validated (non-admin) teachers to MailBluster. # Returns a summary hash with counts. + # Includes a small delay between requests to avoid rate limiting. def sync_all_teachers teachers = Teacher.where(admin: false) .where(application_status: "Validated") @@ -78,6 +79,9 @@ def sync_all_teachers results[:failed] += 1 results[:errors] << "Teacher #{teacher.id} (#{teacher.full_name})" end + + # Small delay between API calls to respect rate limits + sleep(0.1) if results[:synced] + results[:failed] > 0 rescue StandardError => e results[:failed] += 1 results[:errors] << "Teacher #{teacher.id}: #{e.message}" From 27e00883fc885949631f760b53f4c3b964224918 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:49:04 -0700 Subject: [PATCH 28/41] Link synced badge to MailBluster profile in teacher table - Synced badge now links to MailBluster lead profile page - Opens in new tab with tooltip --- app/helpers/teacher_helper.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/helpers/teacher_helper.rb b/app/helpers/teacher_helper.rb index 51588db0..005d50fc 100644 --- a/app/helpers/teacher_helper.rb +++ b/app/helpers/teacher_helper.rb @@ -22,7 +22,12 @@ def email_address_label(email) def mailbluster_sync_status(teacher) if teacher.mailbluster_synced? - 'Synced'.html_safe + url = teacher.mailbluster_profile_url + if url + link_to('Synced'.html_safe, url, target: "_blank", title: "View in MailBluster") + else + 'Synced'.html_safe + end else 'Not Synced'.html_safe end From 1afabdecb38202ac7b5d0c1f1a0f3ddf964f6607 Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:49:59 -0700 Subject: [PATCH 29/41] Add helper specs for MailBluster sync status and email labels - Test mailbluster_sync_status for synced/not-synced states - Test profile link in synced badge - Test email_address_label for primary and bounced badges --- spec/helpers/teacher_helper_spec.rb | 37 +++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/spec/helpers/teacher_helper_spec.rb b/spec/helpers/teacher_helper_spec.rb index 03eaf319..a5dede18 100644 --- a/spec/helpers/teacher_helper_spec.rb +++ b/spec/helpers/teacher_helper_spec.rb @@ -14,4 +14,41 @@ it "helpers ip_history_display displays correctly" do expect(ip_history_display(teacher)).to eq "-" end + + describe "#mailbluster_sync_status" do + it "shows Not Synced badge when teacher has not been synced" do + result = mailbluster_sync_status(teacher) + expect(result).to include("Not Synced") + expect(result).to include("badge-secondary") + end + + it "shows Synced badge when teacher has been synced" do + teacher.update_columns(mailbluster_synced_at: Time.current, mailbluster_id: 123) + result = mailbluster_sync_status(teacher) + expect(result).to include("Synced") + expect(result).to include("badge-success") + end + + it "links Synced badge to MailBluster profile when ID present" do + teacher.update_columns(mailbluster_synced_at: Time.current, mailbluster_id: 456) + result = mailbluster_sync_status(teacher) + expect(result).to include("mailbluster.com") + expect(result).to include("target=\"_blank\"") + end + end + + describe "#email_address_label" do + it "shows primary badge for primary email" do + email = teacher.email_addresses.find_by(primary: true) + result = email_address_label(email) + expect(result).to include("primary") + end + + it "shows bounced badge for bounced email" do + email = teacher.email_addresses.first + email.update_columns(bounced: true) + result = email_address_label(email) + expect(result).to include("bounced") + end + end end From 41e4b4204ced2c27809040628c993ddfd6573e8d Mon Sep 17 00:00:00 2001 From: Hagen Date: Thu, 9 Apr 2026 18:52:50 -0700 Subject: [PATCH 30/41] Update model annotations for email_address_spec - Regenerate annotations to reflect new delivery tracking columns - emails_sent, emails_delivered, bounced columns now in spec header --- spec/models/email_address_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/models/email_address_spec.rb b/spec/models/email_address_spec.rb index 424d5ee5..1b278c23 100644 --- a/spec/models/email_address_spec.rb +++ b/spec/models/email_address_spec.rb @@ -1,5 +1,29 @@ # frozen_string_literal: true +# == Schema Information +# +# Table name: email_addresses +# +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), 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) +# require "rails_helper" RSpec.describe EmailAddress, type: :model do From 8ca29ea2b340bc517bde3ea1c7f06acaef077be8 Mon Sep 17 00:00:00 2001 From: Hagen Date: Sat, 11 Apr 2026 18:25:34 -0700 Subject: [PATCH 31/41] Fix RuboCop offenses in MailblusterService - Remove blank line after private access modifier - Use Ruby 3.1+ shorthand hash syntax for headers --- app/services/mailbluster_service.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/services/mailbluster_service.rb b/app/services/mailbluster_service.rb index 27aba317..26c09044 100644 --- a/app/services/mailbluster_service.rb +++ b/app/services/mailbluster_service.rb @@ -110,7 +110,6 @@ def configured? end private - def api_key ENV["MAILBLUSTER_API_KEY"] end @@ -179,7 +178,7 @@ def headers def post(path, body) HTTParty.post( "#{BASE_URL}#{path}", - headers: headers, + headers:, body: body.to_json ) end @@ -187,14 +186,14 @@ def post(path, body) def get(path) HTTParty.get( "#{BASE_URL}#{path}", - headers: headers + headers: ) end def delete(path) HTTParty.delete( "#{BASE_URL}#{path}", - headers: headers + headers: ) end From 019481a5b6fa2c6aab1cd62bd09a90bd8ff97ad9 Mon Sep 17 00:00:00 2001 From: Hagen Date: Sat, 11 Apr 2026 18:25:40 -0700 Subject: [PATCH 32/41] Fix test data leak from before(:all) loading seeds Change before(:all) to before(:each) in email_templates_controller_spec and teacher_mailer_spec so that seed data is rolled back by transactional fixtures instead of persisting across test examples. --- spec/controllers/email_templates_controller_spec.rb | 2 +- spec/mailers/teacher_mailer_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/email_templates_controller_spec.rb b/spec/controllers/email_templates_controller_spec.rb index 99e82db7..c7cad915 100644 --- a/spec/controllers/email_templates_controller_spec.rb +++ b/spec/controllers/email_templates_controller_spec.rb @@ -5,7 +5,7 @@ RSpec.describe EmailTemplatesController, type: :controller do fixtures :all - before(:all) do + before(:each) do Rails.application.load_seed end diff --git a/spec/mailers/teacher_mailer_spec.rb b/spec/mailers/teacher_mailer_spec.rb index 9ba16098..0adcba92 100644 --- a/spec/mailers/teacher_mailer_spec.rb +++ b/spec/mailers/teacher_mailer_spec.rb @@ -4,7 +4,7 @@ describe TeacherMailer do fixtures :all - before(:all) do + before(:each) do Rails.application.load_seed end it "Sends Welcome Email" do From 1d3c266e20cb01a8281e9021dca8c74538c1c747 Mon Sep 17 00:00:00 2001 From: Hagen Date: Sat, 11 Apr 2026 18:25:44 -0700 Subject: [PATCH 33/41] Expand seed data with diverse teachers and schools Add 13 non-admin teachers across 5 new schools with varied application statuses, roles, languages, and email addresses. Add MAILBLUSTER_API_KEY placeholder to .env.example. --- .env.example | 1 + db/seed_data.rb | 365 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 353 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 9c409a6a..61542eca 100644 --- a/.env.example +++ b/.env.example @@ -20,6 +20,7 @@ S3_BUCKET_NAME=[NEW ONE] S3_REGION=us-west-2 S3_SECRET_ACCESS_KEY=[NEW ONE] SECRET_KEY_BASE=[NEW ONE] +MAILBLUSTER_API_KEY=[NEW ONE] SENTRY_DSN= SNAP_CLIENT_SECRET='ignore' SNAP_CLIENT_URL='https://forum.snap.berkeley.edu/session/sso_provider' diff --git a/db/seed_data.rb b/db/seed_data.rb index d68c0885..43c1f0a1 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -150,18 +150,72 @@ def self.emails end def self.create_schools - school = School.find_or_initialize_by( - name: "UC Berkeley", - city: "Berkeley", - state: "CA", - country: "US" - ) - - school.update!( - website: "https://bjc.berkeley.edu", - grade_level: 4, - school_type: 0 - ) + schools = [ + { + name: "UC Berkeley", + city: "Berkeley", + state: "CA", + country: "US", + website: "https://bjc.berkeley.edu", + grade_level: 4, + school_type: 0 + }, + { + name: "Lincoln High School", + city: "San Francisco", + state: "CA", + country: "US", + website: "https://lincolnhs.org", + grade_level: 2, + school_type: 0 + }, + { + name: "Riverside Middle School", + city: "Austin", + state: "TX", + country: "US", + website: "https://riverside-ms.edu", + grade_level: 1, + school_type: 0 + }, + { + name: "Greenfield Charter Academy", + city: "Portland", + state: "OR", + country: "US", + website: "https://greenfieldcharter.org", + grade_level: 2, + school_type: 2 + }, + { + name: "Eastside Prep", + city: "New York", + state: "NY", + country: "US", + website: "https://eastsideprep.edu", + grade_level: 2, + school_type: 1 + }, + { + name: "Pacific Community College", + city: "Seattle", + state: "WA", + country: "US", + website: "https://pacificcc.edu", + grade_level: 3, + school_type: 0 + } + ] + + schools.each do |attrs| + school = School.find_or_initialize_by( + name: attrs[:name], + city: attrs[:city], + state: attrs[:state], + country: attrs[:country] + ) + school.update!(attrs) + end end def self.teachers @@ -191,7 +245,187 @@ def self.teachers school: School.find_by(name: "UC Berkeley"), email: "lmock@berkeley.edu", - } + }, + { + first_name: "Priya", + last_name: "Ramirez", + admin: false, + status: 0, + snap: "priya_ram", + application_status: "Validated", + personal_website: "https://priyaramirez.dev", + personal_email: "priya.ramirez@gmail.com", + education_level: 1, + languages: ["English", "Spanish"], + school: School.find_by(name: "Lincoln High School"), + email: "pramirez@lincolnhs.org", + }, + { + first_name: "James", + last_name: "Okonkwo", + admin: false, + status: 1, + snap: "jokonkwo_cs", + application_status: "Validated", + personal_website: "https://okonkwo-teaches.com", + personal_email: "james.okonkwo@outlook.com", + education_level: 0, + languages: ["English"], + school: School.find_by(name: "Riverside Middle School"), + email: "jokonkwo@riverside-ms.edu", + }, + { + first_name: "Sofia", + last_name: "Chen", + admin: false, + status: 0, + snap: "sofia_chen_cs", + application_status: "Not Reviewed", + personal_website: "https://sofiacsclass.com", + education_level: 1, + languages: ["English", "Mandarin"], + school: School.find_by(name: "Greenfield Charter Academy"), + email: "schen@greenfieldcharter.org", + }, + { + first_name: "Marcus", + last_name: "Williams", + admin: false, + status: 2, + snap: "mwilliams_bjc", + application_status: "Validated", + personal_website: "https://mwilliamscs.com", + education_level: 1, + languages: ["English"], + school: School.find_by(name: "Eastside Prep"), + email: "mwilliams@eastsideprep.edu", + }, + { + first_name: "Aisha", + last_name: "Patel", + admin: false, + status: 0, + snap: "aisha_p", + application_status: "Info Needed", + personal_website: "https://aishapatel.dev", + personal_email: "aisha.patel@yahoo.com", + education_level: 2, + more_info: "Currently setting up a new CS program at the college.", + languages: ["English", "Hindi"], + school: School.find_by(name: "Pacific Community College"), + email: "apatel@pacificcc.edu", + }, + { + first_name: "Diego", + last_name: "Morales", + admin: false, + status: 5, + snap: "diego_teals", + application_status: "Validated", + personal_website: "https://dmorales-teals.com", + personal_email: "diego.morales@proton.me", + education_level: 1, + languages: ["English", "Spanish"], + school: School.find_by(name: "Lincoln High School"), + email: "dmorales@lincolnhs.org", + }, + { + first_name: "Emily", + last_name: "Nakamura", + admin: false, + status: 8, + snap: "emily_nak", + application_status: "Denied", + personal_website: "https://nakamura-cs.net", + education_level: 0, + verification_notes: "Could not verify school affiliation.", + languages: ["English", "Japanese"], + school: School.find_by(name: "Riverside Middle School"), + email: "enakamura@riverside-ms.edu", + }, + { + first_name: "Tariq", + last_name: "Hassan", + admin: false, + status: 0, + snap: "tariq_h", + application_status: "Validated", + personal_website: "https://tariqhassan.io", + personal_email: "tariq.hassan@gmail.com", + education_level: 1, + languages: ["English", "Arabic"], + school: School.find_by(name: "Eastside Prep"), + email: "thassan@eastsideprep.edu", + }, + { + first_name: "Rachel", + last_name: "Novak", + admin: false, + status: 0, + snap: "rnovak_cs", + application_status: "Validated", + personal_website: "https://rachelnovak.dev", + personal_email: "rachel.novak@gmail.com", + education_level: 1, + languages: ["English", "Czech"], + school: School.find_by(name: "Lincoln High School"), + email: "rnovak@lincolnhs.org", + }, + { + first_name: "Kevin", + last_name: "Tran", + admin: false, + status: 1, + snap: "kevtran_bjc", + application_status: "Not Reviewed", + personal_website: "https://kevintran.me", + education_level: 2, + languages: ["English", "Vietnamese"], + school: School.find_by(name: "Pacific Community College"), + email: "ktran@pacificcc.edu", + }, + { + first_name: "Fatima", + last_name: "Al-Rashid", + admin: false, + status: 7, + snap: "fatima_excite", + application_status: "Validated", + personal_website: "https://fatimaalrashid.org", + personal_email: "fatima.alrashid@icloud.com", + education_level: 1, + languages: ["English", "Arabic", "French"], + school: School.find_by(name: "Greenfield Charter Academy"), + email: "falrashid@greenfieldcharter.org", + }, + { + first_name: "Owen", + last_name: "Burke", + admin: false, + status: 9, + snap: "oburke_home", + application_status: "Info Needed", + personal_website: "https://burkelearning.com", + more_info: "Homeschool cooperative covering 12 families.", + education_level: 1, + languages: ["English"], + school: School.find_by(name: "Riverside Middle School"), + email: "oburke@riverside-ms.edu", + }, + { + first_name: "Mia", + last_name: "Johansson", + admin: false, + status: 0, + snap: "mia_johan", + application_status: "Validated", + personal_website: "https://miajohansson.se", + personal_email: "mia.johansson@hotmail.com", + education_level: 1, + languages: ["English", "Swedish"], + school: School.find_by(name: "Eastside Prep"), + email: "mjohansson@eastsideprep.edu", + }, ] end @@ -222,6 +456,111 @@ def self.email_addresses primary: false, teacher: Teacher.find_by(first_name: "Lauren"), }, + { + email: "pramirez@lincolnhs.org", + primary: true, + teacher: Teacher.find_by(first_name: "Priya"), + }, + { + email: "priya.ramirez@gmail.com", + primary: false, + teacher: Teacher.find_by(first_name: "Priya"), + }, + { + email: "jokonkwo@riverside-ms.edu", + primary: true, + teacher: Teacher.find_by(first_name: "James"), + }, + { + email: "james.okonkwo@outlook.com", + primary: false, + teacher: Teacher.find_by(first_name: "James"), + }, + { + email: "schen@greenfieldcharter.org", + primary: true, + teacher: Teacher.find_by(first_name: "Sofia"), + }, + { + email: "mwilliams@eastsideprep.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Marcus"), + }, + { + email: "apatel@pacificcc.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Aisha"), + }, + { + email: "aisha.patel@yahoo.com", + primary: false, + teacher: Teacher.find_by(first_name: "Aisha"), + }, + { + email: "dmorales@lincolnhs.org", + primary: true, + teacher: Teacher.find_by(first_name: "Diego"), + }, + { + email: "diego.morales@proton.me", + primary: false, + teacher: Teacher.find_by(first_name: "Diego"), + }, + { + email: "enakamura@riverside-ms.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Emily"), + }, + { + email: "thassan@eastsideprep.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Tariq"), + }, + { + email: "tariq.hassan@gmail.com", + primary: false, + teacher: Teacher.find_by(first_name: "Tariq"), + }, + { + email: "rnovak@lincolnhs.org", + primary: true, + teacher: Teacher.find_by(first_name: "Rachel"), + }, + { + email: "rachel.novak@gmail.com", + primary: false, + teacher: Teacher.find_by(first_name: "Rachel"), + }, + { + email: "ktran@pacificcc.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Kevin"), + }, + { + email: "falrashid@greenfieldcharter.org", + primary: true, + teacher: Teacher.find_by(first_name: "Fatima"), + }, + { + email: "fatima.alrashid@icloud.com", + primary: false, + teacher: Teacher.find_by(first_name: "Fatima"), + }, + { + email: "oburke@riverside-ms.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Owen"), + }, + { + email: "mjohansson@eastsideprep.edu", + primary: true, + teacher: Teacher.find_by(first_name: "Mia"), + }, + { + email: "mia.johansson@hotmail.com", + primary: false, + teacher: Teacher.find_by(first_name: "Mia"), + }, ] end end From 9677917f62c98477d2cdb6139d3336ed7ac22cfd Mon Sep 17 00:00:00 2001 From: Hagen Date: Sat, 18 Apr 2026 12:02:37 -0700 Subject: [PATCH 34/41] Hide MailBluster column on index --- app/views/teachers/_table_headers.erb | 3 +++ app/views/teachers/_teacher.erb | 3 +++ app/views/teachers/index.html.erb | 8 ++++---- features/mailbluster_sync.feature | 6 +++--- spec/controllers/teachers_controller_spec.rb | 14 ++++++++++++++ 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb index aa15aee9..210bfeb1 100644 --- a/app/views/teachers/_table_headers.erb +++ b/app/views/teachers/_table_headers.erb @@ -1,6 +1,7 @@ <% if include_id %> ID <% end %> +<% show_mailbluster = local_assigns.fetch(:show_mailbluster, true) %> Name Email Role @@ -8,4 +9,6 @@ School Approved? Created +<% if show_mailbluster %> MB Sync +<% end %> diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index 2667d8eb..547e1174 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -1,6 +1,7 @@ <% if merge_table %> <%= teacher.id %> <% end %> +<% show_mailbluster = local_assigns.fetch(:show_mailbluster, true) %> <% if merge_table %> <%= link_to teacher.full_name, preview_merge_path(@teacher.id, teacher.id), method: :get %> @@ -45,4 +46,6 @@ <%= teacher.created_at.strftime("%m/%d/%Y") %> +<% if show_mailbluster %> <%= mailbluster_sync_status(teacher) %> +<% end %> diff --git a/app/views/teachers/index.html.erb b/app/views/teachers/index.html.erb index 022013b9..7dc5bf95 100644 --- a/app/views/teachers/index.html.erb +++ b/app/views/teachers/index.html.erb @@ -17,13 +17,13 @@ - <%= render 'table_headers', include_id: false %> + <%= render 'table_headers', include_id: false, show_mailbluster: false %> <% @all_teachers.each do |teacher| %> - <%= render 'teacher', teacher: teacher, merge_table: false %> + <%= render 'teacher', teacher: teacher, merge_table: false, show_mailbluster: false %> <% end %> @@ -35,13 +35,13 @@
- <%= render 'table_headers', include_id: false %> + <%= render 'table_headers', include_id: false, show_mailbluster: false %> <% @admins.each do |admin| %> - <%= render 'teacher', teacher: admin, merge_table: false %> + <%= render 'teacher', teacher: admin, merge_table: false, show_mailbluster: false %> <% end %> diff --git a/features/mailbluster_sync.feature b/features/mailbluster_sync.feature index 2098e486..41ed9d80 100644 --- a/features/mailbluster_sync.feature +++ b/features/mailbluster_sync.feature @@ -33,14 +33,14 @@ Feature: MailBluster email sync When I go to the teachers page Then I should see a button named "Sync All to MailBluster" - Scenario: Admin sees MB Sync column in teachers table + Scenario: Admin does not see MB Sync column in teachers table Given I am on the BJC home page Given I have an admin email And I follow "Log In" Then I can log in with Google When I go to the teachers page - Then I should see "MB Sync" - And I should see "Not Synced" + Then I should not see "MB Sync" + And I should not see "Not Synced" Scenario: Admin sees email delivery stats on teacher show page Given I am on the BJC home page diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index cac57ed6..ee0f8b09 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -10,6 +10,20 @@ ApplicationController.any_instance.stub(:require_login).and_return(true) end + describe "GET #index" do + render_views + + it "hides the MailBluster sync column from the teachers index" do + ApplicationController.any_instance.stub(:require_admin).and_return(true) + + get :index + + expect(response).to have_http_status(:ok) + expect(response.body).not_to include("MB Sync") + expect(response.body).not_to include("Not Synced") + end + end + it "should initialize session count to 1 when teachers signs up (submits app)" do ApplicationController.any_instance.stub(:is_admin?).and_return(false) short_app = Teacher.find_by(first_name: "Short") From 02b10601945ea31b90f74e0417859621231b0dae Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Wed, 22 Apr 2026 22:38:21 -0700 Subject: [PATCH 35/41] Remove MB Sync column in dashboard view --- app/views/main/dashboard.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/main/dashboard.html.erb b/app/views/main/dashboard.html.erb index 8f485437..347aa118 100644 --- a/app/views/main/dashboard.html.erb +++ b/app/views/main/dashboard.html.erb @@ -4,14 +4,14 @@
- <%= render 'teachers/table_headers', include_id: false %> + <%= render 'teachers/table_headers', include_id: false, show_mailbluster: false %> <% @unreviewed_teachers.each do |teacher| %> - <%= render 'teachers/teacher', teacher: teacher, merge_table: false %> + <%= render 'teachers/teacher', teacher: teacher, merge_table: false, show_mailbluster: false %>
Actions
<%= button_to("✔️", validate_teacher_path(teacher.id), From 3c2864e8615a0f54562eaaf3316d94476532ed8f Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Thu, 23 Apr 2026 01:00:13 -0700 Subject: [PATCH 36/41] Edit seed data for future integration --- db/seed_data.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/db/seed_data.rb b/db/seed_data.rb index 43c1f0a1..c8c10134 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -398,20 +398,6 @@ def self.teachers school: School.find_by(name: "Greenfield Charter Academy"), email: "falrashid@greenfieldcharter.org", }, - { - first_name: "Owen", - last_name: "Burke", - admin: false, - status: 9, - snap: "oburke_home", - application_status: "Info Needed", - personal_website: "https://burkelearning.com", - more_info: "Homeschool cooperative covering 12 families.", - education_level: 1, - languages: ["English"], - school: School.find_by(name: "Riverside Middle School"), - email: "oburke@riverside-ms.edu", - }, { first_name: "Mia", last_name: "Johansson", From 87855238416e58a01292841900d863cc6876aef5 Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Thu, 23 Apr 2026 01:54:18 -0700 Subject: [PATCH 37/41] update primary and bounced email tags --- app/helpers/teacher_helper.rb | 6 +++--- app/javascript/styles/application.scss | 19 +++++++++++++++++++ app/views/teachers/_teacher.erb | 7 ++++--- app/views/teachers/_teacher_info.html.erb | 5 ++--- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/helpers/teacher_helper.rb b/app/helpers/teacher_helper.rb index 005d50fc..abc95252 100644 --- a/app/helpers/teacher_helper.rb +++ b/app/helpers/teacher_helper.rb @@ -14,10 +14,10 @@ def ip_history_display(teacher) def email_address_label(email) labels = [] - labels << 'primary' if email.primary? - labels << 'bounced' if email.bounced? + labels << '' if email.primary? + labels << '' if email.bounced? return nil if labels.empty? - "  #{labels.join(' ')}".html_safe + labels.join('').html_safe end def mailbluster_sync_status(teacher) diff --git a/app/javascript/styles/application.scss b/app/javascript/styles/application.scss index 5e5e2626..96561e88 100644 --- a/app/javascript/styles/application.scss +++ b/app/javascript/styles/application.scss @@ -162,6 +162,25 @@ footer a:active { } } +.primary-email-dot, +.bounced-email-dot { + display: inline-block; + width: 12px; + height: 12px; + border-radius: 50%; + vertical-align: middle; + margin-right: 4px; + cursor: default; +} + +.primary-email-dot { + background-color: #007bff; +} + +.bounced-email-dot { + background-color: #dc3545; +} + .trapezoid { position: absolute; bottom: -11px; diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index 547e1174..5920a3ba 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -11,9 +11,10 @@ <%= link_to('(Web)', teacher.personal_website, target: '_blank') if teacher.personal_website.present? %>
- <% teacher.email_addresses.each do |email| %> - <%= email.email %><%= email_address_label(email) %> - <% unless email == teacher.email_addresses.last %> + <% sorted_emails = teacher.email_addresses.sort_by { |e| e.primary? ? 0 : 1 } %> + <% sorted_emails.each do |email| %> + <%= email_address_label(email) %><%= email.email %> + <% unless email == sorted_emails.last %>
<% end %> <% end %> diff --git a/app/views/teachers/_teacher_info.html.erb b/app/views/teachers/_teacher_info.html.erb index 8cdda7c0..02779248 100644 --- a/app/views/teachers/_teacher_info.html.erb +++ b/app/views/teachers/_teacher_info.html.erb @@ -35,9 +35,9 @@
Emails:
    - <% teacher.email_addresses.each do |email| %> + <% teacher.email_addresses.sort_by { |e| e.primary? ? 0 : 1 }.each do |email| %> <% end %>
From 5dbaaf005b23ef5172e5c995f79d90be0dccd8de Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Thu, 23 Apr 2026 01:55:01 -0700 Subject: [PATCH 38/41] lint --- app/helpers/teacher_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/teacher_helper.rb b/app/helpers/teacher_helper.rb index abc95252..c31e6a5a 100644 --- a/app/helpers/teacher_helper.rb +++ b/app/helpers/teacher_helper.rb @@ -17,7 +17,7 @@ def email_address_label(email) labels << '' if email.primary? labels << '' if email.bounced? return nil if labels.empty? - labels.join('').html_safe + labels.join("").html_safe end def mailbluster_sync_status(teacher) From 6be173ba9a54591414fc936003e20d1b447ac51a Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Tue, 28 Apr 2026 19:49:54 -0700 Subject: [PATCH 39/41] Add SES bounce count columns and ses_delivery_events table --- ...01_add_bounce_counts_to_email_addresses.rb | 9 ++++++++ ...260424000002_create_ses_delivery_events.rb | 20 +++++++++++++++++ db/schema.rb | 22 ++++++++++++++++++- spec/factories/email_addresses.rb | 21 ++++++++++-------- spec/fixtures/email_addresses.yml | 21 ++++++++++-------- spec/models/email_template_spec.rb | 18 +++++++++++++++ 6 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20260424000001_add_bounce_counts_to_email_addresses.rb create mode 100644 db/migrate/20260424000002_create_ses_delivery_events.rb diff --git a/db/migrate/20260424000001_add_bounce_counts_to_email_addresses.rb b/db/migrate/20260424000001_add_bounce_counts_to_email_addresses.rb new file mode 100644 index 00000000..826cc575 --- /dev/null +++ b/db/migrate/20260424000001_add_bounce_counts_to_email_addresses.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddBounceCountsToEmailAddresses < ActiveRecord::Migration[6.1] + def change + add_column :email_addresses, :soft_bounce_count, :integer, default: 0, null: false + add_column :email_addresses, :hard_bounce_count, :integer, default: 0, null: false + add_column :email_addresses, :last_ses_event_at, :datetime + end +end diff --git a/db/migrate/20260424000002_create_ses_delivery_events.rb b/db/migrate/20260424000002_create_ses_delivery_events.rb new file mode 100644 index 00000000..50c67401 --- /dev/null +++ b/db/migrate/20260424000002_create_ses_delivery_events.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateSesDeliveryEvents < ActiveRecord::Migration[6.1] + def change + create_table :ses_delivery_events do |t| + t.references :email_address, foreign_key: true, null: true + t.string :sns_message_id, null: false + t.string :event_type, null: false + t.string :bounce_type + t.string :recipient_email, null: false + t.datetime :event_occurred_at, null: false + t.jsonb :payload, default: {}, null: false + t.timestamps + + t.index [:sns_message_id, :recipient_email], unique: true, name: "idx_ses_events_dedupe" + t.index :event_type + t.index :recipient_email + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2f7cb58f..56e2f36a 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: 2026_04_10_002004) do +ActiveRecord::Schema.define(version: 2026_04_24_000002) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -63,6 +63,9 @@ t.integer "emails_sent", default: 0, null: false t.integer "emails_delivered", default: 0, null: false t.boolean "bounced", default: false, null: false + t.integer "soft_bounce_count", default: 0, null: false + t.integer "hard_bounce_count", default: 0, null: false + t.datetime "last_ses_event_at" 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" @@ -137,6 +140,22 @@ t.index ["name", "city", "website"], name: "index_schools_on_name_city_and_website" end + create_table "ses_delivery_events", force: :cascade do |t| + t.bigint "email_address_id" + t.string "sns_message_id", null: false + t.string "event_type", null: false + t.string "bounce_type" + t.string "recipient_email", null: false + t.datetime "event_occurred_at", null: false + t.jsonb "payload", default: {}, null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["email_address_id"], name: "index_ses_delivery_events_on_email_address_id" + t.index ["event_type"], name: "index_ses_delivery_events_on_event_type" + t.index ["recipient_email"], name: "index_ses_delivery_events_on_recipient_email" + t.index ["sns_message_id", "recipient_email"], name: "idx_ses_events_dedupe", unique: true + end + create_table "teachers", id: :serial, force: :cascade do |t| t.string "first_name" t.string "last_name" @@ -175,5 +194,6 @@ add_foreign_key "pages", "teachers", column: "last_editor_id" add_foreign_key "pd_registrations", "professional_developments" add_foreign_key "pd_registrations", "teachers" + add_foreign_key "ses_delivery_events", "email_addresses" add_foreign_key "teachers", "schools" end diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb index b78c7d16..65465c1f 100644 --- a/spec/factories/email_addresses.rb +++ b/spec/factories/email_addresses.rb @@ -4,15 +4,18 @@ # # Table name: email_addresses # -# id :bigint not null, primary key -# bounced :boolean default(FALSE), not null -# email :string not null -# emails_delivered :integer default(0), not null -# emails_sent :integer default(0), not null -# primary :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# teacher_id :bigint not null +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# hard_bounce_count :integer default(0), not null +# last_ses_event_at :datetime +# primary :boolean default(FALSE), not null +# soft_bounce_count :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # diff --git a/spec/fixtures/email_addresses.yml b/spec/fixtures/email_addresses.yml index c1632ed6..fc762f21 100644 --- a/spec/fixtures/email_addresses.yml +++ b/spec/fixtures/email_addresses.yml @@ -2,15 +2,18 @@ # # Table name: email_addresses # -# id :bigint not null, primary key -# bounced :boolean default(FALSE), not null -# email :string not null -# emails_delivered :integer default(0), not null -# emails_sent :integer default(0), not null -# primary :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# teacher_id :bigint not null +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# hard_bounce_count :integer default(0), not null +# last_ses_event_at :datetime +# primary :boolean default(FALSE), not null +# soft_bounce_count :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # diff --git a/spec/models/email_template_spec.rb b/spec/models/email_template_spec.rb index c75c2ac7..cca80a83 100644 --- a/spec/models/email_template_spec.rb +++ b/spec/models/email_template_spec.rb @@ -1,5 +1,23 @@ # frozen_string_literal: true +# == Schema Information +# +# Table name: email_templates +# +# id :bigint not null, primary key +# body :text +# format :string +# handler :string +# locale :string +# partial :boolean +# path :string +# required :boolean default(FALSE) +# subject :string +# title :string +# to :string +# created_at :datetime not null +# updated_at :datetime not null +# require "rails_helper" RSpec.describe EmailTemplate, type: :model do From bf07adea1b05591ab4f47e499242da7215d57f90 Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Tue, 28 Apr 2026 19:51:15 -0700 Subject: [PATCH 40/41] Add SesDeliveryEvent model and ses_delivery_events association to EmailAddress --- app/models/email_address.rb | 22 +++-- app/models/ses_delivery_event.rb | 15 ++++ spec/models/email_address_spec.rb | 47 ++++++++-- spec/models/ses_delivery_event_spec.rb | 118 +++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 18 deletions(-) create mode 100644 app/models/ses_delivery_event.rb create mode 100644 spec/models/ses_delivery_event_spec.rb diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 156d4c5d..1313be12 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -4,15 +4,18 @@ # # Table name: email_addresses # -# id :bigint not null, primary key -# bounced :boolean default(FALSE), not null -# email :string not null -# emails_delivered :integer default(0), not null -# emails_sent :integer default(0), not null -# primary :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# teacher_id :bigint not null +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# hard_bounce_count :integer default(0), not null +# last_ses_event_at :datetime +# primary :boolean default(FALSE), not null +# soft_bounce_count :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # @@ -26,6 +29,7 @@ # class EmailAddress < ApplicationRecord belongs_to :teacher + has_many :ses_delivery_events, dependent: :destroy # Rail's bulit-in validation for email format regex validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP } diff --git a/app/models/ses_delivery_event.rb b/app/models/ses_delivery_event.rb new file mode 100644 index 00000000..2dfa1bdc --- /dev/null +++ b/app/models/ses_delivery_event.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class SesDeliveryEvent < ApplicationRecord + EVENT_TYPES = %w[Send Delivery Bounce Complaint].freeze + + belongs_to :email_address, optional: true + + validates :sns_message_id, presence: true + validates :event_type, presence: true + validates :recipient_email, presence: true + validates :event_occurred_at, presence: true + + scope :hard_bounces, -> { where("event_type = 'Complaint' OR (event_type = 'Bounce' AND bounce_type <> 'Transient')") } + scope :soft_bounces, -> { where(event_type: "Bounce", bounce_type: "Transient") } +end diff --git a/spec/models/email_address_spec.rb b/spec/models/email_address_spec.rb index 1b278c23..ab4a4437 100644 --- a/spec/models/email_address_spec.rb +++ b/spec/models/email_address_spec.rb @@ -4,15 +4,18 @@ # # Table name: email_addresses # -# id :bigint not null, primary key -# bounced :boolean default(FALSE), not null -# email :string not null -# emails_delivered :integer default(0), not null -# emails_sent :integer default(0), not null -# primary :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# teacher_id :bigint not null +# id :bigint not null, primary key +# bounced :boolean default(FALSE), not null +# email :string not null +# emails_delivered :integer default(0), not null +# emails_sent :integer default(0), not null +# hard_bounce_count :integer default(0), not null +# last_ses_event_at :datetime +# primary :boolean default(FALSE), not null +# soft_bounce_count :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# teacher_id :bigint not null # # Indexes # @@ -98,6 +101,32 @@ it "is not bounced by default" do expect(email.bounced?).to be false end + + it "has 0 soft_bounce_count by default" do + expect(email.soft_bounce_count).to eq(0) + end + + it "has 0 hard_bounce_count by default" do + expect(email.hard_bounce_count).to eq(0) + end + + it "has nil last_ses_event_at by default" do + expect(email.last_ses_event_at).to be_nil + end + end + + describe "ses_delivery_events association" do + it "destroys associated events when the email address is destroyed" do + event = SesDeliveryEvent.create!( + email_address: email, + sns_message_id: "sns-1", + event_type: "Delivery", + recipient_email: email.email, + event_occurred_at: Time.current + ) + expect { email.destroy! }.to change(SesDeliveryEvent, :count).by(-1) + expect(SesDeliveryEvent.where(id: event.id)).to be_empty + end end end end diff --git a/spec/models/ses_delivery_event_spec.rb b/spec/models/ses_delivery_event_spec.rb new file mode 100644 index 00000000..26fcc115 --- /dev/null +++ b/spec/models/ses_delivery_event_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe SesDeliveryEvent, type: :model do + fixtures :all + + let(:email) { email_addresses(:validated_teacher_email) } + + def build_event(overrides = {}) + SesDeliveryEvent.new({ + email_address: email, + sns_message_id: "sns-#{SecureRandom.hex(4)}", + event_type: "Delivery", + recipient_email: email.email, + event_occurred_at: Time.current, + payload: { any: "json" } + }.merge(overrides)) + end + + describe "validations" do + it "is valid with all required attributes" do + expect(build_event).to be_valid + end + + it "requires sns_message_id" do + event = build_event(sns_message_id: nil) + expect(event).not_to be_valid + expect(event.errors[:sns_message_id]).to be_present + end + + it "requires event_type" do + event = build_event(event_type: nil) + expect(event).not_to be_valid + expect(event.errors[:event_type]).to be_present + end + + it "requires recipient_email" do + event = build_event(recipient_email: nil) + expect(event).not_to be_valid + expect(event.errors[:recipient_email]).to be_present + end + + it "requires event_occurred_at" do + event = build_event(event_occurred_at: nil) + expect(event).not_to be_valid + expect(event.errors[:event_occurred_at]).to be_present + end + + it "allows nil email_address (unknown recipient)" do + event = build_event(email_address: nil) + expect(event).to be_valid + end + end + + describe "uniqueness on (sns_message_id, recipient_email)" do + it "rejects a duplicate row at the DB level" do + attrs = { + sns_message_id: "sns-dup-1", + recipient_email: email.email, + event_type: "Delivery", + event_occurred_at: Time.current + } + SesDeliveryEvent.create!(attrs.merge(email_address: email)) + + expect { + SesDeliveryEvent.create!(attrs.merge(email_address: email)) + }.to raise_error(ActiveRecord::RecordNotUnique) + end + + it "allows the same sns_message_id for different recipients" do + SesDeliveryEvent.create!( + email_address: email, + sns_message_id: "sns-multi", + event_type: "Delivery", + recipient_email: email.email, + event_occurred_at: Time.current + ) + + other = email_addresses(:bob_email) + expect { + SesDeliveryEvent.create!( + email_address: other, + sns_message_id: "sns-multi", + event_type: "Delivery", + recipient_email: other.email, + event_occurred_at: Time.current + ) + }.not_to raise_error + end + end + + describe "payload" do + it "defaults to an empty hash" do + event = SesDeliveryEvent.create!( + email_address: email, + sns_message_id: "sns-payload", + event_type: "Send", + recipient_email: email.email, + event_occurred_at: Time.current + ) + expect(event.reload.payload).to eq({}) + end + + it "persists arbitrary JSON content" do + event = SesDeliveryEvent.create!( + email_address: email, + sns_message_id: "sns-payload-2", + event_type: "Bounce", + bounce_type: "Permanent", + recipient_email: email.email, + event_occurred_at: Time.current, + payload: { "bounce" => { "bounceType" => "Permanent" } } + ) + expect(event.reload.payload).to eq("bounce" => { "bounceType" => "Permanent" }) + end + end +end From 76b8b517ba6c63029613164f265169dfca207cb8 Mon Sep 17 00:00:00 2001 From: Kien Huynh <143785443+kienthuynh@users.noreply.github.com> Date: Tue, 28 Apr 2026 19:53:02 -0700 Subject: [PATCH 41/41] Add SnsMessageVerifier service with SNS signature verification and topic allowlist --- .env.example | 1 + Gemfile | 3 + Gemfile.lock | 4 ++ app/services/sns_message_verifier.rb | 46 +++++++++++++ config/initializers/sns_webhook.rb | 7 ++ spec/services/sns_message_verifier_spec.rb | 76 ++++++++++++++++++++++ 6 files changed, 137 insertions(+) create mode 100644 app/services/sns_message_verifier.rb create mode 100644 config/initializers/sns_webhook.rb create mode 100644 spec/services/sns_message_verifier_spec.rb diff --git a/.env.example b/.env.example index 61542eca..714d611a 100644 --- a/.env.example +++ b/.env.example @@ -21,6 +21,7 @@ S3_REGION=us-west-2 S3_SECRET_ACCESS_KEY=[NEW ONE] SECRET_KEY_BASE=[NEW ONE] MAILBLUSTER_API_KEY=[NEW ONE] +SNS_ALLOWED_TOPIC_ARNS=arn:aws:sns:us-west-2:123456789012:ses-events SENTRY_DSN= SNAP_CLIENT_SECRET='ignore' SNAP_CLIENT_URL='https://forum.snap.berkeley.edu/session/sso_provider' diff --git a/Gemfile b/Gemfile index 33c131c0..a42c1b8b 100644 --- a/Gemfile +++ b/Gemfile @@ -40,6 +40,9 @@ gem "liquid" # Store uploaded files gem "aws-sdk-s3", require: false +# Verify signatures on SNS webhook deliveries +gem "aws-sdk-sns", "~> 1", require: false + # Render images for file uploads in pages gem "image_processing", ">= 1.2" diff --git a/Gemfile.lock b/Gemfile.lock index 4056a055..464f9cda 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -106,6 +106,9 @@ GEM aws-sdk-core (~> 3, >= 3.127.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.4) + aws-sdk-sns (1.55.0) + aws-sdk-core (~> 3, >= 3.127.0) + aws-sigv4 (~> 1.1) aws-sigv4 (1.4.0) aws-eventstream (~> 1, >= 1.0.2) axe-core-api (4.3.2) @@ -569,6 +572,7 @@ DEPENDENCIES activerecord-import annotate aws-sdk-s3 + aws-sdk-sns (~> 1) axe-core-cucumber axe-core-rspec bootsnap (>= 1.4.4) diff --git a/app/services/sns_message_verifier.rb b/app/services/sns_message_verifier.rb new file mode 100644 index 00000000..85dffce4 --- /dev/null +++ b/app/services/sns_message_verifier.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "aws-sdk-sns" + +class SnsMessageVerifier + class VerificationError < StandardError; end + + def self.verify!(raw_body) + new(raw_body).verify! + end + + def initialize(raw_body) + @raw_body = raw_body + end + + def verify! + envelope = parse_envelope + assert_topic_allowed!(envelope["TopicArn"]) + assert_signature_valid!(envelope) + envelope + end + + private + attr_reader :raw_body + + def parse_envelope + JSON.parse(raw_body) + rescue JSON::ParserError => e + raise VerificationError, "Invalid JSON: #{e.message}" + end + + def assert_topic_allowed!(topic_arn) + return if topic_arn.present? && allowed_topic_arns.include?(topic_arn) + raise VerificationError, "Topic ARN #{topic_arn.inspect} is not in the allowlist" + end + + def assert_signature_valid!(envelope) + Aws::SNS::MessageVerifier.new.authenticate!(envelope.to_json) + rescue Aws::SNS::MessageVerifier::VerificationError => e + raise VerificationError, "Signature verification failed: #{e.message}" + end + + def allowed_topic_arns + (ENV["SNS_ALLOWED_TOPIC_ARNS"] || "").split(",").map(&:strip).reject(&:empty?) + end +end diff --git a/config/initializers/sns_webhook.rb b/config/initializers/sns_webhook.rb new file mode 100644 index 00000000..6b358434 --- /dev/null +++ b/config/initializers/sns_webhook.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Rails.application.config.after_initialize do + if ENV["SNS_ALLOWED_TOPIC_ARNS"].to_s.strip.empty? && !Rails.env.test? + Rails.logger.warn("[SNS Webhook] SNS_ALLOWED_TOPIC_ARNS is not set. The SES webhook will reject all incoming messages.") + end +end diff --git a/spec/services/sns_message_verifier_spec.rb b/spec/services/sns_message_verifier_spec.rb new file mode 100644 index 00000000..fa878b4c --- /dev/null +++ b/spec/services/sns_message_verifier_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe SnsMessageVerifier, type: :service do + let(:topic_arn) { "arn:aws:sns:us-west-2:123456789012:ses-events" } + let(:other_topic_arn) { "arn:aws:sns:us-west-2:123456789012:other-topic" } + + let(:envelope) do + { + "Type" => "Notification", + "MessageId" => "abc-123", + "TopicArn" => topic_arn, + "Message" => '{"eventType":"Delivery"}', + "Timestamp" => "2026-04-23T10:00:00.000Z", + "SignatureVersion" => "1", + "Signature" => "placeholder", + "SigningCertURL" => "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-xxx.pem" + } + end + + let(:raw_body) { envelope.to_json } + let(:verifier_double) { instance_double(Aws::SNS::MessageVerifier) } + + before do + allow(Aws::SNS::MessageVerifier).to receive(:new).and_return(verifier_double) + allow(verifier_double).to receive(:authenticate!).and_return(true) + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("SNS_ALLOWED_TOPIC_ARNS").and_return(topic_arn) + end + + describe ".verify!" do + it "returns the parsed envelope when signature is valid and topic is allowlisted" do + expect(described_class.verify!(raw_body)).to eq(envelope) + end + + it "raises VerificationError when the body is not valid JSON" do + expect { described_class.verify!("not json") } + .to raise_error(SnsMessageVerifier::VerificationError, /invalid json/i) + end + + it "raises VerificationError when the signature is invalid" do + allow(verifier_double).to receive(:authenticate!) + .and_raise(Aws::SNS::MessageVerifier::VerificationError.new("bad sig")) + + expect { described_class.verify!(raw_body) } + .to raise_error(SnsMessageVerifier::VerificationError, /signature/i) + end + + it "raises VerificationError when TopicArn is not in the allowlist" do + envelope["TopicArn"] = other_topic_arn + expect { described_class.verify!(envelope.to_json) } + .to raise_error(SnsMessageVerifier::VerificationError, /topic/i) + end + + it "supports a comma-separated allowlist" do + allow(ENV).to receive(:[]).with("SNS_ALLOWED_TOPIC_ARNS") + .and_return("#{other_topic_arn},#{topic_arn}") + + expect(described_class.verify!(raw_body)).to eq(envelope) + end + + it "trims whitespace in the allowlist entries" do + allow(ENV).to receive(:[]).with("SNS_ALLOWED_TOPIC_ARNS") + .and_return(" #{topic_arn} , #{other_topic_arn} ") + + expect(described_class.verify!(raw_body)).to eq(envelope) + end + + it "raises VerificationError when the allowlist is empty" do + allow(ENV).to receive(:[]).with("SNS_ALLOWED_TOPIC_ARNS").and_return("") + expect { described_class.verify!(raw_body) } + .to raise_error(SnsMessageVerifier::VerificationError, /topic/i) + end + end +end