diff --git a/.env.example b/.env.example index 9c409a6a..714d611a 100644 --- a/.env.example +++ b/.env.example @@ -20,6 +20,8 @@ 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] +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/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 diff --git a/app/controllers/email_addresses_controller.rb b/app/controllers/email_addresses_controller.rb index 998ad717..5b152754 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(", ") @@ -28,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/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 35516ea9..dd1b7a76 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -10,10 +10,10 @@ 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] + 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 @@ -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? @@ -173,6 +187,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 @@ -185,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 @@ -208,6 +226,34 @@ 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 + + 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. #{result[:error]}" + 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 diff --git a/app/helpers/teacher_helper.rb b/app/helpers/teacher_helper.rb index 122b159b..c31e6a5a 100644 --- a/app/helpers/teacher_helper.rb +++ b/app/helpers/teacher_helper.rb @@ -13,7 +13,23 @@ def ip_history_display(teacher) end def email_address_label(email) - return unless email.primary? - '  primary'.html_safe + labels = [] + labels << '' if email.primary? + labels << '' if email.bounced? + return nil if labels.empty? + labels.join("").html_safe + end + + def mailbluster_sync_status(teacher) + if teacher.mailbluster_synced? + 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 end end 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/models/email_address.rb b/app/models/email_address.rb index 7bf8dc0d..1313be12 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -4,12 +4,18 @@ # # 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 +# 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 # @@ -23,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 } @@ -31,6 +38,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? 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/app/models/teacher.rb b/app/models/teacher.rb index f7994961..2a726dad 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) @@ -311,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) diff --git a/app/services/mailbluster_service.rb b/app/services/mailbluster_service.rb new file mode 100644 index 00000000..26c09044 --- /dev/null +++ b/app/services/mailbluster_service.rb @@ -0,0 +1,223 @@ +# 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. + # Includes a small delay between requests to avoid rate limiting. + 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 + + # 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}" + end + + results + end + + # Sync a single teacher to MailBluster. + # Returns a hash with :success and optionally :error. + def sync_teacher(teacher) + lead_data = create_or_update_lead(teacher) + 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. + 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:, + body: body.to_json + ) + end + + def get(path) + HTTParty.get( + "#{BASE_URL}#{path}", + headers: + ) + end + + def delete(path) + HTTParty.delete( + "#{BASE_URL}#{path}", + 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 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/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 %> <% end %> +<% show_mailbluster = local_assigns.fetch(:show_mailbluster, true) %> @@ -8,3 +9,6 @@ +<% if show_mailbluster %> + +<% end %> diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index efdec2c5..5920a3ba 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -1,6 +1,7 @@ <% if merge_table %> <% end %> +<% show_mailbluster = local_assigns.fetch(:show_mailbluster, true) %> +<% if show_mailbluster %> + +<% 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:
diff --git a/app/views/teachers/index.html.erb b/app/views/teachers/index.html.erb index d6dddae3..7dc5bf95 100644 --- a/app/views/teachers/index.html.erb +++ b/app/views/teachers/index.html.erb @@ -17,13 +17,13 @@
Actions
<%= button_to("✔️", validate_teacher_path(teacher.id), diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb index f28faf8a..210bfeb1 100644 --- a/app/views/teachers/_table_headers.erb +++ b/app/views/teachers/_table_headers.erb @@ -1,6 +1,7 @@ <% if include_id %>
IDName Email RoleSchool Approved? CreatedMB Sync<%= teacher.id %> <% if merge_table %> <%= link_to teacher.full_name, preview_merge_path(@teacher.id, teacher.id), method: :get %> @@ -10,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 %> @@ -45,3 +47,6 @@ 		<%= teacher.created_at.strftime("%m/%d/%Y") %> <%= mailbluster_sync_status(teacher) %>
- <%= 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 %> @@ -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?" } %> +
+
+
+<% 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 %> +
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 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/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 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/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 20434184..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_02_21_120000) 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" @@ -60,6 +60,12 @@ 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.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" @@ -134,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" @@ -154,9 +176,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" @@ -169,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/db/seed_data.rb b/db/seed_data.rb index d68c0885..c8c10134 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,173 @@ 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: "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 +442,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 diff --git a/features/mailbluster_sync.feature b/features/mailbluster_sync.feature new file mode 100644 index 00000000..41ed9d80 --- /dev/null +++ b/features/mailbluster_sync.feature @@ -0,0 +1,73 @@ +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 | + | 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 + 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 "MailBluster Sync" + And I should see "Not synced" + 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 + 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 a button named "Sync All to MailBluster" + + 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 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 + 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" 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 diff --git a/spec/controllers/email_addresses_controller_spec.rb b/spec/controllers/email_addresses_controller_spec.rb index 95880990..59de396c 100644 --- a/spec/controllers/email_addresses_controller_spec.rb +++ b/spec/controllers/email_addresses_controller_spec.rb @@ -28,3 +28,61 @@ 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 + + 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 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/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index f7b9c5b3..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") @@ -91,6 +105,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 } @@ -342,4 +366,129 @@ 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({ success: 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({ success: false, error: "API error" }) + + 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 + + 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 diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb index 57e34dc9..65465c1f 100644 --- a/spec/factories/email_addresses.rb +++ b/spec/factories/email_addresses.rb @@ -4,12 +4,18 @@ # # 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 +# 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/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..fc762f21 100644 --- a/spec/fixtures/email_addresses.yml +++ b/spec/fixtures/email_addresses.yml @@ -2,12 +2,18 @@ # # 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 +# 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/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/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 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 diff --git a/spec/models/email_address_spec.rb b/spec/models/email_address_spec.rb new file mode 100644 index 00000000..ab4a4437 --- /dev/null +++ b/spec/models/email_address_spec.rb @@ -0,0 +1,132 @@ +# 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 +# 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 +# +# 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 + 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 + + 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/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 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 diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index 6a224df5..dd31ca24 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) @@ -131,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 diff --git a/spec/services/mailbluster_service_spec.rb b/spec/services/mailbluster_service_spec.rb new file mode 100644 index 00000000..eac1545f --- /dev/null +++ b/spec/services/mailbluster_service_spec.rb @@ -0,0 +1,259 @@ +# 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 success hash on successful sync" do + allow(HTTParty).to receive(:post).and_return(success_response) + + result = described_class.sync_teacher(teacher) + expect(result[:success]).to be true + end + + it "returns failure hash on failed sync" do + allow(HTTParty).to receive(:post).and_return(failure_response) + + 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 + + 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 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