olavim / objection-cursor

Cursor based pagination plugin for Objection.js
MIT License
30 stars 8 forks source link

fix previous remaining calculation error #6

Closed idododu closed 5 years ago

idododu commented 5 years ago

when back to previous page, remaining should be total - rs rather than rs - models.length

eg: we have 16 records, and we are on the 2nd page(11 - 16) when click previous btn, rs is equal to 10 since we are moving to records(1 - 10), and models.length is also 10 which cause remaining equals to 0, this happens on sql like select * from user order by name asc, id asc

olavim commented 5 years ago

Thank you for the pull request!

However, this is working as expected. The remaining value should tell you how many results there are remaining in this direction.


So, let's say we have 25 items, and we are on the first page, viewing items 0-9 (limit is 10):

go to next page

go to next page

go to previous page

go to previous page

go to previous page


Could you verify if I understood your problem correctly? The expected behavior is currently documented briefly in https://github.com/olavim/objection-cursor#cursorpagecursor-before. If you think the documentation should be improved, let me know.

idododu commented 5 years ago

in my opinion, if we are visiting the same page, api should return the same pageInfo result no matter the direction is after or before. Otherwise we have to implement direction detect in our codes to calculate start position and end position of current page, which is not convinient. Hope below codes make sense.

// we have 25 records in total

// visit 1st page, api returns below info
const pageInfo = {
  remaining: 15,
  hasNext: true,
  hasPrevious: false,
  total: 25
}

// visit 2nd page, api returns below info
const pageInfo = {
  remaining: 5,
  hasNext: true,
  hasPrevious: true,
  total: 25
}

// when back to the 1st page, api returns below info
const pageInfo = {
  remaining: 0,
  hasNext: true,
  hasPrevious: false,
  total: 25
}

// get current page info
const pagesize = 10
const totalPages = Math.ceil(pageInfo.total / pagesize)

let currentPage
if(pageInfo.remaining == 0) {
  // it is the last page
  currentPage = totalPages
} else {
  /**
   * Here we hope we can use below calculation directly rather
   * currentPage = (pageInfo.total - pageInfo.remaining) / pagesize
   */
  if(direction == 'after') {
    currentPage = (pageInfo.total - pageInfo.remaining) / pagesize
  } else {
    currentPage = totalPages - ((pageInfo.total - pageInfo.remaining) / pagesize)
  }
}

const startPosition = currentPage * pagesize - 9
const endPosition = pageInfo.remaining == 0 ? pageInfo.total : currentPage * pagesize
olavim commented 5 years ago

in my opinion, if we are visiting the same page, api should return the same pageInfo result no matter the direction is after or before.

You might not get the same page info for the same logical page, but unless data has changed, the page info will always be the same for a given cursor. Unless data has changed, an input will give the same output.

But I understand the need for direction detection as it stands. We could deprecate the remaining property and introduce remainingBefore and remainingAfter in its stead. Would this work for you?

idododu commented 5 years ago

forget it.

Do we have any plan to support pagination by current page index rather than cursor?

olavim commented 5 years ago

forget it

I don't understand. remainingBefore and remainingAfter wouldn't solve your problem? I just don't want to change the semantics of remaining, and it's working as I intended it to.

Do we have any plan to support pagination by current page index rather than cursor?

Pagination by page should be well supported by objection already https://vincit.github.io/objection.js/recipes/paging.html#paging.

olavim commented 5 years ago

Features discussed here have been implemented in https://github.com/olavim/objection-cursor/pull/7, together with some additional fixes.