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.

Sign up for GitHub

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

ETQ usager je vois les infos de contact de mon groupe instructeur avant dépôt du dossier, une fois le(s) champ(s) de routage rempli(s) #10430

Merged
merged 5 commits into from
Jul 23, 2024
+51 −30
Merged

ETQ usager je vois les infos de contact de mon groupe instructeur avant dépôt du dossier, une fois le(s) champ(s) de routage rempli(s) #10430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f860745
feat(routing): route when autosave of champ used by routing rules
E-L-T May 17, 2024
f30057e
feat(routing): replace contact information with turbo_stream
E-L-T May 17, 2024
78d1572
test(routing): test routing on autosave
E-L-T Jun 7, 2024
e413c08
chore(routing): update turbo stream only if champ used by routing rul…
E-L-T Jun 14, 2024
a04749c
fix a flaky test
E-L-T Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/controllers/users/dossiers_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,10 @@ def update_dossier_and_compute_errors
@dossier.update_champs_attributes(champs_public_attributes_params, :public, updated_by: current_user.email)
if @dossier.champs.any?(&:changed_for_autosave?)
@dossier.last_champ_updated_at = Time.zone.now

if @dossier.champs_public.any? { _1.changed_for_autosave? && _1.used_by_routing_rules? }
RoutingEngine.compute(@dossier)
end
end

# We save the dossier without validating fields, and if it is successful and the client
Expand Down
30 changes: 30 additions & 0 deletions app/views/shared/dossiers/_update_contact_information.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#contact_information
- service = dossier&.service || procedure.service
- if service.present?
%h3.fr-footer__top-cat= I18n.t('users.procedure_footer.managed_by.header')
.fr-footer__top-link.fr-pb-2w
%span{ lang: :fr }= service.pretty_nom
%div{ lang: :fr }
= render SimpleFormatComponent.new(service.adresse, class_names_map: {paragraph: 'fr-footer__content-desc'})
%h3.fr-footer__top-cat= I18n.t('users.procedure_footer.contact.header')
%ul.fr-footer__top-list
- if dossier.present? && dossier.messagerie_available?
%li
= link_to I18n.t('users.procedure_footer.contact.in_app_mail.link'), messagerie_dossier_path(dossier), class: 'fr-footer__top-link'
- elsif service.present?
%li
%span.fr-footer__top-link
= I18n.t('users.procedure_footer.contact.email.link')
= link_to service.email, "mailto:#{service.email}", class: "fr-footer__top-link"

- if service.present?
- if service.telephone.present? || service.horaires.present?
%li
- horaires = "#{I18n.t('users.procedure_footer.contact.schedule.prefix')}#{formatted_horaires(service.horaires)}"
- if service.telephone.present?
= link_to service.telephone_url, class: 'fr-footer__top-link' do
%p
= I18n.t('users.procedure_footer.contact.phone.link', service_telephone: service.telephone)
- if service.horaires.present?
%p
= horaires
30 changes: 1 addition & 29 deletions app/views/users/_procedure_footer.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,9 @@
%footer.fr-footer.footer-procedure#footer{ role: "contentinfo" }
- service = dossier&.service || procedure.service
.fr-footer__top.fr-mb-0
.fr-container
.fr-grid-row.fr-grid-row--start.fr-grid-row--gutters
.fr-col-12.fr-col-sm-4.fr-col-md-4
- if service.present?
%h3.fr-footer__top-cat= I18n.t('users.procedure_footer.managed_by.header')
.fr-footer__top-link.fr-pb-2w
%span{ lang: :fr }= service.pretty_nom
%div{ lang: :fr }
= render SimpleFormatComponent.new(service.adresse, class_names_map: {paragraph: 'fr-footer__content-desc'})
%h3.fr-footer__top-cat= I18n.t('users.procedure_footer.contact.header')
%ul.fr-footer__top-list
- if dossier.present? && dossier.messagerie_available?
%li
= link_to I18n.t('users.procedure_footer.contact.in_app_mail.link'), messagerie_dossier_path(dossier), class: 'fr-footer__top-link'
- elsif service.present?
%li
%span.fr-footer__top-link
= I18n.t('users.procedure_footer.contact.email.link')
= link_to service.email, "mailto:#{service.email}", class: "fr-footer__top-link"

- if service.present?
- if service.telephone.present? || service.horaires.present?
%li
- horaires = "#{I18n.t('users.procedure_footer.contact.schedule.prefix')}#{formatted_horaires(service.horaires)}"
- if service.telephone.present?
= link_to service.telephone_url, class: 'fr-footer__top-link' do
%p
= I18n.t('users.procedure_footer.contact.phone.link', service_telephone: service.telephone)
- if service.horaires.present?
%p
= horaires
= render partial: 'shared/dossiers/update_contact_information', locals: { dossier: dossier, procedure: procedure }

- politiques = politiques_conservation_de_donnees(procedure)
- if politiques.present?
Expand Down
2 changes: 2 additions & 0 deletions app/views/users/dossiers/update.turbo_stream.haml
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

est-ce qu'on a un moyen (facile) de savoir si le routage / les infos de contact a changé ? Pour éviter de render ce contact information dans 99% des cas où ce ne sera pas nécessaire, que ce soit côté serveur ou côté navigateur et conserver une réactivité maximale de l'autosave (sachant que ce serait déjà largement perfectible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colinux dans le dernier commit j‘ai ajouté une condition pour faire l‘update du footer seulement si on a touché à un champ de routage, mais ça fait péter un test de system : spec/system/users/brouillon_spec.rb:9 . Sachant que ça a l‘air d‘être un flaky test (il échoue sur main en local sur mon poste). J‘ai testé différents fixs, sans succès, je mets ça en pause pour l‘instant. Si jamais tu as une idée...

Copy link
Member

Choose a reason for hiding this comment

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

@colinux Je me demande si c'est une bonne idée d'introduire ce genre d'optimisations ? J'ai l'impression que nous avons beaucoup de conditions complexes dans les turbo_stream. Je veux bien qu'on en parle au point tech. J'aimerais bien qu'on utilise quand c'est possible le nouveau turbo_stream refresh avec morph dans la plupart des cas (l'autosave c'est probablement pas le bon candidat, mais même ici je pense qu'on peut simplifier)

Copy link
Member

Choose a reason for hiding this comment

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

l'optimisation n'étant pas tant pour le temps serveur, mais pour éviter que les navigateurs refassent des manips/comparaisons de DOM inutiles (je pense en particulier aux mobiles low-end chez qui la page de la démarche lag déjà). Cela dit il y a peut-être des astuces turbo/morph que j'ignore qui font que c'est déjà pas cher du tout, ok pour en causer

Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
= turbo_stream.append('contenu', render(Dossiers::InvalidIneligibiliteRulesComponent.new(dossier: @dossier)))
- else @ineligibilite_rules_is_computable
= turbo_stream.remove(dom_id(@dossier, :ineligibilite_rules_broken))
- if @dossier.champs.any? { _1.used_by_routing_rules? && params[:dossier][:champs_public_attributes].key?(_1.stable_id.to_s) }
= turbo_stream.update "contact_information", partial: 'shared/dossiers/update_contact_information', locals: { dossier: @dossier, procedure: @procedure }
12 changes: 12 additions & 0 deletions spec/system/routing/rules_full_scenario_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@
expect(page).to have_text('Le groupe d’instructeurs « artistique » a été créé. ')
expect(procedure.groupe_instructeurs.count).to eq(4)

# add contact_information to all groupes instructeur
procedure.groupe_instructeurs.each { |gi| gi.update!(contact_information: create(:contact_information)) }

# publish
publish_procedure(procedure)
log_out
Expand Down Expand Up @@ -267,9 +270,18 @@ def user_send_dossier(user, groupe)
# the old system should not be present
expect(page).not_to have_selector("#dossier_groupe_instructeur_id")

dossier = user.dossiers.first

expect(dossier.groupe_instructeur_id).to be_nil
expect(page).to have_text(procedure.service.nom)

choose(groupe)
wait_for_autosave

expect(dossier.reload.groupe_instructeur_id).not_to be_nil
expect(page).to have_text(dossier.service.nom)
expect(page).not_to have_text(procedure.service.nom)

click_on 'Déposer le dossier'
expect(page).to have_text('Merci')

Expand Down
3 changes: 2 additions & 1 deletion spec/system/users/brouillon_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,8 @@ def champ_past_value_for(libelle, value)

def champ_for(libelle)
champs = user_dossier.reload.champs_public
champs.find { |c| c.libelle == libelle }
champ = champs.find { |c| c.libelle == libelle }
champ.reload
end

def fill_individual
Expand Down
Loading