Skip to content

Fiche salarié : faciliter le nouvel envoi de la fiche si le SIRET de l'entreprise a changé - #5988

Merged
EwenKorr merged 12 commits into
masterfrom
ewen/siret
May 22, 2025
Merged

Fiche salarié : faciliter le nouvel envoi de la fiche si le SIRET de l'entreprise a changé#5988
EwenKorr merged 12 commits into
masterfrom
ewen/siret

Conversation

@EwenKorr

@EwenKorr EwenKorr commented Apr 17, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

On veut faciliter le renvoi de la fiche salarié lorsque si l'entreprise a un nouveau SIRET.

En fait, on généralise un petit peu, puisque renvoyer des FS peut être utile dans d'autres cas (fiche supprimée côté ASP par exemple).
On affiche donc un bouton "Renvoyer la fiche salarié" sur toutes les FS Intégrées.

Points de vigilance :

  • le siret est celui de l'entreprise mère (employee_record.siret_from_asp_source)

En pratique, le bouton Renvoyer > Avec modification est un lien vers employee_record_views:create (qui gère également la modification).
Le bouton Renvoyer > Sans modification est directement un POST vers la dernière étape du même processus. J'ai trouvé que c'était le plus simple, mais c'est questionnable sans souci.

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?
  • Ajouter l'étiquette « Bug » ?

🏝️ Comment tester ?

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc. Si vous disposez d'une recette jetable, mettre l'URL pour tester dans cette partie.

💻 Captures d'écran

image

@EwenKorr EwenKorr added the ajouté Ajouté dans le changelog. label Apr 17, 2025
@EwenKorr EwenKorr self-assigned this Apr 17, 2025
@EwenKorr
EwenKorr force-pushed the ewen/siret branch 4 times, most recently from b2d8892 to 753d9a6 Compare April 22, 2025 06:48
@notion-workspace

Copy link
Copy Markdown

@EwenKorr
EwenKorr force-pushed the ewen/siret branch 4 times, most recently from 45e88f1 to e059965 Compare April 24, 2025 14:49
Comment thread itou/employee_record/models.py
@EwenKorr
EwenKorr force-pushed the ewen/siret branch 2 times, most recently from 85dfdcb to 48f2be6 Compare April 25, 2025 11:42
@EwenKorr EwenKorr added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Apr 25, 2025
@github-actions

Copy link
Copy Markdown

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

@EwenKorr
EwenKorr force-pushed the ewen/siret branch 2 times, most recently from 039f43b to 7fcc1b9 Compare April 25, 2025 15:12
@EwenKorr EwenKorr added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels Apr 25, 2025
@github-actions

Copy link
Copy Markdown

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

@EwenKorr
EwenKorr force-pushed the ewen/siret branch 6 times, most recently from f28985f to 75d7114 Compare April 28, 2025 15:16
@EwenKorr EwenKorr added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels Apr 29, 2025
@EwenKorr

Copy link
Copy Markdown
Contributor Author

Il y a eu pas mal de petites choses à faire à côté (mise en place de la barre d'action, déplacement de EmployeeRecord.siret_from_asp_source()…).

A posteriori je vois bien que ç'aurait pu aller dans d'autres PR. Aurais-je pu/dû ouvrir des petites PR satellites pour certaines des modifications que vous avez suggérées ?

Ça vaut toujours le coup de le faire maintenant ?

@francoisfreitag

Copy link
Copy Markdown
Member

Excellente réalisation, au fur et à mesure on identifie de mieux en mieux comment découper ses changements, ce qui facilite la vie de tous. 💯 👏

A posteriori je vois bien que ç'aurait pu aller dans d'autres PR. Aurais-je pu/dû ouvrir des petites PR satellites pour certaines des modifications que vous avez suggérées ?

La limite entre multiple PRs et une PR avec tous les commits est ténue et varie selon les préférences individuelles, chaque méthode ayant ses avantages et inconvénients. Sommairement pour de multiples PRs, ~1 commit par PR (opposé à une PR avec plusieurs commits pour une fonctionnalité donnée).

Avantages

  • Petites relectures
  • Généralement des discussions plus courtes et itérations plus rapides
  • CI passe sur la PR, assurant que les tests passent bien sur chaque commit
  • Intégration progressive à la base de code, ce qui permet d’identifier plus vite les régressions en prod

Inconvénients

  • Répétition des tâches GitHub (gestion des PRs, avec un titre, description, lien vers la carte Notion, rebase, ...)
  • Répétition des tâches de MEP (surveillance merge queue, déploiement, Sentry)
  • Changements initiaux artificiels
  • Difficile de voir que le petit changement proposé est la meilleure façon de parvenir à l’objectif
  • Baisse de la vision globale (toutes ces PRs sont liées à ce sujet, même si Notion permet de lister toutes le PRs sur un sujet)

J’ai essayé de rester neutre, j’ai une préférence pour les PRs atomiques et petites.

Je rappelle le mantra “make the change easy, then make the easy change”, soit « faciliter le changement, puis faire le changement facile ».

Ça vaut toujours le coup de le faire maintenant ?

Plutôt non, la PR ayant été relue de multiples fois, j’ai l’impression que les changements apportés ne génèreront plus beaucoup de discussion, qu’on est à l’étape de peaufinage de la PR.

@francoisfreitag

Copy link
Copy Markdown
Member

Le statut « brouillon » de la PR n’est plus nécessaire, si ?

@EwenKorr

Copy link
Copy Markdown
Contributor Author

Le statut « brouillon » de la PR n’est plus nécessaire, si ?

En effet, j'ai oublié de modifier le statut (que j'avais remis temporairement parce que je réorganisais un peu).

@EwenKorr
EwenKorr marked this pull request as ready for review May 13, 2025 11:58
@EwenKorr
EwenKorr force-pushed the ewen/siret branch 2 times, most recently from 8dfc449 to 39a95d1 Compare May 16, 2025 15:27
@EwenKorr EwenKorr added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels May 19, 2025
@github-actions

Copy link
Copy Markdown

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

@rsebille rsebille left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merci pour le découpage, j'ai eu peur en voyant 14 commits mais ça été vite :).

Comment thread tests/www/employee_record_views/test_list.py Outdated
Comment thread tests/www/employee_record_views/test_list.py
Comment thread tests/www/employee_record_views/test_list.py Outdated
EwenKorr added 12 commits May 20, 2025 17:07
instead of the side block.
Showing the same actions in the list view and the summary view is
better, too.
We'll later want to compare the EmployeeRecord.siret (sent to the ASP)
with the current siret of the company (or the mother company if the
current company is an antenna).
…any's

Keeping in mind we're looking at the siret sent to the ASP, so the
mother's company siret in case of an antenna.
If any of the company's employee record, with status=PROCESSED,
has a SIRET that is different from the mother company's SIRET,
show a warning at the top of the list and in the concerned items.
@EwenKorr
EwenKorr added this pull request to the merge queue May 22, 2025
Merged via the queue into master with commit 19961ad May 22, 2025
@EwenKorr
EwenKorr deleted the ewen/siret branch May 22, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC ajouté Ajouté dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants