Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Implements Pagination (fixes #25) #94

Merged
merged 14 commits into from
Feb 9, 2015
Merged

Implements Pagination (fixes #25) #94

merged 14 commits into from
Feb 9, 2015

Conversation

Natim
Copy link
Contributor

@Natim Natim commented Feb 5, 2015

No description provided.

@Natim Natim force-pushed the 25_pagination branch 4 times, most recently from b254d8f to d65c2cd Compare February 6, 2015 09:38
@Natim
Copy link
Contributor Author

Natim commented Feb 6, 2015

r? @leplatrem

'description': "_limit should be an integer"
}
self.request.errors.add(**error_details)
raise errors.json_error(self.request.errors)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have a self.raise_invalid :)

}
self.db.create(self.resource, 'bob', record)
self.resource.request.GET = {}
self.last_response.headers = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be necessary

@leplatrem
Copy link
Contributor

Nice !

I suggest we add a few more tests that cover these parts of the code:

  • pagination_rules in test_backend (small test)
  • lt_ and gt_ in test_filter
  • _to in test_sync
  • _limit not an integer returns 400
  • _token not base64
  • _token not json
  • Take out b64encode(json.dumps(token).encode('utf-8')).decode('utf-8')
  • add some unit tests for expected set of filters returned by _build_pagination_token. There are a lot of internal corner cases in this function, and it definitely deserves a dedicated test suite IHMO

@Natim Natim force-pushed the 25_pagination branch 4 times, most recently from 9830a0c to 31e2a22 Compare February 6, 2015 15:40
limit = int(limit)
except ValueError:
error_details = {
'name': None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name None is default and could be dropped here (maybe your prefer explicit)

@leplatrem
Copy link
Contributor

like @ametaireau would say : oh yeah :) 👍

@leplatrem leplatrem merged commit 98d4674 into master Feb 9, 2015
@Natim Natim deleted the 25_pagination branch February 9, 2015 10:00
@leplatrem leplatrem modified the milestone: 0.2 Feb 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants