-
Notifications
You must be signed in to change notification settings - Fork 80
✨ [FEAT] Add filter by creation year and update year to Report list (refs #4085) #4134
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
Conversation
Passing run #9183 ↗︎Details:
Review all test suite changes for PR #4134 ↗︎ |
db89932
to
d4fb5b8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4134 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 267 267
Lines 20760 20783 +23
=======================================
+ Hits 20424 20447 +23
Misses 336 336 ☔ View full report in Codecov by Sentry. |
e85b891
to
e9de056
Compare
@@ -16,7 +16,7 @@ | |||
|
|||
from . import models as feedback_models | |||
from . import serializers as feedback_serializers | |||
from .filters import ReportFilterSet, ReportNoEmailFilterSet | |||
from .filters import ReportFilterSet, ReportEmailFilterSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ces deux noms ont été échangés
|
||
class Meta(MapEntityFilterSet.Meta): | ||
model = Report | ||
fields = ['activity', 'category', 'status', 'problem_magnitude', 'assigned_user'] | ||
fields = ['activity', 'email', 'category', 'status', 'problem_magnitude', 'assigned_user'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change email in a filter dedicated PR ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reorganized these two filtersets to
- have a more logical inheritance
- not need to duplicate my code by adding the two new filters in both FilterSets
Before :
ReportFilterSet
-> filters on 10 fields
ReportNoEmailFilterSet
-> filters on 9 fields because it does not include 'email' filter, and does not inherit from ReportFilterSet
After :
ReportFilterSet
-> filters on 12 fields
ReportEmailFilterSet
-> filters on same 12 fields + 'email' = 13 fields and does inherit from ReportFilterSet
|
||
class SelectableUserManager(models.Manager): | ||
|
||
def get_queryset(self): | ||
return super().get_queryset().filter(userprofile__isnull=False) | ||
|
||
|
||
class ReportManager(NoDeleteManager, TimestampedChoicesMixin): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use NoDeleteMixin, you should use existing
everywhere objects
is used. In each queryset, else each delete objects is not delete from database, it will be always present in .objects. querysets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manager for Report
already was NoDeleteManager
(implicit thanks to NoDeleteMixin
), so existing
is already called where needed (see views.py
), I only added this here to extend it with my TimestampedChoicesMixin
like I saw here :
class TrekManager(TopologyManager, ProviderChoicesMixin): |
e9de056
to
b82aba3
Compare
b82aba3
to
2b37bae
Compare
Description
Filtrer par année de mise à jour ou de création
Sous forme de Mixin pour le rajouter facilement sur les autres modules à l'avenir
Related Issue
Checklist