Adding "paginator" in "meta" for data collections. #2
Conversation
| @@ -77,16 +77,16 @@ def get_serializable_data(self, request=None): | |||
| included=self.included, | |||
| request=request, | |||
| )) | |||
| return ret | |||
| return ret, dict(total_count=len(self.data)) | |||
There was a problem hiding this comment.
No need to return the meta dictionary to get updated in self.meta. Just update here. The object state is the top-level. There are some other improvements that should be made to support that, but out of scope for this PR.
| if self.meta: | ||
| self.meta.update(total_count) | ||
| else: | ||
| self.meta = total_count |
There was a problem hiding this comment.
This isn't ideal. We need to ensure self.meta is already a dict.
|
@brosner this fixes the "meta" dictionary existence issue. I believe meta works as desired and page[size] as well, but forthcoming tests will prove their correctness. |
Removing "meta" keyword argument in methods so that TopLevel can use it's default "meta" value instead.
|
Yippee @brosner this is ready for review and merging |
grahamu
changed the title
| @@ -43,7 +46,7 @@ def from_validation_error(cls, exc, resource_class): | |||
| errs.append(err) | |||
| return cls(errors=errs) | |||
|
|
|||
| def __init__(self, data=None, errors=None, links=False, included=None, meta=None, linkage=False): | |||
| def __init__(self, data=None, errors=None, links=False, included=None, meta={}, linkage=False): | |||
There was a problem hiding this comment.
This is an ugly no-no in Python. The dictionary (if used) is scoped globally and can have bad side-affects to meta across requests (it can share values across all requests).
There was a problem hiding this comment.
Simple fix: check for None in __init__ and if true then set self.meta to a dictionary created in the method scope.
There was a problem hiding this comment.
Another fix is to leave it None and revert the behavior in serializable. Is there another reason I am not seeing for this change?
There was a problem hiding this comment.
I see the reason. You need self.meta to be a dictionary in get_serializable_data. That makes sense. I say go for the first solution and in serializable don't check if self.meta is not {} but if self.meta (because {} is not {} is always True).
There was a problem hiding this comment.
Yikes, I do remember reading about that bad pattern. Will fix!
No description provided.