From 141e23a38164ee694dd1a9d43a0b1666991c9ff5 Mon Sep 17 00:00:00 2001 From: Xavier J Date: Mon, 30 Nov 2015 17:03:36 +0100 Subject: [PATCH] Code review --- app/controllers/admin/procedures_controller.rb | 4 ++-- .../backoffice/dossiers_controller.rb | 8 ++++---- app/controllers/users/carte_controller.rb | 9 ++++----- app/controllers/users/dossiers_controller.rb | 13 ++++--------- app/views/gestionnaires/sessions/new.html.haml | 2 +- config/initializers/will_paginate.rb | 3 +++ lib/carto/sgmap/api.rb | 6 +----- lib/carto/sgmap/quartier_prioritaire_adapter.rb | 8 +------- spec/controllers/users/carte_controller_spec.rb | 12 +++++++++--- .../users/dossiers_controller_spec.rb | 1 + spec/lib/carto/sgmap/api_spec.rb | 2 +- .../sgmap/quartier_prioritaire_adapter_spec.rb | 17 ++++++++++------- .../dossiers/a_traiter_html.haml_spec.rb | 2 +- .../dossiers/en_attente_html.haml_spec.rb | 2 +- .../dossiers/termine_html.haml_spec.rb | 2 +- .../users/dossiers/a_traiter_html.haml_spec.rb | 2 +- .../users/dossiers/en_attente_html.haml_spec.rb | 2 +- .../users/dossiers/termine_html.haml_spec.rb | 2 +- 18 files changed, 47 insertions(+), 50 deletions(-) create mode 100644 config/initializers/will_paginate.rb diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 623c7e002..95800c633 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -2,12 +2,12 @@ class Admin::ProceduresController < AdminController def index @procedures = current_administrateur.procedures.where(archived: false) - @procedures = @procedures.paginate(:page => params[:page], :per_page => 12) + @procedures = @procedures.paginate(:page => params[:page]) end def archived @procedures_archived = current_administrateur.procedures.where(archived: true) - @procedures_archived = @procedures_archived.paginate(:page => params[:page], :per_page => 12) + @procedures_archived = @procedures_archived.paginate(:page => params[:page]) end def show diff --git a/app/controllers/backoffice/dossiers_controller.rb b/app/controllers/backoffice/dossiers_controller.rb index ecc7bd2e8..16ea989e7 100644 --- a/app/controllers/backoffice/dossiers_controller.rb +++ b/app/controllers/backoffice/dossiers_controller.rb @@ -7,20 +7,20 @@ class Backoffice::DossiersController < ApplicationController def a_traiter @dossiers_a_traiter = current_gestionnaire.dossiers.waiting_for_gestionnaire() - @dossiers_a_traiter = @dossiers_a_traiter.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers_a_traiter = @dossiers_a_traiter.paginate(:page => params[:page]).decorate total_dossiers_per_state end def en_attente @dossiers_en_attente = current_gestionnaire.dossiers.waiting_for_user() - @dossiers_en_attente = @dossiers_en_attente.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers_en_attente = @dossiers_en_attente.paginate(:page => params[:page]).decorate total_dossiers_per_state end def termine @dossiers_termine = current_gestionnaire.dossiers.termine() - @dossiers_termine = @dossiers_termine.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers_termine = @dossiers_termine.paginate(:page => params[:page]).decorate total_dossiers_per_state end @@ -30,7 +30,7 @@ class Backoffice::DossiersController < ApplicationController @dossiers_search, @dossier = Dossier.search(current_gestionnaire, @search_terms) unless @dossiers_search.empty? - @dossiers_search = @dossiers_search.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers_search = @dossiers_search.paginate(:page => params[:page]).decorate end @dossier = @dossier.decorate unless @dossier.nil? diff --git a/app/controllers/users/carte_controller.rb b/app/controllers/users/carte_controller.rb index 0ea4b98ab..c03d89b53 100644 --- a/app/controllers/users/carte_controller.rb +++ b/app/controllers/users/carte_controller.rb @@ -11,10 +11,10 @@ class Users::CarteController < UsersController def save dossier = current_user_dossier - dossier.quartier_prioritaires.all.map(&:destroy) + dossier.quartier_prioritaires.map(&:destroy) - unless params[:json_latlngs] == '' || params[:json_latlngs] == '[]' - qp_list = generate_qp JSON.parse(params[:json_latlngs]); + unless params[:json_latlngs].blank? + qp_list = generate_qp JSON.parse(params[:json_latlngs]) qp_list.each do |key, qp| qp.merge!({dossier_id: dossier.id}) @@ -33,8 +33,7 @@ class Users::CarteController < UsersController body: 'La localisation de la demande a été modifiée. Merci de le prendre en compte.', dossier_id: dossier.id } - commentaire = Commentaire.new commentaire_params - commentaire.save + Commentaire.create commentaire_params redirect_to url_for(controller: :recapitulatif, action: :show, dossier_id: params[:dossier_id]) end end diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 5d9cb834e..5b850d9ec 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -16,7 +16,6 @@ class Users::DossiersController < UsersController end def create - procedure = Procedure.find(params['procedure_id']) @etablissement = Etablissement.new(SIADE::EtablissementAdapter.new(siret).to_params) @entreprise = Entreprise.new(SIADE::EntrepriseAdapter.new(siren).to_params) @@ -30,11 +29,7 @@ class Users::DossiersController < UsersController end end - @dossier = Dossier.create(user: current_user) - @dossier.draft! - - @dossier.procedure = procedure - @dossier.save + @dossier = Dossier.create(user: current_user, state: 'draft', procedure_id: params['procedure_id']) @entreprise.dossier = @dossier @entreprise.save @@ -90,7 +85,7 @@ class Users::DossiersController < UsersController params[:page] = 1 if params[:page].nil? - @dossiers = @dossiers.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers = @dossiers.paginate(:page => params[:page]).decorate total_dossiers_per_state end @@ -100,7 +95,7 @@ class Users::DossiersController < UsersController params[:page] = 1 if params[:page].nil? - @dossiers = @dossiers.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers = @dossiers.paginate(:page => params[:page]).decorate total_dossiers_per_state end @@ -110,7 +105,7 @@ class Users::DossiersController < UsersController params[:page] = 1 if params[:page].nil? - @dossiers = @dossiers.paginate(:page => params[:page], :per_page => 12).decorate + @dossiers = @dossiers.paginate(:page => params[:page]).decorate total_dossiers_per_state end diff --git a/app/views/gestionnaires/sessions/new.html.haml b/app/views/gestionnaires/sessions/new.html.haml index 3fba36f81..75162f9e4 100644 --- a/app/views/gestionnaires/sessions/new.html.haml +++ b/app/views/gestionnaires/sessions/new.html.haml @@ -6,7 +6,7 @@ %br %br #new_user - = form_for @gestionnaire, url: {controller: 'gestionnaires/sessions', action: :create } do |f| + = form_for @gestionnaire, url: gestionnaire_session_path, method: :post do |f| %h4 = f.label :email .input-group diff --git a/config/initializers/will_paginate.rb b/config/initializers/will_paginate.rb new file mode 100644 index 000000000..454b8abfb --- /dev/null +++ b/config/initializers/will_paginate.rb @@ -0,0 +1,3 @@ +require 'will_paginate' + +WillPaginate.per_page = 12 \ No newline at end of file diff --git a/lib/carto/sgmap/api.rb b/lib/carto/sgmap/api.rb index 7a2572730..2377f67c0 100644 --- a/lib/carto/sgmap/api.rb +++ b/lib/carto/sgmap/api.rb @@ -19,10 +19,6 @@ class CARTO::SGMAP::API end def self.base_url - if Rails.env.production? - 'https://apicarto.sgmap.fr' - else - 'https://apicarto.sgmap.fr' - end + 'https://apicarto.sgmap.fr' end end diff --git a/lib/carto/sgmap/quartier_prioritaire_adapter.rb b/lib/carto/sgmap/quartier_prioritaire_adapter.rb index 66c3bd5dc..77fc8c305 100644 --- a/lib/carto/sgmap/quartier_prioritaire_adapter.rb +++ b/lib/carto/sgmap/quartier_prioritaire_adapter.rb @@ -10,7 +10,7 @@ class CARTO::SGMAP::QuartierPrioritaireAdapter def to_params params = {} - data_source[:features].each_with_index do |feature, index| + data_source[:features].each do |feature| qp_code = feature[:properties][:code] params[qp_code] = feature[:properties] @@ -19,10 +19,4 @@ class CARTO::SGMAP::QuartierPrioritaireAdapter params end - - def properties_to_fetch - [:code, - :nom, - :commune] - end end diff --git a/spec/controllers/users/carte_controller_spec.rb b/spec/controllers/users/carte_controller_spec.rb index aa8e0e8fe..01e03c2f2 100644 --- a/spec/controllers/users/carte_controller_spec.rb +++ b/spec/controllers/users/carte_controller_spec.rb @@ -181,14 +181,20 @@ RSpec.describe Users::CarteController, type: :controller do let(:coordinates) { '[]' } subject { JSON.parse(response.body) } - it { expect(subject['quartier_prioritaires']).to eq({}) } + + it 'Quartier Prioritaire Adapter does not call' do + expect(subject['quartier_prioritaires']).to eq({}) + end end context 'when coordinates are informed' do let(:coordinates) { '[[{"lat":48.87442541960633,"lng":2.3859214782714844},{"lat":48.87273183590832,"lng":2.3850631713867183},{"lat":48.87081237174292,"lng":2.3809432983398438},{"lat":48.8712640169951,"lng":2.377510070800781},{"lat":48.87510283703279,"lng":2.3778533935546875},{"lat":48.87544154230615,"lng":2.382831573486328},{"lat":48.87442541960633,"lng":2.3859214782714844}]]' } - subject { JSON.parse(response.body) } - it { expect(subject['quartier_prioritaires']).not_to be_nil } + subject { JSON.parse(response.body)['quartier_prioritaires'] } + it { expect(subject).not_to be_nil } + it { expect(subject['QPCODE1234']['code']).to eq('QPCODE1234') } + it { expect(subject['QPCODE1234']['geometry']['type']).to eq('MultiPolygon') } + it { expect(subject['QPCODE1234']['geometry']['coordinates']).to eq([[[[2.38715792094576, 48.8723062632126], [2.38724851642619, 48.8721392348061]]]]) } end end end diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 6612fdc50..a2ede281a 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -88,6 +88,7 @@ describe Users::DossiersController, type: :controller do it 'creates exercices for dossier' do expect { subject }.to change { Exercice.count }.by(3) + expect(Exercice.last.etablissement).to eq(Dossier.last.etablissement) end it 'links procedure to dossier' do diff --git a/spec/lib/carto/sgmap/api_spec.rb b/spec/lib/carto/sgmap/api_spec.rb index f5f4334ea..5ef32ff88 100644 --- a/spec/lib/carto/sgmap/api_spec.rb +++ b/spec/lib/carto/sgmap/api_spec.rb @@ -23,7 +23,7 @@ describe CARTO::SGMAP::API do context 'when geojson exist' do let(:geojson) { File.read('spec/support/files/geojson/request.json') } let(:status) { 200 } - let(:body) { File.read('spec/support/files/geojson/response.json') } + let(:body) { 'toto' } it 'returns response body' do expect(subject).to eq(body) diff --git a/spec/lib/carto/sgmap/quartier_prioritaire_adapter_spec.rb b/spec/lib/carto/sgmap/quartier_prioritaire_adapter_spec.rb index 7762d75b5..ab9d5f39e 100644 --- a/spec/lib/carto/sgmap/quartier_prioritaire_adapter_spec.rb +++ b/spec/lib/carto/sgmap/quartier_prioritaire_adapter_spec.rb @@ -10,20 +10,23 @@ describe CARTO::SGMAP::QuartierPrioritaireAdapter do to_return(status: status, body: body) end - context 'coordinates ard informed' do - let(:coordinates) { '' } + context 'coordinates are filled' do + let(:coordinates) { '[[2.252728, 43.27151][2.323223, 32.835332]]' } let(:status) { 200 } let(:body) { File.read('spec/support/files/geojson/response.json') } it { expect(subject).to be_a_instance_of(Hash) } - context 'Attributs' do + context 'Attributes' do let(:qp_code) { 'QP057019' } - it { expect(subject[qp_code][:code]).to eq(qp_code) } - it { expect(subject[qp_code][:nom]).to eq('Hauts De Vallières') } - it { expect(subject[qp_code][:commune]).to eq('Metz') } - it { expect(subject[qp_code][:geometry]).to eq({:type=>"MultiPolygon", :coordinates=>[[[[6.2136923480551, 49.1342109827851], [6.21416055031881, 49.1338823553928]]]]}) } + subject { super()[qp_code] } + + it { expect(subject[:code]).to eq(qp_code) } + it { expect(subject[:nom]).to eq('Hauts De Vallières') } + it { expect(subject[:commune]).to eq('Metz') } + + it { expect(subject[:geometry]).to eq({:type=>"MultiPolygon", :coordinates=>[[[[6.2136923480551, 49.1342109827851], [6.21416055031881, 49.1338823553928]]]]}) } end end diff --git a/spec/views/backoffice/dossiers/a_traiter_html.haml_spec.rb b/spec/views/backoffice/dossiers/a_traiter_html.haml_spec.rb index 84e41add7..435578b33 100644 --- a/spec/views/backoffice/dossiers/a_traiter_html.haml_spec.rb +++ b/spec/views/backoffice/dossiers/a_traiter_html.haml_spec.rb @@ -8,7 +8,7 @@ describe 'backoffice/dossiers/a_traiter.html.haml', type: :view do let!(:decorate_dossier) { create(:dossier, :with_user, state: 'initiated', procedure: procedure).decorate } before do - assign(:dossiers_a_traiter, gestionnaire.dossiers.waiting_for_gestionnaire.paginate(:page => 1, :per_page => 12).decorate) + assign(:dossiers_a_traiter, gestionnaire.dossiers.waiting_for_gestionnaire.paginate(:page => 1).decorate) render end diff --git a/spec/views/backoffice/dossiers/en_attente_html.haml_spec.rb b/spec/views/backoffice/dossiers/en_attente_html.haml_spec.rb index 0e2c66cae..264c60f3b 100644 --- a/spec/views/backoffice/dossiers/en_attente_html.haml_spec.rb +++ b/spec/views/backoffice/dossiers/en_attente_html.haml_spec.rb @@ -8,7 +8,7 @@ describe 'backoffice/dossiers/en_attente.html.haml', type: :view do let!(:decorate_dossier) { create(:dossier, :with_user, procedure: procedure, state: 'replied').decorate } before do - assign(:dossiers_en_attente, gestionnaire.dossiers.waiting_for_user.paginate(:page => 1, :per_page => 12).decorate) + assign(:dossiers_en_attente, gestionnaire.dossiers.waiting_for_user.paginate(:page => 1).decorate) render end diff --git a/spec/views/backoffice/dossiers/termine_html.haml_spec.rb b/spec/views/backoffice/dossiers/termine_html.haml_spec.rb index d38dafb91..84ed3780a 100644 --- a/spec/views/backoffice/dossiers/termine_html.haml_spec.rb +++ b/spec/views/backoffice/dossiers/termine_html.haml_spec.rb @@ -8,7 +8,7 @@ describe 'backoffice/dossiers/termine.html.haml', type: :view do let!(:decorate_dossier) { create(:dossier, :with_user, procedure: procedure, state: 'closed').decorate } before do - assign(:dossiers_termine, gestionnaire.dossiers.termine.paginate(:page => 1, :per_page => 12).decorate) + assign(:dossiers_termine, gestionnaire.dossiers.termine.paginate(:page => 1).decorate) render end diff --git a/spec/views/users/dossiers/a_traiter_html.haml_spec.rb b/spec/views/users/dossiers/a_traiter_html.haml_spec.rb index 55effc365..24075b1ba 100644 --- a/spec/views/users/dossiers/a_traiter_html.haml_spec.rb +++ b/spec/views/users/dossiers/a_traiter_html.haml_spec.rb @@ -6,7 +6,7 @@ describe 'users/dossiers/a_traiter.html.haml', type: :view do let!(:decorate_dossier) { create(:dossier, :with_procedure, user: user, state: 'replied').decorate } let!(:decorate_dossier_2) { create(:dossier, :with_procedure, user: user, state: 'initiated', nom_projet: 'projet de test').decorate } - let(:dossiers_list) { user.dossiers.waiting_for_user.paginate(:page => 1, :per_page => 12).decorate } + let(:dossiers_list) { user.dossiers.waiting_for_user.paginate(:page => 1).decorate } before do assign(:dossiers, dossiers_list) diff --git a/spec/views/users/dossiers/en_attente_html.haml_spec.rb b/spec/views/users/dossiers/en_attente_html.haml_spec.rb index 25d49bd75..4a9121bdd 100644 --- a/spec/views/users/dossiers/en_attente_html.haml_spec.rb +++ b/spec/views/users/dossiers/en_attente_html.haml_spec.rb @@ -6,7 +6,7 @@ describe 'users/dossiers/en_attente.html.haml', type: :view do let!(:decorate_dossier) { create(:dossier, :with_procedure, user: user, state: 'initiated', nom_projet: 'projet de test').decorate } let!(:decorate_dossier_2) { create(:dossier, :with_procedure, user: user, state: 'replied').decorate } - let(:dossiers_list) { user.dossiers.waiting_for_gestionnaire.paginate(:page => 1, :per_page => 12).decorate } + let(:dossiers_list) { user.dossiers.waiting_for_gestionnaire.paginate(:page => 1).decorate } before do assign(:dossiers, dossiers_list) diff --git a/spec/views/users/dossiers/termine_html.haml_spec.rb b/spec/views/users/dossiers/termine_html.haml_spec.rb index a2a6f7d04..369f75489 100644 --- a/spec/views/users/dossiers/termine_html.haml_spec.rb +++ b/spec/views/users/dossiers/termine_html.haml_spec.rb @@ -7,7 +7,7 @@ describe 'users/dossiers/termine.html.haml', type: :view do let!(:decorate_dossier_2) { create(:dossier, :with_procedure, user: user, state: 'replied', nom_projet: 'projet terminé').decorate } let!(:dossier_termine) { create(:dossier, :with_procedure, user: user, state: 'closed').decorate } - let(:dossiers_list) { user.dossiers.termine.paginate(:page => 1, :per_page => 12).decorate } + let(:dossiers_list) { user.dossiers.termine.paginate(:page => 1).decorate } before do assign(:dossiers, dossiers_list)