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.

Sign up for GitHub

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

Jump to bottom

fix: remove legacy filters from site route + allow filtering type_sites by name #416

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix: remove legacy filters from site route + allow filtering type_sites by name #416

wants to merge 2 commits into from

Conversation

mathieu-roudaut-crea
Copy link
Contributor

@mathieu-roudaut-crea mathieu-roudaut-crea commented Mar 17, 2025 •
edited
Loading

  • remove legacy filters from filter_according_to_column_type_for_site function
  • allow to filter by type_site name (not only ID)
  • Use Ruff to format imports order

amandine-sahl
amandine-sahl requested changes Apr 1, 2025
View reviewed changes
Copy link
Contributor

@amandine-sahl amandine-sahl left a comment

Choose a reason for hiding this comment

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

Merci pour la contribution. Par contre, ce serait sympatique de rajouter un test

backend/gn_module_monitoring/utils/routes.py
@@ -156,19 +154,6 @@ def query_all_types_site_from_module_id(id_module: int = None):


def filter_according_to_column_type_for_site(query, params):
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce que ça a encore un sens de garder cette fonction ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

De ce que j'en comprends, effectivement on peut la supprimer. Je fais une passe début de semaine prochaine pour vérifier son impact

backend/gn_module_monitoring/monitoring/queries.py
else:
# HACK gestionnaire des sites
# Quand filtre sur type de site envoie une chaine de caractère
params["types_site_label"] = value[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce qu'il ne faudrait pas supprimer types_site de params ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pour l'interface graphique, c'est sûr qu'il ne sert pas.
Si quelqu'un veut faire des filtres directement depuis l'API ça peut être intéressant de le conserver (mais pas obligatoire)

@mathieu-roudaut-crea mathieu-roudaut-crea force-pushed the fix/remove-legacy-filters-from-site-queries branch from c448ea8 to 3512e04 Compare April 7, 2025 13:58
@codecov Codecov
Copy link

codecov bot commented Apr 7, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.

Project coverage is 75.21%. Comparing base (37ca00a) to head (3512e04).
Report is 29 commits behind head on develop.

Files with missing lines Patch % Lines
backend/gn_module_monitoring/monitoring/queries.py 70.83% 7 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #416      +/-   ##
===========================================
- Coverage    75.93%   75.21%   -0.72%     
===========================================
  Files           30       30              
  Lines         2410     2429      +19     
===========================================
- Hits          1830     1827       -3     
- Misses         580      602      +22
Flag Coverage Δ
pytest 75.21% <85.10%> (-0.72%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

@amandine-sahl amandine-sahl amandine-sahl requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mathieu-roudaut-crea @amandine-sahl