Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refacto de la délégation des erreurs dans le UserRdvWizard #4782

Merged
merged 7 commits into from
Nov 19, 2024
5 changes: 3 additions & 2 deletions app/form_models/admin/rdv_wizard_form/step2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ class Admin::RdvWizardForm::Step2
def phone_number_present_for_motif_by_phone
return unless rdv.motif.phone?

users_to_notify = users.map(&:user_to_notify)
errors.add(:phone_number, :missing_for_phone_motif) if users_to_notify.none? { _1.phone_number.present? }
return if users.map(&:user_to_notify).any? { _1.phone_number.present? }

errors.add :base, "Aucun usager n’a de numéro de téléphone renseigné alors que le rendez-vous est téléphonique"
end

def can_receive_notification_for_motif_by_visio
Expand Down
4 changes: 2 additions & 2 deletions app/form_models/user_rdv_wizard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class Base
attr_accessor :rdv, :user

delegate :motif, :starts_at, :users, :service, to: :rdv
delegate :errors, to: :rdv

def initialize(user, attributes)
@user = user
Expand Down Expand Up @@ -81,6 +80,8 @@ def lieu
end

class Step1 < Base
delegate :errors, to: :user

validate :phone_number_present_for_motif_by_phone
validate do
if rdv.requires_ants_predemande_number?
Expand All @@ -89,7 +90,6 @@ class Step1 < Base
ants_pre_demande_number: @user_attributes[:ants_pre_demande_number],
ignore_benign_errors: @user_attributes[:ignore_benign_errors]
)
errors.merge!(@user)
end
end

Expand Down
3 changes: 0 additions & 3 deletions config/locales/models/rdv.fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ fr:
missing_mobile_phone_or_email: "Vous devez indiquer un numéro de téléphone mobile ou une adresse email pour que l'usager puisse recevoir le lien de visioconférence."
starts_at:
format: "L’horaire du RDV %{message}"
phone_number:
missing_for_phone_motif: "Aucun usager n’a de numéro de téléphone renseigné alors que le rendez-vous est téléphonique."
format: "%{message}"
duration_in_min:
must_be_positive: doit être supérieur à 0
lieu:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/models/user.fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ fr:
ants_pre_demande_number: &ants_pre_demande_number_errors
invalid_format: doit comporter 10 chiffres et lettres
unexpected_api_error: n'a pas pu être validé à cause d'une erreur inattendue. Merci de réessayer dans 30 secondes.
phone_number:
missing_for_phone_motif: "Le numéro de téléphone est obligatoire car le RDV aura lieu par téléphone"
format: "%{message}"
warnings:
models:
user:
Expand Down
55 changes: 54 additions & 1 deletion spec/controllers/admin/rdv_wizard_steps_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
end
end

describe "POST create" do
describe "POST step4" do
render_views

subject(:create_request) { post :create, params: params }
Expand Down Expand Up @@ -101,4 +101,57 @@
end
end
end

context "POST step2 avec motif téléphonique et l’usager a un numéro de téléphone" do
render_views
let!(:organisation) { create(:organisation) }
let!(:motif) { create(:motif, :by_phone, organisation:) }
let!(:agent) { create(:agent, service: motif.service, basic_role_in_organisations: [organisation]) }
let!(:user) { create(:user, organisations: [organisation], first_name: "François", last_name: "Fictif", phone_number: "0606060606") }

it "passe à l’étape suivante" do
post(
:create,
params: {
organisation_id: organisation.id,
step: 2,
rdv: {
duration_in_min: 30,
motif_id: motif.id,
starts_at: 2.days.from_now,
user_ids: [user.id],
},
}
)
expect(response.status).to eq(302)
redirect_params = Rack::Utils.parse_query(URI.parse(response.location).query)
expect(redirect_params["step"]).to eq("3")
expect(flash[:error]).to be_nil
end
end

context "POST step2 avec motif téléphonique mais l’usager n’a pas de numéro de téléphone" do
render_views
let!(:organisation) { create(:organisation) }
let!(:motif) { create(:motif, :by_phone, organisation:) }
let!(:agent) { create(:agent, service: motif.service, basic_role_in_organisations: [organisation]) }
let!(:user) { create(:user, organisations: [organisation], first_name: "François", last_name: "Fictif", phone_number: nil) }

it "affiche une erreur et ne passe pas à l’étape suivante" do
post(
:create,
params: {
organisation_id: organisation.id,
step: 2,
rdv: {
duration_in_min: 30,
motif_id: motif.id,
starts_at: 2.days.from_now,
user_ids: [user.id],
},
}
)
expect(response.body).to include("Aucun usager n’a de numéro de téléphone renseigné alors que le rendez-vous est téléphonique")
end
end
end
36 changes: 36 additions & 0 deletions spec/features/users/online_booking/default_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,42 @@
end
end

describe "validation du numéro de téléphone pour les motifs téléphoniques" do
let!(:territory) { create(:territory, departement_number: "24") }
let!(:organisation) { create(:organisation, territory:) }
let!(:user) { create(:user, phone_number: nil) }
let!(:motif) { create(:motif, :by_phone, organisation:) }
let!(:lieu) { create(:lieu, organisation:) }
let!(:plage_ouverture) do
create(:plage_ouverture, :weekdays, first_day: Date.parse("2024-11-04"), motifs: [motif], lieu: lieu, organisation:, start_time: Tod::TimeOfDay.new(8), end_time: Tod::TimeOfDay.new(12))
end

before { travel_to Date.parse("2024-11-03").in_time_zone + 8.hours }
before { login_as(user, scope: :user) }

context "numéro de tel renseigné et valide" do
it "passe à l’étape suivante" do
visit(new_users_rdv_wizard_step_path(step: 1, departement: "24", motif_id: motif.id, lieu_id: lieu.id, starts_at: Time.zone.parse("2024-11-05 08:00")))
expect(page).to have_content("Vos informations")
fill_in :user_phone_number, with: "0130303030"
click_button("Continuer")
expect(page).to have_content("Pour qui prenez-vous rendez-vous")
end
end

context "numéro de tel non renseigné" do
it "reste à l’étape 1 et montre une erreur" do
visit(new_users_rdv_wizard_step_path(step: 1, departement: "24", motif_id: motif.id, lieu_id: lieu.id, starts_at: Time.zone.parse("2024-11-05 08:00")))
expect(page).to have_content("Vos informations")
# page.execute_script(%{document.querySelector('#user_phone_number').removeAttribute("required")})
# cette ligne n’est nécessaire que si on passe le test en JS, ou pour reproduire le test dans votre navigateur
click_button("Continuer")
expect(page).not_to have_content("Pour qui prenez-vous rendez-vous")
expect(page).to have_content("Le numéro de téléphone est obligatoire car le RDV aura lieu par téléphone")
end
end
end

private

def execute_search
Expand Down
2 changes: 1 addition & 1 deletion spec/form_models/user_rdv_wizard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
it "return false with a rdv by_phone and user without phone" do
rdv_wizard = UserRdvWizard::Step1.new(user, attributes)
rdv_wizard.valid?
expect(rdv_wizard.errors.full_messages.join(", ")).to eq("Aucun usager n’a de numéro de téléphone renseigné alors que le rendez-vous est téléphonique.")
expect(rdv_wizard.errors.full_messages.join(", ")).to eq("Le numéro de téléphone est obligatoire car le RDV aura lieu par téléphone")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En effet, dans la grande majorité des cas, ce message plus simple est pertinent. Si vraiment on veut être dans le détail on peut compter le nombre de participants, mais c'est du bonus.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

en fait il y a une différence entre ce qu’on exige côté usagers et côté admin :

  • côté usagers : le user « principal », celui qui prend RDV doit avoir un numéro
  • côté admin : au moins un usager parmi le principal et les proches doit avoir un numéro

end
end
end
Expand Down
Loading