-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add view to retrieve period data #137
Conversation
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.
Just minor comments.
class PeriodAddressSerializer(serializers.Serializer): | ||
start_date = serializers.SerializerMethodField() | ||
end_date = serializers.SerializerMethodField() | ||
holder = serializers.CharField(source="address") |
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.
We should return checksumed addresses.
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.
Done in 5a58928 with gnosis.eth.django. serializers.EthereumAddressField
total_boosted_points = serializers.CharField() | ||
|
||
def get_start_date(self, obj): | ||
return obj.period.start_date |
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.
Would we follow the same format than this serializer?
https://github.com/safe-global/safe-locking-service/pull/137/files#diff-cdb19cb303055a7425c8c774069e3cc6ca3e61cfd8629c3d797255db9b272ed8R20
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.
Changed in 7954e02.
Now accessing it via the source
arg.
@@ -195,3 +196,30 @@ def get(self, *args, **kwargs): | |||
|
|||
serializer = self.serializer_class(queryset) | |||
return Response(status=status.HTTP_200_OK, data=serializer.data) | |||
|
|||
|
|||
class GetAddressPeriodsView(ListAPIView): |
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.
We could add cache_page
here to don´t hit with every request the database.
What do you think?
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.
Added in 5a58928.
To keep it aligned with other endpoints I kept it with the same configuration. In the future we can consider having it as an application config 👍
7b9264b
to
32177d0
Compare
c702cf2
to
5a58928
Compare
5a58928
to
db46c50
Compare
Pull Request Test Coverage Report for Build 9485475859Details
💛 - Coveralls |
What was wrong? 👾
Closes #131
How was it fixed? 🎯
Campaign
holder
- by setting theholder
address as the query parameterThe endpoint is as follows: