Employeur : Historique des SIRET

[rsebille]

🤔 Pourquoi ?

• C'est très utile pour le support (et aussi nous) d'avoir les anciens SIRET d'une entreprise
• On a déjà 2 modèles qui font ce genre de chose mais uniquement si on passe par le .save() et pas tout à faire de la même manière

Lors de l'édition dans des transactions //, le 2ème UPDATE bloque jusqu'à ce que la transaction ayant déjà fait un UPDATE soit COMMIT donc on ne devrais pas avoir de perte de données mais par contre possible qu'on crée des dead locks, à surveiller.

🏝️ Comment tester

Les tests sont normalement parlant, sinon ça fonctionne aussi via psql ou tout autre moyen modifiant le champ ;)

[tonial]

🤩

[xavfernandez]

Je me demande si on ne voudrait pas defer ce champs dans le base_manager vu qu'a priori on ne l'utilisera que dans l'admin ?
Cf https://adamj.eu/tech/2023/12/11/django-always-defer-a-field/

[xavfernandez]

😍

[rsebille]

J'ai rajouté le .defer() car les managers étaient là mais vu qu'a plein d'autre endroit c'est pas pris en compte je sais pas si au final on veux avoir cette différence de comportement, car elle peux surprendre suivant les cas d'utilisations.

Pour les histoires de nom de champs et de DB j'ai utilisé .column plutôt que .get_attname_column() qui semble aussi faire le boulot.

J'en ai également profité pour tester dans psql cette histoire de transaction //, et ai donc mis à jour le corps de la PR avec le résultat.

Lors de l'édition dans des transactions //, le 2ème UPDATE bloque jusqu'à ce que la transaction ayant déjà fait un UPDATE soit COMMIT donc on ne devrais pas avoir de perte de données mais par contre possible qu'on crée des dead locks, à surveiller.

Et avec l'ajout du champs dans l'admin je pense qu'on est sur quelque chose de fusionnable la semaine prochaine.

[xavfernandez]

Pour les histoires de nom de champs et de DB j'ai utilisé .column plutôt que .get_attname_column() qui semble aussi faire le boulot.

Cf https://github.com/django/django/blob/stable/5.1.x/django/db/models/fields/__init__.py#L937 ça me semble bien 👍

[xavfernandez]

nit: comme le champs n'est pas nullable, je me demande si on ne devrait pas mettre un db_default=[] pour ne pas casser la création (improbable) d'une Company pendant le déploiement ?

[rsebille]

Tu m'as mis le doute depuis le temps, mais django gère le truc comme il faut :

❯ ./manage.py sqlmigrate companies 0014
BEGIN;                                                                                                                                                         
--                                                                         
-- Add field fields_history to company
--                                                                                                                                                             
ALTER TABLE "companies_company" ADD COLUMN "fields_history" jsonb[] DEFAULT '{}' NOT NULL;
ALTER TABLE "companies_company" ALTER COLUMN "fields_history" DROP DEFAULT;
...

[xavfernandez]

Euh oui, il gère l'ajout de la colonne :)
Mais après il DROP le DEFAULT: tu as maintenant une colonne NOT NULL sans valeur par défaut.
Donc l'ancien code (qui tourne toujours après le passage de la migration), quand il va tenter un INSERT: 💥 .

Après on peut tout à fait se dire que la création d'entreprise est assez rare pour que ça n'arrive pas pendant le déploiement de cette PR.