-
Notifications
You must be signed in to change notification settings - Fork 5
868 add school email domain api #878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
f5a8796
2cf9498
337551c
b51fcfb
27cc95f
dddfb10
528a359
e180140
71e4b63
d4e13e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Api | ||
| class SchoolEmailDomainsController < ApiController | ||
| before_action :authorize_user | ||
| load_and_authorize_resource :school | ||
| authorize_resource :school_email_domain, class: false | ||
|
|
||
| def index | ||
| render json: school_email_domains, status: :ok | ||
| end | ||
|
|
||
| def create | ||
| result = SchoolEmailDomain::Create.call(school: @school, domain: school_email_domain_params[:domain], token: current_user.token) | ||
| if result.success? | ||
| render json: { domain: result[:school_email_domain].domain }, status: :created | ||
| else | ||
| render json: { error: result[:error], error_code: result[:error_code] }, status: :unprocessable_content | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def school_email_domains | ||
| @school.school_email_domains.order(:created_at).pluck(:domain) | ||
| end | ||
|
|
||
| def school_email_domain_params | ||
| params.expect(school_email_domain: [:domain]) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class SchoolEmailDomain | ||
| class Create | ||
| class << self | ||
| def call(school:, domain:, token:) | ||
| response = OperationResponse.new | ||
| response[:school_email_domain] = nil | ||
| response[:school_email_domain] = build_domain(school, domain) | ||
| school.with_lock do | ||
| response[:school_email_domain].save! | ||
| update_profile(school, token) | ||
| end | ||
| response | ||
| rescue ActiveRecord::RecordInvalid => e | ||
| record = response[:school_email_domain] || e.record | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth fixing this after all - I don't expect it to be a problem but it's a trivial change. It's similar to what's been done here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made this changed and initialised with |
||
| response[:error] = record.errors.full_messages.join(', ') | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| response[:error_code] = domain_error_code(record) | ||
|
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| response | ||
| rescue ActiveRecord::RecordNotUnique | ||
| record = response[:school_email_domain] | ||
|
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| record.errors.add(:domain, :taken) | ||
| response[:error] = record.errors.full_messages.join(', ') | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| response[:error_code] = 'taken' | ||
|
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| response | ||
| rescue StandardError => e | ||
| Sentry.capture_exception(e) # Send unexpected/Profile errors to Sentry | ||
| response[:error] = e.message | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
github-code-quality[bot] marked this conversation as resolved.
Fixed
|
||
| response[:error_code] = 'profile_sync_failed' | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
|
||
| response | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def build_domain(school, domain) | ||
| school.school_email_domains.build(domain:) | ||
| end | ||
|
|
||
| def update_profile(school, token) | ||
| school_email_domains = school.school_email_domains.order(:created_at).pluck(:domain) | ||
| ProfileApiClient.update_school_email_domains(token:, school_id: school.id, school_email_domains:) | ||
|
PetarSimonovic marked this conversation as resolved.
|
||
| end | ||
|
PetarSimonovic marked this conversation as resolved.
|
||
|
|
||
| def domain_error_code(record) | ||
| record.errors.details[:domain].first.fetch(:error).to_s | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe SchoolEmailDomain::Create, type: :unit do | ||
| let(:school) { create(:school) } | ||
| let(:domain) { 'school.edu' } | ||
| let(:token) { UserProfileMock::TOKEN } | ||
|
|
||
| before { stub_profile_api_update_school_email_domains } | ||
|
|
||
| context 'with valid values' do | ||
| it 'returns a successful operation response' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response.success?).to be(true) | ||
| end | ||
|
|
||
| it 'creates a school email domain' do | ||
| expect { described_class.call(school:, domain:, token:) }.to change(SchoolEmailDomain, :count).by(1) | ||
| end | ||
|
|
||
| it 'returns the domain in the operation response' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response[:school_email_domain]).to be_a(SchoolEmailDomain) | ||
| end | ||
|
|
||
| it 'assigns the domain' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response[:school_email_domain].domain).to eq(domain) | ||
| end | ||
|
|
||
| it 'assigns the school' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response[:school_email_domain].school_id).to eq(school.id) | ||
| end | ||
|
|
||
| it 'syncs the domains to Profile' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(ProfileApiClient).to have_received(:update_school_email_domains).with( | ||
| token:, | ||
| school_id: school.id, | ||
| school_email_domains: [domain] | ||
| ) | ||
| end | ||
|
|
||
| it 'locks the school while creating and syncing to Profile' do | ||
| allow(school).to receive(:with_lock).and_call_original | ||
|
|
||
| described_class.call(school:, domain:, token:) | ||
|
|
||
| expect(school).to have_received(:with_lock) | ||
| end | ||
|
|
||
| context 'when multiple domains already exist' do | ||
| before do | ||
| create(:school_email_domain, school:, domain: 'first.edu') | ||
| create(:school_email_domain, school:, domain: 'second.edu') | ||
| create(:school_email_domain, school:, domain: 'third.edu') | ||
| end | ||
|
|
||
| it 'syncs all domains to Profile' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(ProfileApiClient).to have_received(:update_school_email_domains).with( | ||
| token:, | ||
| school_id: school.id, | ||
| school_email_domains: ['first.edu', 'second.edu', 'third.edu', domain] | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| shared_examples 'an invalid record' do | ||
| before { allow(Sentry).to receive(:capture_exception) } | ||
|
|
||
| it 'does not create a school email domain' do | ||
| expect { described_class.call(school:, domain:, token:) }.not_to change(SchoolEmailDomain, :count) | ||
| end | ||
|
|
||
| it 'returns a failed operation response' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response.failure?).to be(true) | ||
| end | ||
|
|
||
| it 'does not send the exception to Sentry' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(Sentry).not_to have_received(:capture_exception).with(kind_of(StandardError)) | ||
| end | ||
|
|
||
| it 'returns the error code in the operation response' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response[:error_code]).to eq(expected_error_code) | ||
| end | ||
|
|
||
| it 'does not attempt to update Profile' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(ProfileApiClient).not_to have_received(:update_school_email_domains) | ||
| end | ||
| end | ||
|
|
||
| context 'when domain is blank' do | ||
| let(:domain) { '' } | ||
| let(:expected_error_code) { 'blank' } | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when domain is not an FQDN' do | ||
| let(:domain) { 'edu' } | ||
| let(:expected_error_code) { 'invalid_host' } | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when domain has an invalid URI' do | ||
| let(:domain) { 'exa mple.com' } | ||
| let(:expected_error_code) { 'invalid_uri' } | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when domain has an invalid public suffix' do | ||
| let(:domain) { 'co.uk' } | ||
| let(:expected_error_code) { 'invalid_public_suffix' } | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when domain is a duplicate' do | ||
| before { create(:school_email_domain, school:, domain:) } | ||
|
|
||
| let(:expected_error_code) { 'taken' } | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when a concurrent request creates the same domain' do | ||
| let(:expected_error_code) { 'taken' } | ||
| let(:school_email_domain) { SchoolEmailDomain.new(school:, domain:) } | ||
|
|
||
| before do | ||
| allow(Sentry).to receive(:capture_exception) | ||
| allow(school.school_email_domains).to receive(:build).with(domain:).and_return(school_email_domain) | ||
| allow(school_email_domain).to receive(:save!).and_raise(ActiveRecord::RecordNotUnique) | ||
| end | ||
|
|
||
| it_behaves_like 'an invalid record' | ||
| end | ||
|
|
||
| context 'when Profile sync fails' do | ||
| let(:profile_error) do | ||
| ProfileApiClient::UnexpectedResponse.new( | ||
| instance_double(Faraday::Response, status: 500, headers: {}, body: '') | ||
| ) | ||
| end | ||
|
|
||
| before do | ||
| allow(Sentry).to receive(:capture_exception) | ||
|
|
||
| allow(ProfileApiClient).to receive(:update_school_email_domains) | ||
| .and_raise(profile_error) | ||
| end | ||
|
|
||
| it 'attempts to sync to Profile' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(ProfileApiClient).to have_received(:update_school_email_domains).once | ||
| end | ||
|
|
||
| it 'does not persist the domain' do | ||
| expect { described_class.call(school:, domain:, token:) } | ||
| .not_to change(SchoolEmailDomain, :count) | ||
| end | ||
|
|
||
| it 'sends the exception to Sentry' do | ||
| described_class.call(school:, domain:, token:) | ||
| expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) | ||
| end | ||
|
|
||
| it 'returns a failed operation response' do | ||
| expect(described_class.call(school:, domain:, token:)).to be_failure | ||
| end | ||
|
|
||
| it 'returns the error code in the operation response' do | ||
| response = described_class.call(school:, domain:, token:) | ||
| expect(response[:error_code]).to eq('profile_sync_failed') | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| FactoryBot.define do | ||
| factory :school_email_domain do | ||
| school | ||
| sequence(:domain) { |n| "domain#{n}.example.edu" } | ||
| end | ||
| end |
Uh oh!
There was an error while loading. Please reload this page.