From 71f1e785ab712ff7d5d6edf1cff7f3a82f291d0d Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Mon, 13 Feb 2023 11:10:14 +0100 Subject: [PATCH 1/6] let annuaire education champ be prefillable --- app/models/type_de_champ.rb | 1 + spec/models/prefill_params_spec.rb | 4 +++- spec/models/type_de_champ_spec.rb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 35da0363d..9fd75e087 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -274,6 +274,7 @@ class TypeDeChamp < ApplicationRecord TypeDeChamp.type_champs.fetch(:repetition), TypeDeChamp.type_champs.fetch(:multiple_drop_down_list), TypeDeChamp.type_champs.fetch(:epci), + TypeDeChamp.type_champs.fetch(:annuaire_education), TypeDeChamp.type_champs.fetch(:dossier_link), TypeDeChamp.type_champs.fetch(:siret), TypeDeChamp.type_champs.fetch(:rna) diff --git a/spec/models/prefill_params_spec.rb b/spec/models/prefill_params_spec.rb index 12876fd52..872cdb5f0 100644 --- a/spec/models/prefill_params_spec.rb +++ b/spec/models/prefill_params_spec.rb @@ -140,6 +140,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ public value that is authorized", :drop_down_list, "value" it_behaves_like "a champ public value that is authorized", :departements, "03" it_behaves_like "a champ public value that is authorized", :communes, ['01', '01457'] + it_behaves_like "a champ public value that is authorized", :annuaire_education, "0050009H" it_behaves_like "a champ public value that is authorized", :multiple_drop_down_list, ["val1", "val2"] it_behaves_like "a champ public value that is authorized", :dossier_link, "1" it_behaves_like "a champ public value that is authorized", :epci, ['01', '200042935'] @@ -182,6 +183,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ private value that is authorized", :siret, "13002526500013" it_behaves_like "a champ private value that is authorized", :departements, "03" it_behaves_like "a champ private value that is authorized", :communes, ['01', '01457'] + it_behaves_like "a champ private value that is authorized", :annuaire_education, "0050009H" it_behaves_like "a champ private value that is authorized", :multiple_drop_down_list, ["val1", "val2"] it_behaves_like "a champ private value that is authorized", :dossier_link, "1" it_behaves_like "a champ private value that is authorized", :epci, ['01', '200042935'] @@ -223,7 +225,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ public value that is unauthorized", :regions, "value" it_behaves_like "a champ public value that is unauthorized", :departements, "value" it_behaves_like "a champ public value that is unauthorized", :communes, "value" - it_behaves_like "a champ public value that is unauthorized", :annuaire_education, "value" + # TODO: SEB validation it_behaves_like "a champ public value that is unauthorized", :annuaire_education, "value" it_behaves_like "a champ public value that is unauthorized", :multiple_drop_down_list, ["value"] context "when the public type de champ is unauthorized because of wrong value format (repetition)" do diff --git a/spec/models/type_de_champ_spec.rb b/spec/models/type_de_champ_spec.rb index d8f9a82d5..d6d364759 100644 --- a/spec/models/type_de_champ_spec.rb +++ b/spec/models/type_de_champ_spec.rb @@ -253,6 +253,7 @@ describe TypeDeChamp do it_behaves_like "a prefillable type de champ", :type_de_champ_checkbox it_behaves_like "a prefillable type de champ", :type_de_champ_drop_down_list it_behaves_like "a prefillable type de champ", :type_de_champ_repetition + it_behaves_like "a prefillable type de champ", :type_de_champ_annuaire_education it_behaves_like "a prefillable type de champ", :type_de_champ_multiple_drop_down_list it_behaves_like "a prefillable type de champ", :type_de_champ_epci it_behaves_like "a prefillable type de champ", :type_de_champ_dossier_link @@ -271,7 +272,6 @@ describe TypeDeChamp do it_behaves_like "a non-prefillable type de champ", :type_de_champ_mesri it_behaves_like "a non-prefillable type de champ", :type_de_champ_carte it_behaves_like "a non-prefillable type de champ", :type_de_champ_address - it_behaves_like "a non-prefillable type de champ", :type_de_champ_annuaire_education end describe '#normalize_libelle' do From 4f4f4baffdd18b38b68478f75a7b7f841ad30e2e Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Wed, 15 Feb 2023 10:42:48 +0100 Subject: [PATCH 2/6] add possible and example values --- config/locales/en.yml | 4 +++- config/locales/fr.yml | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 936f34209..6b42db237 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -148,7 +148,8 @@ en: rna_html: A RNA number repetition_html: A array of hashes with possible values for each field of the repetition. epci_html: An array of the department code and the EPCI one. - dossier_link_html: The file ID, as integer. + annuaire_education_html: An educational institution code, as defined by the Éducation Nationale directory + dossier_link_html: The file ID, as integer examples: title: Example text: Short text @@ -165,6 +166,7 @@ en: date: "2023-02-01" datetime: "2023-02-01T10:30" checkbox: "true" + annuaire_education: "0561383Z" dossier_link: 42 rna: "W503726238" siret: "13002526500013" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 218632afc..f333df668 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -139,7 +139,8 @@ fr: siret_html: Un numéro de SIRET repetition_html: Un tableau de dictionnaires avec les valeurs possibles pour chaque champ de la répétition. epci_html: Un tableau contenant le code de département et celui de l'EPCI. - dossier_link_html: L'identifiant du dossier, sous forme de nombre entier. + annuaire_education_html: Un code d'établissement scolaire, tel que défini par l'Annuaire de l'Éducation Nationale + dossier_link_html: L'identifiant du dossier, sous forme de nombre entier examples: title: Exemple text: Texte court @@ -157,6 +158,7 @@ fr: date: "2023-02-01" datetime: "2023-02-01T10:30" checkbox: "true" + annuaire_education: "0561383Z" dossier_link: 42 rna: "W503726238" siret: "13002526500013" From d7bfb9dc181130fb506de1193f0874a9cb301268 Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Wed, 15 Feb 2023 15:25:15 +0100 Subject: [PATCH 3/6] enrich value from education api --- ...refill_annuaire_education_type_de_champ.rb | 16 ++++++ .../types_de_champ/prefill_type_de_champ.rb | 2 + .../annuaire_education_empty.json | 14 +++++ .../annuaire_education_adapter_spec.rb | 9 +++ ...l_annuaire_education_type_de_champ_spec.rb | 56 +++++++++++++++++++ .../prefill_type_de_champ_spec.rb | 6 ++ 6 files changed, 103 insertions(+) create mode 100644 app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb create mode 100644 spec/fixtures/files/api_education/annuaire_education_empty.json create mode 100644 spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb diff --git a/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb b/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb new file mode 100644 index 000000000..afd256e05 --- /dev/null +++ b/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp < TypesDeChamp::PrefillTypeDeChamp + def to_assignable_attributes(champ, value) + return nil if value.blank? + return nil unless (data = APIEducation::AnnuaireEducationAdapter.new(value.presence).to_params) + + { + id: champ.id, + external_id: data['identifiant_de_l_etablissement'], + value: "#{data['nom_etablissement']}, #{data['nom_commune']} (#{data['identifiant_de_l_etablissement']})" + } + rescue APIEducation::AnnuaireEducationAdapter::InvalidSchemaError + nil + end +end diff --git a/app/models/types_de_champ/prefill_type_de_champ.rb b/app/models/types_de_champ/prefill_type_de_champ.rb index 34dee1206..0dbadbb3c 100644 --- a/app/models/types_de_champ/prefill_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_type_de_champ.rb @@ -27,6 +27,8 @@ class TypesDeChamp::PrefillTypeDeChamp < SimpleDelegator TypesDeChamp::PrefillCommuneTypeDeChamp.new(type_de_champ, revision) when TypeDeChamp.type_champs.fetch(:epci) TypesDeChamp::PrefillEpciTypeDeChamp.new(type_de_champ, revision) + when TypeDeChamp.type_champs.fetch(:annuaire_education) + TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp.new(type_de_champ, revision) else new(type_de_champ, revision) end diff --git a/spec/fixtures/files/api_education/annuaire_education_empty.json b/spec/fixtures/files/api_education/annuaire_education_empty.json new file mode 100644 index 000000000..34e44e091 --- /dev/null +++ b/spec/fixtures/files/api_education/annuaire_education_empty.json @@ -0,0 +1,14 @@ +{ + "nhits": 0, + "parameters": { + "dataset": "fr-en-annuaire-education", + "rows": 1, + "start": 0, + "refine": { + "identifiant_de_l_etablissement": "0341247" + }, + "format": "json", + "timezone": "UTC" + }, + "records": [] +} diff --git a/spec/lib/api_education/annuaire_education_adapter_spec.rb b/spec/lib/api_education/annuaire_education_adapter_spec.rb index 2b29c9b7e..eff7bd789 100644 --- a/spec/lib/api_education/annuaire_education_adapter_spec.rb +++ b/spec/lib/api_education/annuaire_education_adapter_spec.rb @@ -38,4 +38,13 @@ describe APIEducation::AnnuaireEducationAdapter do expect { subject }.to raise_exception(APIEducation::AnnuaireEducationAdapter::InvalidSchemaError) end end + + context "when responds with empty schema" do + let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education_empty.json') } + let(:status) { 200 } + + it '#to_params returns nil' do + expect(subject).to eq(nil) + end + end end diff --git a/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb new file mode 100644 index 000000000..e7bba2828 --- /dev/null +++ b/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp do + let(:procedure) { create(:procedure) } + let(:type_de_champ) { build(:type_de_champ_annuaire_education, procedure: procedure) } + + describe 'ancestors' do + subject { described_class.new(type_de_champ, procedure.active_revision) } + + it { is_expected.to be_kind_of(TypesDeChamp::PrefillTypeDeChamp) } + end + + describe '#to_assignable_attributes' do + let(:champ) { create(:champ_annuaire_education, type_de_champ: type_de_champ) } + subject { described_class.build(type_de_champ, procedure.active_revision).to_assignable_attributes(champ, value) } + + context 'when the value is nil' do + let(:value) { nil } + + it { is_expected.to eq(nil) } + end + + context 'when the value is empty' do + let(:value) { '' } + + it { is_expected.to eq(nil) } + end + + context 'when the value is present' do + let(:value) { '0050009H' } + + before do + stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) + .to_return(body: body, status: 200) + end + + context 'when the annuaire education api responds with a valid schema' do + let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education.json') } + + it { is_expected.to match({ id: champ.id, external_id: '0050009H', value: 'Lycée professionnel Sévigné, Gap (0050009H)' }) } + end + + context "when the annuaire education api responds with invalid schema" do + let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education_invalid.json') } + + it { is_expected.to eq(nil) } + end + + context 'when the annuaire education api responds with empty schema' do + let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education_empty.json') } + + it { is_expected.to eq(nil) } + end + end + end +end diff --git a/spec/models/types_de_champ/prefill_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_type_de_champ_spec.rb index 844115889..1d8e135e5 100644 --- a/spec/models/types_de_champ/prefill_type_de_champ_spec.rb +++ b/spec/models/types_de_champ/prefill_type_de_champ_spec.rb @@ -57,6 +57,12 @@ RSpec.describe TypesDeChamp::PrefillTypeDeChamp, type: :model do it { expect(built).to be_kind_of(TypesDeChamp::PrefillEpciTypeDeChamp) } end + context 'when the type de champ is an annuaire_education' do + let(:type_de_champ) { build(:type_de_champ_annuaire_education) } + + it { expect(built).to be_kind_of(TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp) } + end + context 'when any other type de champ' do let(:type_de_champ) { build(:type_de_champ_date, procedure: procedure) } From 447ea1b9a8c9df174c73c78e02ca75109f9a5b19 Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Thu, 16 Feb 2023 14:11:36 +0100 Subject: [PATCH 4/6] don't prefill with an incorrect value --- spec/models/prefill_params_spec.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/spec/models/prefill_params_spec.rb b/spec/models/prefill_params_spec.rb index 872cdb5f0..4764a1255 100644 --- a/spec/models/prefill_params_spec.rb +++ b/spec/models/prefill_params_spec.rb @@ -6,6 +6,7 @@ RSpec.describe PrefillParams do let(:dossier) { create(:dossier, :brouillon, procedure: procedure) } let(:types_de_champ_public) { [] } let(:types_de_champ_private) { [] } + let(:api_annuaire_education_body) { File.read('spec/fixtures/files/api_education/annuaire_education.json') } subject(:prefill_params_array) { described_class.new(dossier, params).to_a } @@ -17,6 +18,11 @@ RSpec.describe PrefillParams do VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis') + + stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/).to_return( + body: api_annuaire_education_body, + status: 200 + ) end after do @@ -225,7 +231,6 @@ RSpec.describe PrefillParams do it_behaves_like "a champ public value that is unauthorized", :regions, "value" it_behaves_like "a champ public value that is unauthorized", :departements, "value" it_behaves_like "a champ public value that is unauthorized", :communes, "value" - # TODO: SEB validation it_behaves_like "a champ public value that is unauthorized", :annuaire_education, "value" it_behaves_like "a champ public value that is unauthorized", :multiple_drop_down_list, ["value"] context "when the public type de champ is unauthorized because of wrong value format (repetition)" do @@ -251,6 +256,12 @@ RSpec.describe PrefillParams do expect(prefill_params_array).to match([]) end end + + context "when the annuaire education can't find the school for the given value" do + let(:api_annuaire_education_body) { File.read('spec/fixtures/files/api_education/annuaire_education_empty.json') } + + it_behaves_like "a champ public value that is unauthorized", :annuaire_education, "value" + end end private From 8b25503f7e0109e9b6f1949ff6b45f59fad35531 Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Thu, 16 Feb 2023 14:26:05 +0100 Subject: [PATCH 5/6] feature spec cover --- spec/support/shared_examples_for_prefilled_dossier.rb | 1 + spec/system/users/dossier_prefill_get_spec.rb | 8 +++++++- spec/system/users/dossier_prefill_post_spec.rb | 8 +++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/spec/support/shared_examples_for_prefilled_dossier.rb b/spec/support/shared_examples_for_prefilled_dossier.rb index 3a85cac6f..a13538e4c 100644 --- a/spec/support/shared_examples_for_prefilled_dossier.rb +++ b/spec/support/shared_examples_for_prefilled_dossier.rb @@ -26,5 +26,6 @@ shared_examples "the user has got a prefilled dossier, owned by themselves" do expect(page).to have_field(type_de_champ_epci.libelle, with: epci_value.last) expect(page).to have_field(type_de_champ_dossier_link.libelle, with: dossier_link_value) expect(page).to have_selector("input[value='Vonnas (01540)']") + expect(page).to have_content(annuaire_education_value.last) end end diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index 9dbd69b46..3fe04c9f0 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -13,6 +13,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do let(:type_de_champ_datetime) { create(:type_de_champ_datetime, procedure: procedure) } let(:type_de_champ_multiple_drop_down_list) { create(:type_de_champ_multiple_drop_down_list, procedure: procedure) } let(:type_de_champ_epci) { create(:type_de_champ_epci, procedure: procedure) } + let(:type_de_champ_annuaire_education) { create(:type_de_champ_annuaire_education, procedure: procedure) } let(:type_de_champ_dossier_link) { create(:type_de_champ_dossier_link, procedure: procedure) } let(:type_de_champ_commune) { create(:type_de_champ_communes, procedure: procedure) } let(:type_de_champ_repetition) { create(:type_de_champ_repetition, :with_types_de_champ, procedure: procedure) } @@ -36,6 +37,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do let(:integer_repetition_libelle) { sub_type_de_champs_repetition.second.libelle } let(:text_repetition_value) { "First repetition text" } let(:integer_repetition_value) { "42" } + let(:annuaire_education_value) { '0050009H' } let(:entry_path) { commencer_path( @@ -54,7 +56,8 @@ describe 'Prefilling a dossier (with a GET request):', js: true do "champ_#{sub_type_de_champs_repetition.first.to_typed_id_for_query}": text_repetition_value, "champ_#{sub_type_de_champs_repetition.second.to_typed_id_for_query}": integer_repetition_value } - ] + ], + "champ_#{type_de_champ_annuaire_education.to_typed_id_for_query}" => annuaire_education_value ) } @@ -71,6 +74,9 @@ describe 'Prefilling a dossier (with a GET request):', js: true do stub_request(:get, /https:\/\/entreprise.api.gouv.fr\/v2\/associations\//) .to_return(status: 200, body: File.read('spec/fixtures/files/api_entreprise/associations.json')) + stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) + .to_return(status: 200, body: File.read('spec/fixtures/files/api_education/annuaire_education.json')) + VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis') diff --git a/spec/system/users/dossier_prefill_post_spec.rb b/spec/system/users/dossier_prefill_post_spec.rb index b9e3d6a9c..9c2c21d1e 100644 --- a/spec/system/users/dossier_prefill_post_spec.rb +++ b/spec/system/users/dossier_prefill_post_spec.rb @@ -13,6 +13,7 @@ describe 'Prefilling a dossier (with a POST request):', js: true do let(:type_de_champ_datetime) { create(:type_de_champ_datetime, procedure: procedure) } let(:type_de_champ_multiple_drop_down_list) { create(:type_de_champ_multiple_drop_down_list, procedure: procedure) } let(:type_de_champ_epci) { create(:type_de_champ_epci, procedure: procedure) } + let(:type_de_champ_annuaire_education) { create(:type_de_champ_annuaire_education, procedure: procedure) } let(:type_de_champ_dossier_link) { create(:type_de_champ_dossier_link, procedure: procedure) } let(:type_de_champ_repetition) { create(:type_de_champ_repetition, :with_types_de_champ, procedure: procedure) } let(:type_de_champ_commune) { create(:type_de_champ_communes, procedure: procedure) } @@ -36,6 +37,7 @@ describe 'Prefilling a dossier (with a POST request):', js: true do let(:text_repetition_value) { "First repetition text" } let(:integer_repetition_value) { "42" } let(:dossier_link_value) { '42' } + let(:annuaire_education_value) { '0050009H' } before do allow(Rails).to receive(:cache).and_return(memory_store) @@ -50,6 +52,9 @@ describe 'Prefilling a dossier (with a POST request):', js: true do stub_request(:get, /https:\/\/entreprise.api.gouv.fr\/v2\/associations\//) .to_return(status: 200, body: File.read('spec/fixtures/files/api_entreprise/associations.json')) + stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) + .to_return(status: 200, body: File.read('spec/fixtures/files/api_education/annuaire_education.json')) + VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis') @@ -160,7 +165,8 @@ describe 'Prefilling a dossier (with a POST request):', js: true do "champ_#{type_de_champ_multiple_drop_down_list.to_typed_id_for_query}" => multiple_drop_down_list_values, "champ_#{type_de_champ_epci.to_typed_id_for_query}" => epci_value, "champ_#{type_de_champ_dossier_link.to_typed_id_for_query}" => dossier_link_value, - "champ_#{type_de_champ_commune.to_typed_id_for_query}" => commune_value + "champ_#{type_de_champ_commune.to_typed_id_for_query}" => commune_value, + "champ_#{type_de_champ_annuaire_education.to_typed_id_for_query}" => annuaire_education_value }.to_json JSON.parse(session.response.body)["dossier_url"].gsub("http://www.example.com", "") end From f52554b5a3f7ee2fb96e29e15a2a0c8f98312267 Mon Sep 17 00:00:00 2001 From: sebastiencarceles Date: Tue, 28 Feb 2023 13:55:52 +0100 Subject: [PATCH 6/6] review: update value with async fetch 1 - spec cover the job which fetches external data 2 - refactor the job with guard clauses 3 - delegate update operation to the champ itself 4 - annuaire education: override the update operation to let the value be populated by the fetched data 5 - prefilling: don't fetch data synchronously --- app/jobs/champ_fetch_external_data_job.rb | 10 ++-- app/models/champ.rb | 4 ++ app/models/champs/annuaire_education_champ.rb | 11 ++++ ...refill_annuaire_education_type_de_champ.rb | 7 +-- .../champ_fetch_external_data_job_spec.rb | 55 +++++++++++++++++++ spec/models/champ_spec.rb | 8 +++ .../champs/annuaire_education_champ_spec.rb | 42 ++++++++++++++ spec/models/prefill_params_spec.rb | 12 ---- ...l_annuaire_education_type_de_champ_spec.rb | 26 +-------- spec/system/users/dossier_prefill_get_spec.rb | 3 - .../system/users/dossier_prefill_post_spec.rb | 3 - 11 files changed, 127 insertions(+), 54 deletions(-) create mode 100644 spec/jobs/champ_fetch_external_data_job_spec.rb create mode 100644 spec/models/champs/annuaire_education_champ_spec.rb diff --git a/app/jobs/champ_fetch_external_data_job.rb b/app/jobs/champ_fetch_external_data_job.rb index 366586dda..790052863 100644 --- a/app/jobs/champ_fetch_external_data_job.rb +++ b/app/jobs/champ_fetch_external_data_job.rb @@ -1,11 +1,9 @@ class ChampFetchExternalDataJob < ApplicationJob def perform(champ, external_id) - if champ.external_id == external_id && champ.data.nil? - data = champ.fetch_external_data + return if champ.external_id != external_id + return if champ.data.present? + return if (data = champ.fetch_external_data).blank? - if data.present? - champ.update!(data: data) - end - end + champ.update_with_external_data!(data: data) end end diff --git a/app/models/champ.rb b/app/models/champ.rb index c82a3db8f..cadb9e395 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -217,6 +217,10 @@ class Champ < ApplicationRecord raise NotImplemented.new(:fetch_external_data) end + def update_with_external_data!(data:) + update!(data: data) + end + def clone champ_attributes = [:parent_id, :private, :row_id, :type, :type_de_champ_id] value_attributes = private? ? [] : [:value, :value_json, :data, :external_id] diff --git a/app/models/champs/annuaire_education_champ.rb b/app/models/champs/annuaire_education_champ.rb index ffd064d1f..46a3ee389 100644 --- a/app/models/champs/annuaire_education_champ.rb +++ b/app/models/champs/annuaire_education_champ.rb @@ -28,4 +28,15 @@ class Champs::AnnuaireEducationChamp < Champs::TextChamp def fetch_external_data APIEducation::AnnuaireEducationAdapter.new(external_id).to_params end + + def update_with_external_data!(data:) + if data&.is_a?(Hash) && data['nom_etablissement'].present? && data['nom_commune'].present? && data['identifiant_de_l_etablissement'].present? + update!( + data: data, + value: "#{data['nom_etablissement']}, #{data['nom_commune']} (#{data['identifiant_de_l_etablissement']})" + ) + else + update!(data: data) + end + end end diff --git a/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb b/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb index afd256e05..7790458d9 100644 --- a/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_annuaire_education_type_de_champ.rb @@ -3,14 +3,11 @@ class TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp < TypesDeChamp::PrefillTypeDeChamp def to_assignable_attributes(champ, value) return nil if value.blank? - return nil unless (data = APIEducation::AnnuaireEducationAdapter.new(value.presence).to_params) { id: champ.id, - external_id: data['identifiant_de_l_etablissement'], - value: "#{data['nom_etablissement']}, #{data['nom_commune']} (#{data['identifiant_de_l_etablissement']})" + external_id: value, + value: value } - rescue APIEducation::AnnuaireEducationAdapter::InvalidSchemaError - nil end end diff --git a/spec/jobs/champ_fetch_external_data_job_spec.rb b/spec/jobs/champ_fetch_external_data_job_spec.rb new file mode 100644 index 000000000..6f5277727 --- /dev/null +++ b/spec/jobs/champ_fetch_external_data_job_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ChampFetchExternalDataJob, type: :job do + let(:champ) { Struct.new(:external_id, :data).new(champ_external_id, data) } + let(:external_id) { "an ID" } + let(:champ_external_id) { "an ID" } + let(:data) { nil } + let(:fetched_data) { nil } + + subject(:perform_job) { described_class.perform_now(champ, external_id) } + + before do + allow(champ).to receive(:fetch_external_data).and_return(fetched_data) + allow(champ).to receive(:update_with_external_data!) + end + + shared_examples "a champ non-updater" do + it 'does not update the champ' do + perform_job + expect(champ).not_to have_received(:update_with_external_data!) + end + end + + context 'when external_id matches the champ external_id and the champ data is nil' do + it 'fetches external data' do + perform_job + expect(champ).to have_received(:fetch_external_data) + end + + context 'when the fetched data is present' do + let(:fetched_data) { "data" } + + it 'updates the champ' do + perform_job + expect(champ).to have_received(:update_with_external_data!).with(data: fetched_data) + end + end + + context 'when the fetched data is blank' do + it_behaves_like "a champ non-updater" + end + end + + context 'when external_id does not match the champ external_id' do + let(:champ_external_id) { "something else" } + it_behaves_like "a champ non-updater" + end + + context 'when the champ data is present' do + let(:data) { "present" } + it_behaves_like "a champ non-updater" + end +end diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index 2d63881f7..e20c55417 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -602,4 +602,12 @@ describe Champ do end end end + + describe '#update_with_external_data!' do + let(:champ) { create(:champ_siret) } + let(:data) { "data" } + subject { champ.update_with_external_data!(data: data) } + + it { expect { subject }.to change { champ.reload.data }.to(data) } + end end diff --git a/spec/models/champs/annuaire_education_champ_spec.rb b/spec/models/champs/annuaire_education_champ_spec.rb new file mode 100644 index 000000000..8c8f58be5 --- /dev/null +++ b/spec/models/champs/annuaire_education_champ_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Champs::AnnuaireEducationChamp do + describe '#update_with_external_data!' do + let(:champ) { create(:champ_annuaire_education, data: "any data") } + subject { champ.update_with_external_data!(data: data) } + + shared_examples "a data updater (without updating the value)" do |data| + it { expect { subject }.to change { champ.reload.data }.to(data) } + it { expect { subject }.not_to change { champ.reload.value } } + end + + context 'when data is nil' do + let(:data) { nil } + it_behaves_like "a data updater (without updating the value)", nil + end + + context 'when data is empty' do + let(:data) { '' } + it_behaves_like "a data updater (without updating the value)", '' + end + + context 'when data is inconsistent' do + let(:data) { { 'yo' => 'lo' } } + it_behaves_like "a data updater (without updating the value)", { 'yo' => 'lo' } + end + + context 'when data is consistent' do + let(:data) { + { + 'nom_etablissement': "karrigel an ankou", + 'nom_commune' => 'kumun', + 'identifiant_de_l_etablissement' => '666667' + }.with_indifferent_access + } + it { expect { subject }.to change { champ.reload.data }.to(data) } + it { expect { subject }.to change { champ.reload.value }.to('karrigel an ankou, kumun (666667)') } + end + end +end diff --git a/spec/models/prefill_params_spec.rb b/spec/models/prefill_params_spec.rb index 4764a1255..ccea47657 100644 --- a/spec/models/prefill_params_spec.rb +++ b/spec/models/prefill_params_spec.rb @@ -6,7 +6,6 @@ RSpec.describe PrefillParams do let(:dossier) { create(:dossier, :brouillon, procedure: procedure) } let(:types_de_champ_public) { [] } let(:types_de_champ_private) { [] } - let(:api_annuaire_education_body) { File.read('spec/fixtures/files/api_education/annuaire_education.json') } subject(:prefill_params_array) { described_class.new(dossier, params).to_a } @@ -18,11 +17,6 @@ RSpec.describe PrefillParams do VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis') - - stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/).to_return( - body: api_annuaire_education_body, - status: 200 - ) end after do @@ -256,12 +250,6 @@ RSpec.describe PrefillParams do expect(prefill_params_array).to match([]) end end - - context "when the annuaire education can't find the school for the given value" do - let(:api_annuaire_education_body) { File.read('spec/fixtures/files/api_education/annuaire_education_empty.json') } - - it_behaves_like "a champ public value that is unauthorized", :annuaire_education, "value" - end end private diff --git a/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb index e7bba2828..e79629698 100644 --- a/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb +++ b/spec/models/types_de_champ/prefill_annuaire_education_type_de_champ_spec.rb @@ -16,41 +16,17 @@ RSpec.describe TypesDeChamp::PrefillAnnuaireEducationTypeDeChamp do context 'when the value is nil' do let(:value) { nil } - it { is_expected.to eq(nil) } end context 'when the value is empty' do let(:value) { '' } - it { is_expected.to eq(nil) } end context 'when the value is present' do let(:value) { '0050009H' } - - before do - stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) - .to_return(body: body, status: 200) - end - - context 'when the annuaire education api responds with a valid schema' do - let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education.json') } - - it { is_expected.to match({ id: champ.id, external_id: '0050009H', value: 'Lycée professionnel Sévigné, Gap (0050009H)' }) } - end - - context "when the annuaire education api responds with invalid schema" do - let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education_invalid.json') } - - it { is_expected.to eq(nil) } - end - - context 'when the annuaire education api responds with empty schema' do - let(:body) { File.read('spec/fixtures/files/api_education/annuaire_education_empty.json') } - - it { is_expected.to eq(nil) } - end + it { is_expected.to match({ id: champ.id, external_id: '0050009H', value: '0050009H' }) } end end end diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index 3fe04c9f0..206ea1f87 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -74,9 +74,6 @@ describe 'Prefilling a dossier (with a GET request):', js: true do stub_request(:get, /https:\/\/entreprise.api.gouv.fr\/v2\/associations\//) .to_return(status: 200, body: File.read('spec/fixtures/files/api_entreprise/associations.json')) - stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) - .to_return(status: 200, body: File.read('spec/fixtures/files/api_education/annuaire_education.json')) - VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis') diff --git a/spec/system/users/dossier_prefill_post_spec.rb b/spec/system/users/dossier_prefill_post_spec.rb index 9c2c21d1e..ee31b2641 100644 --- a/spec/system/users/dossier_prefill_post_spec.rb +++ b/spec/system/users/dossier_prefill_post_spec.rb @@ -52,9 +52,6 @@ describe 'Prefilling a dossier (with a POST request):', js: true do stub_request(:get, /https:\/\/entreprise.api.gouv.fr\/v2\/associations\//) .to_return(status: 200, body: File.read('spec/fixtures/files/api_entreprise/associations.json')) - stub_request(:get, /https:\/\/data.education.gouv.fr\/api\/records\/1.0/) - .to_return(status: 200, body: File.read('spec/fixtures/files/api_education/annuaire_education.json')) - VCR.insert_cassette('api_geo_departements') VCR.insert_cassette('api_geo_communes') VCR.insert_cassette('api_geo_epcis')