From 949a0c21e69efed0a43eef8012123c07ba12c64f Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 16 Sep 2019 15:58:15 +0200 Subject: [PATCH 01/28] gestion des 2 formats de bouton monavis --- app/validators/mon_avis_embed_validator.rb | 2 +- .../admin/procedures_controller_spec.rb | 23 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/validators/mon_avis_embed_validator.rb b/app/validators/mon_avis_embed_validator.rb index ac84bad24..17e8059bd 100644 --- a/app/validators/mon_avis_embed_validator.rb +++ b/app/validators/mon_avis_embed_validator.rb @@ -1,7 +1,7 @@ class MonAvisEmbedValidator < ActiveModel::Validator def validate(record) # We need to ensure the embed code is not any random string in order to avoid injections - r = Regexp.new('\s*Je donne mon avis\s*', Regexp::MULTILINE) + r = Regexp.new('\s*Je donne mon avis\s*', Regexp::MULTILINE) if record.monavis_embed.present? && !r.match?(record.monavis_embed) record.errors[:base] << "Le code fourni ne correspond pas au format des codes MonAvis reconnus par la plateforme." end diff --git a/spec/controllers/admin/procedures_controller_spec.rb b/spec/controllers/admin/procedures_controller_spec.rb index 0ed1d5613..7df3010d7 100644 --- a/spec/controllers/admin/procedures_controller_spec.rb +++ b/spec/controllers/admin/procedures_controller_spec.rb @@ -796,10 +796,11 @@ describe Admin::ProceduresController, type: :controller do patch :update_monavis, params: { procedure_id: procedure.id, procedure: procedure_params } procedure.reload end + let(:monavis_embed) { <<-MSG - - Je donne mon avis + + Je donne mon avis MSG } @@ -820,6 +821,24 @@ describe Admin::ProceduresController, type: :controller do it { expect(response.body).to include "MonAvis" } end + context 'when the embed code is valid with the original format' do + let(:monavis_embed) { + <<-MSG + + Je donne mon avis + + MSG + } + describe 'the monavis field is updated' do + subject { procedure } + + it { expect(subject.monavis_embed).to eq(monavis_embed) } + end + + it { expect(flash[:notice]).to be_present } + it { expect(response.body).to include "MonAvis" } + end + context 'when the embed code is not valid' do let(:monavis_embed) { 'invalid embed code' } From fc416ee2ec488b2e759e50ef1ad371936edfcd90 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 17 Sep 2019 12:24:24 +0200 Subject: [PATCH 02/28] Fix procedure_locked_spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `login_as administrateur, scope: :administrateur` doesn’t seem to actually do anything, but testing for the _absence_ of content can’t break. - Make the test fail if login fails, - Fix the login, - Write the opposite test, - Update the searched content to the actual text used in the partial --- spec/features/admin/procedure_locked_spec.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/procedure_locked_spec.rb b/spec/features/admin/procedure_locked_spec.rb index 8d0dbe001..eeb76dd44 100644 --- a/spec/features/admin/procedure_locked_spec.rb +++ b/spec/features/admin/procedure_locked_spec.rb @@ -2,17 +2,27 @@ require 'spec_helper' feature 'procedure locked' do let(:administrateur) { create(:administrateur) } - let (:published_at) { nil } - let(:procedure) { create(:procedure, administrateur: administrateur, published_at: published_at) } before do - login_as administrateur, scope: :administrateur + login_as administrateur.user, scope: :user visit admin_procedure_path(procedure) end context 'when procedure is not published' do + let(:procedure) { create(:procedure, administrateur: administrateur) } + scenario 'info label is not present' do - expect(page).not_to have_content('Cette démarche a été publiée, certains éléments ne peuvent plus être modifiés') + expect(page).to have_content('Test et publication') + expect(page).not_to have_content('Cette démarche est publiée, certains éléments ne peuvent plus être modifiés.') + end + end + + context 'when procedure is published' do + let(:procedure) { create(:procedure, :published, administrateur: administrateur) } + + scenario 'info label is present' do + expect(page).to have_content('Publication') + expect(page).to have_content('Cette démarche est publiée, certains éléments ne peuvent plus être modifiés.') end end end From 8e44172e595cd0ccd8503f9baf610540d745f852 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 14:13:32 +0200 Subject: [PATCH 03/28] Remove unused columns and tables --- .../20190917115911_drop_flipflop_features.rb | 8 +++ ...190917120039_remove_carrierwave_columns.rb | 8 +++ .../20190917120047_remove_devise_columns.rb | 31 +++++++++++ .../20190917120856_remove_unused_columns.rb | 6 +++ db/schema.rb | 51 +------------------ 5 files changed, 54 insertions(+), 50 deletions(-) create mode 100644 db/migrate/20190917115911_drop_flipflop_features.rb create mode 100644 db/migrate/20190917120039_remove_carrierwave_columns.rb create mode 100644 db/migrate/20190917120047_remove_devise_columns.rb create mode 100644 db/migrate/20190917120856_remove_unused_columns.rb diff --git a/db/migrate/20190917115911_drop_flipflop_features.rb b/db/migrate/20190917115911_drop_flipflop_features.rb new file mode 100644 index 000000000..57afc9723 --- /dev/null +++ b/db/migrate/20190917115911_drop_flipflop_features.rb @@ -0,0 +1,8 @@ +class DropFlipflopFeatures < ActiveRecord::Migration[5.2] + def change + remove_column :administrateurs, :features + remove_column :instructeurs, :features + + drop_table :flipflop_features + end +end diff --git a/db/migrate/20190917120039_remove_carrierwave_columns.rb b/db/migrate/20190917120039_remove_carrierwave_columns.rb new file mode 100644 index 000000000..41258b967 --- /dev/null +++ b/db/migrate/20190917120039_remove_carrierwave_columns.rb @@ -0,0 +1,8 @@ +class RemoveCarrierwaveColumns < ActiveRecord::Migration[5.2] + def change + remove_columns :procedures, :logo, :logo_secure_token + remove_columns :commentaires, :file, :piece_justificative_id + remove_columns :attestations, :pdf, :content_secure_token + remove_columns :attestation_templates, :logo, :logo_secure_token, :signature, :signature_secure_token + end +end diff --git a/db/migrate/20190917120047_remove_devise_columns.rb b/db/migrate/20190917120047_remove_devise_columns.rb new file mode 100644 index 000000000..951d0a490 --- /dev/null +++ b/db/migrate/20190917120047_remove_devise_columns.rb @@ -0,0 +1,31 @@ +class RemoveDeviseColumns < ActiveRecord::Migration[5.2] + def change + remove_columns :administrateurs, + :encrypted_password, + :reset_password_token, + :reset_password_sent_at, + :remember_created_at, + :sign_in_count, + :current_sign_in_at, + :last_sign_in_at, + :current_sign_in_ip, + :last_sign_in_ip, + :failed_attempts, + :unlock_token, + :locked_at + + remove_columns :instructeurs, + :encrypted_password, + :reset_password_token, + :reset_password_sent_at, + :remember_created_at, + :sign_in_count, + :current_sign_in_at, + :last_sign_in_at, + :current_sign_in_ip, + :last_sign_in_ip, + :failed_attempts, + :unlock_token, + :locked_at + end +end diff --git a/db/migrate/20190917120856_remove_unused_columns.rb b/db/migrate/20190917120856_remove_unused_columns.rb new file mode 100644 index 000000000..50930a3b8 --- /dev/null +++ b/db/migrate/20190917120856_remove_unused_columns.rb @@ -0,0 +1,6 @@ +class RemoveUnusedColumns < ActiveRecord::Migration[5.2] + def change + remove_column :dossiers, :json_latlngs + remove_column :services, :siret + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d0416d1e..330d17d99 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_08_28_073736) do +ActiveRecord::Schema.define(version: 2019_09_17_120856) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -49,26 +49,11 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do create_table "administrateurs", id: :serial, force: :cascade do |t| t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false - t.string "reset_password_token" - t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at" - t.datetime "last_sign_in_at" - t.string "current_sign_in_ip" - t.string "last_sign_in_ip" t.datetime "created_at" t.datetime "updated_at" t.boolean "active", default: false - t.jsonb "features", default: {}, null: false t.string "encrypted_token" - t.integer "failed_attempts", default: 0, null: false - t.string "unlock_token" - t.datetime "locked_at" t.index ["email"], name: "index_administrateurs_on_email", unique: true - t.index ["reset_password_token"], name: "index_administrateurs_on_reset_password_token", unique: true - t.index ["unlock_token"], name: "index_administrateurs_on_unlock_token", unique: true end create_table "administrateurs_instructeurs", id: false, force: :cascade do |t| @@ -130,24 +115,18 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.text "title" t.text "body" t.text "footer" - t.string "logo" - t.string "signature" t.boolean "activated" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "procedure_id" - t.string "logo_secure_token" - t.string "signature_secure_token" t.index ["procedure_id"], name: "index_attestation_templates_on_procedure_id", unique: true end create_table "attestations", id: :serial, force: :cascade do |t| - t.string "pdf" t.string "title" t.integer "dossier_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "content_secure_token" t.index ["dossier_id"], name: "index_attestations_on_dossier_id" end @@ -205,8 +184,6 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.string "body" t.integer "dossier_id" t.datetime "updated_at", null: false - t.integer "piece_justificative_id" - t.string "file" t.bigint "user_id" t.bigint "instructeur_id" t.index ["dossier_id"], name: "index_commentaires_on_dossier_id" @@ -266,7 +243,6 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.datetime "updated_at" t.string "state" t.integer "user_id" - t.text "json_latlngs" t.boolean "archived", default: false t.datetime "en_construction_at" t.datetime "en_instruction_at" @@ -348,13 +324,6 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.index ["user_id"], name: "index_feedbacks_on_user_id" end - create_table "flipflop_features", force: :cascade do |t| - t.string "key", null: false - t.boolean "enabled", default: false, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - end - create_table "flipper_features", force: :cascade do |t| t.string "key", null: false t.datetime "created_at", null: false @@ -443,26 +412,11 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do create_table "instructeurs", id: :serial, force: :cascade do |t| t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false - t.string "reset_password_token" - t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at" - t.datetime "last_sign_in_at" - t.string "current_sign_in_ip" - t.string "last_sign_in_ip" t.datetime "created_at" t.datetime "updated_at" t.text "encrypted_login_token" t.datetime "login_token_created_at" - t.jsonb "features", default: {"enable_email_login_token"=>true}, null: false - t.integer "failed_attempts", default: 0, null: false - t.string "unlock_token" - t.datetime "locked_at" t.index ["email"], name: "index_instructeurs_on_email", unique: true - t.index ["reset_password_token"], name: "index_instructeurs_on_reset_password_token", unique: true - t.index ["unlock_token"], name: "index_instructeurs_on_unlock_token", unique: true end create_table "invites", id: :serial, force: :cascade do |t| @@ -505,9 +459,7 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "euro_flag", default: false - t.string "logo" t.boolean "cerfa_flag", default: false - t.string "logo_secure_token" t.string "lien_site_web" t.string "lien_notice" t.boolean "for_individual", default: false @@ -566,7 +518,6 @@ ActiveRecord::Schema.define(version: 2019_08_28_073736) do t.string "telephone" t.text "horaires" t.text "adresse" - t.string "siret" t.index ["administrateur_id", "nom"], name: "index_services_on_administrateur_id_and_nom", unique: true t.index ["administrateur_id"], name: "index_services_on_administrateur_id" end From ae34e48624688e3032a2fa1e0f97bec31a3c2b66 Mon Sep 17 00:00:00 2001 From: maatinito <15379878+maatinito@users.noreply.github.com> Date: Thu, 12 Sep 2019 14:55:22 -1000 Subject: [PATCH 04/28] [fix #4311] Unable to reinvite admin --- app/controllers/manager/administrateurs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/manager/administrateurs_controller.rb b/app/controllers/manager/administrateurs_controller.rb index 18e30f9a1..f976f3cf6 100644 --- a/app/controllers/manager/administrateurs_controller.rb +++ b/app/controllers/manager/administrateurs_controller.rb @@ -14,7 +14,7 @@ module Manager end def reinvite - Administrateur.find_inactive_by_id(params[:id]).invite!(current_administration.id) + Administrateur.find_inactive_by_id(params[:id]).user.invite_administrateur!(current_administration.id) flash.notice = "Invitation renvoyée" redirect_to manager_administrateur_path(params[:id]) end From e9c6ed80e4beb45823d2c50562c463c1b460008e Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 17 Sep 2019 15:55:35 +0200 Subject: [PATCH 05/28] Migration: Index and make Procedure.path nonnull --- app/models/procedure.rb | 2 +- db/migrate/20190917151652_make_path_nonnull.rb | 6 ++++++ db/schema.rb | 5 +++-- spec/factories/procedure.rb | 1 + 4 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20190917151652_make_path_nonnull.rb diff --git a/app/models/procedure.rb b/app/models/procedure.rb index a93d73a5f..b9760fa41 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -66,7 +66,7 @@ class Procedure < ApplicationRecord validates :description, presence: true, allow_blank: false, allow_nil: false validates :administrateurs, presence: true validate :check_juridique - validates :path, format: { with: /\A[a-z0-9_\-]{3,50}\z/ }, uniqueness: { scope: :aasm_state, case_sensitive: false }, presence: true, allow_blank: false, allow_nil: true + validates :path, presence: true, format: { with: /\A[a-z0-9_\-]{3,50}\z/ }, uniqueness: { scope: [:path, :archived_at, :hidden_at], case_sensitive: false } # FIXME: remove duree_conservation_required flag once all procedures are converted to the new style validates :duree_conservation_dossiers_dans_ds, allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_DUREE_CONSERVATION }, if: :durees_conservation_required validates :duree_conservation_dossiers_hors_ds, allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :durees_conservation_required diff --git a/db/migrate/20190917151652_make_path_nonnull.rb b/db/migrate/20190917151652_make_path_nonnull.rb new file mode 100644 index 000000000..401a13c84 --- /dev/null +++ b/db/migrate/20190917151652_make_path_nonnull.rb @@ -0,0 +1,6 @@ +class MakePathNonnull < ActiveRecord::Migration[5.2] + def change + change_column_null :procedures, :path, false + add_index :procedures, [:path, :archived_at, :hidden_at], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 330d17d99..9914c4180 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_09_17_120856) do +ActiveRecord::Schema.define(version: 2019_09_17_151652) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -480,12 +480,13 @@ ActiveRecord::Schema.define(version: 2019_09_17_120856) do t.string "cadre_juridique" t.boolean "juridique_required", default: true t.boolean "durees_conservation_required", default: true - t.string "path" + t.string "path", null: false t.string "declarative_with_state" 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" t.index ["parent_procedure_id"], name: "index_procedures_on_parent_procedure_id" + t.index ["path", "archived_at", "hidden_at"], name: "index_procedures_on_path_and_archived_at_and_hidden_at", unique: true t.index ["service_id"], name: "index_procedures_on_service_id" end diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 118b1c7d5..23b32efa7 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -11,6 +11,7 @@ FactoryBot.define do duree_conservation_dossiers_hors_ds { 6 } ask_birthday { false } lien_site_web { "https://mon-site.gouv" } + path { SecureRandom.uuid } transient do administrateur {} From 8fa630d2bb03a591ae3c3cb2cc58c41328eae08e Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 17 Sep 2019 15:07:58 +0200 Subject: [PATCH 06/28] Remove Gestionnaire.visible_procedures It is actually the same thing as Gestionnaire.procedures. It already included the procedures with paths as well as the archived procedures, and in production, there were no Gestionnaire for who procedures was returning a different result than visible_procedures (expect for two baddata brouillon procedures with a nil path). In addition, Procedure.path is now nonnull, which means the Procedure.avec_lien scope is pointless. Finally, the current spec showed that the only procedure not visible to the gestion was the one he was not assigned to. --- .../instructeurs/procedures_controller.rb | 4 +-- app/models/instructeur.rb | 4 --- app/models/procedure.rb | 1 - app/views/layouts/_new_header.haml | 2 +- spec/models/instructeur_spec.rb | 25 ------------------- 5 files changed, 3 insertions(+), 33 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 83cc2d03d..fe323dee4 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -7,7 +7,7 @@ module Instructeurs def index @procedures = current_instructeur - .visible_procedures + .procedures .with_attached_logo .includes(:defaut_groupe_instructeur) .order(archived_at: :desc, published_at: :desc, created_at: :desc) @@ -235,7 +235,7 @@ module Instructeurs end def redirect_to_avis_if_needed - if current_instructeur.visible_procedures.count == 0 && current_instructeur.avis.count > 0 + if current_instructeur.procedures.count == 0 && current_instructeur.avis.count > 0 redirect_to instructeur_avis_index_path end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 0657032be..3cdfa45c9 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -24,10 +24,6 @@ class Instructeur < ApplicationRecord has_one :user - def visible_procedures - procedures.merge(Procedure.avec_lien.or(Procedure.archivees)) - end - def follow(dossier) begin followed_dossiers << dossier diff --git a/app/models/procedure.rb b/app/models/procedure.rb index b9760fa41..b425e0b60 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -45,7 +45,6 @@ class Procedure < ApplicationRecord scope :by_libelle, -> { order(libelle: :asc) } scope :created_during, -> (range) { where(created_at: range) } scope :cloned_from_library, -> { where(cloned_from_library: true) } - scope :avec_lien, -> { where.not(path: nil) } scope :declarative, -> { where.not(declarative_with_state: nil) } scope :for_api, -> { diff --git a/app/views/layouts/_new_header.haml b/app/views/layouts/_new_header.haml index eb3c7eb97..440a01b4a 100644 --- a/app/views/layouts/_new_header.haml +++ b/app/views/layouts/_new_header.haml @@ -15,7 +15,7 @@ - if nav_bar_profile == :instructeur && instructeur_signed_in? - current_url = request.path_info %ul.header-tabs - - if current_instructeur.visible_procedures.count > 0 + - if current_instructeur.procedures.count > 0 %li = active_link_to "Démarches", instructeur_procedures_path, active: :inclusive, class: 'tab-link' - if current_instructeur.avis.count > 0 diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 281123918..5b0ba0ed4 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -12,31 +12,6 @@ describe Instructeur, type: :model do assign(procedure_2) end - describe '#visible_procedures' do - let(:procedure_not_assigned) { create :procedure, administrateur: admin } - let(:procedure_with_default_path) { create :procedure, administrateur: admin } - let(:procedure_with_custom_path) { create :procedure, :with_path, administrateur: admin } - let(:procedure_archived_manually) { create :procedure, :archived, administrateur: admin } - let(:procedure_archived_automatically) { create :procedure, :archived_automatically, administrateur: admin } - - before do - assign(procedure_with_default_path) - assign(procedure_with_custom_path) - assign(procedure_archived_manually) - assign(procedure_archived_automatically) - end - - subject { instructeur.visible_procedures } - - it do - expect(subject).not_to include(procedure_not_assigned) - expect(subject).to include(procedure_with_default_path) - expect(subject).to include(procedure_with_custom_path) - expect(subject).to include(procedure_archived_manually) - expect(subject).to include(procedure_archived_automatically) - end - end - describe 'follow' do let(:dossier) { create :dossier } let(:already_followed_dossier) { create :dossier } From b9968b76b0a4767973376a4cd0b2c9005518bf39 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Mon, 16 Sep 2019 16:59:10 +0200 Subject: [PATCH 07/28] =?UTF-8?q?Remove=20=E2=80=9Cavec=5Flien=E2=80=9D=20?= =?UTF-8?q?helpers=20now=20that=20Procedure.path=20cannot=20be=20nil?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/admin_controller.rb | 2 +- .../new_administrateur/administrateur_controller.rb | 2 +- app/helpers/procedure_helper.rb | 10 ++++------ app/models/procedure.rb | 5 ----- app/serializers/procedure_serializer.rb | 10 ++++------ app/views/admin/procedures/show.html.haml | 4 ++-- ...t_panel_admin_procedurescontroller_navbar.html.haml | 2 +- 7 files changed, 13 insertions(+), 22 deletions(-) diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 926bb0ff4..608c1fcef 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -25,7 +25,7 @@ class AdminController < ApplicationController end def reset_procedure - if @procedure.brouillon_avec_lien? + if @procedure.brouillon? @procedure.reset! end end diff --git a/app/controllers/new_administrateur/administrateur_controller.rb b/app/controllers/new_administrateur/administrateur_controller.rb index 2e1ad0eb4..d8828ba1c 100644 --- a/app/controllers/new_administrateur/administrateur_controller.rb +++ b/app/controllers/new_administrateur/administrateur_controller.rb @@ -20,7 +20,7 @@ module NewAdministrateur end def reset_procedure - if @procedure.brouillon_avec_lien? + if @procedure.brouillon? @procedure.reset! end end diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index 52a834f16..38cab945b 100644 --- a/app/helpers/procedure_helper.rb +++ b/app/helpers/procedure_helper.rb @@ -1,11 +1,9 @@ module ProcedureHelper def procedure_lien(procedure) - if procedure.path.present? - if procedure.brouillon_avec_lien? - commencer_test_url(path: procedure.path) - else - commencer_url(path: procedure.path) - end + if procedure.brouillon? + commencer_test_url(path: procedure.path) + else + commencer_url(path: procedure.path) end end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index b425e0b60..42bb93ba6 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -133,11 +133,6 @@ class Procedure < ApplicationRecord !archivee? end - # This method is needed for transition. Eventually this will be the same as brouillon?. - def brouillon_avec_lien? - brouillon? && path.present? - end - def publiee_ou_archivee? publiee? || archivee? end diff --git a/app/serializers/procedure_serializer.rb b/app/serializers/procedure_serializer.rb index c2608a9ff..778e236e6 100644 --- a/app/serializers/procedure_serializer.rb +++ b/app/serializers/procedure_serializer.rb @@ -24,12 +24,10 @@ class ProcedureSerializer < ActiveModel::Serializer end def link - if object.path.present? - if object.brouillon_avec_lien? - commencer_test_url(path: object.path) - else - commencer_url(path: object.path) - end + if object.brouillon? + commencer_test_url(path: object.path) + else + commencer_url(path: object.path) end end diff --git a/app/views/admin/procedures/show.html.haml b/app/views/admin/procedures/show.html.haml index f1e353499..9301644a3 100644 --- a/app/views/admin/procedures/show.html.haml +++ b/app/views/admin/procedures/show.html.haml @@ -44,7 +44,7 @@ .lien-demarche %h3 - - if @procedure.brouillon_avec_lien? + - if @procedure.brouillon? Test et publication - else Publication @@ -58,7 +58,7 @@ %br %br Attention, diffusez toujours le lien complet affiché ci-dessus, et non pas un lien générique vers demarches-simplifiees.fr. Ne dites pas non plus aux usagers de se rendre sur le site générique demarches-simplifiees.fr, donnez-leur toujours le lien complet. - - elsif @procedure.brouillon_avec_lien? + - elsif @procedure.brouillon? - if @procedure.missing_steps.empty? %p Cette démarche est actuellement en test, diff --git a/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_navbar.html.haml b/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_navbar.html.haml index f4b646548..18d34a23d 100644 --- a/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_navbar.html.haml +++ b/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_navbar.html.haml @@ -11,7 +11,7 @@ #procedure-list %a#onglet-infos{ href: url_for(admin_procedure_path(@procedure)) } .procedure-list-element{ class: ('active' if active == 'Informations') } - - if @procedure.brouillon_avec_lien? + - if @procedure.brouillon? Test et publication - else Publication From 92e60321157283b2da293c847505b967776f6843 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Fri, 5 Jul 2019 10:37:29 +0200 Subject: [PATCH 08/28] Remove path availability dead code when creating/editing a procedure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There’s no “path” field anymore in the Procedure form, it can only be set when publishing. --- .../admin/procedures_controller.rb | 21 ------- .../admin/procedures/_unavailable.html.haml | 11 ---- .../procedures/check_availability.js.erb | 11 ---- config/routes.rb | 1 - .../admin/procedures_controller_spec.rb | 57 ------------------- 5 files changed, 101 deletions(-) delete mode 100644 app/views/admin/procedures/_unavailable.html.haml delete mode 100644 app/views/admin/procedures/check_availability.js.erb diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index c2c061e75..1fc919b07 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -43,8 +43,6 @@ class Admin::ProceduresController < AdminController end def edit - @path = @procedure.path || @procedure.default_path - @availability = @procedure.path_availability(current_administrateur, @path) end def destroy @@ -63,13 +61,10 @@ class Admin::ProceduresController < AdminController def new @procedure ||= Procedure.new(for_individual: true) - @availability = Procedure::PATH_AVAILABLE end def create @procedure = Procedure.new(procedure_params.merge(administrateurs: [current_administrateur])) - @path = @procedure.path - @availability = Procedure.path_availability(current_administrateur, @procedure.path) if !@procedure.save flash.now.alert = @procedure.errors.full_messages @@ -87,10 +82,6 @@ class Admin::ProceduresController < AdminController if !@procedure.update(procedure_params) flash.now.alert = @procedure.errors.full_messages - @path = procedure_params[:path] - if @path.present? - @availability = @procedure.path_availability(current_administrateur, @path) - end render 'edit' elsif @procedure.brouillon? reset_procedure @@ -234,18 +225,6 @@ class Admin::ProceduresController < AdminController render json: json_path_list end - def check_availability - path = params[:procedure][:path] - procedure_id = params[:procedure][:id] - - if procedure_id.present? - procedure = current_administrateur.procedures.find(procedure_id) - @availability = procedure.path_availability(current_administrateur, path) - else - @availability = Procedure.path_availability(current_administrateur, path) - end - end - def delete_logo @procedure.logo.purge_later @procedure.logo_active_storage.purge_later diff --git a/app/views/admin/procedures/_unavailable.html.haml b/app/views/admin/procedures/_unavailable.html.haml deleted file mode 100644 index d3c2ce71d..000000000 --- a/app/views/admin/procedures/_unavailable.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -- case availability -- when Procedure::PATH_AVAILABLE_PUBLIEE - Ce lien est déjà utilisé par une de vos démarche. - %br - Si vous voulez l’utiliser, l’ancienne démarche sera archivée lors de la publication de la démarche (plus accessible du public). -- when Procedure::PATH_NOT_AVAILABLE_BROUILLON - Une démarche en test existe déjà avec ce lien. -- when Procedure::PATH_NOT_AVAILABLE - Ce lien est déjà utilisé par une démarche. - %br - Vous ne pouvez pas l’utiliser car il appartient à un autre administrateur. diff --git a/app/views/admin/procedures/check_availability.js.erb b/app/views/admin/procedures/check_availability.js.erb deleted file mode 100644 index f5eed5960..000000000 --- a/app/views/admin/procedures/check_availability.js.erb +++ /dev/null @@ -1,11 +0,0 @@ -<% if @availability == Procedure::PATH_AVAILABLE %> - <%= remove_element('.unavailable-path-message', inner: true) %> -<% else %> - <%= render_to_element('.unavailable-path-message', partial: 'unavailable', locals: { availability: @availability }) %> -<% end %> - -<% if @availability.in?(Procedure::PATH_CAN_PUBLISH) %> - <%= enable_element('button[type=submit]') %> -<% else %> - <%= disable_element('button[type=submit]') %> -<% end %> diff --git a/config/routes.rb b/config/routes.rb index 0bb11a794..9214c6d5d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -160,7 +160,6 @@ Rails.application.routes.draw do get 'procedures/archived' => 'procedures#archived' get 'procedures/draft' => 'procedures#draft' get 'procedures/path_list' => 'procedures#path_list' - get 'procedures/available' => 'procedures#check_availability' resources :procedures do collection do diff --git a/spec/controllers/admin/procedures_controller_spec.rb b/spec/controllers/admin/procedures_controller_spec.rb index 7df3010d7..ede09f394 100644 --- a/spec/controllers/admin/procedures_controller_spec.rb +++ b/spec/controllers/admin/procedures_controller_spec.rb @@ -716,63 +716,6 @@ describe Admin::ProceduresController, type: :controller do end end - describe "GET #check_availability" do - render_views - let(:procedure) { create(:procedure, :with_path, administrateur: admin) } - let(:params) { - { - procedure: { - path: path, - id: procedure.id - } - } - } - let(:path) { generate(:published_path) } - - before do - get :check_availability, params: params, format: 'js' - end - - context 'self path' do - let(:path) { procedure.path } - - it { expect(response.body).to include("innerHTML = ''") } - end - - context 'available path' do - it { expect(response.body).to include("innerHTML = ''") } - end - - context 'my path (brouillon)' do - let(:procedure_owned) { create(:procedure, :with_path, administrateur: admin) } - let(:path) { procedure_owned.path } - - it { - expect(response.body).to include('Une démarche en test existe déjà avec ce lien.') - } - end - - context 'my path' do - let(:procedure_owned) { create(:procedure, :published, administrateur: admin) } - let(:path) { procedure_owned.path } - - it { - expect(response.body).to include('Ce lien est déjà utilisé par une de vos démarche.') - expect(response.body).to include('Si vous voulez l’utiliser, l’ancienne démarche sera archivée') - } - end - - context 'unavailable path' do - let(:procedure_not_owned) { create(:procedure, :with_path, administrateur: create(:administrateur)) } - let(:path) { procedure_not_owned.path } - - it { - expect(response.body).to include('Ce lien est déjà utilisé par une démarche.') - expect(response.body).to include('Vous ne pouvez pas l’utiliser car il appartient à un autre administrateur.') - } - end - end - describe 'PATCH #monavis' do let!(:procedure) { create(:procedure, administrateur: admin) } let(:procedure_params) { From 99f986b815d67d22a898ad4cfa05c22b7d152801 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Fri, 5 Jul 2019 10:37:29 +0200 Subject: [PATCH 09/28] =?UTF-8?q?Cleanup=20Procedure=20retrieval=20in=20?= =?UTF-8?q?=E2=80=9CCommencer=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure to only fetch active (brouillons or published) procedures. --- app/controllers/users/commencer_controller.rb | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/app/controllers/users/commencer_controller.rb b/app/controllers/users/commencer_controller.rb index 9a16ee2aa..841acd58a 100644 --- a/app/controllers/users/commencer_controller.rb +++ b/app/controllers/users/commencer_controller.rb @@ -3,21 +3,21 @@ module Users layout 'procedure_context' def commencer - @procedure = Procedure.publiees.find_by(path: params[:path]) - return procedure_not_found if @procedure.blank? + @procedure = retrieve_procedure + return procedure_not_found if @procedure.blank? || @procedure.brouillon? render 'commencer/show' end def commencer_test - @procedure = Procedure.brouillons.find_by(path: params[:path]) - return procedure_not_found if @procedure.blank? + @procedure = retrieve_procedure + return procedure_not_found if @procedure.blank? || @procedure.publiee? render 'commencer/show' end def sign_in - @procedure = Procedure.find_by(path: params[:path]) + @procedure = retrieve_procedure return procedure_not_found if @procedure.blank? store_user_location!(@procedure) @@ -25,7 +25,7 @@ module Users end def sign_up - @procedure = Procedure.find_by(path: params[:path]) + @procedure = retrieve_procedure return procedure_not_found if @procedure.blank? store_user_location!(@procedure) @@ -33,7 +33,7 @@ module Users end def france_connect - @procedure = Procedure.find_by(path: params[:path]) + @procedure = retrieve_procedure return procedure_not_found if @procedure.blank? store_user_location!(@procedure) @@ -41,23 +41,25 @@ module Users end def procedure_for_help - Procedure.publiees.find_by(path: params[:path]) || Procedure.brouillons.find_by(path: params[:path]) + retrieve_procedure end private + def retrieve_procedure + Procedure.publiees.or(Procedure.brouillons).find_by(path: params[:path]) + end + def procedure_not_found procedure = Procedure.find_by(path: params[:path]) if procedure&.archivee? flash.alert = t('errors.messages.procedure_archived') - elsif procedure&.publiee? - flash.alert = t('errors.messages.procedure_not_draft') else flash.alert = t('errors.messages.procedure_not_found') end - return redirect_to root_path + redirect_to root_path end def store_user_location!(procedure) From 00c37eccb300858017fc4d25f643fe530e121701 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Mon, 16 Sep 2019 17:00:37 +0200 Subject: [PATCH 10/28] Simplify procedure.path and publish event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Get rid of the “reopen” event, merge it with “publish” (it’s the same code) * Remove the “availability” states; “available with brouillon” makes no sense since the brouillons path are always uuids * Instead of checking if publish can happen, just try it and handle the errors --- .../admin/procedures_controller.rb | 32 ++++---- app/models/procedure.rb | 81 ++++++------------- app/views/admin/procedures/publish.js.erb | 2 +- .../admin/procedures_controller_spec.rb | 6 +- 4 files changed, 44 insertions(+), 77 deletions(-) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 1fc919b07..2056ff8ea 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -98,22 +98,24 @@ class Admin::ProceduresController < AdminController lien_site_web = params[:lien_site_web] procedure = current_administrateur.procedures.find(params[:procedure_id]) - procedure.path = path - procedure.lien_site_web = lien_site_web - - if !procedure.validate - flash.alert = 'Lien de la démarche invalide ou lien vers la démarche manquant' - return redirect_to admin_procedures_path - else - procedure.path = nil - end - - if procedure.publish_or_reopen!(current_administrateur, path, lien_site_web) - flash.notice = "Démarche publiée" - redirect_to admin_procedures_path - else - @mine = false + procedure.publish_or_reopen!(current_administrateur, path, lien_site_web) + flash.notice = "Démarche publiée" + redirect_to admin_procedures_path + rescue ActiveRecord::RecordInvalid => e + errors = e.record.errors + if errors.details.key?(:path) + path_error = errors.details.dig(:path, 0, :error) + case path_error + when :invalid + @valid = false + when :taken + @valid = true + @mine = false + end render '/admin/procedures/publish', formats: 'js' + else + flash.alert = errors.full_messages + return redirect_to admin_procedure_path(procedure) end rescue ActiveRecord::RecordNotFound flash.alert = 'Démarche inexistante' diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 42bb93ba6..75f353220 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -74,7 +74,8 @@ class Procedure < ApplicationRecord validates_with MonAvisEmbedValidator before_save :update_juridique_required before_save :update_durees_conservation_required - before_create :ensure_path_exists + after_initialize :ensure_path_exists + before_save :ensure_path_exists after_create :ensure_default_groupe_instructeur include AASM @@ -85,11 +86,8 @@ class Procedure < ApplicationRecord state :archivee state :hidden - event :publish, after: :after_publish, guard: :can_publish? do + event :publish, before: :before_publish, after: :after_publish do transitions from: :brouillon, to: :publiee - end - - event :reopen, after: :after_reopen, guard: :can_publish? do transitions from: :archivee, to: :publiee end @@ -109,10 +107,10 @@ class Procedure < ApplicationRecord end def publish_or_reopen!(administrateur, path, lien_site_web) - if archivee? && may_reopen?(administrateur, path, lien_site_web) - reopen!(administrateur, path, lien_site_web) - elsif may_publish?(administrateur, path, lien_site_web) - reset! + Procedure.transaction do + if brouillon? + reset! + end publish!(administrateur, path, lien_site_web) end end @@ -364,34 +362,20 @@ class Procedure < ApplicationRecord percentile_time(:en_construction_at, :processed_at, 90) end - PATH_AVAILABLE = :available - PATH_AVAILABLE_PUBLIEE = :available_publiee - PATH_NOT_AVAILABLE = :not_available - PATH_NOT_AVAILABLE_BROUILLON = :not_available_brouillon - PATH_CAN_PUBLISH = [PATH_AVAILABLE, PATH_AVAILABLE_PUBLIEE] + def path_available?(administrateur, path) + procedure = Procedure.publiees + .where.not(id: self.id) + .find_by(path: path) - def path_availability(administrateur, path) - Procedure.path_availability(administrateur, path, id) + procedure.blank? || administrateur.owns?(procedure) end - def self.path_availability(administrateur, path, exclude_id = nil) - if exclude_id.present? - procedure = where.not(id: exclude_id).find_by(path: path) - else - procedure = find_by(path: path) - end + def auto_archive_procedure_with_same_path(administrateur, path) + procedure = administrateur.procedures.publiees + .where.not(id: self.id) + .find_by(path: path) - if procedure.blank? - PATH_AVAILABLE - elsif administrateur.owns?(procedure) - if procedure.brouillon? - PATH_NOT_AVAILABLE_BROUILLON - else - PATH_AVAILABLE_PUBLIEE - end - else - PATH_NOT_AVAILABLE - end + procedure&.archive! end def self.find_with_path(path) @@ -491,41 +475,22 @@ class Procedure < ApplicationRecord end end - def claim_path_ownership!(path) - procedure = Procedure.joins(:administrateurs) - .where(administrateurs: { id: administrateur_ids }) - .find_by(path: path) - - if procedure&.publiee? && procedure != self - procedure.archive! - end - - update!(path: path) - end - - def can_publish?(administrateur, path, lien_site_web) - path_availability(administrateur, path).in?(PATH_CAN_PUBLISH) && lien_site_web.present? + def before_publish(administrateur, path, lien_site_web) + auto_archive_procedure_with_same_path(administrateur, path) + update!(path: path, lien_site_web: lien_site_web, archived_at: nil) end def after_publish(administrateur, path, lien_site_web) update!(published_at: Time.zone.now) - - claim_path_ownership!(path) - end - - def after_reopen(administrateur, path, lien_site_web) - update!(published_at: Time.zone.now, archived_at: nil) - - claim_path_ownership!(path) end def after_archive - update!(archived_at: Time.zone.now, path: SecureRandom.uuid) + update!(archived_at: Time.zone.now) end def after_hide now = Time.zone.now - update!(hidden_at: now, path: SecureRandom.uuid) + update!(hidden_at: now) dossiers.update_all(hidden_at: now) end @@ -562,7 +527,7 @@ class Procedure < ApplicationRecord end def ensure_path_exists - if self.path.nil? + if self.path.blank? self.path = SecureRandom.uuid end end diff --git a/app/views/admin/procedures/publish.js.erb b/app/views/admin/procedures/publish.js.erb index 9c11731ef..d5df8f14b 100644 --- a/app/views/admin/procedures/publish.js.erb +++ b/app/views/admin/procedures/publish.js.erb @@ -1 +1 @@ -<%= "togglePathMessage(true, #{@mine})" %> +<%= "togglePathMessage(#{@valid}, #{@mine})" %> diff --git a/spec/controllers/admin/procedures_controller_spec.rb b/spec/controllers/admin/procedures_controller_spec.rb index ede09f394..1f05f798a 100644 --- a/spec/controllers/admin/procedures_controller_spec.rb +++ b/spec/controllers/admin/procedures_controller_spec.rb @@ -383,7 +383,8 @@ describe Admin::ProceduresController, type: :controller do expect(procedure.publiee?).to be_falsey expect(procedure.path).not_to match(path) expect(procedure.lien_site_web).to match(lien_site_web) - expect(response.status).to eq 200 + expect(assigns(:valid)).to eq true + expect(assigns(:mine)).to eq false end it 'previous procedure remains published' do @@ -401,8 +402,7 @@ describe Admin::ProceduresController, type: :controller do expect(procedure.publiee?).to be_falsey expect(procedure.path).not_to match(path) expect(procedure.lien_site_web).to match(lien_site_web) - expect(response).to redirect_to :admin_procedures - expect(flash[:alert]).to have_content 'Lien de la démarche invalide' + expect(assigns(:valid)).to eq false end end end From 8806eab8d6b90871745c23e99a1e1dbe94147323 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Mon, 29 Jul 2019 16:01:13 +0200 Subject: [PATCH 11/28] Properly validate Procedure.lien_site_web is set when publishing --- app/models/procedure.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 75f353220..e3431a3a8 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -64,6 +64,7 @@ class Procedure < ApplicationRecord validates :libelle, presence: true, allow_blank: false, allow_nil: false validates :description, presence: true, allow_blank: false, allow_nil: false validates :administrateurs, presence: true + validates :lien_site_web, presence: true, if: :publiee? validate :check_juridique validates :path, presence: true, format: { with: /\A[a-z0-9_\-]{3,50}\z/ }, uniqueness: { scope: [:path, :archived_at, :hidden_at], case_sensitive: false } # FIXME: remove duree_conservation_required flag once all procedures are converted to the new style From bd1e0aba3856baa8bbe06388617beb6c17499cfb Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Fri, 5 Jul 2019 14:38:20 +0200 Subject: [PATCH 12/28] Add Procedure path suggestion --- .../admin/procedures_controller.rb | 1 + app/models/procedure.rb | 18 +++++- .../admin/procedures/_modal_publish.html.haml | 2 +- spec/models/procedure_spec.rb | 57 +++++++++++++++++-- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 2056ff8ea..20784c050 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -40,6 +40,7 @@ class Admin::ProceduresController < AdminController end def show + @suggested_path = @procedure.suggested_path(current_administrateur) end def edit diff --git a/app/models/procedure.rb b/app/models/procedure.rb index e3431a3a8..61c9a8a44 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -167,8 +167,22 @@ class Procedure < ApplicationRecord types_de_champ_private.map(&:build_champ) end - def default_path - libelle&.parameterize&.first(50) + def path_customized? + !path.match?(/[[:xdigit:]]{8}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{12}/) + end + + def suggested_path(administrateur) + if path_customized? + return path + end + slug = libelle&.parameterize&.first(50) + suggestion = slug + counter = 1 + while !path_available?(administrateur, suggestion) + counter = counter + 1 + suggestion = "#{slug}-#{counter}" + end + suggestion end def organisation_name diff --git a/app/views/admin/procedures/_modal_publish.html.haml b/app/views/admin/procedures/_modal_publish.html.haml index 102e0c234..e1d981e13 100644 --- a/app/views/admin/procedures/_modal_publish.html.haml +++ b/app/views/admin/procedures/_modal_publish.html.haml @@ -21,7 +21,7 @@ %h4 Lien de la démarche %p.center = commencer_url(path: '') - = text_field_tag(:path, @procedure.path || @procedure.default_path, + = text_field_tag(:path, @suggested_path, id: 'procedure_path', placeholder: 'Chemin vers la démarche', data: { autocomplete: 'path' }, diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 0ddc65d61..df3899ef8 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -616,6 +616,22 @@ describe Procedure do it { expect(procedure.archived_at).to eq(now) } end + describe 'path_customized?' do + let(:procedure) { create :procedure } + + subject { procedure.path_customized? } + + context 'when the path is still the default' do + it { is_expected.to be_falsey } + end + + context 'when the path has been changed' do + before { procedure.path = 'custom_path' } + + it { is_expected.to be_truthy } + end + end + describe 'total_dossier' do let(:procedure) { create :procedure } @@ -630,12 +646,45 @@ describe Procedure do it { is_expected.to eq 2 } end - describe '#default_path' do - let(:procedure) { create(:procedure, libelle: 'A long libelle with àccênts, blabla coucou hello un deux trois voila') } + describe 'suggested_path' do + let(:procedure) { create :procedure, aasm_state: :publiee, libelle: 'Inscription au Collège' } - subject { procedure.default_path } + subject { procedure.suggested_path(procedure.administrateurs.first) } - it { is_expected.to eq('a-long-libelle-with-accents-blabla-coucou-hello-un') } + context 'when the path has been customized' do + before { procedure.path = 'custom_path' } + + it { is_expected.to eq 'custom_path' } + end + + context 'when the suggestion does not conflict' do + it { is_expected.to eq 'inscription-au-college' } + end + + context 'when the suggestion conflicts with one procedure' do + before do + create :procedure, aasm_state: :publiee, path: 'inscription-au-college' + end + + it { is_expected.to eq 'inscription-au-college-2' } + end + + context 'when the suggestion conflicts with several procedures' do + before do + create :procedure, aasm_state: :publiee, path: 'inscription-au-college' + create :procedure, aasm_state: :publiee, path: 'inscription-au-college-2' + end + + it { is_expected.to eq 'inscription-au-college-3' } + end + + context 'when the suggestion conflicts with another procedure of the same admin' do + before do + create :procedure, aasm_state: :publiee, path: 'inscription-au-college', administrateurs: procedure.administrateurs + end + + it { is_expected.to eq 'inscription-au-college' } + end end describe ".default_scope" do From c26a701a178dd86424f5797f93bfcde1a465b9dd Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 30 Jul 2019 16:54:43 +0200 Subject: [PATCH 13/28] Refactor and redesign publish modal * remove the autocomplete menu * use ujs to pre-validate the procedure * tweak the UI --- .../old_design/admin_procedures_modal.js | 70 --------------- .../stylesheets/admin_procedures_modal.scss | 24 ----- app/assets/stylesheets/application.scss | 1 - .../admin/procedures_controller.rb | 57 ++++-------- app/javascript/shared/autocomplete.js | 4 - app/models/procedure.rb | 90 ++++++++++--------- .../admin/procedures/_modal_publish.html.haml | 78 +++++++--------- .../procedures/_publish_buttons.html.haml | 13 +++ .../_publish_path_message.html.haml | 11 +++ app/views/admin/procedures/publish.js.erb | 1 - .../admin/procedures/publish_validate.js.haml | 4 + app/views/admin/procedures/show.html.haml | 4 +- config/locales/fr.yml | 2 + config/routes.rb | 2 +- .../admin/procedures_controller_spec.rb | 56 +----------- spec/factories/procedure.rb | 15 ++-- spec/models/procedure_spec.rb | 4 +- .../admin/procedures/show.html.haml_spec.rb | 4 +- 18 files changed, 148 insertions(+), 292 deletions(-) delete mode 100644 app/assets/javascripts/old_design/admin_procedures_modal.js delete mode 100644 app/assets/stylesheets/admin_procedures_modal.scss create mode 100644 app/views/admin/procedures/_publish_buttons.html.haml create mode 100644 app/views/admin/procedures/_publish_path_message.html.haml delete mode 100644 app/views/admin/procedures/publish.js.erb create mode 100644 app/views/admin/procedures/publish_validate.js.haml diff --git a/app/assets/javascripts/old_design/admin_procedures_modal.js b/app/assets/javascripts/old_design/admin_procedures_modal.js deleted file mode 100644 index 5a013cb53..000000000 --- a/app/assets/javascripts/old_design/admin_procedures_modal.js +++ /dev/null @@ -1,70 +0,0 @@ -/* globals $ */ - -$(document).on('turbolinks:load', init_path_modal); - -var PROCEDURE_PATH_SELECTOR = 'input[data-autocomplete=path]'; - -function init_path_modal() { - path_modal_action(); - path_validation_action(); - path_type_init(); - path_validation($(PROCEDURE_PATH_SELECTOR)); -} - -function path_modal_action() { - $('#publish-modal').on('show.bs.modal', function(event) { - $('#publish-modal .modal-body .table .tr-content').hide(); - - var button = $(event.relatedTarget); // Button that triggered the modal - var modal_title = button.data('modal_title'); // Extract info from data-* attributes - var modal_index = button.data('modal_index'); // Extract info from data-* attributes - - var modal = $(this); - modal.find('#publish-modal-title').html(modal_title); - $('#publish-modal .modal-body .table #' + modal_index).show(); - }); -} - -function path_validation_action() { - $(PROCEDURE_PATH_SELECTOR).keyup(function(key) { - if (key.keyCode != 13) path_validation(this); - }); -} - -function togglePathMessage(valid, mine) { - $('#path-messages .message').hide(); - - if (valid === true && mine === true) { - $('#path_is_mine').show(); - } else if (valid === true && mine === false) { - $('#path_is_not_mine').show(); - } else if (valid === false && mine === null) { - $('#path_is_invalid').show(); - } - - if ((valid && mine === null) || mine === true) - $('#publish-modal #publish').removeAttr('disabled'); - else $('#publish-modal #publish').attr('disabled', 'disabled'); -} - -function path_validation(el) { - var valid = validatePath($(el).val()); - toggleErrorClass(el, valid); - togglePathMessage(valid, null); -} - -function toggleErrorClass(node, boolean) { - if (boolean) $(node).removeClass('input-error'); - else $(node).addClass('input-error'); -} - -function validatePath(path) { - var re = /^[a-z0-9_-]{3,50}$/; - return re.test(path); -} - -function path_type_init() { - $(PROCEDURE_PATH_SELECTOR).on('autocomplete:select', function(event) { - togglePathMessage(true, event.detail['mine']); - }); -} diff --git a/app/assets/stylesheets/admin_procedures_modal.scss b/app/assets/stylesheets/admin_procedures_modal.scss deleted file mode 100644 index b6b2aa84e..000000000 --- a/app/assets/stylesheets/admin_procedures_modal.scss +++ /dev/null @@ -1,24 +0,0 @@ -.path-mine-false { - color: #FF0000; -} - -#path-messages { - .message { - display: none; - } -} - -#publish-modal { - .algolia-autocomplete { - width: 300px; - } - - .aa-dropdown-menu { - width: 300px; - } - - // Fix the input not being displayed on the same line than the text before - .aa-input { - vertical-align: initial !important; - } -} diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 5eec7e869..84ef7d75e 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -13,7 +13,6 @@ // = require _card // = require _helpers // = require _turbolinks -// = require admin_procedures_modal // = require admin_type_de_champ // = require carte // = require custom_mails diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 20784c050..3449336e1 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -2,7 +2,7 @@ class Admin::ProceduresController < AdminController include SmartListing::Helper::ControllerExtensions helper SmartListing::Helper - before_action :retrieve_procedure, only: [:show, :edit, :delete_logo, :delete_deliberation, :delete_notice, :monavis, :update_monavis] + before_action :retrieve_procedure, only: [:show, :edit, :delete_logo, :delete_deliberation, :delete_notice, :monavis, :update_monavis, :publish_validate, :publish] def index if current_administrateur.procedures.count != 0 @@ -40,7 +40,8 @@ class Admin::ProceduresController < AdminController end def show - @suggested_path = @procedure.suggested_path(current_administrateur) + @procedure.path = @procedure.suggested_path(current_administrateur) + @current_administrateur = current_administrateur end def edit @@ -94,33 +95,19 @@ class Admin::ProceduresController < AdminController end end - def publish - path = params[:path] - lien_site_web = params[:lien_site_web] - procedure = current_administrateur.procedures.find(params[:procedure_id]) + def publish_validate + @procedure.assign_attributes(publish_params) + end + + def publish + @procedure.assign_attributes(publish_params) + + @procedure.publish_or_reopen!(current_administrateur) - procedure.publish_or_reopen!(current_administrateur, path, lien_site_web) flash.notice = "Démarche publiée" redirect_to admin_procedures_path - rescue ActiveRecord::RecordInvalid => e - errors = e.record.errors - if errors.details.key?(:path) - path_error = errors.details.dig(:path, 0, :error) - case path_error - when :invalid - @valid = false - when :taken - @valid = true - @mine = false - end - render '/admin/procedures/publish', formats: 'js' - else - flash.alert = errors.full_messages - return redirect_to admin_procedure_path(procedure) - end - rescue ActiveRecord::RecordNotFound - flash.alert = 'Démarche inexistante' - redirect_to admin_procedures_path + rescue ActiveRecord::RecordInvalid + render 'publish_validate', formats: :js end def transfer @@ -214,20 +201,6 @@ class Admin::ProceduresController < AdminController @draft_class = 'active' end - def path_list - json_path_list = Procedure - .find_with_path(params[:request]) - .order(:id) - .map do |procedure| - { - label: procedure.path, - mine: current_administrateur.owns?(procedure) - } - end.to_json - - render json: json_path_list - end - def delete_logo @procedure.logo.purge_later @procedure.logo_active_storage.purge_later @@ -256,6 +229,10 @@ class Admin::ProceduresController < AdminController params[:from_new_from_existing].present? end + def publish_params + params.permit(:path, :lien_site_web) + end + def procedure_params editable_params = [:libelle, :description, :organisation, :direction, :lien_site_web, :cadre_juridique, :deliberation, :notice, :web_hook_url, :euro_flag, :logo, :auto_archive_on, :monavis_embed] permited_params = if @procedure&.locked? diff --git a/app/javascript/shared/autocomplete.js b/app/javascript/shared/autocomplete.js index 4aefffd57..e37dec4dc 100644 --- a/app/javascript/shared/autocomplete.js +++ b/app/javascript/shared/autocomplete.js @@ -5,10 +5,6 @@ const sources = [ { type: 'address', url: '/address/suggestions' - }, - { - type: 'path', - url: '/admin/procedures/path_list' } ]; diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 61c9a8a44..16ff6a9ee 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -65,6 +65,7 @@ class Procedure < ApplicationRecord validates :description, presence: true, allow_blank: false, allow_nil: false validates :administrateurs, presence: true validates :lien_site_web, presence: true, if: :publiee? + validate :validate_for_publication, on: :publication validate :check_juridique validates :path, presence: true, format: { with: /\A[a-z0-9_\-]{3,50}\z/ }, uniqueness: { scope: [:path, :archived_at, :hidden_at], case_sensitive: false } # FIXME: remove duree_conservation_required flag once all procedures are converted to the new style @@ -107,12 +108,18 @@ class Procedure < ApplicationRecord end end - def publish_or_reopen!(administrateur, path, lien_site_web) + def publish_or_reopen!(administrateur) Procedure.transaction do if brouillon? reset! end - publish!(administrateur, path, lien_site_web) + + other_procedure = other_procedure_with_path(path) + if other_procedure.present? && administrateur.owns?(other_procedure) + other_procedure.archive! + end + + publish! end end @@ -124,6 +131,44 @@ class Procedure < ApplicationRecord end end + def validate_for_publication + old_attributes = self.slice(:aasm_state, :archived_at) + self.aasm_state = :publiee + self.archived_at = nil + + is_valid = validate + + self.attributes = old_attributes + + is_valid + end + + def suggested_path(administrateur) + if path_customized? + return path + end + slug = libelle&.parameterize&.first(50) + suggestion = slug + counter = 1 + while !path_available?(administrateur, suggestion) + counter = counter + 1 + suggestion = "#{slug}-#{counter}" + end + suggestion + end + + def other_procedure_with_path(path) + Procedure.publiees + .where.not(id: self.id) + .find_by(path: path) + end + + def path_available?(administrateur, path) + procedure = other_procedure_with_path(path) + + procedure.blank? || administrateur.owns?(procedure) + end + def locked? publiee_ou_archivee? end @@ -171,20 +216,6 @@ class Procedure < ApplicationRecord !path.match?(/[[:xdigit:]]{8}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{12}/) end - def suggested_path(administrateur) - if path_customized? - return path - end - slug = libelle&.parameterize&.first(50) - suggestion = slug - counter = 1 - while !path_available?(administrateur, suggestion) - counter = counter + 1 - suggestion = "#{slug}-#{counter}" - end - suggestion - end - def organisation_name service&.nom || organisation end @@ -377,26 +408,6 @@ class Procedure < ApplicationRecord percentile_time(:en_construction_at, :processed_at, 90) end - def path_available?(administrateur, path) - procedure = Procedure.publiees - .where.not(id: self.id) - .find_by(path: path) - - procedure.blank? || administrateur.owns?(procedure) - end - - def auto_archive_procedure_with_same_path(administrateur, path) - procedure = administrateur.procedures.publiees - .where.not(id: self.id) - .find_by(path: path) - - procedure&.archive! - end - - def self.find_with_path(path) - where.not(aasm_state: :archivee).where("path LIKE ?", "%#{path}%") - end - def populate_champ_stable_ids TypeDeChamp.where(procedure: self, stable_id: nil).find_each do |type_de_champ| type_de_champ.update_column(:stable_id, type_de_champ.id) @@ -490,12 +501,11 @@ class Procedure < ApplicationRecord end end - def before_publish(administrateur, path, lien_site_web) - auto_archive_procedure_with_same_path(administrateur, path) - update!(path: path, lien_site_web: lien_site_web, archived_at: nil) + def before_publish + update!(archived_at: nil) end - def after_publish(administrateur, path, lien_site_web) + def after_publish update!(published_at: Time.zone.now) end diff --git a/app/views/admin/procedures/_modal_publish.html.haml b/app/views/admin/procedures/_modal_publish.html.haml index e1d981e13..cfcfaf6cc 100644 --- a/app/views/admin/procedures/_modal_publish.html.haml +++ b/app/views/admin/procedures/_modal_publish.html.haml @@ -1,63 +1,49 @@ #publish-modal.modal{ "aria-labelledby" => "myModalLabel", :role => "dialog", :tabindex => "-1" } .modal-dialog.modal-lg{ :role => "document" } - = form_tag admin_procedure_publish_path(procedure_id: @procedure.id), method: :put, remote: true do + = form_tag admin_procedure_publish_path(procedure_id: procedure.id), method: :put, remote: true do .modal-content .modal-header %button.close{ "aria-label" => "Close", "data-dismiss" => "modal", :type => "button" } %span{ "aria-hidden" => "true" } × %h4#myModalLabel.modal-title - = procedure_modal_text(@procedure, :title) + = procedure_modal_text(procedure, :title) %span#publish-modal-title .modal-body - = procedure_modal_text(@procedure, :body) - - if !@procedure.archivee? - %b - Elle ne pourra plus être modifiée à l’issue de cette publication. - %br - Afin de faciliter l’accès à la démarche, vous êtes invité à personnaliser l’adresse d'accès si vous le souhaitez. - %br + .text-info + %p + = procedure_modal_text(procedure, :body) + %b Elle ne pourra plus être modifiée à l’issue de cette publication. .form-group - %br - %h4 Lien de la démarche - %p.center + %h4 Adresse de la démarche + %p Vous pouvez personnaliser le lien public de la démarche pour en faciliter l’accès. + %p = commencer_url(path: '') - = text_field_tag(:path, @suggested_path, - id: 'procedure_path', - placeholder: 'Chemin vers la démarche', - data: { autocomplete: 'path' }, - class: 'form-control', - maxlength: 50, - style: 'width: 300px; display: inline;') - %br - .alert.alert-info - Attention, diffusez toujours le lien complet affiché ci-dessus, et non pas un lien générique vers demarches-simplifiees.fr. Ne dites pas non plus aux usagers de se rendre sur le site générique demarches-simplifiees.fr, donnez-leur toujours le lien complet. - %br - %br - Prenez quelques minutes pour savoir comment établir une bonne relation avec les usagers de votre démarche : - = link_to "Regarder la vidéo de 5 minutes", - "https://vimeo.com/334463514", - target: "_blank" - + = text_field_tag(:path, procedure.path, + id: 'procedure_path', + placeholder: 'chemin-de-la-démarche', + required: true, + class: 'form-control', + pattern: '^[a-z0-9_-]{3,50}$', + title: "De 3 à 50 caractères; minuscules, chiffres et tiret seulement", + data: { remote: true, debounce: true, url: admin_procedure_publish_validate_path(procedure) }, + autocomplete: 'off', + style: 'width: 300px; display: inline;') + = render 'publish_path_message', procedure: procedure, administrateur: administrateur + .text-info + Attention, diffusez toujours le lien complet affiché ci-dessus, et non pas un lien générique vers demarches-simplifiees.fr. Ne dites pas non plus aux usagers de se rendre sur le site générique demarches-simplifiees.fr, donnez-leur toujours le lien complet. .form-group - %h4 Où les usagers trouveront-ils le lien vers cette démarche ? * + %h4 Diffusion de la démarche + %p Où les utilisateurs trouveront-ils le lien de la démarche ? Typiquement, il s’agit d’une page de votre site web. %p.center - = text_field_tag(:lien_site_web, @procedure.lien_site_web, + = text_field_tag(:lien_site_web, procedure.lien_site_web, required: true, class: 'form-control', + autocomplete: 'off', placeholder: 'https://exemple.gouv.fr/ma_demarche') - - #path-messages - #path_is_mine.text-warning.center.message - Ce lien est déjà utilisé par une de vos démarche. - %br - Si vous voulez l’utiliser, l’ancienne démarche sera archivée (plus accessible du public). - #path_is_not_mine.text-danger.center.message - Ce lien est déjà utilisé par une démarche. - %br - Vous ne pouvez pas l’utiliser car il appartient à un autre administrateur. - #path_is_invalid.text-danger.center.message - Ce lien - = t('activerecord.errors.models.procedure.attributes.path.invalid') + .text-info + Prenez quelques minutes pour savoir comment établir une bonne relation avec les usagers de votre démarche : + = link_to "Regarder la vidéo de 5 minutes.", + "https://vimeo.com/334463514", + target: "_blank" .modal-footer - = submit_tag procedure_modal_text(@procedure, :submit), class: %w(btn btn btn-success), disabled: :disabled, id: 'publish' - = button_tag "Annuler", class: %w(btn btn btn-default), data: { dismiss: :modal }, id: 'cancel' + = render 'publish_buttons', procedure: procedure, administrateur: administrateur diff --git a/app/views/admin/procedures/_publish_buttons.html.haml b/app/views/admin/procedures/_publish_buttons.html.haml new file mode 100644 index 000000000..6630763d8 --- /dev/null +++ b/app/views/admin/procedures/_publish_buttons.html.haml @@ -0,0 +1,13 @@ +#publish-buttons + = button_tag "Annuler", class: %w(btn btn-default), data: { dismiss: :modal } + + - procedure.validate(:publication) + - errors = procedure.errors + -# Ignore the :taken error if the path can be claimed + - if errors.details[:path]&.pluck(:error)&.include?(:taken) && procedure.path_available?(administrateur, procedure.path) + - errors.delete(:path) + + - options = { class: %w(btn btn-success), id: 'publish' } + - if errors.details[:path].present? + - options[:disabled] = :disabled + = submit_tag procedure_modal_text(@procedure, :submit), options diff --git a/app/views/admin/procedures/_publish_path_message.html.haml b/app/views/admin/procedures/_publish_path_message.html.haml new file mode 100644 index 000000000..5ed0a1dcd --- /dev/null +++ b/app/views/admin/procedures/_publish_path_message.html.haml @@ -0,0 +1,11 @@ +#publish-path-message + - procedure.validate(:publication) + - errors = procedure.errors + -# Ignore the :taken error if the path can be claimed, and instead display the :taken_can_be_claimed error message. + - if errors.details[:path]&.pluck(:error)&.include?(:taken) && procedure.path_available?(administrateur, procedure.path) + .alert.alert-warning + = errors.full_message('Le lien public', errors.generate_message(:path, :taken_can_be_claimed)) + - elsif errors.messages[:path].present? + -# Display the actual errors for :path + .alert.alert-danger + = errors.full_message('Le lien public', errors.messages[:path].first) diff --git a/app/views/admin/procedures/publish.js.erb b/app/views/admin/procedures/publish.js.erb deleted file mode 100644 index d5df8f14b..000000000 --- a/app/views/admin/procedures/publish.js.erb +++ /dev/null @@ -1 +0,0 @@ -<%= "togglePathMessage(#{@valid}, #{@mine})" %> diff --git a/app/views/admin/procedures/publish_validate.js.haml b/app/views/admin/procedures/publish_validate.js.haml new file mode 100644 index 000000000..74c0681c3 --- /dev/null +++ b/app/views/admin/procedures/publish_validate.js.haml @@ -0,0 +1,4 @@ += render_to_element("#publish-path-message", partial: 'publish_path_message', outer: true, + locals: { procedure: @procedure, administrateur: current_administrateur }) += render_to_element("#publish-buttons", partial: 'publish_buttons', outer: true, + locals: { procedure: @procedure, administrateur: current_administrateur }) diff --git a/app/views/admin/procedures/show.html.haml b/app/views/admin/procedures/show.html.haml index 9301644a3..ccd27f46f 100644 --- a/app/views/admin/procedures/show.html.haml +++ b/app/views/admin/procedures/show.html.haml @@ -1,7 +1,7 @@ = render partial: 'admin/closed_mail_template_attestation_inconsistency_alert' .row.white-back #procedure_show - = render partial: '/admin/procedures/modal_publish' + = render 'modal_publish', procedure: @procedure, administrateur: @current_administrateur = render partial: '/admin/procedures/modal_transfer' - if @procedure.brouillon? @@ -117,7 +117,7 @@ - if @procedure.missing_steps.include?(:service) %p.alert.alert-danger Vous devez renseigner les coordonnées de votre Service administratif avant de pouvoir publier votre démarche. - = link_to 'Cliquez ici.', (current_administrateur.services.present? ? url_for(services_path(procedure_id: @procedure.id)) : url_for(new_service_path(procedure_id: @procedure.id))) + = link_to 'Cliquez ici.', (@current_administrateur.services.present? ? url_for(services_path(procedure_id: @procedure.id)) : url_for(new_service_path(procedure_id: @procedure.id))) - if @procedure.missing_steps.include?(:instructeurs) %p.alert.alert-danger diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f48b60ed6..c44b18967 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -163,6 +163,8 @@ fr: procedure: attributes: path: + taken: est déjà utilisé par une démarche. Vous ne pouvez pas l’utiliser car il appartient à un autre administrateur. + taken_can_be_claimed: est identique à celui d’une autre de vos démarches publiées. Si vous publiez cette démarche, l’ancienne sera archivée et ne sera plus accessible au public. invalid: n'est pas valide. Il doit comporter au moins 3 caractères, au plus 50 caractères et seuls les caractères a-z, 0-9, '_' et '-' sont autorisés. errors: diff --git a/config/routes.rb b/config/routes.rb index 9214c6d5d..e48f85db9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -159,7 +159,6 @@ Rails.application.routes.draw do patch 'activate' => '/administrateurs/activate#create' get 'procedures/archived' => 'procedures#archived' get 'procedures/draft' => 'procedures#draft' - get 'procedures/path_list' => 'procedures#path_list' resources :procedures do collection do @@ -175,6 +174,7 @@ Rails.application.routes.draw do resources :mail_templates, only: [:index, :edit, :update] put 'archive' => 'procedures#archive', as: :archive + get 'publish_validate' => 'procedures#publish_validate', as: :publish_validate put 'publish' => 'procedures#publish', as: :publish post 'transfer' => 'procedures#transfer', as: :transfer put 'clone' => 'procedures#clone', as: :clone diff --git a/spec/controllers/admin/procedures_controller_spec.rb b/spec/controllers/admin/procedures_controller_spec.rb index 1f05f798a..b5c5fc87c 100644 --- a/spec/controllers/admin/procedures_controller_spec.rb +++ b/spec/controllers/admin/procedures_controller_spec.rb @@ -340,7 +340,7 @@ describe Admin::ProceduresController, type: :controller do context 'when admin is the owner of the procedure' do before do - put :publish, params: { procedure_id: procedure.id, path: path, lien_site_web: lien_site_web } + put :publish, format: :js, params: { procedure_id: procedure.id, path: path, lien_site_web: lien_site_web } procedure.reload procedure2.reload end @@ -383,8 +383,6 @@ describe Admin::ProceduresController, type: :controller do expect(procedure.publiee?).to be_falsey expect(procedure.path).not_to match(path) expect(procedure.lien_site_web).to match(lien_site_web) - expect(assigns(:valid)).to eq true - expect(assigns(:mine)).to eq false end it 'previous procedure remains published' do @@ -402,7 +400,6 @@ describe Admin::ProceduresController, type: :controller do expect(procedure.publiee?).to be_falsey expect(procedure.path).not_to match(path) expect(procedure.lien_site_web).to match(lien_site_web) - expect(assigns(:valid)).to eq false end end end @@ -419,8 +416,7 @@ describe Admin::ProceduresController, type: :controller do end it 'fails' do - expect(response).to redirect_to :admin_procedures - expect(flash[:alert]).to have_content 'Démarche inexistante' + expect(response).to have_http_status(404) end end @@ -573,54 +569,6 @@ describe Admin::ProceduresController, type: :controller do end end - describe 'GET #path_list' do - let!(:procedure) { create(:procedure, :published, administrateur: admin) } - let(:admin2) { create(:administrateur) } - let!(:procedure2) { create(:procedure, :published, administrateur: admin2) } - let!(:procedure3) { create(:procedure, :published, administrateur: admin2) } - - subject { get :path_list } - - let(:body) { JSON.parse(response.body) } - - describe 'when no params' do - before do - subject - end - - it { expect(response.status).to eq(200) } - it { expect(body.size).to eq(3) } - it { expect(body.first['label']).to eq(procedure.path) } - it { expect(body.first['mine']).to be_truthy } - it { expect(body.second['label']).to eq(procedure2.path) } - it { expect(body.second['mine']).to be_falsy } - end - - context 'filtered' do - before do - subject - end - - subject { get :path_list, params: { request: URI.encode(procedure2.path) } } - - it { expect(response.status).to eq(200) } - it { expect(body.size).to eq(1) } - it { expect(body.first['label']).to eq(procedure2.path) } - it { expect(body.first['mine']).to be_falsy } - end - - context 'when procedure is archived' do - let!(:procedure3) { create(:procedure, :archived, administrateur: admin2) } - before do - subject - end - - it 'do not return on the json' do - expect(body.size).to eq(2) - end - end - end - describe 'POST #transfer' do let!(:procedure) { create :procedure, :with_service, administrateur: admin } diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 23b32efa7..3cfdd52dc 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -49,7 +49,8 @@ FactoryBot.define do after(:build) do |procedure, _evaluator| procedure.for_individual = true procedure.types_de_champ << create(:type_de_champ, libelle: 'Texte obligatoire', mandatory: true) - procedure.publish!(procedure.administrateurs.first, generate(:published_path), procedure.lien_site_web) + procedure.path = generate(:published_path) + procedure.publish! end end @@ -148,13 +149,15 @@ FactoryBot.define do trait :published do after(:build) do |procedure, _evaluator| - procedure.publish!(procedure.administrateurs.first, generate(:published_path), procedure.lien_site_web) + procedure.path = generate(:published_path) + procedure.publish! end end trait :archived do after(:build) do |procedure, _evaluator| - procedure.publish!(procedure.administrateurs.first, generate(:published_path), procedure.lien_site_web) + procedure.path = generate(:published_path) + procedure.publish! procedure.archive! end end @@ -163,14 +166,16 @@ FactoryBot.define do # For now the behavior is the same than :archived # (it may be different in the future though) after(:build) do |procedure, _evaluator| - procedure.publish!(procedure.administrateurs.first, generate(:published_path), procedure.lien_site_web) + procedure.path = generate(:published_path) + procedure.publish! procedure.archive! end end trait :hidden do after(:build) do |procedure, _evaluator| - procedure.publish!(procedure.administrateurs.first, generate(:published_path), procedure.lien_site_web) + procedure.path = generate(:published_path) + procedure.publish! procedure.hide! end end diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index df3899ef8..c95879de4 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -547,12 +547,12 @@ describe Procedure do end describe '#publish!' do - let(:procedure) { create(:procedure) } + let(:procedure) { create(:procedure, path: 'example-path') } let(:now) { Time.zone.now.beginning_of_minute } before do Timecop.freeze(now) - procedure.publish!(procedure.administrateurs.first, "example-path", procedure.lien_site_web) + procedure.publish! end after { Timecop.return } diff --git a/spec/views/admin/procedures/show.html.haml_spec.rb b/spec/views/admin/procedures/show.html.haml_spec.rb index f98d1eb91..8f724c6a7 100644 --- a/spec/views/admin/procedures/show.html.haml_spec.rb +++ b/spec/views/admin/procedures/show.html.haml_spec.rb @@ -41,7 +41,7 @@ describe 'admin/procedures/show.html.haml', type: :view do describe 'procedure is published' do before do - procedure.publish!(procedure.administrateurs.first, 'fake_path', procedure.lien_site_web) + procedure.publish! procedure.reload render end @@ -59,7 +59,7 @@ describe 'admin/procedures/show.html.haml', type: :view do describe 'procedure is archived' do before do - procedure.publish!(procedure.administrateurs.first, 'fake_path', procedure.lien_site_web) + procedure.publish! procedure.archive! procedure.reload render From 543f9894c1661e9254c6776aa88388e4adceeb25 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 13 Aug 2019 15:43:23 +0200 Subject: [PATCH 14/28] =?UTF-8?q?Make=20sure=20the=20=E2=80=9Ccommencer?= =?UTF-8?q?=E2=80=9D=20link=20uses=20the=20proper=20test=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/admin/procedures_controller.rb | 5 +++++ app/views/admin/procedures/show.html.haml | 4 ++-- spec/views/admin/procedures/show.html.haml_spec.rb | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 3449336e1..05230f168 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -40,6 +40,11 @@ class Admin::ProceduresController < AdminController end def show + if @procedure.brouillon? + @procedure_lien = commencer_test_url(path: @procedure.path) + else + @procedure_lien = commencer_url(path: @procedure.path) + end @procedure.path = @procedure.suggested_path(current_administrateur) @current_administrateur = current_administrateur end diff --git a/app/views/admin/procedures/show.html.haml b/app/views/admin/procedures/show.html.haml index ccd27f46f..13c81bac4 100644 --- a/app/views/admin/procedures/show.html.haml +++ b/app/views/admin/procedures/show.html.haml @@ -54,7 +54,7 @@ - elsif @procedure.publiee? Cette démarche est publiée, certains éléments ne peuvent plus être modifiés. Pour y accéder vous pouvez utiliser le lien : - = link_to procedure_lien(@procedure), sanitize_url(procedure_lien(@procedure)), target: :blank, rel: :noopener + = link_to @procedure_lien, sanitize_url(@procedure_lien), target: :blank, rel: :noopener %br %br Attention, diffusez toujours le lien complet affiché ci-dessus, et non pas un lien générique vers demarches-simplifiees.fr. Ne dites pas non plus aux usagers de se rendre sur le site générique demarches-simplifiees.fr, donnez-leur toujours le lien complet. @@ -63,7 +63,7 @@ %p Cette démarche est actuellement en test, pour y accéder vous pouvez utiliser le lien : - = link_to procedure_lien(@procedure), sanitize_url(procedure_lien(@procedure)), target: :blank, rel: :noopener + = link_to @procedure_lien, sanitize_url(@procedure_lien), target: :blank, rel: :noopener %p Toute personne ayant la connaissance de ce lien pourra ainsi remplir des dossiers de test sur votre démarche. %br diff --git a/spec/views/admin/procedures/show.html.haml_spec.rb b/spec/views/admin/procedures/show.html.haml_spec.rb index 8f724c6a7..c1ec1bffe 100644 --- a/spec/views/admin/procedures/show.html.haml_spec.rb +++ b/spec/views/admin/procedures/show.html.haml_spec.rb @@ -6,6 +6,7 @@ describe 'admin/procedures/show.html.haml', type: :view do before do assign(:procedure, procedure) + assign(:procedure_lien, commencer_url(path: procedure.path)) end describe 'procedure is draft' do From 78da268dedcb5e17454f3875938bc66f623ea6cc Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Wed, 14 Aug 2019 18:05:01 +0200 Subject: [PATCH 15/28] Improve feature tests for procedure cloning and creation --- spec/features/admin/procedure_cloning_spec.rb | 38 +++++++++++++------ .../features/admin/procedure_creation_spec.rb | 7 +++- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/spec/features/admin/procedure_cloning_spec.rb b/spec/features/admin/procedure_cloning_spec.rb index eb908a012..74442d89f 100644 --- a/spec/features/admin/procedure_cloning_spec.rb +++ b/spec/features/admin/procedure_cloning_spec.rb @@ -7,23 +7,39 @@ feature 'As an administrateur I wanna clone a procedure', js: true do let(:administrateur) { create(:administrateur) } before do + create :procedure, :with_service, + aasm_state: :publiee, published_at: Time.zone.now, + administrateurs: [administrateur], + libelle: 'libellé de la procédure', + path: 'libelle-de-la-procedure' login_as administrateur.user, scope: :user - visit new_from_existing_admin_procedures_path end - context 'Cloning procedure' do - before 'Create procedure' do - page.find_by_id('from-scratch').click - fill_in_dummy_procedure_details - page.find_by_id('save-procedure').click - end - - scenario 'Cloning' do - visit admin_procedures_draft_path + context 'Cloning a procedure owned by the current admin' do + scenario do + visit admin_procedures_path expect(page.find_by_id('procedures')['data-item-count']).to eq('1') page.all('.clone-btn').first.click visit admin_procedures_draft_path - expect(page.find_by_id('procedures')['data-item-count']).to eq('2') + expect(page.find_by_id('procedures')['data-item-count']).to eq('1') + click_on Procedure.last.libelle + expect(page).to have_current_path(admin_procedure_path(Procedure.last)) + + find('#publish-procedure').click + + within '#publish-modal' do + expect(find_field('procedure_path').value).to eq 'libelle-de-la-procedure' + expect(page).to have_text('ancienne sera archivée') + fill_in 'lien_site_web', with: 'http://some.website' + click_on 'publish' + end + + page.refresh + + visit admin_procedures_archived_path + expect(page.find_by_id('procedures')['data-item-count']).to eq('1') + visit admin_procedures_draft_path + expect(page.find_by_id('procedures')['data-item-count']).to eq('0') end end end diff --git a/spec/features/admin/procedure_creation_spec.rb b/spec/features/admin/procedure_creation_spec.rb index afd26564c..5abfec21c 100644 --- a/spec/features/admin/procedure_creation_spec.rb +++ b/spec/features/admin/procedure_creation_spec.rb @@ -98,11 +98,16 @@ feature 'As an administrateur I wanna create a new procedure', js: true do click_on Procedure.last.libelle expect(page).to have_current_path(admin_procedure_path(Procedure.last)) + expect(page).to have_content('en test') + # Only check the path even though the link is the complete URL + # (Capybara runs the app on an arbitrary host/port.) + expect(page).to have_link(nil, href: /#{commencer_test_path(Procedure.last.path)}/) + expect(page).to have_selector('#publish-procedure', visible: true) find('#publish-procedure').click within '#publish-modal' do - expect(page).to have_field('procedure_path') + expect(find_field('procedure_path').value).to eq 'libelle-de-la-procedure' fill_in 'lien_site_web', with: 'http://some.website' click_on 'publish' end From 3e2985b3057ceda7377e6e485402f9cf9922b928 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 30 Jul 2019 14:37:00 +0200 Subject: [PATCH 16/28] First attempt at procedure stats fixes #3945, #3946, #3948 --- .../new_design/procedure_show.scss | 12 ++++--- app/assets/stylesheets/new_design/stats.scss | 2 +- .../instructeurs/procedures_controller.rb | 18 +++++++++++ .../instructeurs/procedures/show.html.haml | 4 ++- .../instructeurs/procedures/stats.html.haml | 32 +++++++++++++++++++ config/routes.rb | 1 + 6 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 app/views/instructeurs/procedures/stats.html.haml diff --git a/app/assets/stylesheets/new_design/procedure_show.scss b/app/assets/stylesheets/new_design/procedure_show.scss index c329189f9..5f9c31bb9 100644 --- a/app/assets/stylesheets/new_design/procedure_show.scss +++ b/app/assets/stylesheets/new_design/procedure_show.scss @@ -2,6 +2,13 @@ @import "common"; @import "constants"; +.procedure-header { + a.header-link { + display: inline-block; + margin-bottom: 1 * $default-padding; + } +} + #procedure-show { h1 { color: $black; @@ -9,11 +16,6 @@ margin-bottom: 1 * $default-padding; } - a.notifications { - display: inline-block; - margin-bottom: 1 * $default-padding; - } - .dossiers-table { margin-top: $default-spacer; margin-bottom: 3 * $default-spacer; diff --git a/app/assets/stylesheets/new_design/stats.scss b/app/assets/stylesheets/new_design/stats.scss index 9eb1e5d2d..4b9b8b58e 100644 --- a/app/assets/stylesheets/new_design/stats.scss +++ b/app/assets/stylesheets/new_design/stats.scss @@ -53,7 +53,7 @@ $stat-card-half-horizontal-spacing: 4 * $default-space; .stat-card-half { width: calc((100% - #{$stat-card-half-horizontal-spacing}) / 2); - margin-right: 3 * $default-space; + margin-right: $stat-card-half-horizontal-spacing; } .stat-card-title { diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index fe323dee4..5f2304c55 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -205,6 +205,24 @@ module Instructeurs redirect_to instructeur_procedure_path(procedure) end + def stats + @procedure = procedure + @usual_traitement_time = procedure.usual_traitement_time + + @dossiers_funnel = [ + ['Démarrés', procedure.dossiers.count], + ['Déposés', procedure.dossiers.state_not_brouillon.count], + ['Instruction débutée', procedure.dossiers.state_instruction_commencee.count], + ['Traités', procedure.dossiers.state_termine.count] + ] + + @termines_states = [ + ['Acceptés', procedure.dossiers.where(state: :accepte).count], + ['Refusés', procedure.dossiers.where(state: :refuse).count], + ['Classés sans suite', procedure.dossiers.where(state: :sans_suite).count] + ] + end + private def find_field(table, column) diff --git a/app/views/instructeurs/procedures/show.html.haml b/app/views/instructeurs/procedures/show.html.haml index 6dc7f9292..6ae142f29 100644 --- a/app/views/instructeurs/procedures/show.html.haml +++ b/app/views/instructeurs/procedures/show.html.haml @@ -9,7 +9,9 @@ .procedure-header %h1= procedure_libelle @procedure - = link_to 'configurez vos notifications', email_notifications_instructeur_procedure_path(@procedure), class: 'notifications' + = link_to 'gestion des notifications', email_notifications_instructeur_procedure_path(@procedure), class: 'header-link' + | + = link_to 'statistiques', stats_instructeur_procedure_path(@procedure), class: 'header-link' %ul.tabs diff --git a/app/views/instructeurs/procedures/stats.html.haml b/app/views/instructeurs/procedures/stats.html.haml new file mode 100644 index 000000000..4346d99ff --- /dev/null +++ b/app/views/instructeurs/procedures/stats.html.haml @@ -0,0 +1,32 @@ +- title = "Statistiques · #{@procedure.libelle}" +- content_for(:title, title) + += render partial: 'new_administrateur/breadcrumbs', + locals: { steps: [link_to(@procedure.libelle, procedure_path(@procedure)), + 'Statistiques'] } + +.statistiques + %h1.new-h1= title + .stat-cards + - if @usual_traitement_time.present? + .stat-card.big-number-card + %span.big-number-card-title TEMPS DE TRAITEMENT USUEL + %span.big-number-card-number + = distance_of_time_in_words(@usual_traitement_time) + %span.big-number-card-detail + 90% des demandes du mois dernier ont été traitées en moins de #{distance_of_time_in_words(@usual_traitement_time)}. + + .stat-cards + .stat-card.stat-card-half.pull-left + %span.stat-card-title AVANCÉE DES DOSSIERS + .chart-container + .chart + = area_chart @dossiers_funnel + + .stat-card.stat-card-half.pull-left + %span.stat-card-title TAUX D’ACCEPTATION + .chart-container + .chart + - colors = %w(#C3D9FF #0069CC #1C7EC9) # from _colors.scss + = pie_chart @termines_states, colors: colors + diff --git a/config/routes.rb b/config/routes.rb index e48f85db9..bf0542172 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -285,6 +285,7 @@ Rails.application.routes.draw do post 'add_filter' get 'remove_filter' => 'procedures#remove_filter', as: 'remove_filter' get 'download_dossiers' + get 'stats' get 'email_notifications' patch 'update_email_notifications' From 06d60cd9434236fef279e341f362fd253a101f18 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Mon, 16 Sep 2019 16:30:45 +0200 Subject: [PATCH 17/28] Cache requests in procedures/stats --- .../instructeurs/procedures_controller.rb | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 5f2304c55..8e431ca19 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -207,20 +207,9 @@ module Instructeurs def stats @procedure = procedure - @usual_traitement_time = procedure.usual_traitement_time - - @dossiers_funnel = [ - ['Démarrés', procedure.dossiers.count], - ['Déposés', procedure.dossiers.state_not_brouillon.count], - ['Instruction débutée', procedure.dossiers.state_instruction_commencee.count], - ['Traités', procedure.dossiers.state_termine.count] - ] - - @termines_states = [ - ['Acceptés', procedure.dossiers.where(state: :accepte).count], - ['Refusés', procedure.dossiers.where(state: :refuse).count], - ['Classés sans suite', procedure.dossiers.where(state: :sans_suite).count] - ] + @usual_traitement_time = stats_usual_traitement_time(@procedure) + @dossiers_funnel = stats_dossiers_funnel(@procedure) + @termines_states = stats_termines_states(@procedure) end private @@ -301,5 +290,31 @@ module Instructeurs end EVAL end + + def stats_usual_traitement_time(procedure) + Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_usual_traitement_time", expires_in: 12.hours) do + procedure.usual_traitement_time + end + end + def stats_dossiers_funnel(procedure) + Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_dossiers_funnel", expires_in: 12.hours) do + [ + ['Démarrés', procedure.dossiers.count], + ['Déposés', procedure.dossiers.state_not_brouillon.count], + ['Instruction débutée', procedure.dossiers.state_instruction_commencee.count], + ['Traités', procedure.dossiers.state_termine.count] + ] + end + end + + def stats_termines_states(procedure) + Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_termines_states", expires_in: 12.hours) do + [ + ['Acceptés', procedure.dossiers.where(state: :accepte).count], + ['Refusés', procedure.dossiers.where(state: :refuse).count], + ['Classés sans suite', procedure.dossiers.where(state: :sans_suite).count] + ] + end + end end end From 0c98f29d59b7def344d27c55f674dbf12b974ba8 Mon Sep 17 00:00:00 2001 From: Nicolas Bouilleaud Date: Tue, 17 Sep 2019 15:52:38 +0200 Subject: [PATCH 18/28] Move the cached procedure stats queries to a ProcedureStatsConcern --- .../instructeurs/procedures_controller.rb | 32 ++----------------- .../concerns/procedure_stats_concern.rb | 30 +++++++++++++++++ app/models/procedure.rb | 2 ++ 3 files changed, 35 insertions(+), 29 deletions(-) create mode 100644 app/models/concerns/procedure_stats_concern.rb diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 8e431ca19..ffc646152 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -207,9 +207,9 @@ module Instructeurs def stats @procedure = procedure - @usual_traitement_time = stats_usual_traitement_time(@procedure) - @dossiers_funnel = stats_dossiers_funnel(@procedure) - @termines_states = stats_termines_states(@procedure) + @usual_traitement_time = @procedure.stats_usual_traitement_time + @dossiers_funnel = @procedure.stats_dossiers_funnel + @termines_states = @procedure.stats_termines_states end private @@ -290,31 +290,5 @@ module Instructeurs end EVAL end - - def stats_usual_traitement_time(procedure) - Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_usual_traitement_time", expires_in: 12.hours) do - procedure.usual_traitement_time - end - end - def stats_dossiers_funnel(procedure) - Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_dossiers_funnel", expires_in: 12.hours) do - [ - ['Démarrés', procedure.dossiers.count], - ['Déposés', procedure.dossiers.state_not_brouillon.count], - ['Instruction débutée', procedure.dossiers.state_instruction_commencee.count], - ['Traités', procedure.dossiers.state_termine.count] - ] - end - end - - def stats_termines_states(procedure) - Rails.cache.fetch("#{procedure.cache_key_with_version}/stats_termines_states", expires_in: 12.hours) do - [ - ['Acceptés', procedure.dossiers.where(state: :accepte).count], - ['Refusés', procedure.dossiers.where(state: :refuse).count], - ['Classés sans suite', procedure.dossiers.where(state: :sans_suite).count] - ] - end - end end end diff --git a/app/models/concerns/procedure_stats_concern.rb b/app/models/concerns/procedure_stats_concern.rb new file mode 100644 index 000000000..7e487c6a4 --- /dev/null +++ b/app/models/concerns/procedure_stats_concern.rb @@ -0,0 +1,30 @@ +module ProcedureStatsConcern + extend ActiveSupport::Concern + + def stats_usual_traitement_time + Rails.cache.fetch("#{cache_key_with_version}/stats_usual_traitement_time", expires_in: 12.hours) do + usual_traitement_time + end + end + + def stats_dossiers_funnel + Rails.cache.fetch("#{cache_key_with_version}/stats_dossiers_funnel", expires_in: 12.hours) do + [ + ['Démarrés', dossiers.count], + ['Déposés', dossiers.state_not_brouillon.count], + ['Instruction débutée', dossiers.state_instruction_commencee.count], + ['Traités', dossiers.state_termine.count] + ] + end + end + + def stats_termines_states + Rails.cache.fetch("#{cache_key_with_version}/stats_termines_states", expires_in: 12.hours) do + [ + ['Acceptés', dossiers.where(state: :accepte).count], + ['Refusés', dossiers.where(state: :refuse).count], + ['Classés sans suite', dossiers.where(state: :sans_suite).count] + ] + end + end +end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 16ff6a9ee..6fcd57717 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -3,6 +3,8 @@ require Rails.root.join('lib', 'percentile') class Procedure < ApplicationRecord self.ignored_columns = ['logo', 'logo_secure_token'] + include ProcedureStatsConcern + MAX_DUREE_CONSERVATION = 36 has_many :types_de_champ, -> { root.public_only.ordered }, inverse_of: :procedure, dependent: :destroy From d2a78abc3a9ae1236846ec876e5894e1c63bcc42 Mon Sep 17 00:00:00 2001 From: pedong Date: Wed, 14 Aug 2019 18:54:45 +0200 Subject: [PATCH 19/28] [fix #3975] set buttons floating in dossier page --- app/assets/stylesheets/new_design/forms.scss | 51 +++++++++++++++++++ app/views/shared/dossiers/_edit.html.haml | 2 +- .../dossiers/brouillon.html.haml_spec.rb | 2 +- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/new_design/forms.scss b/app/assets/stylesheets/new_design/forms.scss index e4f412ce7..2b26b4ff8 100644 --- a/app/assets/stylesheets/new_design/forms.scss +++ b/app/assets/stylesheets/new_design/forms.scss @@ -361,6 +361,57 @@ } } + .send-dossier-wrapper { + display: flex; + width: 100%; + margin-top: $default-padding; + background: #FFFFFF; + position: fixed; + bottom: 0px; + border: 1px solid #CCCCCC; + border-top-left-radius: 5px; + border-top-right-radius: 5px; + border-bottom: none; + padding: 30px; + padding-bottom: 5px; + padding-top: 5px; + left: 0px; + margin: 0 auto; + + .button { + margin-top: $default-padding; + margin-bottom: 0; + } + + // Wide layout: align buttons on a single row + @media (min-width: 550px) { + flex-direction: row; + + .button:not(:first-of-type) { + margin-left: $default-spacer; + } + + // If there are more than one button, align the "Send" button to the right + .button:not(:first-of-type).send { + margin-left: auto; + } + } + + // Narrow layout: stack buttons vertically + @media (max-width: 550px) { + flex-direction: column-reverse; + align-items: center; + + .button { + width: 100%; + max-width: 350px; + line-height: 30px; + margin-left: none; + margin-right: none; + } + } + } + .send-notice { @include notice-text-style; margin-bottom: $default-padding; diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index 01cfa4f2a..cfe6c2217 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -32,7 +32,7 @@ locals: { champ: champ, form: champ_form } - if !apercu - .send-wrapper + .send-dossier-wrapper - if dossier.brouillon? = f.button 'Enregistrer le brouillon', formnovalidate: true, diff --git a/spec/views/users/dossiers/brouillon.html.haml_spec.rb b/spec/views/users/dossiers/brouillon.html.haml_spec.rb index 2d95b631a..505ae734d 100644 --- a/spec/views/users/dossiers/brouillon.html.haml_spec.rb +++ b/spec/views/users/dossiers/brouillon.html.haml_spec.rb @@ -21,7 +21,7 @@ describe 'users/dossiers/brouillon.html.haml', type: :view do end it 'affiche les boutons de validation' do - expect(rendered).to have_selector('.send-wrapper') + expect(rendered).to have_selector('.send-dossier-wrapper') end it 'prépare le footer' do From 09a0f21cacbe53f2f7780ac82234801f95c9ce66 Mon Sep 17 00:00:00 2001 From: pedong Date: Wed, 28 Aug 2019 18:42:30 +0200 Subject: [PATCH 20/28] use same look with .champs-editor.buttons --- app/assets/stylesheets/new_design/forms.scss | 8 +++++++- app/assets/stylesheets/new_design/new_footer.scss | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/new_design/forms.scss b/app/assets/stylesheets/new_design/forms.scss index 2b26b4ff8..201ec1c16 100644 --- a/app/assets/stylesheets/new_design/forms.scss +++ b/app/assets/stylesheets/new_design/forms.scss @@ -375,14 +375,19 @@ padding: 30px; padding-bottom: 5px; padding-top: 5px; - left: 0px; margin: 0 auto; + margin-left: -30px; + max-width: 1100px; .button { margin-top: $default-padding; margin-bottom: 0; } + @media(max-width: 1100px) { + margin-left: -16px; + } + // Wide layout: align buttons on a single row @media (min-width: 550px) { flex-direction: row; @@ -401,6 +406,7 @@ @media (max-width: 550px) { flex-direction: column-reverse; align-items: center; + margin-left: -16px; .button { width: 100%; diff --git a/app/assets/stylesheets/new_design/new_footer.scss b/app/assets/stylesheets/new_design/new_footer.scss index a879bd4b4..d3048e7a9 100644 --- a/app/assets/stylesheets/new_design/new_footer.scss +++ b/app/assets/stylesheets/new_design/new_footer.scss @@ -109,7 +109,7 @@ footer { margin-bottom: 30px; &:last-child { - margin-bottom: 0; + margin-bottom: 35px; } // In this case, the bottom margin is defined directly on each individual column From 974c1b1150b8badcc7e9ba3af8ee531fbf1fdd98 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 12 Sep 2019 14:16:15 +0200 Subject: [PATCH 21/28] stylesheets: move the dossier floating bar CSS to dossier_edit.scss forms.scss is supposed to be for generic elements. --- .../stylesheets/new_design/dossier_edit.scss | 57 +++++++++++++++++++ app/assets/stylesheets/new_design/forms.scss | 57 ------------------- app/views/shared/dossiers/_edit.html.haml | 2 +- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/app/assets/stylesheets/new_design/dossier_edit.scss b/app/assets/stylesheets/new_design/dossier_edit.scss index 3b631c66b..ef8d07cb3 100644 --- a/app/assets/stylesheets/new_design/dossier_edit.scss +++ b/app/assets/stylesheets/new_design/dossier_edit.scss @@ -45,4 +45,61 @@ padding: 20px; border-radius: 4px; } + + .send-dossier-actions-bar { + display: flex; + width: 100%; + margin-top: $default-padding; + background: #FFFFFF; + position: fixed; + bottom: 0px; + border: 1px solid #CCCCCC; + border-top-left-radius: 5px; + border-top-right-radius: 5px; + border-bottom: none; + padding: 30px; + padding-bottom: 5px; + padding-top: 5px; + margin: 0 auto; + margin-left: -30px; + max-width: 1100px; + + .button { + margin-top: $default-padding; + margin-bottom: 0; + } + + @media(max-width: 1100px) { + margin-left: -16px; + } + + // Wide layout: align buttons on a single row + @media (min-width: 550px) { + flex-direction: row; + + .button:not(:first-of-type) { + margin-left: $default-spacer; + } + + // If there are more than one button, align the "Send" button to the right + .button:not(:first-of-type).send { + margin-left: auto; + } + } + + // Narrow layout: stack buttons vertically + @media (max-width: 550px) { + flex-direction: column-reverse; + align-items: center; + margin-left: -16px; + + .button { + width: 100%; + max-width: 350px; + line-height: 30px; + margin-left: none; + margin-right: none; + } + } + } } diff --git a/app/assets/stylesheets/new_design/forms.scss b/app/assets/stylesheets/new_design/forms.scss index 201ec1c16..e4f412ce7 100644 --- a/app/assets/stylesheets/new_design/forms.scss +++ b/app/assets/stylesheets/new_design/forms.scss @@ -361,63 +361,6 @@ } } - .send-dossier-wrapper { - display: flex; - width: 100%; - margin-top: $default-padding; - background: #FFFFFF; - position: fixed; - bottom: 0px; - border: 1px solid #CCCCCC; - border-top-left-radius: 5px; - border-top-right-radius: 5px; - border-bottom: none; - padding: 30px; - padding-bottom: 5px; - padding-top: 5px; - margin: 0 auto; - margin-left: -30px; - max-width: 1100px; - - .button { - margin-top: $default-padding; - margin-bottom: 0; - } - - @media(max-width: 1100px) { - margin-left: -16px; - } - - // Wide layout: align buttons on a single row - @media (min-width: 550px) { - flex-direction: row; - - .button:not(:first-of-type) { - margin-left: $default-spacer; - } - - // If there are more than one button, align the "Send" button to the right - .button:not(:first-of-type).send { - margin-left: auto; - } - } - - // Narrow layout: stack buttons vertically - @media (max-width: 550px) { - flex-direction: column-reverse; - align-items: center; - margin-left: -16px; - - .button { - width: 100%; - max-width: 350px; - line-height: 30px; - margin-left: none; - margin-right: none; - } - } - } - .send-notice { @include notice-text-style; margin-bottom: $default-padding; diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index cfe6c2217..7da3f5bc5 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -32,7 +32,7 @@ locals: { champ: champ, form: champ_form } - if !apercu - .send-dossier-wrapper + .send-dossier-actions-bar - if dossier.brouillon? = f.button 'Enregistrer le brouillon', formnovalidate: true, From 1f8f38f492cef4b5c706888f790d6fcc37990250 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 12 Sep 2019 14:28:11 +0200 Subject: [PATCH 22/28] stylesheets: align the buttons vertically --- app/assets/stylesheets/new_design/dossier_edit.scss | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/new_design/dossier_edit.scss b/app/assets/stylesheets/new_design/dossier_edit.scss index ef8d07cb3..56023ada8 100644 --- a/app/assets/stylesheets/new_design/dossier_edit.scss +++ b/app/assets/stylesheets/new_design/dossier_edit.scss @@ -57,16 +57,16 @@ border-top-left-radius: 5px; border-top-right-radius: 5px; border-bottom: none; - padding: 30px; - padding-bottom: 5px; - padding-top: 5px; + padding-top: 0px; + padding-bottom: $default-spacer; + padding-right: 30px; + padding-left: 30px; margin: 0 auto; margin-left: -30px; max-width: 1100px; .button { - margin-top: $default-padding; - margin-bottom: 0; + margin-top: $default-spacer; } @media(max-width: 1100px) { From 4b883a57bfeddc6fab10851ab96757112f68b394 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 12 Sep 2019 13:56:24 +0000 Subject: [PATCH 23/28] stylesheets: make the actions bar sticky --- .../stylesheets/new_design/dossier_edit.scss | 66 ++++++++----------- .../stylesheets/new_design/new_footer.scss | 4 -- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/app/assets/stylesheets/new_design/dossier_edit.scss b/app/assets/stylesheets/new_design/dossier_edit.scss index 56023ada8..ad0490cfd 100644 --- a/app/assets/stylesheets/new_design/dossier_edit.scss +++ b/app/assets/stylesheets/new_design/dossier_edit.scss @@ -47,59 +47,49 @@ } .send-dossier-actions-bar { + // scss-lint:disable VendorPrefix + position: -webkit-sticky; // This is needed on Safari (tested on 12.1) + // scss-lint:enable VendorPrefix + position: sticky; + bottom: 0; + display: flex; - width: 100%; + flex-direction: row; + align-items: center; margin-top: $default-padding; + margin-left: -$default-padding; + margin-right: -$default-padding; + margin-bottom: 0; + padding-top: 0; + padding-bottom: $default-spacer; + padding-right: $default-padding; + padding-left: $default-padding; background: #FFFFFF; - position: fixed; - bottom: 0px; border: 1px solid #CCCCCC; border-top-left-radius: 5px; border-top-right-radius: 5px; border-bottom: none; - padding-top: 0px; - padding-bottom: $default-spacer; - padding-right: 30px; - padding-left: 30px; - margin: 0 auto; - margin-left: -30px; - max-width: 1100px; .button { - margin-top: $default-spacer; + min-height: 38px; + line-height: 16px; } - @media(max-width: 1100px) { - margin-left: -16px; + // If there are more than one button, align the "Send" button to the right + .button:not(:first-of-type).send { + margin-left: auto; } - // Wide layout: align buttons on a single row - @media (min-width: 550px) { - flex-direction: row; - - .button:not(:first-of-type) { - margin-left: $default-spacer; - } - - // If there are more than one button, align the "Send" button to the right - .button:not(:first-of-type).send { - margin-left: auto; - } + // Normal layout + @media (min-width: 500px) { + padding-top: $default-spacer * 2; + padding-bottom: $default-spacer * 2; } - // Narrow layout: stack buttons vertically - @media (max-width: 550px) { - flex-direction: column-reverse; - align-items: center; - margin-left: -16px; - - .button { - width: 100%; - max-width: 350px; - line-height: 30px; - margin-left: none; - margin-right: none; - } + // Compact layout + @media (max-width: 500px) { + padding-top: $default-spacer; + padding-bottom: $default-spacer; } } } diff --git a/app/assets/stylesheets/new_design/new_footer.scss b/app/assets/stylesheets/new_design/new_footer.scss index d3048e7a9..092ee442b 100644 --- a/app/assets/stylesheets/new_design/new_footer.scss +++ b/app/assets/stylesheets/new_design/new_footer.scss @@ -108,10 +108,6 @@ footer { .footer-row { margin-bottom: 30px; - &:last-child { - margin-bottom: 35px; - } - // In this case, the bottom margin is defined directly on each individual column &.footer-columns { margin-bottom: 0; From 11b4698ef3735b3e5715ead23f82f5549411acaa Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 16:09:19 +0200 Subject: [PATCH 24/28] Fix test --- spec/views/users/dossiers/brouillon.html.haml_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/views/users/dossiers/brouillon.html.haml_spec.rb b/spec/views/users/dossiers/brouillon.html.haml_spec.rb index 505ae734d..5a1802c1e 100644 --- a/spec/views/users/dossiers/brouillon.html.haml_spec.rb +++ b/spec/views/users/dossiers/brouillon.html.haml_spec.rb @@ -21,7 +21,7 @@ describe 'users/dossiers/brouillon.html.haml', type: :view do end it 'affiche les boutons de validation' do - expect(rendered).to have_selector('.send-dossier-wrapper') + expect(rendered).to have_selector('.send-dossier-actions-bar') end it 'prépare le footer' do From 91815ad7093a00384e50978494249da5a5e18c63 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 10 Jul 2019 19:04:10 +0200 Subject: [PATCH 25/28] Upload through proxy --- .../service/ds_proxy_service.rb | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/app/lib/active_storage/service/ds_proxy_service.rb b/app/lib/active_storage/service/ds_proxy_service.rb index fe540d591..bb516d287 100644 --- a/app/lib/active_storage/service/ds_proxy_service.rb +++ b/app/lib/active_storage/service/ds_proxy_service.rb @@ -1,6 +1,7 @@ module ActiveStorage # Wraps an ActiveStorage::Service to route direct upload and direct download URLs through our proxy, - # thus avoiding exposing the storage provider’s URL to our end-users. + # thus avoiding exposing the storage provider’s URL to our end-users. It also overrides upload and + # object_for methods to fetch and put blobs through encryption proxy. class Service::DsProxyService < SimpleDelegator attr_reader :wrapped @@ -23,8 +24,37 @@ module ActiveStorage publicize(url) end + # This method is responsible for writing to object storage. We directly use direct upload + # url to ensure we upload through encryption proxy. + def upload(key, io, checksum: nil, **) + wrapped.send(:instrument, :upload, key: key, checksum: checksum) do + url = url_for_direct_upload(key, expires_in: 1.hour) + data = Fog::Storage.parse_data(io) + + headers = { 'Content-Type' => wrapped.send(:guess_content_type, io) }.merge(data[:headers]) + if checksum + headers['ETag'] = wrapped.send(:convert_base64digest_to_hexdigest, checksum) + end + + response = Typhoeus::Request.new( + url, + method: :put, + body: data[:body].read, + headers: headers + ).run + + if response.success? + response + else + raise ActiveStorage::IntegrityError + end + end + end + private + # This method is responsible for reading from object storage. We use url method + # on the service to ensure we download through encryption proxy. def object_for(key, &block) blob_url = url(key) if block_given? From d338f9b9f303c03c9f1d9ca089656b1ac2849b7f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 12 Sep 2019 17:21:43 +0200 Subject: [PATCH 26/28] Factory: can create a dossier with a defined gi --- spec/factories/dossier.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index f828ff27f..98cf72929 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -14,7 +14,10 @@ FactoryBot.define do else procedure = create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private) end - dossier.groupe_instructeur = procedure.defaut_groupe_instructeur + + if dossier.groupe_instructeur.nil? + dossier.groupe_instructeur = procedure.defaut_groupe_instructeur + end end trait :with_entreprise do From dec42e4886384b48037f717bc2bde422e7dec548 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 12 Sep 2019 17:26:59 +0200 Subject: [PATCH 27/28] Instructeur procedure index: show stats by procedure Also sum the count from differents groupe_instructeur from the same procedure --- .../instructeurs/procedures_controller.rb | 19 ++--- .../instructeurs/procedures/index.html.haml | 10 +-- .../procedures_controller_spec.rb | 80 +++++++++++++++---- 3 files changed, 80 insertions(+), 29 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index ffc646152..3d7353b32 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -12,19 +12,20 @@ module Instructeurs .includes(:defaut_groupe_instructeur) .order(archived_at: :desc, published_at: :desc, created_at: :desc) - groupe_instructeurs = current_instructeur.groupe_instructeurs.where(procedure: @procedures) + dossiers = current_instructeur.dossiers.joins(:groupe_instructeur) + @dossiers_count_per_procedure = dossiers.all_state.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_a_suivre_count_per_procedure = dossiers.without_followers.en_cours.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_archived_count_per_procedure = dossiers.archived.group('groupe_instructeurs.procedure_id').count + @dossiers_termines_count_per_procedure = dossiers.termine.group('groupe_instructeurs.procedure_id').reorder(nil).count - dossiers = current_instructeur.dossiers - @dossiers_count_per_groupe_instructeur = dossiers.all_state.group(:groupe_instructeur_id).reorder(nil).count - @dossiers_a_suivre_count_per_groupe_instructeur = dossiers.without_followers.en_cours.group(:groupe_instructeur_id).reorder(nil).count - @dossiers_archived_count_per_groupe_instructeur = dossiers.archived.group(:groupe_instructeur_id).count - @dossiers_termines_count_per_groupe_instructeur = dossiers.termine.group(:groupe_instructeur_id).reorder(nil).count + groupe_ids = current_instructeur.groupe_instructeurs.pluck(:id) - @followed_dossiers_count_per_groupe_instructeur = current_instructeur + @followed_dossiers_count_per_procedure = current_instructeur .followed_dossiers + .joins(:groupe_instructeur) .en_cours - .where(groupe_instructeur: groupe_instructeurs) - .group(:groupe_instructeur_id) + .where(groupe_instructeur_id: groupe_ids) + .group('groupe_instructeurs.procedure_id') .reorder(nil) .count end diff --git a/app/views/instructeurs/procedures/index.html.haml b/app/views/instructeurs/procedures/index.html.haml index 95c8341a0..305732605 100644 --- a/app/views/instructeurs/procedures/index.html.haml +++ b/app/views/instructeurs/procedures/index.html.haml @@ -19,7 +19,7 @@ %li %object = link_to(instructeur_procedure_path(p, statut: 'a-suivre')) do - - a_suivre_count = @dossiers_a_suivre_count_per_groupe_instructeur[p.defaut_groupe_instructeur.id] || 0 + - a_suivre_count = @dossiers_a_suivre_count_per_procedure[p.id] || 0 .stats-number = a_suivre_count .stats-legend @@ -29,7 +29,7 @@ = link_to(instructeur_procedure_path(p, statut: 'suivis')) do - if current_instructeur.notifications_per_procedure[p.id].present? %span.notifications{ 'aria-label': "notifications" } - - followed_count = @followed_dossiers_count_per_groupe_instructeur[p.defaut_groupe_instructeur.id] || 0 + - followed_count = @followed_dossiers_count_per_procedure[p.id] || 0 .stats-number = followed_count .stats-legend @@ -39,7 +39,7 @@ = link_to(instructeur_procedure_path(p, statut: 'traites')) do - if current_instructeur.notifications_per_procedure(:termine)[p.id].present? %span.notifications{ 'aria-label': "notifications" } - - termines_count = @dossiers_termines_count_per_groupe_instructeur[p.defaut_groupe_instructeur.id] || 0 + - termines_count = @dossiers_termines_count_per_procedure[p.id] || 0 .stats-number = termines_count .stats-legend @@ -47,7 +47,7 @@ %li %object = link_to(instructeur_procedure_path(p, statut: 'tous')) do - - dossier_count = @dossiers_count_per_groupe_instructeur[p.defaut_groupe_instructeur.id] || 0 + - dossier_count = @dossiers_count_per_procedure[p.id] || 0 .stats-number = dossier_count .stats-legend @@ -55,7 +55,7 @@ %li %object = link_to(instructeur_procedure_path(p, statut: 'archives')) do - - archived_count = @dossiers_archived_count_per_groupe_instructeur[p.defaut_groupe_instructeur.id] || 0 + - archived_count = @dossiers_archived_count_per_procedure[p.id] || 0 .stats-number = archived_count .stats-legend diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index a6a0b8246..740d61024 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -125,11 +125,11 @@ describe Instructeurs::ProceduresController, type: :controller do let(:state) { Dossier.states.fetch(:brouillon) } before { subject } - it { expect(assigns(:dossiers_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:dossiers_a_suivre_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:dossiers_archived_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:followed_dossiers_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:dossiers_termines_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } + it { expect(assigns(:dossiers_count_per_procedure)[procedure.id]).to eq(nil) } + it { expect(assigns(:dossiers_a_suivre_count_per_procedure)[procedure.id]).to eq(nil) } + it { expect(assigns(:dossiers_archived_count_per_procedure)[procedure.id]).to eq(nil) } + it { expect(assigns(:followed_dossiers_count_per_procedure)[procedure.id]).to eq(nil) } + it { expect(assigns(:dossiers_termines_count_per_procedure)[procedure.id]).to eq(nil) } end context "with not draft state on multiple procedures" do @@ -149,17 +149,67 @@ describe Instructeurs::ProceduresController, type: :controller do subject end - it { expect(assigns(:dossiers_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(3) } - it { expect(assigns(:dossiers_a_suivre_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(3) } - it { expect(assigns(:followed_dossiers_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:dossiers_archived_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(1) } - it { expect(assigns(:dossiers_termines_count_per_groupe_instructeur)[procedure.defaut_groupe_instructeur.id]).to eq(nil) } + it { expect(assigns(:dossiers_count_per_procedure)[procedure.id]).to eq(3) } + it { expect(assigns(:dossiers_a_suivre_count_per_procedure)[procedure.id]).to eq(3) } + it { expect(assigns(:followed_dossiers_count_per_procedure)[procedure.id]).to eq(nil) } + it { expect(assigns(:dossiers_archived_count_per_procedure)[procedure.id]).to eq(1) } + it { expect(assigns(:dossiers_termines_count_per_procedure)[procedure.id]).to eq(nil) } - it { expect(assigns(:dossiers_count_per_groupe_instructeur)[procedure2.defaut_groupe_instructeur.id]).to eq(3) } - it { expect(assigns(:dossiers_a_suivre_count_per_groupe_instructeur)[procedure2.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:followed_dossiers_count_per_groupe_instructeur)[procedure2.defaut_groupe_instructeur.id]).to eq(1) } - it { expect(assigns(:dossiers_archived_count_per_groupe_instructeur)[procedure2.defaut_groupe_instructeur.id]).to eq(nil) } - it { expect(assigns(:dossiers_termines_count_per_groupe_instructeur)[procedure2.defaut_groupe_instructeur.id]).to eq(1) } + it { expect(assigns(:dossiers_count_per_procedure)[procedure2.id]).to eq(3) } + it { expect(assigns(:dossiers_a_suivre_count_per_procedure)[procedure2.id]).to eq(nil) } + it { expect(assigns(:followed_dossiers_count_per_procedure)[procedure2.id]).to eq(1) } + it { expect(assigns(:dossiers_archived_count_per_procedure)[procedure2.id]).to eq(nil) } + it { expect(assigns(:dossiers_termines_count_per_procedure)[procedure2.id]).to eq(1) } + end + end + + context "with a routed procedure" do + let!(:procedure) { create(:procedure, :published) } + let!(:gi_p1_1) { procedure.defaut_groupe_instructeur } + let!(:gi_p1_2) { GroupeInstructeur.create(label: '2', procedure: procedure) } + + context 'with multiple dossiers en construction on each group' do + before do + alternate_gis = 0.upto(20).map { |i| i.even? ? gi_p1_1 : gi_p1_2 } + + alternate_gis.take(4).each { |gi| create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_construction), groupe_instructeur: gi) } + + alternate_gis.take(6).each do |gi| + instructeur.followed_dossiers << create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction), groupe_instructeur: gi) + end + + alternate_gis.take(10).each { |gi| create(:dossier, procedure: procedure, state: Dossier.states.fetch(:sans_suite), groupe_instructeur: gi) } + alternate_gis.take(14).each { |gi| create(:dossier, procedure: procedure, state: Dossier.states.fetch(:sans_suite), archived: true, groupe_instructeur: gi) } + end + + context 'when an instructeur belongs to the 2 gi' do + before do + instructeur.groupe_instructeurs << gi_p1_1 << gi_p1_2 + + subject + end + + it { expect(assigns(:dossiers_a_suivre_count_per_procedure)[procedure.id]).to eq(4) } + it { expect(assigns(:followed_dossiers_count_per_procedure)[procedure.id]).to eq(6) } + it { expect(assigns(:dossiers_termines_count_per_procedure)[procedure.id]).to eq(10) } + it { expect(assigns(:dossiers_count_per_procedure)[procedure.id]).to eq(4 + 6 + 10) } + it { expect(assigns(:dossiers_archived_count_per_procedure)[procedure.id]).to eq(14) } + end + + context 'when an instructeur only belongs to one of them gi' do + before do + instructeur.groupe_instructeurs << gi_p1_1 + + subject + end + + it { expect(assigns(:dossiers_a_suivre_count_per_procedure)[procedure.id]).to eq(2) } + # An instructeur cannot follow a dossier which belongs to another groupe + it { expect(assigns(:followed_dossiers_count_per_procedure)[procedure.id]).to eq(3) } + it { expect(assigns(:dossiers_termines_count_per_procedure)[procedure.id]).to eq(5) } + it { expect(assigns(:dossiers_count_per_procedure)[procedure.id]).to eq(2 + 3 + 5) } + it { expect(assigns(:dossiers_archived_count_per_procedure)[procedure.id]).to eq(7) } + end end end end From 8fcf1353f3e4d9b6672a8e3365f37b675ce5d225 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 28 Aug 2019 14:27:41 +0200 Subject: [PATCH 28/28] Remove duplicated attachments --- .../admin/attestation_templates_controller.rb | 19 +---- .../admin/procedures_controller.rb | 1 - .../instructeurs/dossiers_controller.rb | 9 +-- app/controllers/users/dossiers_controller.rb | 2 - app/mailers/application_mailer.rb | 7 +- app/models/attestation.rb | 3 - app/models/attestation_template.rb | 71 ++++--------------- app/models/procedure.rb | 8 --- .../attestation_templates/edit.html.haml | 8 +-- .../attestation_templates/show.pdf.prawn | 32 +++++---- .../admin/procedures/_informations.html.haml | 2 +- .../_show.html.haml | 4 +- .../2019_05_29_migrate_commentaire_pj.rake | 38 ---------- .../attestation_templates_controller_spec.rb | 20 +++--- spec/models/attestation_template_spec.rb | 8 ++- 15 files changed, 59 insertions(+), 173 deletions(-) delete mode 100644 lib/tasks/2019_05_29_migrate_commentaire_pj.rake diff --git a/app/controllers/admin/attestation_templates_controller.rb b/app/controllers/admin/attestation_templates_controller.rb index 729c66f9b..f50cba788 100644 --- a/app/controllers/admin/attestation_templates_controller.rb +++ b/app/controllers/admin/attestation_templates_controller.rb @@ -40,33 +40,20 @@ class Admin::AttestationTemplatesController < AdminController end def preview - @title = activated_attestation_params[:title] - @body = activated_attestation_params[:body] - @footer = activated_attestation_params[:footer] - @created_at = Time.zone.now - - # In a case of a preview, when the user does not change its images, - # the images are not uploaded and thus should be retrieved from previous - # attestation_template - @logo = activated_attestation_params[:logo] || @procedure.attestation_template&.proxy_logo - @signature = activated_attestation_params[:signature] || @procedure.attestation_template&.proxy_signature + @attestation = (@procedure.attestation_template || AttestationTemplate.new).render_attributes_for(activated_attestation_params) render 'admin/attestation_templates/show', formats: [:pdf] end def delete_logo - attestation_template = @procedure.attestation_template - attestation_template.logo.purge_later - attestation_template.logo_active_storage.purge_later + @procedure.attestation_template.logo.purge_later flash.notice = 'le logo a bien été supprimée' redirect_to edit_admin_procedure_attestation_template_path(@procedure) end def delete_signature - attestation_template = @procedure.attestation_template - attestation_template.signature.purge_later - attestation_template.signature_active_storage.purge_later + @procedure.attestation_template.signature.purge_later flash.notice = 'la signature a bien été supprimée' redirect_to edit_admin_procedure_attestation_template_path(@procedure) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 05230f168..6ddd005e0 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -208,7 +208,6 @@ class Admin::ProceduresController < AdminController def delete_logo @procedure.logo.purge_later - @procedure.logo_active_storage.purge_later flash.notice = 'le logo a bien été supprimé' redirect_to edit_admin_procedure_path(@procedure) diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index c213a0255..ac8343631 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -16,18 +16,11 @@ module Instructeurs def attestation if dossier.attestation.pdf.attached? redirect_to url_for(dossier.attestation.pdf) - elsif dossier.attestation.pdf_active_storage.attached? - redirect_to url_for(dossier.attestation.pdf_active_storage) end end def apercu_attestation - @title = dossier.procedure.attestation_template.title_for_dossier(dossier) - @body = dossier.procedure.attestation_template.body_for_dossier(dossier) - @footer = dossier.procedure.attestation_template.footer - @created_at = Time.zone.now - @logo = dossier.procedure.attestation_template&.proxy_logo - @signature = dossier.procedure.attestation_template&.proxy_signature + @attestation = dossier.procedure.attestation_template.render_attributes_for(dossier: dossier) render 'admin/attestation_templates/show', formats: [:pdf] end diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 6eda2b032..71fd1c713 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -50,8 +50,6 @@ module Users def attestation if dossier.attestation.pdf.attached? redirect_to url_for(dossier.attestation.pdf) - elsif dossier.attestation.pdf_active_storage.attached? - redirect_to url_for(dossier.attestation.pdf_active_storage) end end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index ab6a5865e..3504f1a7a 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -6,17 +6,12 @@ class ApplicationMailer < ActionMailer::Base # Attach the procedure logo to the email (if any). # Returns the attachment url. def attach_logo(procedure) - return nil if !procedure.logo? - if procedure.logo.attached? logo_filename = procedure.logo.filename.to_s attachments.inline[logo_filename] = procedure.logo.download - elsif procedure.logo_active_storage.attached? - logo_filename = procedure.logo_active_storage.filename.to_s - attachments.inline[logo_filename] = procedure.logo_active_storage.download + attachments[logo_filename].url end - attachments[logo_filename].url rescue StandardError => e # A problem occured when reading logo, maybe the logo is missing and we should clean the procedure to remove logo reference ? Raven.extra_context(procedure_id: procedure.id) diff --git a/app/models/attestation.rb b/app/models/attestation.rb index bd599de4c..ee296b6a5 100644 --- a/app/models/attestation.rb +++ b/app/models/attestation.rb @@ -4,13 +4,10 @@ class Attestation < ApplicationRecord belongs_to :dossier has_one_attached :pdf - has_one_attached :pdf_active_storage def pdf_url if pdf.attached? Rails.application.routes.url_helpers.url_for(pdf) - elsif pdf_active_storage.attached? - Rails.application.routes.url_helpers.url_for(pdf_active_storage) end end end diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index 0f31cc36f..d61ebc1fb 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -7,9 +7,7 @@ class AttestationTemplate < ApplicationRecord belongs_to :procedure has_one_attached :logo - has_one_attached :logo_active_storage has_one_attached :signature - has_one_attached :signature_active_storage validates :footer, length: { maximum: 190 } @@ -54,14 +52,6 @@ class AttestationTemplate < ApplicationRecord # we don't want to run virus scanner on duplicated file metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } ) - elsif logo_active_storage.attached? - attestation_template.logo.attach( - io: StringIO.new(logo_active_storage.download), - filename: logo_active_storage.filename.to_s, - content_type: logo_active_storage.content_type, - # we don't want to run virus scanner on duplicated file - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - ) end if signature.attached? @@ -72,65 +62,34 @@ class AttestationTemplate < ApplicationRecord # we don't want to run virus scanner on duplicated file metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } ) - elsif signature_active_storage.attached? - attestation_template.signature.attach( - io: StringIO.new(signature_active_storage.download), - filename: signature_active_storage.filename.to_s, - content_type: signature_active_storage.content_type, - # we don't want to run virus scanner on duplicated file - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - ) end attestation_template end - def logo? - logo.attached? || logo_active_storage.attached? - end - - def signature? - signature.attached? || signature_active_storage.attached? - end - def logo_url if logo.attached? Rails.application.routes.url_helpers.url_for(logo) - elsif logo_active_storage.attached? - Rails.application.routes.url_helpers.url_for(logo_active_storage) end end def signature_url if signature.attached? Rails.application.routes.url_helpers.url_for(signature) - elsif signature_active_storage.attached? - Rails.application.routes.url_helpers.url_for(signature_active_storage) end end - def proxy_logo - if logo.attached? - logo - elsif logo_active_storage.attached? - logo_active_storage - end - end + def render_attributes_for(params = {}) + dossier = params.fetch(:dossier, false) - def proxy_signature - if signature.attached? - signature - elsif signature_active_storage.attached? - signature_active_storage - end - end - - def title_for_dossier(dossier) - replace_tags(title, dossier) - end - - def body_for_dossier(dossier) - replace_tags(body, dossier) + { + created_at: Time.zone.now, + title: dossier ? replace_tags(title, dossier) : params.fetch(:title, ''), + body: dossier ? replace_tags(body, dossier) : params.fetch(:body, ''), + footer: params.fetch(:footer, footer), + logo: params.fetch(:logo, logo.attached? ? logo : nil), + signature: params.fetch(:signature, signature.attached? ? signature : nil) + } end private @@ -147,14 +106,8 @@ class AttestationTemplate < ApplicationRecord end def build_pdf(dossier) - action_view = ActionView::Base.new(ActionController::Base.view_paths, - logo: proxy_logo, - title: title_for_dossier(dossier), - body: body_for_dossier(dossier), - signature: proxy_signature, - footer: footer, - created_at: Time.zone.now) - + attestation = render_attributes_for(dossier: dossier) + action_view = ActionView::Base.new(ActionController::Base.view_paths, attestation: attestation) attestation_view = action_view.render(file: 'admin/attestation_templates/show', formats: [:pdf]) StringIO.new(attestation_view) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 6fcd57717..4688cc070 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -32,7 +32,6 @@ class Procedure < ApplicationRecord has_one :defaut_groupe_instructeur, -> { where(label: GroupeInstructeur::DEFAULT_LABEL) }, class_name: 'GroupeInstructeur', inverse_of: :procedure has_one_attached :logo - has_one_attached :logo_active_storage has_one_attached :notice has_one_attached :deliberation @@ -304,7 +303,6 @@ class Procedure < ApplicationRecord clone_attachment(:piece_justificative_template, original, kopy) elsif original.is_a?(Procedure) clone_attachment(:logo, original, kopy) - clone_attachment(:logo_active_storage, original, kopy) clone_attachment(:notice, original, kopy) clone_attachment(:deliberation, original, kopy) end @@ -471,15 +469,9 @@ class Procedure < ApplicationRecord end end - def logo? - logo.attached? || logo_active_storage.attached? - end - def logo_url if logo.attached? Rails.application.routes.url_helpers.url_for(logo) - elsif logo_active_storage.attached? - Rails.application.routes.url_helpers.url_for(logo_active_storage) else ActionController::Base.helpers.image_url("marianne.svg") end diff --git a/app/views/admin/attestation_templates/edit.html.haml b/app/views/admin/attestation_templates/edit.html.haml index 764c6f91b..4239d89e7 100644 --- a/app/views/admin/attestation_templates/edit.html.haml +++ b/app/views/admin/attestation_templates/edit.html.haml @@ -17,11 +17,11 @@ celle-ci est également disponible au téléchargement depuis l’espace personnel de l’usager. .image-upload - - if @attestation_template.logo? + - if @attestation_template.logo.attached? = image_tag @attestation_template.logo_url, class: 'thumbnail' .form-group = f.label :logo, "Logo de l'attestation" - - if @attestation_template.logo? + - if @attestation_template.logo.attached? = link_to 'Supprimer le logo', admin_procedure_attestation_template_logo_path(@procedure), method: :delete = f.file_field :logo, accept: 'image/png, image/jpg, image/jpeg' %p.help-block @@ -54,11 +54,11 @@ = tag[:description] .image-upload - - if @attestation_template.signature? + - if @attestation_template.signature.attached? = image_tag @attestation_template.signature_url, class: 'thumbnail' .form-group = f.label :signature, "Tampon de l'attestation" - - if @attestation_template.signature? + - if @attestation_template.signature.attached? = link_to 'Supprimer le tampon', admin_procedure_attestation_template_signature_path(@procedure), method: :delete = f.file_field :signature, accept: 'image/png, image/jpg, image/jpeg' %p.help-block diff --git a/app/views/admin/attestation_templates/show.pdf.prawn b/app/views/admin/attestation_templates/show.pdf.prawn index ff40b1ec5..6ea60f129 100644 --- a/app/views/admin/attestation_templates/show.pdf.prawn +++ b/app/views/admin/attestation_templates/show.pdf.prawn @@ -19,6 +19,14 @@ max_logo_width = body_width max_logo_height = 50.mm max_signature_size = 50.mm +title = @attestation.fetch(:title) +body = @attestation.fetch(:body) +footer = @attestation.fetch(:footer) +created_at = @attestation.fetch(:created_at) + +logo = @attestation[:logo] +signature = @attestation[:signature] + prawn_document(margin: [top_margin, right_margin, bottom_margin, left_margin], page_size: page_size) do |pdf| pdf.font_families.update( 'liberation serif' => { normal: Rails.root.join('lib/prawn/fonts/liberation_serif/LiberationSerif-Regular.ttf' )}) pdf.font 'liberation serif' @@ -29,30 +37,30 @@ prawn_document(margin: [top_margin, right_margin, bottom_margin, left_margin], p body_height = pdf.cursor - footer_height pdf.bounding_box([0, pdf.cursor], width: body_width, height: body_height) do - if @logo.present? - logo_file = if @logo.is_a?(ActiveStorage::Attached::One) - @logo.download + if logo.present? + logo_file = if logo.is_a?(ActiveStorage::Attached::One) + logo.download else - @logo.read + logo.read end pdf.image StringIO.new(logo_file), fit: [max_logo_width , max_logo_height], position: :center end pdf.fill_color grey - pdf.pad_top(40) { pdf.text "le #{l(@created_at, format: '%e %B %Y')}", size: 10, align: :right, character_spacing: -0.5 } + pdf.pad_top(40) { pdf.text "le #{l(created_at, format: '%e %B %Y')}", size: 10, align: :right, character_spacing: -0.5 } pdf.fill_color black - pdf.pad_top(40) { pdf.text @title, size: 18, character_spacing: -0.2 } + pdf.pad_top(40) { pdf.text title, size: 18, character_spacing: -0.2 } pdf.fill_color grey - pdf.pad_top(30) { pdf.text @body, size: 10, character_spacing: -0.2, align: :justify } + pdf.pad_top(30) { pdf.text body, size: 10, character_spacing: -0.2, align: :justify } - if @signature.present? + if signature.present? pdf.pad_top(40) do - signature_file = if @signature.is_a?(ActiveStorage::Attached::One) - @signature.download + signature_file = if signature.is_a?(ActiveStorage::Attached::One) + signature.download else - @signature.read + signature.read end pdf.image StringIO.new(signature_file), fit: [max_signature_size , max_signature_size], position: :right end @@ -62,6 +70,6 @@ prawn_document(margin: [top_margin, right_margin, bottom_margin, left_margin], p pdf.repeat(:all) do pdf.move_cursor_to footer_height - 10 pdf.fill_color grey - pdf.text @footer, align: :center, size: 8 + pdf.text footer, align: :center, size: 8 end end diff --git a/app/views/admin/procedures/_informations.html.haml b/app/views/admin/procedures/_informations.html.haml index bcd59d4a8..0184c07bf 100644 --- a/app/views/admin/procedures/_informations.html.haml +++ b/app/views/admin/procedures/_informations.html.haml @@ -77,7 +77,7 @@ .row .col-md-6 %h4 Logo de la démarche - - if @procedure.logo? + - if @procedure.logo.attached? = image_tag @procedure.logo_url, { style: 'height: 40px; display: inline; margin-right: 6px;', id: 'preview_procedure_logo' } \- diff --git a/app/views/fields/attestation_template_field/_show.html.haml b/app/views/fields/attestation_template_field/_show.html.haml index e3f3f706b..4512b51bc 100644 --- a/app/views/fields/attestation_template_field/_show.html.haml +++ b/app/views/fields/attestation_template_field/_show.html.haml @@ -5,7 +5,7 @@ %strong Logo %p - - if field.data.logo? + - if field.data.logo.attached? = image_tag field.data.logo_url - else Aucun @@ -20,7 +20,7 @@ %strong Signature %p - - if field.data.signature? + - if field.data.signature.attached? = image_tag field.data.signature_url - else Aucun diff --git a/lib/tasks/2019_05_29_migrate_commentaire_pj.rake b/lib/tasks/2019_05_29_migrate_commentaire_pj.rake deleted file mode 100644 index 60cb5d668..000000000 --- a/lib/tasks/2019_05_29_migrate_commentaire_pj.rake +++ /dev/null @@ -1,38 +0,0 @@ -namespace :'2019_05_29_migrate_commentaire_pj' do - task run: :environment do - commentaires = Commentaire.where - .not(file: nil) - .left_joins(:piece_jointe_attachment) - .where('active_storage_attachments.id IS NULL') - .order(:created_at) - - limit = ENV['LIMIT'] - if limit - commentaires.limit!(limit.to_i) - end - - progress = ProgressReport.new(commentaires.count) - commentaires.find_each do |commentaire| - if commentaire.file.present? - dossier = Dossier.unscope(where: :hidden_at).find(commentaire.dossier_id) - uri = URI.parse(URI.escape(commentaire.file_url)) - response = Typhoeus.get(uri) - if response.success? - filename = commentaire.file.filename || commentaire.file_identifier - updated_at = commentaire.updated_at - dossier_updated_at = dossier.updated_at - commentaire.piece_jointe.attach( - io: StringIO.new(response.body), - filename: filename, - content_type: commentaire.file.content_type, - metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } - ) - commentaire.update_column(:updated_at, updated_at) - dossier.update_column(:updated_at, dossier_updated_at) - end - end - progress.inc - end - progress.finish - end -end diff --git a/spec/controllers/admin/attestation_templates_controller_spec.rb b/spec/controllers/admin/attestation_templates_controller_spec.rb index f5c6bffcd..11c9c094c 100644 --- a/spec/controllers/admin/attestation_templates_controller_spec.rb +++ b/spec/controllers/admin/attestation_templates_controller_spec.rb @@ -30,13 +30,13 @@ describe Admin::AttestationTemplatesController, type: :controller do context 'with an interlaced png' do let(:upload_params) { { logo: interlaced_logo } } - it { expect(assigns(:logo).read).to eq(uninterlaced_logo.read) } + it { expect(assigns(:attestation)[:logo].read).to eq(uninterlaced_logo.read) } end context 'if an attestation template does not exist on the procedure' do let(:attestation_template) { nil } it { expect(subject.status).to eq(200) } - it { expect(assigns).to include(upload_params.stringify_keys) } + it { expect(assigns(:attestation)).to include(upload_params) } end context 'if an attestation template exists on the procedure' do @@ -48,18 +48,18 @@ describe Admin::AttestationTemplatesController, type: :controller do end it { expect(subject.status).to eq(200) } - it { expect(assigns).to include(upload_params.stringify_keys) } - it { expect(assigns[:created_at]).to eq(Time.zone.now) } - it { expect(assigns(:logo).download).to eq(logo2.read) } - it { expect(assigns(:signature).download).to eq(signature2.read) } + it { expect(assigns(:attestation)).to include(upload_params) } + it { expect(assigns(:attestation)[:created_at]).to eq(Time.zone.now) } + it { expect(assigns(:attestation)[:logo].download).to eq(logo2.read) } + it { expect(assigns(:attestation)[:signature].download).to eq(signature2.read) } end context 'with empty logo' do it { expect(subject.status).to eq(200) } - it { expect(assigns).to include(upload_params.stringify_keys) } - it { expect(assigns[:created_at]).to eq(Time.zone.now) } - it { expect(assigns(:logo)).to eq(nil) } - it { expect(assigns(:signature)).to eq(nil) } + it { expect(assigns(:attestation)).to include(upload_params) } + it { expect(assigns(:attestation)[:created_at]).to eq(Time.zone.now) } + it { expect(assigns(:attestation)[:logo]).to eq(nil) } + it { expect(assigns(:attestation)[:signature]).to eq(nil) } end end end diff --git a/spec/models/attestation_template_spec.rb b/spec/models/attestation_template_spec.rb index 471b591d8..8cd9c6990 100644 --- a/spec/models/attestation_template_spec.rb +++ b/spec/models/attestation_template_spec.rb @@ -159,9 +159,11 @@ describe AttestationTemplate, type: :model do .update(value: 'libelle2') end - it { expect(view_args[:title]).to eq('title libelle1') } - it { expect(view_args[:body]).to eq('body libelle2') } - it { expect(attestation.title).to eq('title libelle1') } + it do + expect(view_args[:attestation][:title]).to eq('title libelle1') + expect(view_args[:attestation][:body]).to eq('body libelle2') + expect(attestation.title).to eq('title libelle1') + end end end end