refacto: better error management

app/components/editable_champ/rna_component/rna_component.html.haml

@@ -9,4 +9,4 @@
   class: "width-33-desktop",
   maxlength: 10
 .rna-info{ id: dom_id(@champ, :rna_info) }
-  = render 'shared/champs/rna/association', champ: @champ, network_error: false, rna: @champ.value
+  = render 'shared/champs/rna/association', champ: @champ, error: nil

app/controllers/champs/rna_controller.rb

@@ -3,7 +3,6 @@ class Champs::RNAController < ApplicationController
 
   def show
     @champ = policy_scope(Champ).find(params[:champ_id])
-    @rna = read_param_value(@champ.input_name, 'value')
+    @error = @champ.association_fetch_error_key unless @champ.fetch_association!(read_param_value(@champ.input_name, 'value'))
-    @network_error = @champ.fetch_association!(@rna).present?
   end
 end

app/models/concerns/rna_champ_association_fetchable_concern.rb

@@ -1,18 +1,27 @@
 module RNAChampAssociationFetchableConcern
   extend ActiveSupport::Concern
 
+  attr_reader :association_fetch_error_key
+
   def fetch_association!(rna)
-    data = APIEntreprise::RNAAdapter.new(rna, procedure_id).to_params
-    update!(data: data, value: rna)
-    nil
-  rescue APIEntreprise::API::Error, ActiveRecord::RecordInvalid => error
-    self.data = nil
     self.value = rna
-    save(validate: false)
+
-    if error.try(:network_error?) && !APIEntrepriseService.api_up?
+    return clear_association!(:empty) if rna.empty?
-      :network_error
+    return clear_association!(:invalid) unless valid?
-    else
+    return clear_association!(:not_found) if (data = APIEntreprise::RNAAdapter.new(rna, procedure_id).to_params).blank?
-      nil
+
-    end
+    update!(data: data)
+  rescue APIEntreprise::API::Error => error
+    error_key = :network_error if error.try(:network_error?) && !APIEntrepriseService.api_up?
+    clear_association!(error_key)
+  end
+
+  private
+
+  def clear_association!(error)
+    @association_fetch_error_key = error
+    self.data = nil
+    save!(context: :brouillon)
+    false
   end
 end

app/views/champs/rna/show.turbo_stream.haml

@@ -1 +1 @@
-= turbo_stream.update dom_id(@champ, :rna_info), partial: 'shared/champs/rna/association', locals: { champ: @champ, network_error: @network_error, rna: @rna }
+= turbo_stream.update dom_id(@champ, :rna_info), partial: 'shared/champs/rna/association', locals: { champ: @champ, error: @error }

app/views/shared/champs/rna/_association.html.haml

@@ -1,6 +1,11 @@
-- if network_error
+- case error
-  %p.pt-1= t('.network_error')
+- when :invalid
-- elsif @rna.present? && champ.data.blank?
+  %p.pt-1
+    Le numéro RNA doit commencer par un W majuscule suivi de 9 chiffres
+- when :not_found
   %p.pt-1= t('.not_found')
-- elsif champ.value.present?
+- when :network_error
-  %p.pt-1= t('.data_fetched', title: champ.title)
+  %p.pt-1= t('.network_error')
+- else
+  - if champ.value.present?
+    %p.pt-1= t('.data_fetched', title: champ.title)

spec/controllers/champs/rna_controller_spec.rb

@@ -38,7 +38,7 @@ describe Champs::RNAController, type: :controller do
         subject! { get :show, params: params, format: :turbo_stream }
 
         it 'clears the data on the model' do
-          expect(champ.reload.data).to eq({})
+          expect(champ.reload.data).to be_nil
         end
 
         it 'clears any information or error message' do
@@ -58,7 +58,7 @@ describe Champs::RNAController, type: :controller do
         end
 
         it 'displays a “RNA is invalid” error message' do
-          expect(response.body).to include("Aucun établissement trouvé")
+          expect(response.body).to include("Le numéro RNA doit commencer par un W majuscule suivi de 9 chiffres")
         end
       end
 
@@ -71,7 +71,7 @@ describe Champs::RNAController, type: :controller do
 
         it 'clears the data on the model' do
           champ.reload
-          expect(champ.data).to eq({})
+          expect(champ.data).to be_nil
         end
 
         it 'displays a “RNA is invalid” error message' do

spec/models/concern/rna_champ_association_fetchable_concern_spec.rb

@@ -1,6 +1,6 @@
 RSpec.describe RNAChampAssociationFetchableConcern do
   describe '.fetch_association!' do
-    let!(:champ) { create(:champ_rna, data: "not nil data") }
+    let!(:champ) { create(:champ_rna, data: "not nil data", value: 'W173847273') }
 
     before do
       stub_request(:get, /https:\/\/entreprise.api.gouv.fr\/v2\/associations\//)
@@ -10,12 +10,17 @@ RSpec.describe RNAChampAssociationFetchableConcern do
 
     subject(:fetch_association!) { champ.fetch_association!(rna) }
 
-    shared_examples "an association fetcher" do |expected_result, _expected_value, expected_data|
+    shared_examples "an association fetcher" do |expected_result, expected_error, expected_value, expected_data|
-      it { expect(fetch_association!).to eq(expected_result) }
+      it { expect { fetch_association! }.to change { champ.reload.value }.to(expected_value) }
-
-      it { expect { fetch_association! }.to change { champ.reload.value }.to(rna) }
 
       it { expect { fetch_association! }.to change { champ.reload.data }.to(expected_data) }
+
+      it { expect(fetch_association!).to eq(expected_result) }
+
+      it 'populates the association_fetch_error_key when an error occurs' do
+        fetch_association!
+        expect(champ.association_fetch_error_key).to eq(expected_error)
+      end
     end
 
     context 'when the RNA is empty' do
@@ -23,7 +28,7 @@ RSpec.describe RNAChampAssociationFetchableConcern do
       let(:status) { 422 }
       let(:body) { '' }
 
-      it_behaves_like "an association fetcher", nil, '', {}
+      it_behaves_like "an association fetcher", false, :empty, '', nil
     end
 
     context 'when the RNA is invalid' do
@@ -31,7 +36,7 @@ RSpec.describe RNAChampAssociationFetchableConcern do
       let(:status) { 422 }
       let(:body) { '' }
 
-      it_behaves_like "an association fetcher", nil, '1234', nil
+      it_behaves_like "an association fetcher", false, :invalid, '1234', nil
     end
 
     context 'when the RNA is unknow' do
@@ -39,7 +44,7 @@ RSpec.describe RNAChampAssociationFetchableConcern do
       let(:status) { 404 }
       let(:body) { '' }
 
-      it_behaves_like "an association fetcher", nil, 'W111111111', {}
+      it_behaves_like "an association fetcher", false, :not_found, 'W111111111', nil
     end
 
     context 'when the API is unavailable due to network error' do
@@ -49,7 +54,7 @@ RSpec.describe RNAChampAssociationFetchableConcern do
 
       before { expect(APIEntrepriseService).to receive(:api_up?).and_return(false) }
 
-      it_behaves_like "an association fetcher", :network_error, 'W595001988', nil
+      it_behaves_like "an association fetcher", false, :network_error, 'W595001988', nil
     end
 
     context 'when the RNA informations are retrieved successfully' do
@@ -57,7 +62,7 @@ RSpec.describe RNAChampAssociationFetchableConcern do
       let(:status) { 200 }
       let(:body) { File.read('spec/fixtures/files/api_entreprise/associations.json') }
 
-      it_behaves_like "an association fetcher", nil, 'W595001988', {
+      it_behaves_like "an association fetcher", true, nil, 'W595001988', {
         "association_id" => "W595001988",
         "association_titre" => "UN SUR QUATRE",
         "association_objet" => "valoriser, transmettre et partager auprès des publics les plus larges possibles, les bienfaits de l'immigration, la richesse de la diversité et la curiosité de l'autre autrement",