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

feat: add missing attachements on SensitivityArea APIs #3348

Merged
merged 11 commits into from
Feb 1, 2023

Conversation

lpofredc
Copy link
Contributor

@lpofredc lpofredc commented Dec 7, 2022

PR to fix missing attachments in SensitiveAreas APIs.

Tests updated to check if attachments are in API responses.

@cypress
Copy link

cypress bot commented Dec 7, 2022

Passing run #5721 ↗︎

0 24 0 0 Flakiness 0

Details:

Merge 1576a13 into 250db52...
Project: Geotrek-admin Commit: 06350a0863 ℹ️
Status: Passed Duration: 03:40 💡
Started: Feb 1, 2023 9:30 AM Ended: Feb 1, 2023 9:33 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Base: 98.28% // Head: 98.28% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (1576a13) compared to base (250db52).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3348   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         291      292    +1     
  Lines       21142    21152   +10     
=======================================
+ Hits        20779    20789   +10     
  Misses        363      363           
Impacted Files Coverage Δ
geotrek/api/v2/serializers.py 99.89% <100.00%> (+<0.01%) ⬆️
geotrek/api/v2/views/sensitivity.py 100.00% <100.00%> (ø)
geotrek/sensitivity/mixins.py 100.00% <100.00%> (ø)
geotrek/sensitivity/serializers.py 100.00% <100.00%> (ø)
geotrek/sensitivity/views.py 98.40% <100.00%> (-0.10%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Chatewgne Chatewgne self-requested a review December 9, 2022 13:32
Copy link
Contributor

@Chatewgne Chatewgne left a comment

Choose a reason for hiding this comment

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

Please add Prefetch in SensitiveAreaViewSet queryset (here : https://github.com/GeotrekCE/Geotrek-admin/blob/master/geotrek/api/v2/views/sensitivity.py#L35) for performances as follows

        queryset = sensitivity_models.SensitiveArea.objects.existing() \
            .filter(published=True) \
            .select_related('species', 'structure') \
            .prefetch_related('species__practices',
                              Prefetch('attachments',
                                       queryset=Attachment.objects.select_related('license', 'filetype', 'filetype__structure'))) \
            .alias(geom_type=GeometryType(F('geom')))

@Chatewgne
Copy link
Contributor

Also please add an entry to describe this PR in latest changelog https://github.com/GeotrekCE/Geotrek-admin/blob/master/docs/changelog.rst

geotrek/sensitivity/views.py Outdated Show resolved Hide resolved
@lpofredc
Copy link
Contributor Author

Changes done.

There will probably be conflicts to manage with other PR I purposed due to simultaneous changes on serializers and viewsets.

@submarcos
Copy link
Member

submarcos commented Jan 27, 2023

Hello @lpofredc, on a effectué quelques modifs et créé un début de guide contribution. Peux-tu rebaser sur master et repush ton code pour lancer les tests ? On va essayer de faire en sorte que tout se lance bien dans la CI pour les contributions externes

Merci

geotrek/sensitivity/mixins.py Show resolved Hide resolved
geotrek/sensitivity/mixins.py Outdated Show resolved Hide resolved
geotrek/sensitivity/mixins.py Outdated Show resolved Hide resolved
@submarcos submarcos merged commit 7623070 into GeotrekCE:master Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants