From ee57d005077eba7b3d28f4bc86108a964f108194 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 27 Mar 2024 11:37:30 +0100 Subject: [PATCH] refactor(champs): add new champ find strategy --- app/controllers/champs/champ_controller.rb | 22 ++++++ app/policies/dossier_policy.rb | 39 +++++++++++ spec/policies/dossier_policy_spec.rb | 79 ++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 app/controllers/champs/champ_controller.rb create mode 100644 app/policies/dossier_policy.rb create mode 100644 spec/policies/dossier_policy_spec.rb diff --git a/app/controllers/champs/champ_controller.rb b/app/controllers/champs/champ_controller.rb new file mode 100644 index 000000000..4322055be --- /dev/null +++ b/app/controllers/champs/champ_controller.rb @@ -0,0 +1,22 @@ +class Champs::ChampController < ApplicationController + before_action :authenticate_logged_user! + before_action :set_champ + + private + + def find_champ + if params[:champ_id].present? + policy_scope(Champ) + .includes(:type_de_champ, dossier: :champs) + .find(params[:champ_id]) + else + dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id]) + type_de_champ = dossier.revision.types_de_champ.find_by!(stable_id: params[:stable_id]) + dossier.champ_for_update(type_de_champ, params[:row_id]) + end + end + + def set_champ + @champ = find_champ + end +end diff --git a/app/policies/dossier_policy.rb b/app/policies/dossier_policy.rb new file mode 100644 index 000000000..c50b43f0f --- /dev/null +++ b/app/policies/dossier_policy.rb @@ -0,0 +1,39 @@ +class DossierPolicy < ApplicationPolicy + # Scope for WRITING to a dossier. + # + # (If the need for a scope to READ a dossier emerges, we can implement another scope + # in this file, following this example: https://github.com/varvet/pundit/issues/368#issuecomment-196111115) + class Scope < ApplicationScope + def resolve + if user.blank? + return scope.none + end + + # The join must be the same for all elements of the WHERE clause. + # + # NB: here we want to do `.left_outer_joins(:invites, { :groupe_instructeur: :instructeurs })`, + # but for some reasons ActiveRecord <= 5.2 generates bogus SQL. Hence the manual version of it below. + joined_scope = scope + .joins('LEFT OUTER JOIN invites ON invites.dossier_id = dossiers.id OR invites.dossier_id = dossiers.editing_fork_origin_id') + .joins('LEFT OUTER JOIN groupe_instructeurs ON groupe_instructeurs.id = dossiers.groupe_instructeur_id') + .joins('LEFT OUTER JOIN assign_tos ON assign_tos.groupe_instructeur_id = groupe_instructeurs.id') + .joins('LEFT OUTER JOIN instructeurs ON instructeurs.id = assign_tos.instructeur_id') + + # Users can access public champs on their own dossiers. + resolved_scope = joined_scope.where(user_id: user.id) + + # Invited users can access public champs on dossiers they are invited to + invite_clause = joined_scope.where('invites.user_id': user.id) + resolved_scope = resolved_scope.or(invite_clause) + + if instructeur.present? + # Additionnaly, instructeurs can access private champs + # on dossiers they are allowed to instruct. + instructeur_clause = joined_scope.where('instructeurs.id': instructeur.id) + resolved_scope = resolved_scope.or(instructeur_clause) + end + + resolved_scope.or(joined_scope.where(for_procedure_preview: true)) + end + end +end diff --git a/spec/policies/dossier_policy_spec.rb b/spec/policies/dossier_policy_spec.rb new file mode 100644 index 000000000..dc0a25aca --- /dev/null +++ b/spec/policies/dossier_policy_spec.rb @@ -0,0 +1,79 @@ +describe DossierPolicy do + let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) } + let(:dossier) { create(:dossier, procedure: procedure, user: dossier_owner) } + let(:dossier_owner) { create(:user) } + + let(:signed_in_user) { create(:user) } + let(:account) { { user: signed_in_user } } + + subject { Pundit.policy_scope(account, Dossier) } + + shared_examples_for 'they can access dossier' do + it { expect(subject.find_by(id: dossier.id)).to eq(dossier) } + end + + shared_examples_for 'they can’t access dossier' do + it { expect(subject.find_by(id: dossier.id)).to eq(nil) } + end + + context 'when an user only has user rights' do + context 'as the dossier owner' do + let(:signed_in_user) { dossier_owner } + + it_behaves_like 'they can access dossier' + end + + context 'as a person invited on the dossier' do + let(:invite) { create(:invite, :with_user, dossier: dossier) } + let(:signed_in_user) { invite.user } + + it_behaves_like 'they can access dossier' + end + + context 'as another user' do + let(:signed_in_user) { create(:user) } + + it_behaves_like 'they can’t access dossier' + end + end + + context 'when the user also has instruction rights' do + let(:instructeur) { create(:instructeur, user: signed_in_user) } + let(:account) { { user: signed_in_user, instructeur: instructeur } } + + context 'as the dossier instructeur and owner' do + let(:signed_in_user) { dossier_owner } + before { instructeur.assign_to_procedure(dossier.procedure) } + + it_behaves_like 'they can access dossier' + end + + context 'as the dossier instructeur (but not owner)' do + let(:signed_in_user) { create(:user) } + before { instructeur.assign_to_procedure(dossier.procedure) } + + it_behaves_like 'they can access dossier' + end + + context 'as an instructeur not assigned to the procedure' do + let(:signed_in_user) { create(:user) } + + it_behaves_like 'they can’t access dossier' + end + end + + context 'when the champ is on a forked dossier' do + let(:signed_in_user) { dossier_owner } + let(:origin) { create(:dossier, procedure: procedure, user: dossier_owner) } + let(:dossier) { origin.find_or_create_editing_fork(dossier_owner) } + + it_behaves_like 'they can access dossier' + + context 'when the user is invited on the origin dossier' do + let(:invite) { create(:invite, :with_user, dossier: origin) } + let(:signed_in_user) { invite.user } + + it_behaves_like 'they can access dossier' + end + end +end