From afaee17c3a72044840e1b7a383225746c9435568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Ferrandis?= Date: Thu, 5 Dec 2024 12:03:59 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=C3=89dition=20en=20masse=20des=20motifs=20?= =?UTF-8?q?(#4854)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement checkboxes and button + blank batch edit page * Move motifs table to a partial * Implement form for name and service * Implement duration * Implement other fields * Ajout du bouton de retour * Don't display checkboxes and button in archived tab * Disable button until several motifs are checked * Add color attr * Display different values for name * Show different values for all attrs * Move JS to .js file * Use motif name as label * Implement check all * Fix bug in duration field * Add spec for batch select * Factorize batch update action * Gérer les erreurs mais pas super bien * Plein de belles choses * Plein d'autres belles choses * Put label on all columns * Add hints to instruction fields * Clarifier l'affichage disabled * Fine tune form inputs * Enfin améliorer le message de validation d'unicité de motif * Revert "Enfin améliorer le message de validation d'unicité de motif" This reverts commit abc939eb4847b3c6074ea6fa25ac5a86d2e9155f. * Afficher les valeurs triées par fréquence décroissante (plus logique) --- .../admin/territories/motifs_controller.rb | 45 +++++ app/javascript/application_agent.js | 1 + app/javascript/components/motifs_table.js | 29 ++++ .../motifs/_motifs_table.html.slim | 72 ++++++++ .../territories/motifs/batch_edit.html.slim | 115 ++++++++++++ .../admin/territories/motifs/index.html.slim | 68 +------- config/locales/views/admin_motifs.fr.yml | 1 + config/locales/views/configuration.yml | 2 + config/routes.rb | 4 + .../territory_admin_can_manage_motifs_spec.rb | 164 ++++++++++++++++++ 10 files changed, 440 insertions(+), 61 deletions(-) create mode 100644 app/javascript/components/motifs_table.js create mode 100644 app/views/admin/territories/motifs/_motifs_table.html.slim create mode 100644 app/views/admin/territories/motifs/batch_edit.html.slim diff --git a/app/controllers/admin/territories/motifs_controller.rb b/app/controllers/admin/territories/motifs_controller.rb index 31c21ebabe..40251079df 100644 --- a/app/controllers/admin/territories/motifs_controller.rb +++ b/app/controllers/admin/territories/motifs_controller.rb @@ -20,6 +20,51 @@ def index end end + def batch_edit + @motifs = Motif.where(id: params[:motif_ids]) + @motifs.each do |motif| + authorize(motif, :edit?, policy_class: Agent::MotifPolicy) + end + end + + UPDATABLE_ATTRS = %i[ + name + service_id + default_duration_in_min + color + restriction_for_rdv + instruction_for_rdv + custom_cancel_warning_message + ].freeze + + def batch_update # rubocop:disable Metrics/PerceivedComplexity + @motifs = Motif.where(id: params[:motif_ids]) + @motifs.each do |motif| + authorize(motif, :update?, policy_class: Agent::MotifPolicy) + end + + permitted_params = params.permit(*UPDATABLE_ATTRS).compact_blank + Motif.transaction do + @motifs.each do |motif| + motif.update(permitted_params) + end + + raise ActiveRecord::Rollback unless @motifs.all?(&:valid?) + end + + invalid_motifs = @motifs.select(&:invalid?) + + if invalid_motifs.none? + flash[:success] = "Motifs modifiés" + else + flash[:error] = invalid_motifs.map do |invalid_motif| + "Motif #{invalid_motif.id} : #{invalid_motif.errors.full_messages.join(', ')}" + end.join("
") + end + + redirect_to batch_edit_admin_territory_motifs_path(motif_ids: params[:motif_ids]) + end + def new skip_authorization @motif = Motif.new diff --git a/app/javascript/application_agent.js b/app/javascript/application_agent.js index b400d8a887..0775badb1e 100644 --- a/app/javascript/application_agent.js +++ b/app/javascript/application_agent.js @@ -27,6 +27,7 @@ import { DestroyButton } from './components/destroy-button' import { Tooltips } from './components/tooltips' import { PlageOuverture } from './components/plage_ouverture.js' import {CheckAll, UnCheckAll} from './components/check-all' +import './components/motifs_table' import './components/calendar' import './components/browser-detection' import './components/clear-field-on-focus.js' diff --git a/app/javascript/components/motifs_table.js b/app/javascript/components/motifs_table.js new file mode 100644 index 0000000000..0d9f165d73 --- /dev/null +++ b/app/javascript/components/motifs_table.js @@ -0,0 +1,29 @@ +document.addEventListener("turbolinks:load", function () { + batchEditCheckboxes().forEach(e => e.addEventListener("change", refreshButtonState)) + + const trigger = triggerCheckbox() + if(trigger) { + trigger.addEventListener("change", event => { + batchEditCheckboxes().forEach(input => input.checked = trigger.checked) + refreshButtonState() + }) + } +}); + +function refreshButtonState() { + const disabled = Array.from(batchEditCheckboxes()).filter(c => c.checked).length < 2; + batchEditButton().disabled = disabled; + batchEditButton().classList.toggle("btn-outline-primary", !disabled) +} + +function batchEditCheckboxes() { + return document.querySelectorAll(".js-batch-edit-checkbox") +} + +function batchEditButton() { + return document.querySelector(".js-batch-edit-button") +} + +function triggerCheckbox() { + return document.querySelector(".js-trigger-checkbox") +} diff --git a/app/views/admin/territories/motifs/_motifs_table.html.slim b/app/views/admin/territories/motifs/_motifs_table.html.slim new file mode 100644 index 0000000000..7ed3622efb --- /dev/null +++ b/app/views/admin/territories/motifs/_motifs_table.html.slim @@ -0,0 +1,72 @@ +/ # locals: (motifs:, display_actions: false, display_checkboxes: false) + +table.table.table-centered.table-bordered.table-sm.mb-0[class=(display_checkboxes ? "table-hover" : "")] + thead.thead-light + tr + - if display_checkboxes + th[scope="col"] + input.js-trigger-checkbox[type="checkbox"] + + th[scope="col"] + / Color columns + th[scope="col"] + | Nom + th[scope="col"] + | Service + th[scope="col"] + | Organisation + th[scope="col"] + | Type + th[scope="col"] + | Durée + + - if display_actions + th[scope="col"] + tbody + - motifs.each do |motif| + - motif_policy = Agent::MotifPolicy.new(current_agent, motif) + tr + - if display_checkboxes + td + input.js-batch-edit-checkbox[id="edit_checkbox#{motif.id}" type="checkbox" form="batch_edit_form" name="motif_ids[]" value=motif.id] + td + span.badge.badge-pill style="background: #{motif.color};"   + td + label.mb-0[for="edit_checkbox#{motif.id}"] + = motif.name + = motif_badges(motif) + td + label.mb-0[for="edit_checkbox#{motif.id}"] + = motif.service.short_name + td + - if motif_policy.show? && display_actions + = link_to(motif.organisation.name, admin_organisation_motifs_path(organisation_id: motif.organisation.id)) + - else + = motif.organisation.name + td + label.mb-0[for="edit_checkbox#{motif.id}"] + = Motif.human_attribute_value(:location_type, motif.location_type) + td + label.mb-0[for="edit_checkbox#{motif.id}"] + = "#{motif.default_duration_in_min}" + - if display_actions + td.p-0 + .d-flex + - if @current_tab != :archived + = link_to edit_admin_organisation_motif_path(motif.organisation, motif), class: "btn btn-link", title: t("admin.motifs.actions.edit"), target: :_blank do + i.fa.fa-edit + - if motif.archived? + - if motif_policy.unarchive? + = link_to unarchive_admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :post, title: t("admin.motifs.actions.unarchive") do + i.fa.fa-arrow-up-from-bracket + - if motif_policy.destroy? + - if motif.destroyable? + = link_to admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :delete, title: t("admin.motifs.actions.destroy"), data: { confirm: t("admin.motifs.actions.destroy_confirm") } do + i.fa.fa-trash-alt + - else + button.btn.btn-link [disabled="disabled" title=t("admin.motifs.actions.not_destroyable_explanation")] + i.fa.fa-trash-alt + - else + - if motif_policy.archive? + = link_to archive_admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :post, title: t("admin.motifs.actions.archive"), data: { confirm: t("admin.motifs.actions.archive_confirm") } do + i.fa.fa-box-archive diff --git a/app/views/admin/territories/motifs/batch_edit.html.slim b/app/views/admin/territories/motifs/batch_edit.html.slim new file mode 100644 index 0000000000..566286e7d2 --- /dev/null +++ b/app/views/admin/territories/motifs/batch_edit.html.slim @@ -0,0 +1,115 @@ += territory_navigation(t("admin.territories.motifs.batch_edit.title"), [link_to(t("admin.territories.motifs.index.title"), admin_territory_motifs_path)]) + +h1 Modification en masse de #{@motifs.size} motifs + += render partial: "admin/territories/motifs/motifs_table", locals: { motifs: @motifs } + +.card.mt-4.mb-4.p-3 + = form_with(url: batch_update_admin_territory_motifs_path, method: :post, html: { id: "name_form" }) do |f| + - @motifs.each do |motif| + = f.hidden_field "motif_ids[]", value: motif.id + + .form-group + - unique_values = @motifs.map(&:name).tally + = f.label "Nom du motif", for: :name, class: "text-bold" + - if unique_values.size > 1 + p Ces motifs ont des noms différents : + ul.mt-0 + - unique_values.sort_by(&:last).reverse.each do |value, nb_of_occurrences| + li #{value} (#{nb_of_occurrences}) + - else + small.form-text.text-muted.mb-2 Ces #{@motifs.size} motifs ont le même nom + .input-group + = f.text_field :name, value: (unique_values.size > 1 ? "" : unique_values.keys.first), class: "form-control", required: true + .input-group-append + = f.submit "Appliquer à ces #{@motifs.size} motifs", class: "btn btn-primary" + +.card.p-3.mb-4 + = form_with(url: batch_update_admin_territory_motifs_path, method: :post, html: { id: "service_form" }) do |f| + - @motifs.each do |motif| + = f.hidden_field "motif_ids[]", value: motif.id + + .form-group + - unique_values = @motifs.map(&:service).tally + = f.label "Service associé", for: :service_id, class: "text-bold" + - if unique_values.size > 1 + p.mb-1 Ces motifs sont associés à des services différents : + ul.mt-0 + - unique_values.sort_by(&:last).reverse.each do |value, nb_of_occurrences| + li #{value.name} (#{nb_of_occurrences}) + - else + small.form-text.text-muted.mb-2 Ces #{@motifs.size} motifs sont associés au même service + .input-group + = f.select :service_id, current_territory.services.reject(&:secretariat?).map { [_1.name, _1.id] }, { selected: (unique_values.size > 1 ? "" : unique_values.keys.first.id), include_blank: true, required: true }, { class: "custom-select" } + .input-group-append + = f.submit "Appliquer à ces #{@motifs.size} motifs", class: "btn btn-primary" + +.card.p-3.mb-4 + = form_with(url: batch_update_admin_territory_motifs_path, method: :post, html: { id: "duration_form" }) do |f| + - @motifs.each do |motif| + = f.hidden_field "motif_ids[]", value: motif.id + + .form-group + - unique_values = @motifs.map(&:default_duration_in_min).tally + = f.label "Durée par défaut en minutes", for: :default_duration_in_min, class: "text-bold" + - if unique_values.size > 1 + p.mb-1 Ces motifs on des durées par défaut différentes : + ul.mt-0 + - unique_values.sort_by(&:last).reverse.each do |value, nb_of_occurrences| + li #{value} (#{nb_of_occurrences}) + - else + small.form-text.text-muted.mb-2 Ces #{@motifs.size} motifs ont la même durée par défaut + .input-group + = f.number_field :default_duration_in_min, value: (unique_values.keys.size > 1 ? nil : unique_values.keys.first), class: "form-control", required: true + .input-group-append + = f.submit "Appliquer à ces #{@motifs.size} motifs", class: "btn btn-primary" + +.card.p-3.mb-4 + = form_with(url: batch_update_admin_territory_motifs_path, method: :post, html: { id: "color_form" }) do |f| + - @motifs.each do |motif| + = f.hidden_field "motif_ids[]", value: motif.id + + .form-group + - current_colors = @motifs.map(&:color).uniq + / Cette datalist permet au color picker natif de suggérer les couleurs existantes dans sa palette ! + datalist#current_colors + - current_colors.each do |color| + option[value=color] + = f.label "Couleur associée", for: :color, class: "text-bold" + .input-group + = f.color_field :color, value: current_colors.first, class: "form-control", list: "current_colors", required: true + .input-group-append + = f.submit "Appliquer à ces #{@motifs.size} motifs", class: "btn btn-primary" + +- %i[restriction_for_rdv instruction_for_rdv custom_cancel_warning_message].each do |instruction_attr_name| + .card.p-3.mb-4 + = form_with(url: batch_update_admin_territory_motifs_path, method: :post, html: { id: "#{instruction_attr_name}_form" }) do |f| + - @motifs.each do |motif| + = f.hidden_field "motif_ids[]", value: motif.id + + .form-group + - unique_values = @motifs.map(&instruction_attr_name).tally + = f.label t("activerecord.attributes.motif.#{instruction_attr_name}"), for: instruction_attr_name, class: "text-bold" + - if unique_values.size > 1 + p.mb-1 Ces motifs ont des textes différents : + ul.mt-0 + - unique_values.sort_by(&:last).reverse.each do |value, nb_of_occurrences| + li + - if value.present? + = "#{value}" + - else + em pas de texte + = " (#{nb_of_occurrences})" + - else + small.form-text.text-muted.mb-2 Ces #{@motifs.size} motifs ont le même texte + = f.text_area instruction_attr_name, value: (unique_values.size > 1 ? nil : unique_values.keys.first), class: "form-control" + .mt-1 + = link_to image_path("motif_form/#{instruction_attr_name}.png"), target: "_blank" do + span> Voir un exemple + i.fa.fa-external-link-alt> + + .float-right + = f.submit "Appliquer à ces #{@motifs.size} motifs", class: "btn btn-primary" + +.rdv-text-align-center.mb-4 + = link_to("Retour", admin_territory_motifs_path, class: "btn btn-outline-primary") diff --git a/app/views/admin/territories/motifs/index.html.slim b/app/views/admin/territories/motifs/index.html.slim index 88357508bd..5df111f1e4 100644 --- a/app/views/admin/territories/motifs/index.html.slim +++ b/app/views/admin/territories/motifs/index.html.slim @@ -47,7 +47,12 @@ .col-12 .float-right - a.btn.btn-primary.mb-2[href=new_admin_territory_motif_path] = t("admin.motifs.actions.new") + .d-flex.flex-gap-1em + - if @current_tab != :archived + form#batch_edit_form[action=batch_edit_admin_territory_motifs_path method="get"] + button.btn.mb-2.js-batch-edit-button[role="submit" disabled=true] = t("admin.motifs.actions.batch_edit") + + a.btn.btn-primary.mb-2[href=new_admin_territory_motif_path] = t("admin.motifs.actions.new") ul.nav.nav-tabs - [[:active, "Actifs", @filtered_motifs.active.count], [:archived, "Archivés", @filtered_motifs.archived.count]].each do |value, label, count| @@ -73,66 +78,7 @@ - if @motifs_page.any? = paginate @motifs_page, theme: "twitter-bootstrap-4" - - table.table.table-centered.table-bordered.table-hover.table-sm.mb-0 - thead.thead-light - = form_with url: admin_territory_motifs_path(current_territory), method: :get do |form| - tr - th[scope="col"] - th[scope="col"] - | Nom - th[scope="col"] - | Service - th[scope="col"] - | Organisation - th[scope="col"] - | Type - th[scope="col"] - | Durée - th[scope="col"] - | Action - tbody - - @motifs_page.each do |motif| - - motif_policy = Agent::MotifPolicy.new(current_agent, motif) - tr - td - span.badge.badge-pill style="background: #{motif.color};"   - td - = motif.name - = motif_badges(motif) - td - = motif.service.short_name - td - - if motif_policy.show? - = link_to(motif.organisation.name, admin_organisation_motifs_path(organisation_id: motif.organisation.id)) - - else - = motif.organisation.name - td - = Motif.human_attribute_value(:location_type, motif.location_type) - td - = "#{motif.default_duration_in_min}" - td.p-0 - .d-flex - - if @current_tab != :archived - = link_to edit_admin_organisation_motif_path(motif.organisation, motif), class: "btn btn-link", title: t("admin.motifs.actions.edit"), target: :_blank do - i.fa.fa-edit - - - if motif.archived? - - if motif_policy.unarchive? - = link_to unarchive_admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :post, title: t("admin.motifs.actions.unarchive") do - i.fa.fa-arrow-up-from-bracket - - if motif_policy.destroy? - - if motif.destroyable? - = link_to admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :delete, title: t("admin.motifs.actions.destroy"), data: { confirm: t("admin.motifs.actions.destroy_confirm") } do - i.fa.fa-trash-alt - - else - button.btn.btn-link [disabled="disabled" title=t("admin.motifs.actions.not_destroyable_explanation")] - i.fa.fa-trash-alt - - else - - if motif_policy.archive? - = link_to archive_admin_territory_motif_path(current_territory, motif), class: "btn btn-link", method: :post, title: t("admin.motifs.actions.archive"), data: { confirm: t("admin.motifs.actions.archive_confirm") } do - i.fa.fa-box-archive - + = render partial: "admin/territories/motifs/motifs_table", locals: { motifs: @motifs_page, display_actions: true, display_checkboxes: @current_tab != :archived } .mt-2 = paginate @motifs_page, theme: "twitter-bootstrap-4" - else diff --git a/config/locales/views/admin_motifs.fr.yml b/config/locales/views/admin_motifs.fr.yml index 461197d777..c33c90651e 100644 --- a/config/locales/views/admin_motifs.fr.yml +++ b/config/locales/views/admin_motifs.fr.yml @@ -11,6 +11,7 @@ fr: archiving_explanation: "Nouveau : vous pouvez désormais archiver les motifs que vous ne souhaitez plus utiliser. Un motif archivé ne sera plus proposé lors de la prise de RDV. Il peut être réactivé à tout moment ou supprimé définitivement si aucun RDV n'est rattaché." actions: new: Créer un motif + batch_edit: Modifier les motifs edit: Modifier duplicate: Dupliquer archive: Archiver diff --git a/config/locales/views/configuration.yml b/config/locales/views/configuration.yml index 9d62eb2b50..de87711a62 100644 --- a/config/locales/views/configuration.yml +++ b/config/locales/views/configuration.yml @@ -111,6 +111,8 @@ fr: motifs: index: title: Motifs + batch_edit: + title: Modifier les motifs motif_fields: edit: title: Champs des motifs diff --git a/config/routes.rb b/config/routes.rb index c7b2b97af2..2a95aea3bc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -156,6 +156,10 @@ end resources :teams, except: :show resources :motifs, only: %i[index new create destroy] do + collection do + get :batch_edit + post :batch_update + end member do post :archive post :unarchive diff --git a/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb b/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb index f25053f923..e3eb0cdb22 100644 --- a/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb +++ b/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb @@ -123,6 +123,170 @@ end end + describe "batch edit" do + let!(:organisation_a) { create(:organisation, territory: territory) } + let!(:organisation_b) { create(:organisation, territory: territory) } + let!(:organisation_c) { create(:organisation, territory: territory) } + + before do + agent.roles.create!(organisation: organisation_a, access_level: AgentRole::ACCESS_LEVEL_ADMIN) + agent.roles.create!(organisation: organisation_b, access_level: AgentRole::ACCESS_LEVEL_ADMIN) + agent.roles.create!(organisation: organisation_c, access_level: AgentRole::ACCESS_LEVEL_ADMIN) + end + + describe "motif selection" do + let!(:motif_a) { create(:motif, organisation: organisation_a) } + let!(:motif_b) { create(:motif, organisation: organisation_b) } + let!(:motif_c) { create(:motif, organisation: organisation_c) } + + it "works manually", js: true do + visit admin_territory_motifs_path(territory) + find(%([type="checkbox"][value="#{motif_a.id}"])).check + find(%([type="checkbox"][value="#{motif_c.id}"])).check + click_on "Modifier les motifs" + + expect(page).to have_content("Modifier les motifs") + expect(page).to have_content(motif_a.name) + expect(page).not_to have_content(motif_b.name) + expect(page).to have_content(motif_c.name) + + # On vérifie qu'on arrive sur la page de modification en masse ; + # cette page est testée dans les exemples ci-dessous. + expect(page).to have_current_path(batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_c.id])) + end + + it "works with batch check", js: true do + visit admin_territory_motifs_path(territory) + find("input.js-trigger-checkbox").check + click_on "Modifier les motifs" + + expect(page).to have_content("Modifier les motifs") + expect(page).to have_content(motif_a.name) + expect(page).to have_content(motif_b.name) + expect(page).to have_content(motif_c.name) + + # On vérifie qu'on arrive sur la page de modification en masse ; + # cette page est testée dans les exemples ci-dessous. + expect(page).to have_current_path(batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id, motif_c.id])) + end + end + + describe "updating name" do + let!(:motif_a) { create(:motif, organisation: organisation_a, name: "Nom avec faute A") } + let!(:motif_b) { create(:motif, organisation: organisation_b, name: "Nom avec faute B") } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#name_form") do + fill_in "Nom du motif", with: "Nom corrigé" + click_on "Appliquer" + expect(motif_a.reload.name).to eq("Nom corrigé") + expect(motif_b.reload.name).to eq("Nom corrigé") + end + end + end + + describe "updating service" do + let!(:service_pmi) { create(:service, :pmi).tap { territory.services << _1 } } + let!(:service_social) { create(:service, :social).tap { territory.services << _1 } } + let!(:motif_a) { create(:motif, organisation: organisation_a, service: service_pmi) } + let!(:motif_b) { create(:motif, organisation: organisation_b, service: service_pmi) } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#service_form") do + select service_social.name, from: "Service" + click_on "Appliquer" + expect(motif_a.reload.service).to eq(service_social) + expect(motif_b.reload.service).to eq(service_social) + end + end + end + + describe "updating duration" do + let!(:motif_a) { create(:motif, organisation: organisation_a, default_duration_in_min: 30) } + let!(:motif_b) { create(:motif, organisation: organisation_b, default_duration_in_min: 60) } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#duration_form") do + fill_in "Durée par défaut en minutes", with: "45" + click_on "Appliquer" + expect(motif_a.reload.default_duration_in_min).to eq(45) + expect(motif_b.reload.default_duration_in_min).to eq(45) + end + end + end + + describe "updating color" do + let!(:motif_a) { create(:motif, organisation: organisation_a, color: "#FFFFFF") } + let!(:motif_b) { create(:motif, organisation: organisation_b, color: "#EEEEEE") } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + # Voir https://youtu.be/B0hpCzggOLM, si vous aviez la ref, bravo ! + within("#color_form") do + fill_in "Couleur", with: "#000000" + click_on "Appliquer" + expect(motif_a.reload.color).to eq("#000000") + expect(motif_b.reload.color).to eq("#000000") + end + end + end + + describe "updating restriction_for_rdv" do + let!(:motif_a) { create(:motif, organisation: organisation_a, restriction_for_rdv: "toto") } + let!(:motif_b) { create(:motif, organisation: organisation_b, restriction_for_rdv: "tata") } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#restriction_for_rdv_form") do + fill_in "Instructions à accepter avant la prise du rendez-vous", with: "titi" + click_on "Appliquer" + expect(motif_a.reload.restriction_for_rdv).to eq("titi") + expect(motif_b.reload.restriction_for_rdv).to eq("titi") + end + end + end + + describe "updating instruction_for_rdv" do + let!(:motif_a) { create(:motif, organisation: organisation_a, instruction_for_rdv: "toto") } + let!(:motif_b) { create(:motif, organisation: organisation_b, instruction_for_rdv: "tata") } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#instruction_for_rdv_form") do + fill_in "Indications affichées après la confirmation du rendez-vous", with: "titi" + click_on "Appliquer" + expect(motif_a.reload.instruction_for_rdv).to eq("titi") + expect(motif_b.reload.instruction_for_rdv).to eq("titi") + end + end + end + + describe "updating custom_cancel_warning_message" do + let!(:motif_a) { create(:motif, organisation: organisation_a, custom_cancel_warning_message: "toto") } + let!(:motif_b) { create(:motif, organisation: organisation_b, custom_cancel_warning_message: "tata") } + + it "works" do + visit batch_edit_admin_territory_motifs_path(territory_id: territory.id, motif_ids: [motif_a.id, motif_b.id]) + + within("#custom_cancel_warning_message_form") do + fill_in "Message d’avertissement en cas d’annulation", with: "titi" + click_on "Appliquer" + expect(motif_a.reload.custom_cancel_warning_message).to eq("titi") + expect(motif_b.reload.custom_cancel_warning_message).to eq("titi") + end + end + end + end + describe "archiving" do let!(:organisation) { create(:organisation, territory: territory) } let!(:motif) { create(:motif, organisation: organisation) } From 3d4fdd05de6cbaa2cfb1b07b050be5646c420a24 Mon Sep 17 00:00:00 2001 From: Victor Mours Date: Thu, 5 Dec 2024 15:50:22 +0100 Subject: [PATCH 2/3] =?UTF-8?q?Suppression=20de=20la=20page=20de=20pr?= =?UTF-8?q?=C3=A9sentation=20de=20RDV=20Mairie=20legacy=20(#4872)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove legacy RDV Mairie page * remove legacy spec --- app/controllers/static_pages_controller.rb | 6 +- app/models/domain.rb | 2 +- .../presentation_for_mairie.html.slim | 57 ------------------- .../accessibility/public_pages_spec.rb | 7 --- 4 files changed, 6 insertions(+), 66 deletions(-) delete mode 100644 app/views/static_pages/presentation_for_mairie.html.slim diff --git a/app/controllers/static_pages_controller.rb b/app/controllers/static_pages_controller.rb index 721296b478..93c2190f53 100644 --- a/app/controllers/static_pages_controller.rb +++ b/app/controllers/static_pages_controller.rb @@ -25,7 +25,11 @@ def contact def domaines; end def presentation_for_agents - render current_domain.presentation_for_agents_template_name, layout: "application_base" + if current_domain == Domain::RDV_MAIRIE + redirect_to root_path # La landing page pour RDV Service Public s'adresse aux agents + else + render current_domain.presentation_for_agents_template_name, layout: "application_base" + end end def microsoft_domain_verification diff --git a/app/models/domain.rb b/app/models/domain.rb index faa52b15c3..4de79aebc7 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -67,7 +67,7 @@ class Domain public_logo_path: "/logo_rdv_service_public.png", dark_logo_path: "logos/logo_sombre_rdv_service_public.svg", name: "RDV Service Public", - presentation_for_agents_template_name: "presentation_for_mairie", + presentation_for_agents_template_name: nil, # C'est la homepage qui joue ce rôle pour ce domaine address_selection_template_name: nil, search_banner_template_name: "search/banners/rdv_mairie", online_reservation_with_public_link: true, diff --git a/app/views/static_pages/presentation_for_mairie.html.slim b/app/views/static_pages/presentation_for_mairie.html.slim deleted file mode 100644 index 805f1c4728..0000000000 --- a/app/views/static_pages/presentation_for_mairie.html.slim +++ /dev/null @@ -1,57 +0,0 @@ -section.py-5.rdv-background-color-flat-blue-ecume - .fr-container - .row - .col-md-9.m-auto - h1.rdv-color-white.mb-3 Faciliter la gestion des rendez-vous - .mt-4= link_to "Connectez-vous", new_agent_session_path, class: "btn text-white btn-outline-white" - p.mt-4.rdv-color-white - | RDV Mairie est un outil de prise de rendez-vous en ligne, - simplifiant votre organisation et rappelant aux usagers leurs rendez-vous. -section.py-5 - .fr-container - .rdv-text-align-center.m-auto - h2.mb-4 RDV Mairie offre des fonctionnalités adaptées à vos besoins : - .divider - .row - .col-md-4 - .w-50.p-3.m-auto - = image_tag "welcome/illu-prise-rdv.svg", alt: "" - .fr-text--xl Définir vos plages de disponibilités - p - | Informez vos usagers et vos collègues - des créneaux disponibles. - - .col-md-4 - .w-50.p-3.m-auto - = image_tag "welcome/illu-rappel-rdv.svg", alt: "" - .fr-text--xl Envoyer une notification de rappel - p - | Vos usagers recevront un SMS et/ou un - e-mail de rappel de RDV. Ils pourront - également modifier ou annuler le RDV. - - .col-md-4 - .w-50.p-3.m-auto - = image_tag "welcome/illu-gestion.svg", alt: "" - .fr-text--xl Importer vos RDVs sur votre agenda - p - | Synchronisez RDV Mairie et votre - agenda du quotidien. - -section.rdv-background-color-alt-green-menthe.py-5 - .fr-container.fr-grid-row - .fr-col-12.fr-col-md-8.mx-auto - h2.mb-3 Qui sommes-nous ? - p - | Les produits RDV Solidarités, RDV Aide Numérique et RDV Mairie sont portés par l’une des équipes de - =< link_to("l’Incubateur des Territoires qui", "https://incubateur.anct.gouv.fr/", target: "_blank") - | suit une approche - =< link_to("beta.gouv.fr", "https://beta.gouv.fr", target: "_blank") - | . L’équipe développe une solution numérique de gestion des RDVs qui répond au mieux aux - besoins des utilisateurs. - p - | Initialement déployée auprès des services médico-sociaux, la solution numérique s’est élargie auprès des conseillers - numériques France Service. Aujourd’hui, le produit est en phase de généralisation auprès des mairies. - Découvrez - =< link_to("notre histoire", "https://beta.gouv.fr/startups/lapins.html", target: "_blank") - | . diff --git a/spec/features/accessibility/public_pages_spec.rb b/spec/features/accessibility/public_pages_spec.rb index d51773e71b..d6bfb31c0e 100644 --- a/spec/features/accessibility/public_pages_spec.rb +++ b/spec/features/accessibility/public_pages_spec.rb @@ -27,13 +27,6 @@ expect(page).to be_axe_clean end - it "presentation page for RDV Mairie is accessible" do - visit "http://www.rdv-mairie-test.localhost/presentation_agent" - expect(page).to have_current_path("/presentation_agent") - # TODO: make it accessible - # expect(page).to be_axe_clean - end - it "mds_path page is accessible" do expect_page_to_be_axe_clean(mds_path) end From 6aba2c4c94efedd7504e07887eb6f17a83e66565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Ferrandis?= Date: Thu, 5 Dec 2024 15:55:13 +0100 Subject: [PATCH 3/3] =?UTF-8?q?Am=C3=A9liorer=20le=20message=20de=20valida?= =?UTF-8?q?tion=20d'unicit=C3=A9=20de=20motif=20(#4869)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Améliorer le message de validation d'unicité de motif * Fix specs? * Clarifier un petit peu la spec --- app/form_models/admin/create_motifs.rb | 6 +----- app/models/motif.rb | 15 ++++++++++++--- .../features/agents/agent_can_crud_motifs_spec.rb | 2 +- .../territory_admin_can_manage_motifs_spec.rb | 4 ++-- spec/form_models/admin/create_motifs_spec.rb | 4 ++-- spec/models/motif_spec.rb | 8 ++++---- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/app/form_models/admin/create_motifs.rb b/app/form_models/admin/create_motifs.rb index 68271d4a11..b1a84fe2a3 100644 --- a/app/form_models/admin/create_motifs.rb +++ b/app/form_models/admin/create_motifs.rb @@ -37,11 +37,7 @@ def organisations_are_present def motifs_are_valid motifs.select(&:invalid?).each do |motif| motif.errors.each do |motif_error| - if motif_error.attribute == :name && motif_error.type == :taken - errors.add(:base, "Un motif du même nom, même service et même type existe déjà dans #{motif.organisation.name}") - else - errors.import(motif_error) unless errors.added?(motif_error.attribute, motif_error.type) # deduplicate errors - end + errors.import(motif_error) unless errors.added?(motif_error.attribute, motif_error.type, **motif_error.options) # deduplicate errors end end end diff --git a/app/models/motif.rb b/app/models/motif.rb index a64462951b..d6d3b68b53 100644 --- a/app/models/motif.rb +++ b/app/models/motif.rb @@ -58,9 +58,7 @@ def lieux # Validation validates :visibility_type, inclusion: { in: VISIBILITY_TYPES } validates :sectorisation_level, inclusion: { in: SECTORISATION_TYPES } - validates :name, presence: true, uniqueness: { scope: %i[organisation location_type service], - conditions: -> { where(deleted_at: nil) }, } - + validates :name, presence: true validates :color, :default_duration_in_min, :min_public_booking_delay, :max_public_booking_delay, presence: true validates :min_public_booking_delay, numericality: { greater_than_or_equal_to: 30.minutes, less_than_or_equal_to: 1.year.minutes } validates :max_public_booking_delay, numericality: { greater_than_or_equal_to: 30.minutes, less_than_or_equal_to: 1.year.minutes } @@ -70,6 +68,7 @@ def lieux validate :not_at_home_if_collectif validate :cant_change_once_rdvs_exist validate :cant_be_for_secretariat_and_follow_up + validate :unique_in_org # Scopes scope :active, lambda { |active = true| @@ -293,4 +292,14 @@ def cant_be_for_secretariat_and_follow_up errors.add(:for_secretariat, :cant_be_enabled_if_follow_up) end end + + def unique_in_org + if Motif.active.where.not(id: id).exists?(organisation_id:, name:, service_id:, location_type:) + errors.add( + :base, + :duplicate_detected, + message: %(Il existe déjà dans #{organisation.name} un motif #{human_attribute_value(:location_type)} nommé "#{name}" pour le service #{service.name}) + ) + end + end end diff --git a/spec/features/agents/agent_can_crud_motifs_spec.rb b/spec/features/agents/agent_can_crud_motifs_spec.rb index d24dfa122d..9482642c4a 100644 --- a/spec/features/agents/agent_can_crud_motifs_spec.rb +++ b/spec/features/agents/agent_can_crud_motifs_spec.rb @@ -156,7 +156,7 @@ it "explains why the motif can't be un-archived" do visit admin_organisation_motif_path(motif.organisation, motif) expect { click_on "Réactiver" }.not_to change { motif.reload.archived? }.from(true) - expect(page).to have_content("Nom est déjà utilisé : un motif du même type et du même service porte déjà ce nom dans cette organisation.") + expect(page).to have_content("Il existe déjà dans #{motif.organisation.name} un motif") end end end diff --git a/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb b/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb index e3eb0cdb22..e65d3cecec 100644 --- a/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb +++ b/spec/features/territory_admins/territory_admin_can_manage_motifs_spec.rb @@ -118,7 +118,7 @@ fill_in "Couleur associée", with: "#123456" expect { click_on "Créer le motif" }.not_to change(Motif, :count) - expect(page).to have_content("Un motif du même nom, même service et même type existe déjà dans Arques") + expect(page).to have_content(%(Il existe déjà dans Arques un motif Sur place nommé "Consultation prénatale" pour le service PMI)) end end end @@ -328,7 +328,7 @@ it "explains why the motif can't be un-archived" do visit admin_territory_motifs_path(territory, current_tab: "archived") expect { click_on "Réactiver" }.not_to change { motif.reload.archived? }.from(true) - expect(page).to have_content("Nom est déjà utilisé : un motif du même type et du même service porte déjà ce nom dans cette organisation.") + expect(page).to have_content(%(Il existe déjà dans #{organisation.name} un motif)) end end end diff --git a/spec/form_models/admin/create_motifs_spec.rb b/spec/form_models/admin/create_motifs_spec.rb index f862ebff15..3d37bd9d1f 100644 --- a/spec/form_models/admin/create_motifs_spec.rb +++ b/spec/form_models/admin/create_motifs_spec.rb @@ -39,8 +39,8 @@ expect(form).to be_invalid expected_errors = [ "Default duration in min doit être rempli(e)", - "Un motif du même nom, même service et même type existe déjà dans Ma première orga", - "Un motif du même nom, même service et même type existe déjà dans Ma seconde orga", + "Il existe déjà dans Ma première orga un motif Sur place nommé \"#{valid_motif.name}\" pour le service #{valid_motif.service.name}", + "Il existe déjà dans Ma seconde orga un motif Sur place nommé \"#{valid_motif.name}\" pour le service #{valid_motif.service.name}", ] expect(form.errors.to_a).to match_array(expected_errors) expect(form.save).to be_falsey diff --git a/spec/models/motif_spec.rb b/spec/models/motif_spec.rb index 9862078117..6a627b2c06 100644 --- a/spec/models/motif_spec.rb +++ b/spec/models/motif_spec.rb @@ -1,7 +1,7 @@ RSpec.describe Motif, type: :model do let(:secretariat) { create(:service, :secretariat) } let(:motif) { create(:motif, organisation: organisation) } - let!(:organisation) { create(:organisation) } + let!(:organisation) { create(:organisation, name: "Mon orga") } it "have a valid factory" do expect(build(:motif)).to be_valid @@ -20,13 +20,13 @@ describe "uniqueness" do subject { motif.dup } - let(:service) { build(:service) } + let(:service) { build(:service, name: "PMI") } let(:motif) { create(:motif, name: "name", location_type: :home, service: service, organisation: organisation) } it do expect(subject).not_to be_valid - expect(subject.errors.details).to eq({ name: [{ error: :taken, value: "name" }] }) - expect(subject.errors.full_messages.to_sentence).to eq "Nom est déjà utilisé : un motif du même type et du même service porte déjà ce nom dans cette organisation." + expect(subject.errors.details).to eq({ base: [{ error: :duplicate_detected }] }) + expect(subject.errors.full_messages.to_sentence).to eq(%(Il existe déjà dans Mon orga un motif À domicile nommé "name" pour le service PMI)) end end