Skip to content

Commit

Permalink
Remove email_address from provider model
Browse files Browse the repository at this point in the history
  • Loading branch information
Jamie authored and JamieCleare2525 committed Jan 16, 2025
1 parent 763c0cc commit afe172d
Show file tree
Hide file tree
Showing 34 changed files with 126 additions and 58 deletions.
2 changes: 1 addition & 1 deletion app/mailers/claims/provider_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class Claims::ProviderMailer < Claims::ApplicationMailer
def sampling_checks_required(provider_sampling)
@provider_sampling = provider_sampling

notify_email to: @provider_sampling.provider_email_address,
notify_email to: @provider_sampling.provider_email_addresses,
subject: t(".subject"),
body: t(
".body",
Expand Down
4 changes: 2 additions & 2 deletions app/mailers/placements/school_user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def partnership_created_notification(user, source_organisation, partner_organisa
@user_name = user.first_name
@school_name = partner_organisation.name
@provider_name = source_organisation.name
@provider_email_address = source_organisation.email_address
@provider_email_address = source_organisation.primary_email_address
@sign_in_url = sign_in_url
@service_name = service_name

Expand All @@ -20,7 +20,7 @@ def partnership_destroyed_notification(user, source_organisation, partner_organi
@user_name = user.first_name
@school_name = partner_organisation.name
@provider_name = source_organisation.name
@provider_email_address = source_organisation.email_address
@provider_email_address = source_organisation.primary_email_address
placements_school = partner_organisation.becomes(Placements::School)
@placements = placements_school.placements.where(provider: source_organisation).decorate.map do |placement|
{ title: placement.title, url: placements_school_placement_url(placements_school, placement) }
Expand Down
1 change: 0 additions & 1 deletion app/models/claims/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down
2 changes: 1 addition & 1 deletion app/models/claims/provider_sampling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Claims::ProviderSampling < ApplicationRecord

scope :order_by_provider_name, -> { joins(:provider).order(providers: { name: :asc }) }

delegate :email_address, :name, to: :provider, prefix: true
delegate :email_addresses, :name, to: :provider, prefix: true

def downloaded?
downloaded_at.present?
Expand Down
1 change: 0 additions & 1 deletion app/models/placements/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down
5 changes: 4 additions & 1 deletion app/models/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down Expand Up @@ -63,6 +62,10 @@ class Provider < ApplicationRecord

accepts_nested_attributes_for :provider_email_addresses, allow_destroy: true

def primary_email_address
provider_email_addresses.primary.last.email_address
end

def email_addresses
provider_email_addresses.pluck(:email_address).uniq
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/provider_email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
#
# id :uuid not null, primary key
# email_address :string
# primary :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
# provider_id :uuid not null
#
# Indexes
#
# index_provider_email_addresses_on_primary (primary)
# index_provider_email_addresses_on_provider_id (provider_id)
# unique_provider_email (email_address,provider_id) UNIQUE
#
Expand All @@ -22,4 +24,6 @@ class ProviderEmailAddress < ApplicationRecord

validates :email_address, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP }
validates :email_address, uniqueness: { scope: :provider_id, case_sensitive: false }

scope :primary, -> { where(primary: true).order(created_at: :desc) }
end
6 changes: 6 additions & 0 deletions app/models/school.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,10 @@ def address
def primary?
phase == PRIMARY_PHASE
end

def email_addresses
return [] if email_address.nil?

[email_address]
end
end
2 changes: 1 addition & 1 deletion app/services/publish_teacher_training/provider/importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ def fetch_providers(link = nil)
provider_type: provider_attributes["provider_type"],
ukprn: provider_attributes["ukprn"],
urn: provider_attributes["urn"],
email_address: provider_attributes["email"], # TODO: Remove once 'email_address' is removed from providers tables
telephone: provider_attributes["telephone"],
website: provider_attributes["website"],
address1: provider_attributes["street_address_1"],
Expand Down Expand Up @@ -74,6 +73,7 @@ def upsert_emails_attributes
emails_attributes << {
email_address: record[:email_address],
provider_id: provider_by_code(record[:code]).id,
primary: true,
}
end
emails_attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
<% end %>
<% summary_list.with_row do |row| %>
<% row.with_key(text: t(".email_address")) %>
<% if organisation.email_address.present? %>
<% if organisation.email_addresses.present? %>
<% row.with_value do %>
<%= govuk_mail_to(organisation.email_address, organisation.email_address) %>
<% organisation.email_addresses.each do |email_address| %>
<%= govuk_mail_to(email_address, email_address) %>
<% end %>
<% end %>
<% else %>
<% row.with_value(**summary_row_value) %>
Expand Down
1 change: 1 addition & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,4 @@ shared:
- provider_id
- created_at
- updated_at
- primary
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RemoveEmailAddressFromProviders < ActiveRecord::Migration[7.2]
def change
safety_assured { remove_column :providers, :email_address, :string }
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddPrimaryToProviderEmailAddresses < ActiveRecord::Migration[7.2]
disable_ddl_transaction!

def change
add_column :provider_email_addresses, :primary, :boolean, default: false
add_index :provider_email_addresses, :primary, algorithm: :concurrently
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class AssignPrimaryTrueToExistingProviderEmailAddresses < ActiveRecord::Migration[7.2]
def up
ProviderEmailAddress.update_all(primary: true)
end

def down
raise ActiveRecord::IrreversibleMigration
end
end
5 changes: 3 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2025_01_14_161953) do
ActiveRecord::Schema[7.2].define(version: 2025_01_16_091921) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
enable_extension "plpgsql"
Expand Down Expand Up @@ -377,7 +377,9 @@
t.uuid "provider_id", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.boolean "primary", default: false
t.index ["email_address", "provider_id"], name: "unique_provider_email", unique: true
t.index ["primary"], name: "index_provider_email_addresses_on_primary"
t.index ["provider_id"], name: "index_provider_email_addresses_on_provider_id"
end

Expand Down Expand Up @@ -409,7 +411,6 @@
t.string "name", default: "", null: false
t.string "ukprn"
t.string "urn"
t.string "email_address"
t.string "telephone"
t.string "website"
t.string "address1"
Expand Down
2 changes: 2 additions & 0 deletions spec/factories/provider_email_addresses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
#
# id :uuid not null, primary key
# email_address :string
# primary :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
# provider_id :uuid not null
#
# Indexes
#
# index_provider_email_addresses_on_primary (primary)
# index_provider_email_addresses_on_provider_id (provider_id)
# unique_provider_email (email_address,provider_id) UNIQUE
#
Expand Down
21 changes: 20 additions & 1 deletion spec/factories/providers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down Expand Up @@ -65,6 +64,26 @@
trait :niot do
name { "NIoT: National Institute of Teaching, founded by the School-Led Development Trust" }
end

transient do
email_addresses { [] }
end

after(:create) do |provider, evaluator|
if evaluator.email_addresses.present?
evaluator.email_addresses.each_with_index do |email_address, i|
create(:provider_email_address,
email_address:,
provider:,
primary: i.zero?)
end
else
create(:provider_email_address,
email_address: "#{provider.name.split(" ").join("_").downcase.gsub(/[^a-zA-Z0-9-]_/, "")}@example.com",
provider:,
primary: true)
end
end
end

factory :claims_provider, class: "Claims::Provider", parent: :provider
Expand Down
4 changes: 2 additions & 2 deletions spec/mailers/claims/provider_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
describe "#sampling_checks_required" do
subject(:sampling_checks_required_email) { described_class.sampling_checks_required(provider_sampling) }

let(:provider) { build(:claims_provider, email_address: "[email protected]") }
let(:provider) { create(:claims_provider) }
let(:provider_sampling) { create(:provider_sampling, provider:) }
let(:url_for_csv) { "https://example.com" }
let(:service_name) { "Claim funding for mentor training" }
Expand All @@ -15,7 +15,7 @@
end

it "sends the sampling checks required email" do
expect(sampling_checks_required_email.to).to contain_exactly(provider.email_address)
expect(sampling_checks_required_email.to).to match_array(provider.email_addresses)
expect(sampling_checks_required_email.subject).to eq("ITT mentor claims need to be assured")
expect(sampling_checks_required_email.body.to_s.squish).to eq(<<~EMAIL.squish)
Dear #{provider.name},
Expand Down
10 changes: 5 additions & 5 deletions spec/mailers/placements/school_user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
described_class.partnership_created_notification(user, source_organisation, partner_organisation)
end

let(:source_organisation) { create(:placements_provider, name: "Provider 1", email_address: "[email protected]") }
let(:source_organisation) { create(:placements_provider, name: "Provider 1") }
let(:partner_organisation) { create(:placements_school, name: "School 1") }
let(:user) { create(:placements_user, schools: [partner_organisation]) }

Expand All @@ -109,7 +109,7 @@
## What happens next?
You can now assign them to your placements.
Contact the provider on [#{source_organisation.email_address}](mailto:#{source_organisation.email_address}) if you have any questions.
Contact the provider on [#{source_organisation.email_addresses.first}](mailto:#{source_organisation.email_addresses.first}) if you have any questions.
## Your account
[Sign in to Manage school placements](http://placements.localhost/sign-in)
Expand All @@ -124,7 +124,7 @@
described_class.partnership_destroyed_notification(user, source_organisation, partner_organisation)
end

let(:source_organisation) { create(:placements_provider, name: "Provider 1", email_address: "[email protected]") }
let(:source_organisation) { create(:placements_provider, name: "Provider 1") }
let(:partner_organisation) { create(:placements_school, name: "School 1") }

let(:user) { create(:placements_user, schools: [partner_organisation]) }
Expand All @@ -143,7 +143,7 @@
## What happens next?
You will no longer be able to assign placements to this provider unless they add you again or you add them to your list of providers.
If you think this is a mistake, contact them on [#{source_organisation.email_address}](mailto:#{source_organisation.email_address}).
If you think this is a mistake, contact them on [#{source_organisation.email_addresses.first}](mailto:#{source_organisation.email_addresses.first}).
## Your account
[Sign in to Manage school placements](http://placements.localhost/sign-in)
Expand Down Expand Up @@ -171,7 +171,7 @@
- [#{placement.decorate.title}](http://placements.localhost/schools/#{partner_organisation.id}/placements/#{placement.id})
We recommend you speak to the provider to avoid confusion about placing trainees at your school. Contact them on [#{source_organisation.email_address}](mailto:#{source_organisation.email_address}).
We recommend you speak to the provider to avoid confusion about placing trainees at your school. Contact them on [#{source_organisation.email_addresses.first}](mailto:#{source_organisation.email_addresses.first}).
## Your account
[Sign in to Manage school placements](http://placements.localhost/sign-in)
Expand Down
2 changes: 1 addition & 1 deletion spec/models/claims/provider_sampling_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
end

describe "delegations" do
it { is_expected.to delegate_method(:email_address).to(:provider).with_prefix }
it { is_expected.to delegate_method(:email_addresses).to(:provider).with_prefix }
it { is_expected.to delegate_method(:name).to(:provider).with_prefix }
end

Expand Down
1 change: 0 additions & 1 deletion spec/models/claims/provider_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down
1 change: 0 additions & 1 deletion spec/models/placements/provider_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down
2 changes: 2 additions & 0 deletions spec/models/provider_email_address_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
#
# id :uuid not null, primary key
# email_address :string
# primary :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
# provider_id :uuid not null
#
# Indexes
#
# index_provider_email_addresses_on_primary (primary)
# index_provider_email_addresses_on_provider_id (provider_id)
# unique_provider_email (email_address,provider_id) UNIQUE
#
Expand Down
1 change: 0 additions & 1 deletion spec/models/provider_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# city :string
# code :string not null
# county :string
# email_address :string
# name :string default(""), not null
# placements_service :boolean default(FALSE)
# postcode :string
Expand Down
20 changes: 20 additions & 0 deletions spec/models/school_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,24 @@
expect(school.primary?).to be(false)
end
end

describe "#email_addresses" do
subject(:email_addresses) { school.email_addresses }

context "when the school does not have an email address" do
let(:school) { create(:school) }

it "returns an empty array" do
expect(email_addresses).to eq([])
end
end

context "when the school does have an email address" do
let(:school) { create(:school, email_address: "[email protected]") }

it "returns an array containing the school's email address" do
expect(email_addresses).to contain_exactly("[email protected]")
end
end
end
end
Loading

0 comments on commit afe172d

Please sign in to comment.