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

API: ajout des filtres departement et departement_postes sur la route /api/v1/siaes/ #4270

Merged
merged 5 commits into from
Jun 25, 2024

Conversation

xavfernandez
Copy link
Contributor

🤔 Pourquoi ?

Certains départements souhaitent pouvoir facilement récupérer la liste des entreprises opérant sur leur territoire.

🍰 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.

💻 Captures d'écran

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ 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.

@xavfernandez xavfernandez added the ajouté Ajouté dans le changelog. label Jun 20, 2024
@xavfernandez xavfernandez self-assigned this Jun 20, 2024
field_name="department", choices=list(DEPARTMENTS.items()), help_text="Département de la structure"
)

departement_postes = ChoiceFilter(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Si vous avez un meilleur nom de filtre, je suis preneur 😬

Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Ça m’a l’air très bien 👍

response, "Le paramètre `distance_max_km` doit être compris entre 0 et 100", status_code=400
)
assert response.status_code == 400
assert response.json() == {"distance_max_km": ["Assurez-vous que cette valeur est inférieure ou égale à 100."]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Faudrait-il également une mention dans la release note de ce changement ? C’est un breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certes mais comme on est sur un cas d'erreur et que j'espère qu'aucun système automatique ne s'amuse à envoyer des données invalides j'aurais tendance à laisser tel quel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Juste une mention dans le changelog ne me semble pas trop lourde, et évite des surprises aux utilisateurs ?

Copy link
Contributor Author

@xavfernandez xavfernandez Jun 24, 2024

Choose a reason for hiding this comment

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

Mais comment ajouter des choses dans le changelog en dehors du titre de cette PR ?
Je rajoute ainsi qu'une modification du format de certaines erreurs à la fin du titre ?

Copy link
Contributor

Choose a reason for hiding this comment

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

nvim CHANGELOG.md ? La génération automatique est bien dans la plupart des cas, mais c’est pas la seule manière ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est un coup à péter la prochaine PR de changelog non ?

cls.company_with_jobs = CompanyFactory(
with_jobs=True, romes=("N1101", "N1105", "N1103", "N4105"), department="44", coords=cls.saint_andre.coords
)

def setUp(self):
super().setUp()
self.client = APIClient()
Copy link
Contributor

Choose a reason for hiding this comment

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

À se demander si le setUp est encore utile...

itou/api/siae_api/viewsets.py Outdated Show resolved Hide resolved
itou/api/siae_api/viewsets.py Outdated Show resolved Hide resolved
)
# Here the department filter has been applied
# Here the department/job_department filters have been applied
Copy link
Contributor

@francoisfreitag francoisfreitag Jun 24, 2024

Choose a reason for hiding this comment

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

departement_postes pour correspondre au nom du filtre ?

Copy link
Contributor

Choose a reason for hiding this comment

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

postes_dans_le_departement ?

tests/api/siae_api/tests.py Show resolved Hide resolved
@xavfernandez xavfernandez force-pushed the xfernandez/api_cleanup branch 2 times, most recently from 9bd11ce to cee5cc3 Compare June 24, 2024 14:20
@xavfernandez xavfernandez force-pushed the xfernandez/api_cleanup branch from cee5cc3 to b144e94 Compare June 25, 2024 09:54
@xavfernandez xavfernandez enabled auto-merge June 25, 2024 09:54
@xavfernandez xavfernandez added this pull request to the merge queue Jun 25, 2024
Merged via the queue into master with commit 0c8fe7d Jun 25, 2024
11 checks passed
@xavfernandez xavfernandez deleted the xfernandez/api_cleanup branch June 25, 2024 10:09
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