From caa90613fd0e892d81b1ad0e3d23ff9406295870 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 25 Jul 2019 10:46:40 +0200 Subject: [PATCH 01/15] =?UTF-8?q?ajout=20d'un=20confirm=20=C3=A0=20la=20su?= =?UTF-8?q?ppression=20d'un=20champ?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../TypesDeChampEditor/components/TypeDeChamp.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js b/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js index 751f7702b..3b5b01647 100644 --- a/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js +++ b/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js @@ -68,9 +68,13 @@ const TypeDeChamp = sortableElement(
From 12f1a29b68bca46968562af648412867006c5dfe Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 25 Jul 2019 11:13:30 +0200 Subject: [PATCH 02/15] update tests --- db/schema.rb | 4 +++- .../new_administrateur/types_de_champ_spec.rb | 13 +++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index b38ea35e9..69ef5b9e6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_07_17_132452) do +ActiveRecord::Schema.define(version: 2019_07_17_151228) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -430,6 +430,7 @@ ActiveRecord::Schema.define(version: 2019_07_17_132452) do t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" + t.text "message" end create_table "module_api_cartos", id: :serial, force: :cascade do |t| @@ -500,6 +501,7 @@ ActiveRecord::Schema.define(version: 2019_07_17_132452) do t.boolean "durees_conservation_required", default: true t.string "path" t.string "declarative_with_state" + t.text "monavis" t.text "monavis_embed" t.index ["declarative_with_state"], name: "index_procedures_on_declarative_with_state" t.index ["hidden_at"], name: "index_procedures_on_hidden_at" diff --git a/spec/features/new_administrateur/types_de_champ_spec.rb b/spec/features/new_administrateur/types_de_champ_spec.rb index 2046fd10e..0fecd6759 100644 --- a/spec/features/new_administrateur/types_de_champ_spec.rb +++ b/spec/features/new_administrateur/types_de_champ_spec.rb @@ -11,7 +11,9 @@ feature 'As an administrateur I can edit types de champ', js: true do end it "Add a new champ" do - click_on 'Supprimer' + page.accept_alert do + click_on 'Supprimer' + end within '.buttons' do click_on 'Ajouter un champ' @@ -48,7 +50,9 @@ feature 'As an administrateur I can edit types de champ', js: true do expect(page).to have_selector('#champ-3-libelle') within '.type-de-champ[data-index="2"]' do - click_on 'Supprimer' + page.accept_alert do + click_on 'Supprimer' + end end expect(page).not_to have_selector('#champ-3-libelle') @@ -68,8 +72,9 @@ feature 'As an administrateur I can edit types de champ', js: true do blur expect(page).to have_content('Formulaire enregistré') page.refresh - - click_on 'Supprimer' + page.accept_alert do + click_on 'Supprimer' + end expect(page).to have_content('Formulaire enregistré') expect(page).to have_content('Supprimer', count: 1) page.refresh From 5a9468aa95a36e0f94038ad3a01d9327b8e3179a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 30 Jul 2019 15:10:01 +0200 Subject: [PATCH 03/15] Fix repetition data export --- app/services/procedure_export_v2_service.rb | 12 +++++++----- spec/services/procedure_export_v2_service_spec.rb | 11 ++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app/services/procedure_export_v2_service.rb b/app/services/procedure_export_v2_service.rb index 2252136dd..be5bf8ff0 100644 --- a/app/services/procedure_export_v2_service.rb +++ b/app/services/procedure_export_v2_service.rb @@ -53,15 +53,17 @@ class ProcedureExportV2Service [dossier.champs, dossier.champs_private] .flatten .select { |champ| champ.is_a?(Champs::RepetitionChamp) } - end + end.group_by(&:libelle) end def champs_repetables_options - champs_repetables.map do |champ| + champs_repetables.map do |libelle, champs| [ - champ.libelle, - champ.rows.each_with_index.map do |champs, index| - Champs::RepetitionChamp::Row.new(index: index + 1, dossier_id: champ.dossier_id.to_s, champs: champs) + libelle, + champs.flat_map do |champ| + champ.rows.each_with_index.map do |champs, index| + Champs::RepetitionChamp::Row.new(index: index + 1, dossier_id: champ.dossier_id.to_s, champs: champs) + end end ] end diff --git a/spec/services/procedure_export_v2_service_spec.rb b/spec/services/procedure_export_v2_service_spec.rb index 5790307bb..de36d0057 100644 --- a/spec/services/procedure_export_v2_service_spec.rb +++ b/spec/services/procedure_export_v2_service_spec.rb @@ -155,8 +155,13 @@ describe ProcedureExportV2Service do end context 'with repetitions' do - let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) } - let(:champ_repetition) { dossier.champs.find { |champ| champ.type_champ == 'repetition' } } + let!(:dossiers) do + [ + create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure), + create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) + ] + end + let(:champ_repetition) { dossiers.first.champs.find { |champ| champ.type_champ == 'repetition' } } it 'should have sheets' do expect(subject.sheets.map(&:name)).to eq(['Dossiers', 'Etablissements', 'Avis', champ_repetition.libelle]) @@ -172,7 +177,7 @@ describe ProcedureExportV2Service do end it 'should have data' do - expect(repetition_sheet.data.size).to eq(2) + expect(repetition_sheet.data.size).to eq(4) end end end From ab1170bcfc3fb712129d62e5933eb5238c59690e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 30 Jul 2019 15:39:20 +0200 Subject: [PATCH 04/15] Fix export dossier n+1 query --- app/models/dossier.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 7d55e97df..f13fb26db 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -108,7 +108,24 @@ class Dossier < ApplicationRecord scope :en_construction, -> { not_archived.state_en_construction } scope :en_instruction, -> { not_archived.state_en_instruction } scope :termine, -> { not_archived.state_termine } - scope :downloadable_sorted, -> { state_not_brouillon.includes(:etablissement, :user, :individual, :followers_gestionnaires, :avis, champs: { etablissement: [:champ], type_de_champ: :drop_down_list }, champs_private: { etablissement: [:champ], type_de_champ: :drop_down_list }).order(en_construction_at: 'asc') } + scope :downloadable_sorted, -> { + state_not_brouillon + .includes( + :user, + :individual, + :followers_gestionnaires, + :avis, + etablissement: :champ, + champs: { + etablissement: :champ, + type_de_champ: :drop_down_list + }, + champs_private: { + etablissement: :champ, + type_de_champ: :drop_down_list + } + ).order(en_construction_at: 'asc') + } scope :en_cours, -> { not_archived.state_en_construction_ou_instruction } scope :without_followers, -> { left_outer_joins(:follows).where(follows: { id: nil }) } scope :followed_by, -> (gestionnaire) { joins(:follows).where(follows: { gestionnaire: gestionnaire }) } From 8c72470a6c98e75a5c384bf55e81bfb29e155ec9 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 30 Jul 2019 15:39:42 +0200 Subject: [PATCH 05/15] Add default etablissement champ name --- app/models/etablissement.rb | 2 +- spec/services/procedure_export_v2_service_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/etablissement.rb b/app/models/etablissement.rb index 2dc07760a..98584e672 100644 --- a/app/models/etablissement.rb +++ b/app/models/etablissement.rb @@ -120,6 +120,6 @@ class Etablissement < ApplicationRecord end def libelle_for_export - champ&.libelle + champ&.libelle || 'Dossier' end end diff --git a/spec/services/procedure_export_v2_service_spec.rb b/spec/services/procedure_export_v2_service_spec.rb index de36d0057..d0dfa759a 100644 --- a/spec/services/procedure_export_v2_service_spec.rb +++ b/spec/services/procedure_export_v2_service_spec.rb @@ -93,6 +93,9 @@ describe ProcedureExportV2Service do context 'with etablissement' do let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_entreprise, procedure: procedure) } + let(:dossier_etablissement) { etablissements_sheet.data[1] } + let(:champ_etablissement) { etablissements_sheet.data[0] } + it 'should have headers' do expect(etablissements_sheet.headers).to eq([ "Dossier ID", @@ -132,6 +135,8 @@ describe ProcedureExportV2Service do it 'should have data' do expect(etablissements_sheet.data.size).to eq(2) + expect(dossier_etablissement[1]).to eq("Dossier") + expect(champ_etablissement[1]).to eq("siret") end end From fc75580a3ce7c0d2ddd8aea759c46232d12a95ac Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 27 Jun 2019 16:26:07 +0200 Subject: [PATCH 06/15] Start using pundit --- Gemfile | 1 + Gemfile.lock | 3 + app/controllers/application_controller.rb | 15 +++-- app/controllers/champs/carte_controller.rb | 10 +--- .../champs/repetition_controller.rb | 5 +- app/models/gestionnaire.rb | 4 ++ app/policies/application_policy.rb | 49 ++++++++++++++++ app/policies/champ_policy.rb | 24 ++++++++ app/policies/type_de_champ_policy.rb | 13 +++++ spec/policies/champ_policy_spec.rb | 56 +++++++++++++++++++ spec/policies/type_de_champ_policy_spec.rb | 23 ++++++++ 11 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 app/policies/application_policy.rb create mode 100644 app/policies/champ_policy.rb create mode 100644 app/policies/type_de_champ_policy.rb create mode 100644 spec/policies/champ_policy_spec.rb create mode 100644 spec/policies/type_de_champ_policy_spec.rb diff --git a/Gemfile b/Gemfile index 7ffa37c1c..4bc7bd8ca 100644 --- a/Gemfile +++ b/Gemfile @@ -47,6 +47,7 @@ gem 'prawn' # PDF Generation gem 'prawn_rails' gem 'premailer-rails' gem 'puma' # Use Puma as the app server +gem 'pundit' gem 'rack-mini-profiler' gem 'rails' gem 'rails-i18n' # Locales par défaut diff --git a/Gemfile.lock b/Gemfile.lock index b925daa7a..8b597a428 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -431,6 +431,8 @@ GEM pry (~> 0.10) public_suffix (3.0.3) puma (3.12.0) + pundit (2.0.1) + activesupport (>= 3.0.0) rack (2.0.6) rack-mini-profiler (1.0.1) rack (>= 1.2.0) @@ -749,6 +751,7 @@ DEPENDENCIES premailer-rails pry-byebug puma + pundit rack-mini-profiler rails rails-controller-testing diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9b58e571d..f405328a9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,6 @@ class ApplicationController < ActionController::Base include TrustedDeviceConcern + include Pundit MAINTENANCE_MESSAGE = 'Le site est actuellement en maintenance. Il sera à nouveau disponible dans un court instant.' @@ -41,12 +42,18 @@ class ApplicationController < ActionController::Base logged_user.present? end - def logged_user_ids - logged_users.map(&:id) - end - helper_method :logged_in? + def pundit_user + if administrateur_signed_in? + current_administrateur + elsif gestionnaire_signed_in? + current_gestionnaire + else + current_user + end + end + protected def authenticate_logged_user! diff --git a/app/controllers/champs/carte_controller.rb b/app/controllers/champs/carte_controller.rb index d214f30d0..1410a9cde 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -14,15 +14,9 @@ class Champs::CarteController < ApplicationController end @champ = if params[:champ_id].present? - Champ - .joins(:dossier) - .where(dossiers: { user_id: logged_user_ids }) - .find(params[:champ_id]) + policy_scope(Champ).find(params[:champ_id]) else - TypeDeChamp - .joins(:procedure) - .where(procedures: { administrateur_id: logged_user_ids }) - .find(params[:type_de_champ_id]).champ.build + policy_scope(TypeDeChamp).find(params[:type_de_champ_id]).champ.build end geo_areas = [] diff --git a/app/controllers/champs/repetition_controller.rb b/app/controllers/champs/repetition_controller.rb index c5d8b12ea..ab0596abb 100644 --- a/app/controllers/champs/repetition_controller.rb +++ b/app/controllers/champs/repetition_controller.rb @@ -2,10 +2,7 @@ class Champs::RepetitionController < ApplicationController before_action :authenticate_logged_user! def show - @champ = Champ - .joins(:dossier) - .where(dossiers: { user_id: logged_user_ids }) - .find(params[:champ_id]) + @champ = policy_scope(Champ).find(params[:champ_id]) @position = params[:position] row = (@champ.champs.empty? ? 0 : @champ.champs.last.row) + 1 diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 1c94f5070..ea633c7e8 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -225,6 +225,10 @@ class Gestionnaire < ApplicationRecord end end + def user + User.find_by(email: email) + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 000000000..eefe976c6 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,49 @@ +class ApplicationPolicy + attr_reader :user, :record + + def initialize(user, record) + @user = user + @record = record + end + + def index? + false + end + + def show? + false + end + + def create? + false + end + + def new? + create? + end + + def update? + false + end + + def edit? + update? + end + + def destroy? + false + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb new file mode 100644 index 000000000..601fa4529 --- /dev/null +++ b/app/policies/champ_policy.rb @@ -0,0 +1,24 @@ +class ChampPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user.is_a?(User) + scope + .joins(:dossier) + .where({ dossiers: { user_id: user.id } }) + elsif user.is_a?(Gestionnaire) + scope_with_join = scope.joins(dossier: :follows) + scope_with_left_join = scope.left_joins(dossier: :follows) + + if user.user + scope_with_left_join + .where({ dossiers: { user_id: user.user.id } }) + .or(scope_with_left_join.where(dossiers: { follows: { gestionnaire_id: user.id } })) + else + scope_with_join.where(dossiers: { follows: { gestionnaire_id: user.id } }) + end + else + scope.none + end + end + end +end diff --git a/app/policies/type_de_champ_policy.rb b/app/policies/type_de_champ_policy.rb new file mode 100644 index 000000000..3fab51906 --- /dev/null +++ b/app/policies/type_de_champ_policy.rb @@ -0,0 +1,13 @@ +class TypeDeChampPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user.is_a?(Administrateur) + scope + .joins(procedure: [:administrateurs]) + .where({ administrateurs: { id: user.id } }) + else + scope.none + end + end + end +end diff --git a/spec/policies/champ_policy_spec.rb b/spec/policies/champ_policy_spec.rb new file mode 100644 index 000000000..2e06f464c --- /dev/null +++ b/spec/policies/champ_policy_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe ChampPolicy do + let(:user) { create(:user) } + let(:dossier) { create(:dossier, user: user) } + let!(:champ) { create(:champ_text, dossier: dossier) } + + let(:pundit_user) { user } + subject { Pundit.policy_scope(pundit_user, Champ) } + + context 'when the user has only user rights' do + context 'cannot access champs for other dossiers' do + let(:pundit_user) { create(:user) } + + it { expect(subject.find_by(id: champ.id)).to eq(nil) } + end + + context 'can access champs for its own dossiers' do + it { + expect(subject.find(champ.id)).to eq(champ) + } + end + end + + context 'when the user has only gestionnaire rights' do + context 'can access champs for dossiers it follows' do + let(:dossier) { create(:dossier, :followed) } + let(:pundit_user) { dossier.followers_gestionnaires.first } + + it { expect(subject.find(champ.id)).to eq(champ) } + end + end + + context 'when the user has user and gestionnaire rights' do + let(:pundit_user) { dossier.followers_gestionnaires.first } + let(:dossier) { create(:dossier, :followed) } + + let(:user) { create(:user, email: pundit_user.email) } + let(:dossier2) { create(:dossier, user: user) } + let!(:champ_2) { create(:champ_text, dossier: dossier2) } + + context 'can access champs for dossiers it follows' do + it do + expect(pundit_user.user).to eq(user) + expect(subject.find(champ.id)).to eq(champ) + end + end + + context 'can access champs for its own dossiers' do + it do + expect(pundit_user.user).to eq(user) + expect(subject.find(champ_2.id)).to eq(champ_2) + end + end + end +end diff --git a/spec/policies/type_de_champ_policy_spec.rb b/spec/policies/type_de_champ_policy_spec.rb new file mode 100644 index 000000000..ca3e7e635 --- /dev/null +++ b/spec/policies/type_de_champ_policy_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe TypeDeChampPolicy do + let(:procedure) { create(:procedure) } + let!(:type_de_champ) { create(:type_de_champ_text, procedure: procedure) } + + let(:pundit_user) { create(:user) } + subject { Pundit.policy_scope(pundit_user, TypeDeChamp) } + + context 'when the user has only user rights' do + it 'can not access' do + expect(subject.find_by(id: type_de_champ.id)).to eq(nil) + end + end + + context 'when the user has administrateur rights' do + let(:pundit_user) { procedure.administrateurs.first } + + it 'can access' do + expect(subject.find(type_de_champ.id)).to eq(type_de_champ) + end + end +end From 25db21467d04bb1eaf5a153fbaee2c7f875e22f3 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 31 Jul 2019 12:25:40 +0200 Subject: [PATCH 07/15] Stop using Flipflop as switch for Fog --- app/helpers/procedure_helper.rb | 2 +- app/models/commentaire.rb | 2 +- app/uploaders/attestation_template_logo_uploader.rb | 4 ++-- app/uploaders/attestation_template_signature_uploader.rb | 4 ++-- app/uploaders/attestation_uploader.rb | 4 ++-- app/uploaders/commentaire_file_uploader.rb | 2 +- app/uploaders/procedure_logo_uploader.rb | 6 +++--- config/features.rb | 2 -- config/secrets.yml | 1 + spec/spec_helper.rb | 8 -------- 10 files changed, 13 insertions(+), 22 deletions(-) diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index 2c09b70e9..d542ea0e2 100644 --- a/app/helpers/procedure_helper.rb +++ b/app/helpers/procedure_helper.rb @@ -26,7 +26,7 @@ module ProcedureHelper if logo.blank? ActionController::Base.helpers.image_url("marianne.svg") else - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] RemoteDownloader.new(logo.filename).url else LocalDownloader.new(logo.path, 'logo').url diff --git a/app/models/commentaire.rb b/app/models/commentaire.rb index 61e45d5f9..bbf492eba 100644 --- a/app/models/commentaire.rb +++ b/app/models/commentaire.rb @@ -52,7 +52,7 @@ class Commentaire < ApplicationRecord if piece_jointe.virus_scanner.safe? Rails.application.routes.url_helpers.url_for(piece_jointe) end - elsif Flipflop.remote_storage? + elsif Rails.application.secrets.fog[:enabled] RemoteDownloader.new(file.path).url elsif file&.url # FIXME: this is horrible but used only in dev and will be removed after migration diff --git a/app/uploaders/attestation_template_logo_uploader.rb b/app/uploaders/attestation_template_logo_uploader.rb index 49be9500b..a74bc8dc6 100644 --- a/app/uploaders/attestation_template_logo_uploader.rb +++ b/app/uploaders/attestation_template_logo_uploader.rb @@ -4,7 +4,7 @@ class AttestationTemplateLogoUploader < BaseUploader end # Choose what kind of storage to use for this uploader: - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] storage :fog else storage :file @@ -13,7 +13,7 @@ class AttestationTemplateLogoUploader < BaseUploader # Override the directory where uploaded files will be stored. # This is a sensible default for uploaders that are meant to be mounted: def store_dir - if !Flipflop.remote_storage? + if !Rails.application.secrets.fog[:enabled] "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end end diff --git a/app/uploaders/attestation_template_signature_uploader.rb b/app/uploaders/attestation_template_signature_uploader.rb index 12110868d..50be72cae 100644 --- a/app/uploaders/attestation_template_signature_uploader.rb +++ b/app/uploaders/attestation_template_signature_uploader.rb @@ -4,7 +4,7 @@ class AttestationTemplateSignatureUploader < BaseUploader end # Choose what kind of storage to use for this uploader: - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] storage :fog else storage :file @@ -13,7 +13,7 @@ class AttestationTemplateSignatureUploader < BaseUploader # Override the directory where uploaded files will be stored. # This is a sensible default for uploaders that are meant to be mounted: def store_dir - if !Flipflop.remote_storage? + if !Rails.application.secrets.fog[:enabled] "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end end diff --git a/app/uploaders/attestation_uploader.rb b/app/uploaders/attestation_uploader.rb index db4304e4a..ddfe1aeab 100644 --- a/app/uploaders/attestation_uploader.rb +++ b/app/uploaders/attestation_uploader.rb @@ -4,7 +4,7 @@ class AttestationUploader < BaseUploader end # Choose what kind of storage to use for this uploader: - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] storage :fog else storage :file @@ -13,7 +13,7 @@ class AttestationUploader < BaseUploader # Override the directory where uploaded files will be stored. # This is a sensible default for uploaders that are meant to be mounted: def store_dir - if !Flipflop.remote_storage? + if !Rails.application.secrets.fog[:enabled] "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end end diff --git a/app/uploaders/commentaire_file_uploader.rb b/app/uploaders/commentaire_file_uploader.rb index 250cd23d5..93131fb94 100644 --- a/app/uploaders/commentaire_file_uploader.rb +++ b/app/uploaders/commentaire_file_uploader.rb @@ -3,7 +3,7 @@ class CommentaireFileUploader < BaseUploader Rails.root.join("public") end - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] storage :fog else storage :file diff --git a/app/uploaders/procedure_logo_uploader.rb b/app/uploaders/procedure_logo_uploader.rb index 68a110993..0d774d514 100644 --- a/app/uploaders/procedure_logo_uploader.rb +++ b/app/uploaders/procedure_logo_uploader.rb @@ -4,7 +4,7 @@ class ProcedureLogoUploader < BaseUploader end # Choose what kind of storage to use for this uploader: - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] storage :fog else storage :file @@ -13,7 +13,7 @@ class ProcedureLogoUploader < BaseUploader # Override the directory where uploaded files will be stored. # This is a sensible default for uploaders that are meant to be mounted: def store_dir - if !Flipflop.remote_storage? + if !Rails.application.secrets.fog[:enabled] "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end end @@ -27,7 +27,7 @@ class ProcedureLogoUploader < BaseUploader def filename if file.present? if original_filename.present? || model.logo_secure_token - if Flipflop.remote_storage? + if Rails.application.secrets.fog[:enabled] filename = "#{model.class.to_s.underscore}-#{secure_token}.#{file.extension&.downcase}" else filename = "logo-#{secure_token}.#{file.extension&.downcase}" diff --git a/config/features.rb b/config/features.rb index d4bb54e21..c10a4a8e7 100644 --- a/config/features.rb +++ b/config/features.rb @@ -29,8 +29,6 @@ Flipflop.configure do end group :production do - feature :remote_storage, - default: ENV['FOG_ENABLED'] == 'enabled' feature :insee_api_v3, default: true feature :weekly_overview, diff --git a/config/secrets.yml b/config/secrets.yml index 7f3166b96..11001e502 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -34,6 +34,7 @@ defaults: &defaults pipedrive: key: <%= ENV['PIPEDRIVE_KEY'] %> fog: + enabled: <%= ENV['FOG_ENABLED'] == 'enabled' %> openstack_tenant: <%= ENV['FOG_OPENSTACK_TENANT'] %> openstack_api_key: <%= ENV['FOG_OPENSTACK_API_KEY'] %> openstack_username: <%= ENV['FOG_OPENSTACK_USERNAME'] %> diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cfaa3683c..27dd26382 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -147,14 +147,6 @@ RSpec.configure do |config| Typhoeus::Expectation.clear ActionMailer::Base.deliveries.clear - - if Flipflop.remote_storage? - VCR.use_cassette("ovh_storage_init") do - CarrierWave.configure do |config| - config.fog_credentials = { provider: 'OpenStack' } - end - end - end } RSpec::Matchers.define :have_same_attributes_as do |expected, options| From 6cfad01d12ef9a580d808054bbfd6fd7bc1e249e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 31 Jul 2019 12:26:07 +0200 Subject: [PATCH 08/15] Stop using Flipflop as switch for weekly_overview --- app/jobs/weekly_overview_job.rb | 2 +- config/application.rb | 2 ++ config/features.rb | 2 -- spec/jobs/weekly_overview_job_spec.rb | 6 ++++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/jobs/weekly_overview_job.rb b/app/jobs/weekly_overview_job.rb index 7e2a3c83e..2d8c09f65 100644 --- a/app/jobs/weekly_overview_job.rb +++ b/app/jobs/weekly_overview_job.rb @@ -3,7 +3,7 @@ class WeeklyOverviewJob < ApplicationJob def perform(*args) # Feature flipped to avoid mails in staging due to unprocessed dossier - if Flipflop.weekly_overview? + if Rails.application.config.ds_weekly_overview Gestionnaire.all .map { |gestionnaire| [gestionnaire, gestionnaire.last_week_overview] } .reject { |_, overview| overview.nil? } diff --git a/config/application.rb b/config/application.rb index 97e20adce..be464bbf2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -39,5 +39,7 @@ module TPS # Make main application helpers available in administrate Administrate::ApplicationController.helper(TPS::Application.helpers) end + + config.ds_weekly_overview = ENV['APP_NAME'] == 'tps' end end diff --git a/config/features.rb b/config/features.rb index c10a4a8e7..1fa9ba3ee 100644 --- a/config/features.rb +++ b/config/features.rb @@ -31,8 +31,6 @@ Flipflop.configure do group :production do feature :insee_api_v3, default: true - feature :weekly_overview, - default: ENV['APP_NAME'] == 'tps' feature :pre_maintenance_mode feature :maintenance_mode end diff --git a/spec/jobs/weekly_overview_job_spec.rb b/spec/jobs/weekly_overview_job_spec.rb index b730ec534..abde4e76f 100644 --- a/spec/jobs/weekly_overview_job_spec.rb +++ b/spec/jobs/weekly_overview_job_spec.rb @@ -8,7 +8,10 @@ RSpec.describe WeeklyOverviewJob, type: :job do context 'if the feature is enabled' do before do - Flipflop::FeatureSet.current.test!.switch!(:weekly_overview, true) + Rails.application.config.ds_weekly_overview = true + end + after do + Rails.application.config.ds_weekly_overview = false end context 'with one gestionnaire with one overview' do @@ -35,7 +38,6 @@ RSpec.describe WeeklyOverviewJob, type: :job do context 'if the feature is disabled' do before do - Flipflop::FeatureSet.current.test!.switch!(:weekly_overview, false) allow(Gestionnaire).to receive(:all) WeeklyOverviewJob.new.perform end From fba195c0b5a181fe79e4c4be6977cf5aecd8b55e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 31 Jul 2019 16:09:28 +0200 Subject: [PATCH 09/15] Add attestation url to dossier on api --- app/models/attestation.rb | 9 +++++++++ app/models/dossier.rb | 4 +++- app/serializers/dossier_serializer.rb | 20 +++++++++++++++---- .../api/v1/dossiers_controller_spec.rb | 6 +++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/models/attestation.rb b/app/models/attestation.rb index 605ed11a4..6d9cdc357 100644 --- a/app/models/attestation.rb +++ b/app/models/attestation.rb @@ -2,4 +2,13 @@ class Attestation < ApplicationRecord belongs_to :dossier mount_uploader :pdf, AttestationUploader + + def pdf_url + if Rails.application.secrets.fog[:enabled] + RemoteDownloader.new(pdf.path).url + elsif pdf&.url + # FIXME: this is horrible but used only in dev and will be removed after migration + File.join(LOCAL_DOWNLOAD_URL, pdf.url) + end + end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index f13fb26db..3aee61aa1 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -144,7 +144,9 @@ class Dossier < ApplicationRecord :etablissement, piece_justificative_file_attachment: :blob ], - avis: [], + justificatif_motivation_attachment: :blob, + attestation: [], + avis: { piece_justificative_file_attachment: :blob }, etablissement: [], individual: [], user: []) diff --git a/app/serializers/dossier_serializer.rb b/app/serializers/dossier_serializer.rb index ce90de67c..dbd3e03ad 100644 --- a/app/serializers/dossier_serializer.rb +++ b/app/serializers/dossier_serializer.rb @@ -14,6 +14,9 @@ class DossierSerializer < ActiveModel::Serializer :motivation, :instructeurs + attribute :attestation, if: :include_attestation? + attribute :justificatif_motivation, if: :include_justificatif_motivation? + has_one :individual has_one :entreprise has_one :etablissement @@ -22,7 +25,6 @@ class DossierSerializer < ActiveModel::Serializer has_many :champs_private has_many :pieces_justificatives has_many :types_de_piece_justificative - has_one :justificatif_motivation has_many :avis has_many :champs, serializer: ChampSerializer @@ -53,10 +55,12 @@ class DossierSerializer < ActiveModel::Serializer PiecesJustificativesService.serialize_champs_as_pjs(object) end + def attestation + object.attestation.pdf_url + end + def justificatif_motivation - if object.justificatif_motivation.attached? - Rails.application.routes.url_helpers.url_for(object.justificatif_motivation) - end + Rails.application.routes.url_helpers.url_for(object.justificatif_motivation) end def types_de_piece_justificative @@ -102,4 +106,12 @@ class DossierSerializer < ActiveModel::Serializer def processed_at object.processed_at&.in_time_zone('UTC') end + + def include_attestation? + object.accepte? + end + + def include_justificatif_motivation? + object.justificatif_motivation.attached? + end end diff --git a/spec/controllers/api/v1/dossiers_controller_spec.rb b/spec/controllers/api/v1/dossiers_controller_spec.rb index bb2fd8a2f..2ea28a353 100644 --- a/spec/controllers/api/v1/dossiers_controller_spec.rb +++ b/spec/controllers/api/v1/dossiers_controller_spec.rb @@ -154,10 +154,10 @@ describe API::V1::DossiersController do context 'when dossier exists and belongs to procedure' do let(:procedure_id) { procedure.id } let(:date_creation) { Time.zone.local(2008, 9, 1, 10, 5, 0) } - let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, :en_construction, procedure: procedure, motivation: "Motivation") } } + let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, :with_attestation, :accepte, procedure: procedure, motivation: "Motivation") } } let(:dossier_id) { dossier.id } let(:body) { JSON.parse(retour.body, symbolize_names: true) } - let(:field_list) { [:id, :created_at, :updated_at, :archived, :individual, :entreprise, :etablissement, :cerfa, :types_de_piece_justificative, :pieces_justificatives, :champs, :champs_private, :commentaires, :state, :simplified_state, :initiated_at, :processed_at, :received_at, :motivation, :email, :instructeurs, :justificatif_motivation, :avis] } + let(:field_list) { [:id, :created_at, :updated_at, :archived, :individual, :entreprise, :etablissement, :cerfa, :types_de_piece_justificative, :pieces_justificatives, :champs, :champs_private, :commentaires, :state, :simplified_state, :initiated_at, :processed_at, :received_at, :motivation, :email, :instructeurs, :attestation, :avis] } subject { body[:dossier] } it 'return REST code 200', :show_in_doc do @@ -165,7 +165,7 @@ describe API::V1::DossiersController do end it { expect(subject[:id]).to eq(dossier.id) } - it { expect(subject[:state]).to eq('initiated') } + it { expect(subject[:state]).to eq('closed') } it { expect(subject[:created_at]).to eq('2008-09-01T08:05:00.000Z') } it { expect(subject[:updated_at]).to eq('2008-09-01T08:05:00.000Z') } it { expect(subject[:archived]).to eq(dossier.archived) } From b2669158925e22abd22896e4eade8ed082d7fcc4 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 11 Jul 2019 10:28:44 +0200 Subject: [PATCH 10/15] Add repetitions to api --- app/models/champ.rb | 3 ++- app/models/champs/repetition_champ.rb | 12 ++++++++++-- app/models/dossier.rb | 10 ++++++++-- app/serializers/champ_serializer.rb | 9 +++++++++ app/serializers/row_serializer.rb | 5 +++++ app/services/procedure_export_v2_service.rb | 6 +----- .../api/v1/dossiers_controller_spec.rb | 16 ++++++++++++++++ 7 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 app/serializers/row_serializer.rb diff --git a/app/models/champ.rb b/app/models/champ.rb index 91835499f..4c091e04f 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -5,10 +5,11 @@ class Champ < ApplicationRecord has_many :commentaires has_one_attached :piece_justificative_file - # We declare champ specific relationships (Champs::CarteChamp and Champs::SiretChamp) + # We declare champ specific relationships (Champs::CarteChamp, Champs::SiretChamp and Champs::RepetitionChamp) # here because otherwise we can't easily use includes in our queries. has_many :geo_areas, dependent: :destroy belongs_to :etablissement, dependent: :destroy + has_many :champs, -> { ordered }, foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy delegate :libelle, :type_champ, :order_place, :mandatory?, :description, :drop_down_list, :exclude_from_export?, :exclude_from_view?, :repetition?, to: :type_de_champ diff --git a/app/models/champs/repetition_champ.rb b/app/models/champs/repetition_champ.rb index 2173fe232..4d95e574d 100644 --- a/app/models/champs/repetition_champ.rb +++ b/app/models/champs/repetition_champ.rb @@ -1,6 +1,4 @@ class Champs::RepetitionChamp < Champ - has_many :champs, -> { ordered }, foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy - accepts_nested_attributes_for :champs, allow_destroy: true def rows @@ -21,11 +19,21 @@ class Champs::RepetitionChamp < Champ # The user cannot enter any information here so it doesn’t make much sense to search end + def rows_for_export + rows.each.with_index(1).map do |champs, index| + Champs::RepetitionChamp::Row.new(index: index, dossier_id: dossier_id.to_s, champs: champs) + end + end + class Row < Hashie::Dash property :index property :dossier_id property :champs + def read_attribute_for_serialization(attribute) + self[attribute] + end + def spreadsheet_columns [ ['Dossier ID', :dossier_id], diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 3aee61aa1..30bbe5312 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -137,12 +137,18 @@ class Dossier < ApplicationRecord champs: [ :geo_areas, :etablissement, - piece_justificative_file_attachment: :blob + piece_justificative_file_attachment: :blob, + champs: [ + piece_justificative_file_attachment: :blob + ] ], champs_private: [ :geo_areas, :etablissement, - piece_justificative_file_attachment: :blob + piece_justificative_file_attachment: :blob, + champs: [ + piece_justificative_file_attachment: :blob + ] ], justificatif_motivation_attachment: :blob, attestation: [], diff --git a/app/serializers/champ_serializer.rb b/app/serializers/champ_serializer.rb index 76d1ca76d..670979c81 100644 --- a/app/serializers/champ_serializer.rb +++ b/app/serializers/champ_serializer.rb @@ -8,6 +8,7 @@ class ChampSerializer < ActiveModel::Serializer has_many :geo_areas, if: :include_geo_areas? has_one :etablissement, if: :include_etablissement? has_one :entreprise, if: :include_etablissement? + has_many :rows, serializer: RowSerializer, if: :include_rows? def value case object @@ -35,6 +36,10 @@ class ChampSerializer < ActiveModel::Serializer object.etablissement&.entreprise end + def rows + object.rows_for_export + end + def include_etablissement? object.is_a?(Champs::SiretChamp) end @@ -43,6 +48,10 @@ class ChampSerializer < ActiveModel::Serializer object.is_a?(Champs::CarteChamp) end + def include_rows? + object.is_a?(Champs::RepetitionChamp) + end + private def legacy_type_de_champ diff --git a/app/serializers/row_serializer.rb b/app/serializers/row_serializer.rb new file mode 100644 index 000000000..b7aa2affd --- /dev/null +++ b/app/serializers/row_serializer.rb @@ -0,0 +1,5 @@ +class RowSerializer < ActiveModel::Serializer + has_many :champs, serializer: ChampSerializer + + attribute :index, key: :id +end diff --git a/app/services/procedure_export_v2_service.rb b/app/services/procedure_export_v2_service.rb index be5bf8ff0..2a14615fa 100644 --- a/app/services/procedure_export_v2_service.rb +++ b/app/services/procedure_export_v2_service.rb @@ -60,11 +60,7 @@ class ProcedureExportV2Service champs_repetables.map do |libelle, champs| [ libelle, - champs.flat_map do |champ| - champ.rows.each_with_index.map do |champs, index| - Champs::RepetitionChamp::Row.new(index: index + 1, dossier_id: champ.dossier_id.to_s, champs: champs) - end - end + champs.flat_map(&:rows_for_export) ] end end diff --git a/spec/controllers/api/v1/dossiers_controller_spec.rb b/spec/controllers/api/v1/dossiers_controller_spec.rb index 2ea28a353..955e2dac5 100644 --- a/spec/controllers/api/v1/dossiers_controller_spec.rb +++ b/spec/controllers/api/v1/dossiers_controller_spec.rb @@ -235,6 +235,22 @@ describe API::V1::DossiersController do it { expect(subject[:type_champ]).to eq('text') } end end + + describe 'repetition' do + let(:procedure) { create(:procedure, administrateur: admin) } + let(:champ) { build(:champ_repetition) } + let(:dossier) { create(:dossier, :en_construction, champs: [champ], procedure: procedure) } + + subject { super().first[:rows] } + + it 'should have rows' do + expect(subject.size).to eq(2) + expect(subject[0][:id]).to eq(1) + expect(subject[0][:champs].size).to eq(2) + expect(subject[0][:champs].map { |c| c[:value] }).to eq(['text', '42']) + expect(subject[0][:champs].map { |c| c[:type_de_champ][:type_champ] }).to eq(['text', 'number']) + end + end end describe 'champs_private' do From 38b48f4217a4d3aae9b6611f6c9c59421e91e9b1 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 29 Jul 2019 10:57:21 +0200 Subject: [PATCH 11/15] transition from accepte to instruction as superadmin --- .../gestionnaires/dossiers_controller.rb | 8 ++++-- .../manager/dossiers_controller.rb | 10 +++++++ app/models/dossier.rb | 5 ++++ app/views/manager/dossiers/show.html.erb | 3 ++ config/routes.rb | 1 + .../gestionnaires/dossiers_controller_spec.rb | 28 ++++++++++++++++++- .../manager/dossiers_controller_spec.rb | 13 +++++++++ 7 files changed, 65 insertions(+), 3 deletions(-) diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index adbc9e237..096b954e6 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -109,8 +109,12 @@ module Gestionnaires if dossier.en_instruction? flash.notice = 'Le dossier est déjà en instruction.' else - flash.notice = "Le dossier #{dossier.id} a été repassé en instruction." - dossier.repasser_en_instruction!(current_gestionnaire) + if dossier.accepte? && !administration_signed_in? + flash.notice = 'Il n\'est pas possible de repasser un dossier accepte en instruction.' + else + flash.notice = "Le dossier #{dossier.id} a été repassé en instruction." + dossier.repasser_en_instruction!(current_gestionnaire) + end end render partial: 'state_button_refresh', locals: { dossier: dossier } diff --git a/app/controllers/manager/dossiers_controller.rb b/app/controllers/manager/dossiers_controller.rb index 35b7c785c..8fd264a62 100644 --- a/app/controllers/manager/dossiers_controller.rb +++ b/app/controllers/manager/dossiers_controller.rb @@ -30,6 +30,16 @@ module Manager redirect_to manager_dossier_path(dossier) end + def repasser_en_instruction + dossier = Dossier.find(params[:id]) + dossier.repasser_en_instruction(current_administration) + + logger.info("Le dossier #{dossier.id} est repassé en instruction par #{current_administration.email}") + flash[:notice] = "Le dossier #{dossier.id} est repassé en instruction." + + redirect_to manager_dossier_path(dossier) + end + private def unfiltered_list? diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 30bbe5312..10f6d0b1c 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1,5 +1,6 @@ class Dossier < ApplicationRecord include DossierFilteringConcern + include Devise::Controllers::Helpers enum state: { brouillon: 'brouillon', @@ -86,6 +87,7 @@ class Dossier < ApplicationRecord event :repasser_en_instruction, after: :after_repasser_en_instruction do transitions from: :refuse, to: :en_instruction transitions from: :sans_suite, to: :en_instruction + transitions from: :accepte, to: :en_instruction end end @@ -228,6 +230,9 @@ class Dossier < ApplicationRecord !procedure.archivee? && brouillon? end + def can_retransition_to_en_instruction? + end + def can_be_updated_by_user? brouillon? || en_construction? end diff --git a/app/views/manager/dossiers/show.html.erb b/app/views/manager/dossiers/show.html.erb index 1dcae61cc..b8a1ec640 100644 --- a/app/views/manager/dossiers/show.html.erb +++ b/app/views/manager/dossiers/show.html.erb @@ -28,6 +28,9 @@ as well as a link to its edit page.
+ <% if dossier.accepte? %> + <%= link_to 'Repasser en instruction', repasser_en_instruction_manager_dossier_path(dossier), method: :post, class: 'button', data: { confirm: "Confirmez vous le passage en instruction du dossier ?" } %> + <% end %> <% if dossier.hidden_at.nil? %> <%= link_to 'Supprimer le dossier', hide_manager_dossier_path(dossier), method: :post, class: 'button', data: { confirm: "Confirmez vous la suppression du dossier ?" } %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index d13ff6051..1a4bd3c6c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,6 +16,7 @@ Rails.application.routes.draw do resources :dossiers, only: [:index, :show] do post 'hide', on: :member + post 'repasser_en_instruction', on: :member end resources :administrateurs, only: [:index, :show, :new, :create] do diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index 0f327221a..9efeb267b 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -4,6 +4,8 @@ describe Gestionnaires::DossiersController, type: :controller do render_views let(:gestionnaire) { create(:gestionnaire) } + let(:administrateur) { create(:administrateur) } + let(:administration) { create(:administration) } let(:gestionnaires) { [gestionnaire] } let(:procedure) { create(:procedure, :published, gestionnaires: gestionnaires) } let(:dossier) { create(:dossier, :en_construction, procedure: procedure) } @@ -153,9 +155,10 @@ describe Gestionnaires::DossiersController, type: :controller do describe '#repasser_en_instruction' do let(:dossier) { create(:dossier, :refuse, procedure: procedure) } + let(:current_user) { gestionnaire } before do - sign_in gestionnaire + sign_in current_user post :repasser_en_instruction, params: { procedure_id: procedure.id, dossier_id: dossier.id }, format: 'js' @@ -173,6 +176,29 @@ describe Gestionnaires::DossiersController, type: :controller do expect(response).to have_http_status(:ok) end end + + context 'when the dossier is accepte' do + let(:dossier) { create(:dossier, :accepte, procedure: procedure) } + + it 'it is not possible to go back to en_instruction as gestionnaire' do + expect(dossier.reload.state).to eq(Dossier.states.fetch(:accepte)) + expect(response).to have_http_status(:ok) + end + context 'as administrateur' do + let (:current_user) { administrateur } + it 'it is not possible to go back to en_instruction' do + expect(dossier.reload.state).to eq(Dossier.states.fetch(:accepte)) + expect(response).to have_http_status(:ok) + end + end + context 'as superadmin' do + let (:current_user) { administration } + it 'it is not possible to go back to en_instruction' do + expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_instruction)) + expect(response).to have_http_status(:ok) + end + end + end end describe '#terminer' do diff --git a/spec/controllers/manager/dossiers_controller_spec.rb b/spec/controllers/manager/dossiers_controller_spec.rb index e3c5f3cad..cc835e6d6 100644 --- a/spec/controllers/manager/dossiers_controller_spec.rb +++ b/spec/controllers/manager/dossiers_controller_spec.rb @@ -11,4 +11,17 @@ describe Manager::DossiersController, type: :controller do it { expect(dossier.hidden_at).not_to be_nil } end + + describe '#repasser_en_instruction' do + let(:administration) { create :administration } + let!(:dossier) { create(:dossier, :accepte) } + + before do + sign_in administration + post :repasser_en_instruction, params: { id: dossier.id } + dossier.reload + end + + it { expect(dossier.en_instruction?).to be true } + end end From 2f82dd9a3ddb6d174378428dd2e1891444fb552d Mon Sep 17 00:00:00 2001 From: Keirua Date: Tue, 30 Jul 2019 08:42:22 +0200 Subject: [PATCH 12/15] improve typography and texts Co-Authored-By: Nicolas Bouilleaud --- app/controllers/gestionnaires/dossiers_controller.rb | 2 +- spec/controllers/gestionnaires/dossiers_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index 096b954e6..8cea6f1f6 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -110,7 +110,7 @@ module Gestionnaires flash.notice = 'Le dossier est déjà en instruction.' else if dossier.accepte? && !administration_signed_in? - flash.notice = 'Il n\'est pas possible de repasser un dossier accepte en instruction.' + flash.notice = 'Il n’est pas possible de repasser un dossier accepté en instruction.' else flash.notice = "Le dossier #{dossier.id} a été repassé en instruction." dossier.repasser_en_instruction!(current_gestionnaire) diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index 9efeb267b..2f02eb67a 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -193,7 +193,7 @@ describe Gestionnaires::DossiersController, type: :controller do end context 'as superadmin' do let (:current_user) { administration } - it 'it is not possible to go back to en_instruction' do + it 'it is possible to go back to en_instruction' do expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_instruction)) expect(response).to have_http_status(:ok) end From f5d3818e3cdd27ed120f84c77c597376a8feb3af Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 30 Jul 2019 10:12:57 +0200 Subject: [PATCH 13/15] remove unnecessary retransition methods --- app/controllers/gestionnaires/dossiers_controller.rb | 2 +- app/models/dossier.rb | 4 ---- spec/controllers/gestionnaires/dossiers_controller_spec.rb | 4 ++-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index 8cea6f1f6..a830229a4 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -109,7 +109,7 @@ module Gestionnaires if dossier.en_instruction? flash.notice = 'Le dossier est déjà en instruction.' else - if dossier.accepte? && !administration_signed_in? + if dossier.accepte? flash.notice = 'Il n’est pas possible de repasser un dossier accepté en instruction.' else flash.notice = "Le dossier #{dossier.id} a été repassé en instruction." diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 10f6d0b1c..913976add 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1,6 +1,5 @@ class Dossier < ApplicationRecord include DossierFilteringConcern - include Devise::Controllers::Helpers enum state: { brouillon: 'brouillon', @@ -230,9 +229,6 @@ class Dossier < ApplicationRecord !procedure.archivee? && brouillon? end - def can_retransition_to_en_instruction? - end - def can_be_updated_by_user? brouillon? || en_construction? end diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index 2f02eb67a..986ae1e46 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -193,8 +193,8 @@ describe Gestionnaires::DossiersController, type: :controller do end context 'as superadmin' do let (:current_user) { administration } - it 'it is possible to go back to en_instruction' do - expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_instruction)) + it 'it is not possible to go back to en_instruction' do + expect(dossier.reload.state).to eq(Dossier.states.fetch(:accepte)) expect(response).to have_http_status(:ok) end end From c75e39884e2833bf21ed86cd586b6fb54620f4ed Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 19 Dec 2018 11:09:13 +0100 Subject: [PATCH 14/15] Save selection utilisateur as geo area --- app/controllers/champs/carte_controller.rb | 6 +- app/models/champs/carte_champ.rb | 80 ++++++++++++++-------- app/models/geo_area.rb | 4 ++ app/serializers/dossier_serializer.rb | 4 +- app/services/api_carto_service.rb | 6 ++ spec/factories/geo_area.rb | 4 ++ spec/models/champs/carte_champ_spec.rb | 12 ++-- spec/serializers/champ_serializer_spec.rb | 24 ++++--- 8 files changed, 94 insertions(+), 46 deletions(-) diff --git a/app/controllers/champs/carte_controller.rb b/app/controllers/champs/carte_controller.rb index 1410a9cde..47c15da93 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -55,11 +55,15 @@ class Champs::CarteController < ApplicationController end end + selection_utilisateur = ApiCartoService.generate_selection_utilisateur(coordinates) + selection_utilisateur[:source] = GeoArea.sources.fetch(:selection_utilisateur) + geo_areas << selection_utilisateur + @champ.geo_areas = geo_areas.map do |geo_area| GeoArea.new(geo_area) end - @champ.value = GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) + @champ.value = coordinates.to_json end if @champ.persisted? diff --git a/app/models/champs/carte_champ.rb b/app/models/champs/carte_champ.rb index 7992d50e1..cb11520e6 100644 --- a/app/models/champs/carte_champ.rb +++ b/app/models/champs/carte_champ.rb @@ -19,6 +19,10 @@ class Champs::CarteChamp < Champ end end + def selection_utilisateur + geo_areas.find(&:selection_utilisateur?) + end + def cadastres? type_de_champ&.cadastres && type_de_champ.cadastres != '0' end @@ -45,6 +49,49 @@ class Champs::CarteChamp < Champ def geo_json @geo_json ||= begin + geo_area = selection_utilisateur + + if geo_area + geo_area.geometry + else + geo_json_from_value + end + end + end + + def selection_utilisateur_size + if geo_json.present? + geo_json['coordinates'].size + else + 0 + end + end + + def to_render_data + { + position: position, + selection: user_geo_area&.geometry, + quartiersPrioritaires: quartiers_prioritaires? ? quartiers_prioritaires.as_json(except: :properties) : [], + cadastres: cadastres? ? cadastres.as_json(except: :properties) : [], + parcellesAgricoles: parcelles_agricoles? ? parcelles_agricoles.as_json(except: :properties) : [] + } + end + + def user_geo_area + geo_area = selection_utilisateur + + if geo_area.present? + geo_area + elsif geo_json_from_value.present? + GeoArea.new( + geometry: geo_json_from_value, + source: GeoArea.sources.fetch(:selection_utilisateur) + ) + end + end + + def geo_json_from_value + @geo_json_from_value ||= begin parsed_value = value.blank? ? nil : JSON.parse(value) # We used to store in the value column a json array with coordinates. if parsed_value.is_a?(Array) @@ -62,34 +109,11 @@ class Champs::CarteChamp < Champ end end - def selection_utilisateur_size - if geo_json.present? - geo_json['coordinates'].size - else - 0 - end - end - - def to_render_data - { - position: position, - selection: geo_json, - quartiersPrioritaires: quartiers_prioritaires? ? quartiers_prioritaires.as_json(except: :properties) : [], - cadastres: cadastres? ? cadastres.as_json(except: :properties) : [], - parcellesAgricoles: parcelles_agricoles? ? parcelles_agricoles.as_json(except: :properties) : [] - } - end - - def user_geo_area - if geo_json.present? - GeoArea.new( - geometry: geo_json, - source: GeoArea.sources.fetch(:selection_utilisateur) - ) - end - end - def for_api - geo_json&.to_json + nil + end + + def for_export + nil end end diff --git a/app/models/geo_area.rb b/app/models/geo_area.rb index 05ad92ebd..7109d0313 100644 --- a/app/models/geo_area.rb +++ b/app/models/geo_area.rb @@ -30,4 +30,8 @@ class GeoArea < ApplicationRecord scope :quartiers_prioritaires, -> { where(source: sources.fetch(:quartier_prioritaire)) } scope :cadastres, -> { where(source: sources.fetch(:cadastre)) } scope :parcelles_agricoles, -> { where(source: sources.fetch(:parcelle_agricole)) } + + def selection_utilisateur? + source == self.class.sources.fetch(:selection_utilisateur) + end end diff --git a/app/serializers/dossier_serializer.rb b/app/serializers/dossier_serializer.rb index dbd3e03ad..358a88a21 100644 --- a/app/serializers/dossier_serializer.rb +++ b/app/serializers/dossier_serializer.rb @@ -39,7 +39,9 @@ class DossierSerializer < ActiveModel::Serializer if champ_carte.present? carto_champs = champ_carte.geo_areas.to_a - carto_champs << champ_carte.user_geo_area + if !carto_champs.find(&:selection_utilisateur?) + carto_champs << champ_carte.user_geo_area + end champs += carto_champs.compact end end diff --git a/app/services/api_carto_service.rb b/app/services/api_carto_service.rb index ec0f64dfe..9bca82a6c 100644 --- a/app/services/api_carto_service.rb +++ b/app/services/api_carto_service.rb @@ -22,4 +22,10 @@ class ApiCartoService ).results end end + + def self.generate_selection_utilisateur(coordinates) + { + geometry: JSON.parse(GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates)) + } + end end diff --git a/spec/factories/geo_area.rb b/spec/factories/geo_area.rb index 33b2decde..12e87c679 100644 --- a/spec/factories/geo_area.rb +++ b/spec/factories/geo_area.rb @@ -9,5 +9,9 @@ FactoryBot.define do nom { 'XYZ' } commune { 'Paris' } end + + trait :selection_utilisateur do + source { GeoArea.sources.fetch(:selection_utilisateur) } + end end end diff --git a/spec/models/champs/carte_champ_spec.rb b/spec/models/champs/carte_champ_spec.rb index 8d0875a88..6beed9886 100644 --- a/spec/models/champs/carte_champ_spec.rb +++ b/spec/models/champs/carte_champ_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' describe Champs::CarteChamp do let(:champ) { Champs::CarteChamp.new(value: value) } let(:value) { '' } - let(:geo_json) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } 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 }]] } - let(:parsed_geo_json) { JSON.parse(geo_json) } + let(:geo_json_as_string) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } + let(:geo_json) { JSON.parse(geo_json_as_string) } describe '#to_render_data' do subject { champ.to_render_data } @@ -47,15 +47,15 @@ describe Champs::CarteChamp do context 'when the value is coordinates' do let(:value) { coordinates.to_json } - let(:selection) { parsed_geo_json } + let(:selection) { geo_json } it { is_expected.to eq(render_data) } end context 'when the value is geojson' do - let(:value) { geo_json } + let(:value) { geo_json.to_json } - let(:selection) { parsed_geo_json } + let(:selection) { geo_json } it { is_expected.to eq(render_data) } end @@ -89,7 +89,7 @@ describe Champs::CarteChamp do end context 'when the value is geojson' do - let(:value) { geo_json } + let(:value) { geo_json.to_json } it { is_expected.to eq(1) } end diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index f2b4d2775..07ff5e10e 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -26,9 +26,9 @@ describe ChampSerializer do context 'when type champ is carte' do let(:champ) { create(:champ_carte, value: value, geo_areas: [geo_area].compact) } let(:value) { nil } - let(:geo_area) { create(:geo_area, geometry: parsed_geo_json) } - let(:parsed_geo_json) { JSON.parse(geo_json) } - let(:geo_json) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } + let(:geo_area) { create(:geo_area, geometry: geo_json) } + let(:geo_json_as_string) { GeojsonService.to_json_polygon_for_selection_utilisateur(coordinates) } + let(:geo_json) { JSON.parse(geo_json_as_string) } 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 }]] } let(:serialized_champ) { @@ -49,10 +49,14 @@ describe ChampSerializer do let(:serialized_id) { -1 } let(:serialized_description) { "" } let(:serialized_order_place) { -1 } - let(:serialized_value) { parsed_geo_json } + let(:serialized_value) { geo_json } context 'and geo_area is selection_utilisateur' do + let(:geo_area) { create(:geo_area, :selection_utilisateur, geometry: geo_json) } + context 'value is empty' do + let(:geo_area) { nil } + context 'when value is nil' do let(:value) { nil } @@ -85,7 +89,7 @@ describe ChampSerializer do end context 'when value is geojson' do - let(:value) { geo_json } + let(:value) { geo_json.to_json } it { expect(subject).to eq(serialized_champ) } end @@ -105,7 +109,7 @@ describe ChampSerializer do let(:serialized_order_place) { champ.order_place } let(:serialized_libelle) { champ.libelle } let(:serialized_type_champ) { champ.type_champ } - let(:serialized_value) { geo_json } + let(:serialized_value) { nil } context 'when value is coordinates' do let(:value) { coordinates.to_json } @@ -114,7 +118,7 @@ describe ChampSerializer do end context 'when value is geojson' do - let(:value) { geo_json } + let(:value) { geo_json.to_json } it { expect(subject).to eq(serialized_champ) } end @@ -147,7 +151,7 @@ describe ChampSerializer do it { expect(subject[:geo_areas].first).to include( source: GeoArea.sources.fetch(:cadastre), - geometry: parsed_geo_json, + geometry: geo_json, numero: '42', feuille: 'A11' ) @@ -165,13 +169,13 @@ describe ChampSerializer do end context 'and geo_area is quartier_prioritaire' do - let(:geo_area) { create(:geo_area, :quartier_prioritaire, geometry: parsed_geo_json) } + let(:geo_area) { create(:geo_area, :quartier_prioritaire, geometry: geo_json) } context 'new_api' do it { expect(subject[:geo_areas].first).to include( source: GeoArea.sources.fetch(:quartier_prioritaire), - geometry: parsed_geo_json, + geometry: geo_json, nom: 'XYZ', commune: 'Paris' ) From 586141a5961c511ed2fed9ce098c2b3b5cddca00 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 31 Jul 2019 17:54:51 +0200 Subject: [PATCH 15/15] Add selection_utilisateur geo_area to all champs carte with value --- .../20190731152733_migrate_geo_area_data.rake | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 lib/tasks/deployment/20190731152733_migrate_geo_area_data.rake diff --git a/lib/tasks/deployment/20190731152733_migrate_geo_area_data.rake b/lib/tasks/deployment/20190731152733_migrate_geo_area_data.rake new file mode 100644 index 000000000..43cde414b --- /dev/null +++ b/lib/tasks/deployment/20190731152733_migrate_geo_area_data.rake @@ -0,0 +1,28 @@ +namespace :after_party do + desc 'Deployment task: migrate_geo_area_data' + task migrate_geo_area_data: :environment do + puts "Running deploy task 'migrate_geo_area_data'" + + progress = ProgressReport.new(Champs::CarteChamp.count) + + Champs::CarteChamp.includes(:geo_areas).find_each do |champ| + geo_area = champ.geo_areas.find(&:selection_utilisateur?) + geo_json = champ.geo_json_from_value + + if geo_area.blank? && geo_json.present? + GeoArea.create( + champ: champ, + geometry: geo_json, + source: GeoArea.sources.fetch(:selection_utilisateur) + ) + progress.inc + end + end + + progress.finish + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord.create version: '20190731152733' + end +end