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

Employeur : Utilisation d'un drapeau pour rendre une entreprise non recherchable #4938

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

rsebille
Copy link
Contributor

🤔 Pourquoi ?

Plus simple à comprendre et utiliser, et ça évite les effets de bord si on souhaite toucher aux coordonnées géographique pour d'autre raison (ce qui est mon cas :D).


Question philosophique du jour : Les assertSnapshotQueries() font pas mal de bruit à droite et à gauche lors de l'ajout d'un nouveau champ, je me suis donc demander si c'était pas mieux de couper le commit en deux pour séparer l'ajout du champ du reste des changements mais je me dit que ça ne rentre pas dans la logique d'avoir des commits les plus atomiques et donc facilement revertable.

@rsebille rsebille self-assigned this Oct 16, 2024
@@ -75,6 +75,7 @@ def form_valid(self, form):

siaes = (
Company.objects.active()
.filter(is_searchable=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Il faudrait aussi rajouter ce filtre sur le queryset job_descriptions juste en dessous non ?
Et passer de :

.exclude(company__block_job_applications=True)

à

.exclude(company__block_job_applications=True, is_searchable=False)

Copy link
Contributor Author

@rsebille rsebille Oct 17, 2024

Choose a reason for hiding this comment

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

Oui c'est mieux !
J'inverse au passage la logique (filter() -> exclude()) pour Company pour avoir la même chose dans les deux queryset et pour coller au défaut qui est qu'on peux rechercher. En fait non :)

@rsebille rsebille force-pushed the rsebille/companies-searchable-flag branch from 10d4a7c to 0a9e112 Compare October 17, 2024 09:02
Copy link
Contributor

@xavfernandez xavfernandez left a comment

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 voudrait pas des tests utilisant self.URL_JOBS également ?

@rsebille rsebille added ajouté Ajouté dans le changelog. and removed modifié labels Oct 21, 2024
@rsebille rsebille added this pull request to the merge queue Oct 21, 2024
Merged via the queue into master with commit 2f71741 Oct 21, 2024
16 checks passed
@rsebille rsebille deleted the rsebille/companies-searchable-flag branch October 21, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ajouté Ajouté dans le changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants