Skip to content

Commit

Permalink
Fix more mapper problems related to new attributes (#1162)
Browse files Browse the repository at this point in the history
Co-authored-by: pskl <[email protected]>
  • Loading branch information
tnicolas1 and pskl authored Oct 21, 2024
1 parent bdb8211 commit 857e278
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 38 deletions.
6 changes: 3 additions & 3 deletions app/apis/students_api/sygne/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ module Sygne
class Api < StudentsApi::Base
class << self
def establishment_students_endpoint(params)
base_url +
format("etablissements/%s/eleves?statut=ST&annee-scolaire=#{SchoolYear.current.start_year}&etat-scolarisation=true", # rubocop:disable Layout/LineLength
params[:uai])
query = { statut: "ST", "annee-scolaire": params[:school_year], "etat-scolarisation": true }.to_query

base_url + format("etablissements/%s/eleves?#{query}", params[:uai])
end

def student_endpoint(params)
Expand Down
4 changes: 2 additions & 2 deletions app/apis/students_api/sygne/mappers/classe_mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ class ClasseMapper < Dry::Transformer::Pipe
define! do
deep_symbolize_keys

rename_keys(codeMefRatt: :mef_code, classe: :label)
rename_keys(codeMefRatt: :mef_code, classe: :label, anneeScolaire: :year)

map_value(:mef_code, Dry::Transformer::Coercions[:to_string])

map_value :mef_code, ->(value) { value.chop }

accept_keys %i[label mef_code]
accept_keys %i[label mef_code year]
end
end
end
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/schoolings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
class SchoolingsController < ApplicationController
include RoleCheck

before_action :set_schooling, only: [:update]
before_action :authenticate_user!, :set_classe, :set_schooling
before_action :check_director, :update_confirmed_director!, :check_confirmed_director,
only: %i[abrogate_decision update]
Expand All @@ -27,7 +26,7 @@ def confirm_removal; end
def confirm_removal_cancellation; end

def remove
@schooling.update(removed_at: params[:value])
@schooling.update!(removed_at: params[:value])

redirect_to school_year_class_path(selected_school_year, @classe),
notice: t("flash.schooling.removed", name: @schooling.student, classe: @schooling.classe.label)
Expand All @@ -53,7 +52,7 @@ def update # rubocop:disable Metrics/AbcSize
def set_schooling
@schooling = Schooling.find(params[:id])
rescue ActiveRecord::RecordNotFound
redirect_to @classe, alert: t("errors.schoolings.not_found")
redirect_to school_year_classes_path(selected_school_year), alert: t("errors.schoolings.not_found")
end

def schooling_params
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def choose_redirect_page!
end

def fetch_students_for!(establishments)
ActiveJob.perform_all_later(establishments.map { |e| Sync::ClassesJob.new(e) })
ActiveJob.perform_all_later(establishments.map { |e| Sync::ClassesJob.new(e, selected_school_year) })
end

def fetch_establishments!
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/sync/classes_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ClassesJob < ApplicationJob
establishment.update!(fetching_students: false)
end

def perform(establishment)
def perform(establishment, school_year)
# NOTE: there is a bug in Sygne where students are removed from classes
# earlier than they should be so we disable student list fetching for now
# (only in production because we want to keep our tests intact)
Expand All @@ -28,7 +28,7 @@ def perform(establishment)
api = establishment.students_api

api
.fetch_resource(:establishment_students, uai: establishment.uai)
.fetch_resource(:establishment_students, uai: establishment.uai, school_year: school_year.start_year)
.then { |data| api.mapper.new(data, establishment.uai).parse! }
end
end
Expand Down
2 changes: 2 additions & 0 deletions app/models/student/mappers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def parse!
begin
map_schooling!(classe, student, entry)
rescue StandardError => e
raise e unless Rails.env.production?

Sentry.capture_exception(
SchoolingParsingError.new(
"Schooling parsing failed for #{uai}: #{e.message}"
Expand Down
2 changes: 1 addition & 1 deletion config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ fr:
1CAP1: CAP en 1 an
1CAP2: 1ère année de CAP
1CAP2A: 1ère année de CAP agricole
2CAP2: 2ène année de CAP
2CAP2: 2ème année de CAP
2CAP2A: 2ème année de CAP agricole
2CAP3: 2ème année CAP en 3 ans
3CAP3: 3ème année de CAP en 3 ans
Expand Down
2 changes: 1 addition & 1 deletion features/step_definitions/api_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
end

Sachantque("les élèves de l'établissement {string} sont rafraîchis") do |uai|
Sync::ClassesJob.new(Establishment.find_by(uai: uai)).perform_now
Sync::ClassesJob.new(Establishment.find_by(uai: uai), SchoolYear.current).perform_now
end

Sachantque("l'API SYGNE renvoie un élève avec l'INE {string} qui a quitté l'établissement {string}") do |ine, uai|
Expand Down
2 changes: 1 addition & 1 deletion features/step_definitions/job_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
end

Quand("la liste des élèves de l'établissement {string} est rafraîchie") do |uai|
Sync::ClassesJob.perform_later(Establishment.find_by(uai: uai))
Sync::ClassesJob.perform_later(Establishment.find_by(uai: uai), SchoolYear.current)
end

# NOTE: pas très élégant mais comme le job parent
Expand Down
8 changes: 4 additions & 4 deletions spec/apis/students_api/sygne/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@

before do
mock_sygne_token
mock_sygne_students_endpoint("007", {}.to_json)
mock_sygne_students_endpoint("007", {}.to_json, 2022)
mock_sygne_student_endpoint_with("007", {}.to_json)
mock_sygne_student_schoolings_endpoint("123", {}.to_json)
end

describe "endpoints" do
specify "establishment students endpoint" do
expect(api.establishment_students_endpoint(uai: "007")).to(
expect(api.establishment_students_endpoint(uai: "007", school_year: 2022)).to(
end_with(
"etablissements/007/eleves?statut=ST&annee-scolaire=#{SchoolYear.current.start_year}&etat-scolarisation=true"
"etablissements/007/eleves?annee-scolaire=2022&etat-scolarisation=true&statut=ST"
)
)
end
Expand All @@ -32,7 +32,7 @@
end

[
[:establishment_students, { uai: "007" }],
[:establishment_students, { uai: "007", school_year: 2022 }],
[:student, { ine: "007" }],
[:student_schoolings, { ine: "007" }]
].each do |resource, params|
Expand Down
11 changes: 7 additions & 4 deletions spec/jobs/sync/classes_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
include ActiveJob::TestHelper

let(:establishment) { create(:establishment, :sygne_provider) }
let(:school_year) { SchoolYear.current }
let(:api_double) { class_double(StudentsApi::Sygne::Api) }
let(:mapper_double) { instance_double(Student::Mappers::Sygne) }

before do
allow(StudentsApi).to receive(:api_for).with("sygne").and_return(api_double)

allow(api_double).to receive(:fetch_resource).with(:establishment_students, uai: establishment.uai)
allow(api_double).to receive(:fetch_resource).with(:establishment_students,
uai: establishment.uai,
school_year: school_year.start_year)
allow(api_double)
.to receive(:mapper)
.and_return(class_double(Student::Mappers::Sygne, new: mapper_double))
Expand All @@ -21,13 +24,13 @@
end

it "calls the matchingStudentsApi proxy" do
described_class.perform_now(establishment)
described_class.perform_now(establishment, school_year)

expect(StudentsApi).to have_received(:api_for).with("sygne")
end

it "maps and parse the results" do
described_class.perform_now(establishment)
described_class.perform_now(establishment, school_year)

expect(mapper_double).to have_received(:parse!)
end
Expand All @@ -39,7 +42,7 @@

it "rescues and retry" do
perform_enqueued_jobs do
described_class.perform_now(establishment)
described_class.perform_now(establishment, school_year)
rescue Faraday::UnauthorizedError # rubocop:disable Lint/SuppressedException
end

Expand Down
29 changes: 16 additions & 13 deletions spec/models/asp/payment_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,36 +166,39 @@
end

describe "eligible_for_rejected_or_unpaid_auto_retry?" do
let(:p_r_rejected) { create(:asp_payment_request, :rejected) }
let(:p_r_rejected_rib) do
create(:asp_payment_request, :rejected, reason: "Test d'une raison de blocage d'un paiement bancaire")
end
let(:p_r_unpaid) { create(:asp_payment_request, :unpaid) }
let(:p_r_unpaid_rib) do
create(:asp_payment_request, :unpaid, reason: "Test d'une raison de blocage d'un paiement bancaire")
end

context "when the payment request is in 'rejected' state without a RIB reason" do
let(:p_r) { create(:asp_payment_request, :rejected) }

it "returns false" do
expect(p_r_rejected.eligible_for_rejected_or_unpaid_auto_retry?).to be false
expect(p_r.eligible_for_rejected_or_unpaid_auto_retry?).to be false
end
end

context "when the payment request is in 'rejected' state with a RIB reason" do
let(:p_r) do
create(:asp_payment_request, :rejected, reason: "Test d'une raison de blocage d'un paiement bancaire")
end

it "returns true" do
expect(p_r_rejected_rib.eligible_for_rejected_or_unpaid_auto_retry?).to be true
expect(p_r.eligible_for_rejected_or_unpaid_auto_retry?).to be true
end
end

context "when the payment request is in 'unpaid' state without a RIB reason" do
let(:p_r) { create(:asp_payment_request, :unpaid) }

it "returns false" do
expect(p_r_unpaid.eligible_for_rejected_or_unpaid_auto_retry?).to be false
expect(p_r.eligible_for_rejected_or_unpaid_auto_retry?).to be false
end
end

context "when the payment request is in 'unpaid' state with a RIB reason" do
let(:p_r) do
create(:asp_payment_request, :unpaid, reason: "Test d'une raison de blocage d'un paiement bancaire")
end

it "returns true" do
expect(p_r_unpaid_rib.eligible_for_rejected_or_unpaid_auto_retry?).to be true
expect(p_r.eligible_for_rejected_or_unpaid_auto_retry?).to be true
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/support/webmock_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ def mock_sygne_student_schoolings_endpoint(ine, payload)
.to_return(status: 200, body: payload, headers: { "Content-Type" => "application/json" })
end

def mock_sygne_students_endpoint(uai, payload)
url = StudentsApi::Sygne::Api.establishment_students_endpoint(uai: uai)
def mock_sygne_students_endpoint(uai, payload, school_year = SchoolYear.current.start_year)
url = StudentsApi::Sygne::Api.establishment_students_endpoint(uai: uai, school_year: school_year)

WebMock.stub_request(:get, url)
.with(
query: { "etat-scolarisation" => true },
query: { "etat-scolarisation" => true, "annee-scolaire" => school_year },
headers: {
"Accept" => "*/*",
"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3",
Expand Down

0 comments on commit 857e278

Please sign in to comment.