-
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
base: main
Are you sure you want to change the base?
Changes from all commits
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,48 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class SchoolEmailDomain | ||
| class Create | ||
| class << self | ||
| def call(school:, domain:, token:) | ||
| response = OperationResponse.new | ||
| response[:school_email_domain] = build_domain(school, domain) | ||
| SchoolEmailDomain.transaction do | ||
| response[:school_email_domain].save! | ||
| update_profile(school, token) | ||
| end | ||
| response | ||
| rescue ActiveRecord::RecordInvalid => e | ||
| record = response[:school_email_domain] || e.record | ||
|
|
||
| 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
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| response[:error_code] = 'profile_sync_failed' | ||
|
PetarSimonovic marked this conversation as resolved.
Dismissed
|
||
| 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:) | ||
|
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. Concurrent creates desync ProfileMedium Severity Under concurrent domain creation, Reviewed by Cursor Bugbot for commit e24d6af. Configure here. |
||
| 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,179 @@ | ||
| # 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 | ||
|
|
||
| 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.