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

Feature ouidou/admin creation delegation gestionnaire page children management #9566

Conversation

@cmayran
Copy link

cmayran commented Oct 9, 2023

@tchak et @krichtof ready for review

@tchak tchak marked this pull request as draft October 10, 2023 09:17
@seb-by-ouidou seb-by-ouidou force-pushed the feature-ouidou/admin_creation_delegation_gestionnaire_page_children_management branch from 1d454c3 to 528c51c Compare October 26, 2023 10:56
@seb-by-ouidou seb-by-ouidou marked this pull request as ready for review October 26, 2023 10:57
@seb-by-ouidou
Copy link
Contributor Author

@tchak et @krichtof ready for review

@krichtof
Copy link
Contributor

Il y a plusieurs tests unitaires qui ne passent pas.
@seb-by-ouidou tu peux jeter un oeil ?

@seb-by-ouidou
Copy link
Contributor Author

@krichtof etrangement il ne joue pas la migration avant le test. aurais tu une idee de pourquoi ? je vais aussi degarder de mon coté

@krichtof
Copy link
Contributor

@seb-by-ouidou Je crois que tu as oublié de commiter db/schema.rb ;)

@seb-by-ouidou
Copy link
Contributor Author

@krichtof fixed

db/schema.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@krichtof krichtof left a comment

Choose a reason for hiding this comment

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

Merci pour cette PR :)
Juste quelques demandes de correction mineures avant approbation.

gestionnaire = Gestionnaire.find(params[:id])

if !gestionnaire.can_be_deleted?
fail "Impossible de supprimer ce gestionnaire car il est gestionnaire d'un groupe racine"
Copy link
Contributor

Choose a reason for hiding this comment

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

Peux-tu plutôt utiliser flash[:alert] ?
(je crois qu'on utilise fail une ou deux fois dans le manager mais nous allons également le corriger)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

belongs_to :groupe_gestionnaire, optional: true # parent
has_many :children, class_name: "GroupeGestionnaire", inverse_of: :groupe_gestionnaire
# belongs_to :groupe_gestionnaire, optional: true # parent
# has_many :children, class_name: "GroupeGestionnaire", inverse_of: :groupe_gestionnaire
has_many :administrateurs
Copy link
Contributor

Choose a reason for hiding this comment

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

Peux-tu du coup supprimer ces 2 lignes plutôt que de les laisser en commentaire ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@krichtof krichtof added this pull request to the merge queue Nov 8, 2023
Merged via the queue into demarches-simplifiees:main with commit a6ea607 Nov 8, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants