Add pagination for .all methods #96

2019-04-12T19:24:41Z

fsmanuel commented

2019-04-12 19:24:41 +00:00

(Migrated from github.com)

I removed rsvp as a dependency as we only use Promise.all

Since the setup only works for me with yarn it would be great if someone can run npm install and push the package-lock.json into this branch.

I removed rsvp as a dependency as we only use Promise.all Since the setup only works for me with yarn it would be great if someone can run `npm install` and push the package-lock.json into this branch.

bumi (Migrated from github.com) approved these changes 2019-04-13 07:22:04 +00:00

bumi (Migrated from github.com) left a comment

looks great!
awesome that those for loops are gone.

looks great! awesome that those for loops are gone.

lib/utils/pagination.js

						
				@@ -0,0 +34,4 @@

				  return Array.from({ length: size }, mapFunction);

				}

				module.exports = function paged(recordCount, options = {}) {

bumi (Migrated from github.com) commented

2019-04-13 07:18:26 +00:00

the call would be somthing like:
page(100, { order: 'desc', page: { size: 10, number: 1} }) ?

was wondering if that is less verbose?
page(100, { order: 'desc', per_page: 10, page: 1 }) - but that's probably my ruby will_paginate head :D

the call would be somthing like: `page(100, { order: 'desc', page: { size: 10, number: 1} })` ? was wondering if that is less verbose? `page(100, { order: 'desc', per_page: 10, page: 1 })` - but that's probably my ruby will_paginate head :D

fsmanuel (Migrated from github.com) reviewed 2019-04-13 10:29:53 +00:00

lib/utils/pagination.js

						
				@@ -0,0 +34,4 @@

				  return Array.from({ length: size }, mapFunction);

				}

				module.exports = function paged(recordCount, options = {}) {

fsmanuel (Migrated from github.com) commented

2019-04-13 10:29:52 +00:00

Yeah, I'm not sure which one is better. I took it from JSON:API spec but I don't care how we do it.
@skddc what is your preference?

Yeah, I'm not sure which one is better. I took it from [JSON:API spec](https://jsonapi.org/format/#fetching-pagination) but I don't care how we do it. @skddc what is your preference?

fsmanuel commented

2019-04-13 10:38:51 +00:00

(Migrated from github.com)

One thing I noticed is that we don't return any information on how many records/pages we have. This is not a problem if we use infinite scrolling as the code always checks that the results are in range. But if we want to have a pagination with pages in the frontend that might be a problem. We can always use functions.*Count but would duplicate logic...
To make it easier I'll add a count property on the contract and we can iterate from there.

One thing I noticed is that we don't return any information on how many records/pages we have. This is not a problem if we use infinite scrolling as the code always checks that the results are in range. But if we want to have a pagination with pages in the frontend that might be a problem. We can always use `functions.*Count` but would duplicate logic... To make it easier I'll add a `count` property on the contract and we can iterate from there.

fsmanuel commented

2019-04-13 11:07:28 +00:00

(Migrated from github.com)

Updated the code. Not sure if Record is the best name...

Updated the code. Not sure if `Record` is the best name...

raucao commented

2019-04-13 11:30:33 +00:00

(Migrated from github.com)

Very cool stuff. I'll work on using the contracts in production before getting to this, so that the kredits on these PRs can actually be recorded. ;)

🎉 1

bumi (Migrated from github.com) reviewed 2019-04-13 18:02:18 +00:00

lib/utils/pagination.js

						
				@@ -0,0 +34,4 @@

				  return Array.from({ length: size }, mapFunction);

				}

				module.exports = function paged(recordCount, options = {}) {

bumi (Migrated from github.com) commented

2019-04-13 18:02:18 +00:00

ah ok, then I guess JSON:API spec is better!

bumi commented

2019-04-13 18:08:32 +00:00

(Migrated from github.com)

I think we can optimize if we run in that problem, so a count function should be enough now.

so let's make this the first recorded github contribution on or rinkeby production org.
I will merge this once hubot is deployed and configured...(and I hope it then works :D)

I think we can optimize if we run in that problem, so a `count` function should be enough now. so let's make this the first recorded github contribution on or rinkeby production org. I will merge this once hubot is deployed and configured...(and I hope it then works :D)

fsmanuel commented

2019-04-14 12:07:41 +00:00

(Migrated from github.com)

@bumi What do you need for my contributor profile? Can we take my old one?

bumi commented

2019-04-14 17:02:21 +00:00

(Migrated from github.com)

see contributor schema.
ethereum address, name, github user+id, maybe wiki username

see contributor schema. ethereum address, name, github user+id, maybe wiki username

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/contracts#96