RKPaginator - Misleading documentation #1117

Closed
JD- opened this Issue Dec 31, 2012 · 3 comments

Comments

Projects
None yet
3 participants
@JD-

JD- commented Dec 31, 2012

Documentation for RKPaginator suggests this pagination mapping :

RKObjectMapping *paginationMapping = [RKObjectMapping mappingForClass:[RKPaginator class]];
[paginationMapping addAttributeMappingsFromDictionary:@{
    @"pagination.per_page",        @"perPage",
    @"pagination.total_pages",     @"pageCount",
    @"pagination.total_objects",   @"objectCount",
}];

Using this mapping, on the first call to the paginator, I get this error :
* Assertion failure in -[RKPaginator pageCount] [xxxx]/Pods/RestKit/Code/Network/RKPaginator.m:130

This error seems pretty logical to me since NSAssert([self hasPageCount]) (l130) is called during mapping, while pageCount is not initiliazed at that moment.

Having a look at RKPaginatorTest.m, I saw that a different mapping was used for testing :

 [paginationMapping addPropertyMapping:[RKAttributeMapping attributeMappingFromKeyPath:@"current_page" toKeyPath:@"currentPage"]];
    [paginationMapping addPropertyMapping:[RKAttributeMapping attributeMappingFromKeyPath:@"per_page" toKeyPath:@"perPage"]];
    [paginationMapping addPropertyMapping:[RKAttributeMapping attributeMappingFromKeyPath:@"total_entries" toKeyPath:@"objectCount"]];

This one does not use " pageCount " and works fine for me.

Maybe a simple correction of the API documentation would be a good fix.

@cmorss

This comment has been minimized.

Show comment Hide comment
@cmorss

cmorss Dec 31, 2012

Just ran across this same issue. Agreed, docs could use some updating. Thanks for pointing it out.

cmorss commented Dec 31, 2012

Just ran across this same issue. Agreed, docs could use some updating. Thanks for pointing it out.

@blakewatters

This comment has been minimized.

Show comment Hide comment
@blakewatters

blakewatters Dec 31, 2012

Member

Thanks for the head's up -- will update it.

Member

blakewatters commented Dec 31, 2012

Thanks for the head's up -- will update it.

@ghost ghost assigned blakewatters Jan 1, 2013

@blakewatters

This comment has been minimized.

Show comment Hide comment
@blakewatters

blakewatters Jan 2, 2013

Member

Fixed in a30263b

Member

blakewatters commented Jan 2, 2013

Fixed in a30263b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment