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

Bugfix: le RDV doit afficher l'adresse du responsable, pas du proche #4812

Merged
merged 3 commits into from
Nov 28, 2024
Merged

Bugfix: le RDV doit afficher l'adresse du responsable, pas du proche #4812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c1f33e
Bugfix: le RDV affiche l'adresse du proche plutôt que celle du respon…
francois-ferrandis Nov 14, 2024
62dcdb7
Merge branch 'production' into frf/rdv-address-bug
francois-ferrandis Nov 27, 2024
2bfb804
Merge branch 'production' into frf/rdv-address-bug
francois-ferrandis Nov 28, 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
5 changes: 5 additions & 0 deletions app/models/concerns/rdv/address_concern.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,9 @@ def address_without_personal_information
Motif.human_attribute_value(:location_type, :phone)
end
end

def user_for_home_rdv
proches, responsables = users.partition(&:responsible_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

élégant 👏 j’avais un doute que ça soit une légère régression de perfs de ne plus faire de distinction sur le loaded? mais après réfléxion j’imagine que rails ne va pas recharger les users si tu appelles partition dessus 🤔

Copy link
Contributor Author

@francois-ferrandis francois-ferrandis Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

On aurait pu continuer de faire la distinction sur le loaded?, mais fait le pari que si l'association n'est pas chargée, ça ne coûte pas beaucoup plus cher de charger tous les users, puisque le nombre d'usager souvent 1, parfois 2-3 (c'est pour les RDVs individuels).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, et aussi :

  • on peut faire le pari que tous les #users seront utilisés ailleurs aussi
  • c'est facile de preloader l'association #users, d'ailleurs on le fait dans Admin::RdvsController
  • le code est plus simple ici si on ne fait pas cette tentative d'optimisation (et on peut utiliser ce magnifique partition que j'aime)

(responsables + proches).select(&:address).first || users.first
end
end
5 changes: 0 additions & 5 deletions app/models/rdv.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,6 @@ def creneaux_available(date_range)
CreneauxSearch::Calculator.available_slots(motif, lieu, date_range)
end

def user_for_home_rdv
responsibles = users.loaded? ? users.select(&:responsible_id) : users.where.not(responsible_id: [nil])
[responsibles, users].flatten.select(&:address).first || users.first
end

# Ces plages d'ouvertures sont utilisé pour afficher des infos
# s'il y a un chevauchement avec le RDV.
#
Expand Down
14 changes: 7 additions & 7 deletions spec/models/rdv_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,17 +186,17 @@
end

context "when rdv is at home" do
let(:responsible) { create(:user) }
let(:child) { create(:user, responsible: responsible) }
let(:rdv) { create(:rdv, :at_home, users: [child]) }
let(:responsable) { create(:user, address: "1 rue du Responsable, 74000 Annecy") }
let(:proche) { create(:user, responsible: responsable, address: "33 impasse du Proche, 42300 Roanne") }
let(:rdv) { create(:rdv, :at_home, users: [responsable, proche]) }

it { is_expected.to eq responsible.address }
it { is_expected.to eq responsable.address }
end

context "when rdv is by phone" do
let(:responsible) { create(:user) }
let(:child) { create(:user, responsible: responsible) }
let(:rdv) { create(:rdv, :by_phone, users: [child]) }
let(:responsable) { create(:user) }
let(:proche) { create(:user, responsible: responsable) }
let(:rdv) { create(:rdv, :by_phone, users: [responsable, proche]) }

it { is_expected.to be_blank }
end
Expand Down
Loading