refactor dossiers controller

app/controllers/dossiers_controller.rb

@@ -5,27 +5,26 @@ class DossiersController < ApplicationController
     @etablissement =  @dossier.etablissement
     @entreprise =  @dossier.entreprise.decorate
   rescue ActiveRecord::RecordNotFound
-    redirect_to url_for({controller: :start, action: :error_dossier})
+    redirect_to url_for(controller: :start, action: :error_dossier)
   end
 
   def create
 
     @dossier_id = params[:pro_dossier_id].strip
 
-    if @dossier_id != ""
+    if dossier_id_is_present?
 
       @dossier = Dossier.find(@dossier_id)
 
       if @dossier.siret == params[:siret]
-        redirect_to url_for({controller: :recapitulatif, action: :show, dossier_id: @dossier_id})
+        redirect_to url_for(controller: :recapitulatif, action: :show, dossier_id: @dossier_id)
       else
-        redirect_to url_for({controller: :start, action: :error_dossier})
+        redirect_to url_for(controller: :start, action: :error_dossier)
       end
     else
 
-
-      @etablissement = Etablissement.new(SIADE::EtablissementAdapter.new(params[:siret]).to_params)
-      @entreprise = Entreprise.new(SIADE::EntrepriseAdapter.new(params[:siret][0..-6]).to_params)
+      @etablissement = Etablissement.new(SIADE::EtablissementAdapter.new(siret).to_params)
+      @entreprise = Entreprise.new(SIADE::EntrepriseAdapter.new(siren).to_params)
       @dossier = Dossier.create
 
       @entreprise.dossier = @dossier
@@ -35,37 +34,29 @@ class DossiersController < ApplicationController
       @etablissement.entreprise = @entreprise
       @etablissement.save
 
-      redirect_to url_for({controller: :dossiers, action: :show, id: @dossier.id})
+      redirect_to url_for(controller: :dossiers, action: :show, id: @dossier.id)
     end
 
   rescue RestClient::ResourceNotFound
-    redirect_to url_for({ controller: :start, action: :error_siret })
+    redirect_to url_for(controller: :start, action: :error_siret)
   rescue ActiveRecord::RecordNotFound
-    redirect_to url_for({controller: :start, action: :error_dossier})
+    redirect_to url_for(controller: :start, action: :error_dossier)
   end
 
   def update
 
     @dossier = Dossier.find(params[:id])
-    return error if update_params[:autorisation_donnees] == '0'
-    @dossier.update_attributes(update_params)
-
-    if @dossier.autorisation_donnees
+    if checked_autorisation_donnees?
+      @dossier.update_attributes(update_params)
       redirect_to url_for({controller: :demandes, action: :show, dossier_id: @dossier.id})
     else
       @etablissement =  @dossier.etablissement
       @entreprise =  @dossier.entreprise.decorate
-
-      self.error
+      flash.now.alert = 'Les conditions sont obligatoires.'
+      render 'show'
     end
   end
 
-  def error
-    show
-    flash.now.alert = 'Les conditions sont obligatoires.'
-    render 'show'
-  end
-
   private
 
   def update_params
@@ -73,7 +64,11 @@ class DossiersController < ApplicationController
   end
 
   def dossier_id_is_present?
+    @dossier_id != ""
+  end
 
+  def checked_autorisation_donnees?
+    update_params[:autorisation_donnees] == '1'
   end
 
   def siret

spec/controllers/dossiers_controller_spec.rb

@@ -106,28 +106,29 @@ RSpec.describe DossiersController, type: :controller do
   end
 
   describe 'PUT #update' do
+    before do
+      put :update, :id => dossier_id, dossier: { autorisation_donnees: autorisation_donnees }
+    end
     context 'when Checkbox is checked' do
+      let(:autorisation_donnees) { '1' }
       it 'redirects to demande' do
-        put :update, :id => dossier_id, dossier: { autorisation_donnees: '1' }
-        expect(response).to redirect_to("/dossiers/#{dossier_id}/demande")
+        expect(response).to redirect_to(controller: :demandes, action: :show, dossier_id: dossier.id)
       end
 
       it 'update dossier' do
-        put :update, :id => dossier_id, dossier: { autorisation_donnees: '1' }
-        dossier = Dossier.find(dossier_id)
+        dossier.reload
         expect(dossier.autorisation_donnees).to be_truthy
       end
     end
 
     context 'when Checkbox is not checked' do
+      let(:autorisation_donnees) { '0' }
       it 'uses flash alert to display message' do
-        put :update, :id => dossier_id, dossier: { autorisation_donnees: '0' }
         expect(flash[:alert]).to have_content('Les conditions sont obligatoires.')
       end
 
       it "doesn't update dossier autorisation_donnees" do
-        put :update, :id => dossier_id, dossier: { autorisation_donnees: '0' }
-        dossier = Dossier.find(dossier_id)
+        dossier.reload
         expect(dossier.autorisation_donnees).to be_falsy
       end
     end