Skip to content

Employeur: Notifier les membres de l'entreprise lorsque les fiches de poste sont dépubliées. - #6830

Merged
tonial merged 2 commits into
masterfrom
alaurent/deactivate_old_jobapp_2
Sep 11, 2025
Merged

tonial merged 2 commits into
masterfrom
alaurent/deactivate_old_jobapp_2

Conversation

@tonial

@tonial tonial commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

https://www.notion.so/gip-inclusion/Fiches-de-poste-Mises-en-place-des-R-gles-de-gestion-de-d-publication-automatique-Notifs-emails--1915f321b6048046acd6f99596e3e917?v=18a5f321b604811bad80000cf7934cee&source=copy_link

🍰 Comment ?

Décrivez en quelques mots la solution retenue et mise en oeuvre, les difficultés ou problèmes rencontrés. Attirez l'attention sur les décisions d'architecture ou de conception importantes.

🚨 À 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

@tonial tonial self-assigned this Sep 9, 2025
@tonial tonial added the modifié Modifié dans le changelog. label Sep 9, 2025
@tonial
tonial changed the base branch from master to alaurent/deactivate_old_jobapp September 9, 2025 08:14
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from d1658ec to 10e3a9e Compare September 9, 2025 08:15
Comment thread itou/companies/notifications.py Outdated
'Command.execute[utils/command.py]',
]),
'sql': '''
SELECT "jobs_appellation"."updated_at",

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.

Tu pourrais .select_related("appellation") sur JobDescription.objects pour éviter cette requête je pense (et le N+1 associé).
Et potentiellement .select_related("location") pour le {{ job_description.display_location }} du sujet.
=> Ça vaudrait ptet le coup d'avoir plus de fiches de poste avec ou sans location pour détecter les N+1 éventuels.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

J'ai mis des location différentes pour les 2 fiches de poste.

Comment thread itou/companies/notifications.py Outdated
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from 10e3a9e to 2d141e9 Compare September 9, 2025 12:32
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from 2d141e9 to f5af508 Compare September 9, 2025 12:34
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp branch 2 times, most recently from c74ab63 to 1dd665b Compare September 9, 2025 12:47
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from f5af508 to 40f75f3 Compare September 9, 2025 12:48
Base automatically changed from alaurent/deactivate_old_jobapp to master September 9, 2025 13:06
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch 2 times, most recently from d6137e6 to c5f557b Compare September 9, 2025 13:09
.exclude(last_employer_update_at__gte=timezone.now() - DEACTIVATION_DELAY)
.update(is_active=False)
.select_related("company", "appellation", "location")
.prefetch_related("company__members")

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.

Je me demande si on ne devrait pas mettre une BATCH_SIZE pour éviter de renvoyer trop de mail d'un coup.
J'ai l'impression qu'il y a en moyenne ~3 membres par SIAE et actuellement il y a 1670 fiches de poste ayant été mises à jour le 22 mai donc potentiellement 4500 emails (avec 2 SELECT & 1 INSERT pour chaque) seront créés dans une seule transaction vers le 20 août...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

un batch_size pour limiter le nombre de fiches de poste traitées par jour ?

@tonial
tonial requested a review from xavfernandez September 9, 2025 14:19
).send()
self.logger.info(f"Deactivated {deactivated_nb} JobDescriptions")
# More than one day delay
if old_job_descriptions_qs.count() > BATCH_SIZE:

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.

Suggested change
if old_job_descriptions_qs.count() > BATCH_SIZE:
if old_job_descriptions_qs.count() >= BATCH_SIZE:

Plutôt, voir == même.

self.logger.info(f"Deactivated {deactivated_nb} JobDescriptions")
# More than one day delay
if old_job_descriptions_qs.count() > BATCH_SIZE:
self.logger.warning("Too many old JobDescriptions to deactivate")

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.

Oui mais en vrai personne ne verra ce log.
Et je ne pense pas qu'on souhaite être notifié sur sentry pour 3 fiches de poste en trop.
On pourrait peut-être compter le nombre de fiches de poste restantes à traiter et rajouter un autre log en erreur si le reliquat dépasse un certain seuil genre BATCH_SIZE ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

C'est ce que je fais : je désactive jusqu'à 200 fiches, et après je compte combien il en reste (qui n'ont pas été désactivée).
Donc l'alerte arrive quand il y a plus d'un jour de retard de traitement

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.

Ah oui effectivement, j'ai lu trop vite 👍
Du coup, je verrai bien un logger.error pour qu'un dev aille voir s'il faut augmenter la taille du batch ou s'il y a un autre soucis.

@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from 3a07bfe to 4788e3d Compare September 10, 2025 04:10
@tonial
tonial requested a review from xavfernandez September 10, 2025 04:10
@tonial
tonial force-pushed the alaurent/deactivate_old_jobapp_2 branch from 4788e3d to 954014f Compare September 10, 2025 05:37
@tonial
tonial added this pull request to the merge queue Sep 11, 2025
Merged via the queue into master with commit 65d1ba7 Sep 11, 2025
14 checks passed
@tonial
tonial deleted the alaurent/deactivate_old_jobapp_2 branch September 11, 2025 08:34
@sentry

sentry Bot commented Sep 12, 2025

Copy link
Copy Markdown

Issues attributed to commits in this pull request

This pull request was merged and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants