-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(rgpd): partition motifs by org type #2540
base: staging
Are you sure you want to change the base?
feat(rgpd): partition motifs by org type #2540
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci @Michaelvilleneuve 👍 . La logique m'a l'air ok, je t'ai juste fait qq suggestions sur les endroits où on pourrait la déplacer.
On est sûr qu'on avait aucune configuration sur des catégories non autorisées en prod du coup ?
app/models/category_configuration.rb
Outdated
def motif_is_authorized_for_organisation | ||
return if motif_category.authorized_for_organisation?(organisation) | ||
|
||
errors.add(:base, "La catégorie de motif n'est pas autorisée pour cette organisation") | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je me demande si cette validation devrait être faite ici au niveau du model. On essaie de garder au niveau du model plutôt les validations liées au format de la donnée que l'on commit en DB.
Ici la validation concerne la logique métier de deux ressources associées, je pense que ça a plus sa place dans le service MotifCatgories::Create
.
create(:motif_category, name: "Autre", short_name: "autre", motif_category_type: "autre") | ||
end | ||
|
||
it "allows to select authorized motif categories" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ce serait bien d'avoir un test qui vérifie qu'on ne puisse pas créer une config sur une catégorie non autorisée
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je pense pas que ce soit utile ce serait redondant avec le TU
Cette PR empêche l'ajout de motifs non désirés en restreignant les organisations à n'utiliser que les motifs autorisés pour leur structure.
Fix #2460