Feat: added metadata on pagination responses#115
Feat: added metadata on pagination responses#115csantero merged 3 commits intoJSONAPIdotNET:masterfrom
Conversation
spike83
commented
Aug 8, 2016
- metadata to indicate the count of pages and total count of records
- acceptance tests pagination in combination with filters and sorting
|
@spike83 I think it would be better to move these changes into a derived class in the I also noticed you are calling |
|
Thanks for your feedback. I'll improve that. |
20825a1 to
370365b
Compare
| writer.WritePropertyName(MetaKeyName); | ||
| MetadataFormatter.Serialize(document.Metadata, writer); | ||
| } | ||
|
|
There was a problem hiding this comment.
@spike83 is it necessary to move the metadata in the JSON document?
There was a problem hiding this comment.
No it's not functionally necessary. But the specs and most samples start with the top-level metadata. I haven't seen any constarint which would enforce it.
Should I move it back?
|
@spike83 Merging the other PR introduced some conflicts here. |
|
@csantero i will fix them |
- metadata to indicate the count of pages and total count of records - acceptance tests pagination in combination with filters and sorting
reduced the number of count calls to save roundtrips
06efe26 to
eaea639
Compare
|
@csantero rebased to your master |
| { | ||
| var count = filteredQuery.CountAsync(cancellationToken); | ||
| metadata.MetaObject.Add("total-pages", (int)Math.Ceiling((decimal) await count / paginationResult.PageSize)); | ||
| metadata.MetaObject.Add("total-count", await count); |
There was a problem hiding this comment.
@spike83 By using two awaits this will cause the exact same query to run twice. You should move the await to line 46.
|
@csantero is this now ok for merging? |
|
Thanks |