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

[TECH] Uniformise le code des fonctionnalités d'organisation sur PixAdmin (PIX-16319). #11272

Merged

Conversation

frinyvonnick
Copy link
Member

@frinyvonnick frinyvonnick commented Jan 29, 2025

🥞 Problème

Il existe dans la base de code plusieurs manière d'activer des fonctionnalités sur une organisation.

🥓 Proposition

Dans cette PR, on uniformise la manière d'afficher et mettre à jour les fonctionnalités d'une organisation sur PixAdmin via le model OrganizationForAdmin.

🧃 Remarques

La valeur pour la feature attestation est en dur. Il faudra faire évoluer à l'ajout de l'attestation parentalité.

😋 Pour tester

Aller sur PixAdmin
Aller sur la page d'une organisation
Vérifier le bon affichage des features dans l'encart "fonctionnalités"
Vérifier le bon fonctionnement lors de l'édition et l'ajout/retrait des features
(note : il est normal que certaines fonctionnalités se soient pas éditable)

@frinyvonnick frinyvonnick self-assigned this Jan 29, 2025
@frinyvonnick frinyvonnick changed the title [TECH] Pix 16319 refactor feature code in pix admin [TECH] Uniformise le code des fonctionnalités d'organisation sur PixAdmin (PIX-16319). Jan 29, 2025
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch from 7d9f149 to 5d06af8 Compare January 30, 2025 17:11
@frinyvonnick frinyvonnick force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch from 5d06af8 to 434bb1b Compare January 31, 2025 09:42
@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch 2 times, most recently from 7e43d56 to 6cb9817 Compare February 4, 2025 08:30
@Alexandre-Monney Alexandre-Monney self-assigned this Feb 4, 2025
@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch 2 times, most recently from 9326e38 to d6c5cb9 Compare February 4, 2025 14:03
@Alexandre-Monney Alexandre-Monney marked this pull request as ready for review February 4, 2025 14:32
@Alexandre-Monney Alexandre-Monney requested a review from a team as a code owner February 4, 2025 14:32
@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch 2 times, most recently from a36930a to 1e4a5d5 Compare February 5, 2025 10:21
@alicegoarnisson
Copy link
Contributor

Prescription: Func OK 🦦

@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch from 1e4a5d5 to bb66a2e Compare February 6, 2025 08:46
this.form.identityProviderForCampaigns =
this.args.organization.identityProviderForCampaigns ?? this.noIdentityProviderOption.value;

this.form.features = JSON.parse(JSON.stringify(this.args.organization.features));
Copy link
Contributor

Choose a reason for hiding this comment

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

C'est étrange comme façon de faire, pourquoi doit-on faire ça ?

Copy link
Member Author

Choose a reason for hiding this comment

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

On doit faire ça parce que sinon on travaille sur la même référence et on veut pouvoir annuler les changements dans le formulaire donc il faut faire une copie 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

En fouillant un peu, je viens de voir qu'il existait en natif depuis window la méthode structuredClone()
Je vais essayer de voir si je peux pas plutôt utiliser ça a la place du .parse/.stringify

Copy link
Contributor

Choose a reason for hiding this comment

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

La méthode structuredClone() marche très bien pour notre cas, je change par ça. Ça rends + explicite à mon avis

@Alexandre-Monney Alexandre-Monney force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch from bb66a2e to a156aa4 Compare February 10, 2025 16:03
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Alexandre-Monney Alexandre-Monney added Tech Review OK and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Feb 13, 2025
@Alexandre-Monney Alexandre-Monney added the Func Review OK PO validated functionally the PR label Feb 13, 2025
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-16319-refactor-feature-code-in-pix-admin branch from a156aa4 to ab38dc1 Compare February 13, 2025 15:14
@pix-service-auto-merge pix-service-auto-merge merged commit 2bc535e into dev Feb 13, 2025
8 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-16319-refactor-feature-code-in-pix-admin branch February 13, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants