PageInfo.hasPreviousPage is false on second page

[daironmichel]

Hi guys,

I'm using graphene==1.1.3 with graphene-django==1.2.1. I'm following the Relay specification and I'm having an issue with pagination on connections.

When paginating forward using the first and after arguments getting the first page is working fine but from the second page forward hasPreviousPage comes back false. For example, on a list of 20 users the endCursor for the first page of 5 items is "abc5". If I ask for the second page of 5 items:

query { users(first: 5, after: "abc5"){ 
  pageInfo {hasPreviousPage hasNextPage startCursor endCursor} 
}}

I get back:

{ "data": { "users": {
    "pageInfo": {
      "hasPreviousPage": false,    <-- This should be true
      "hasNextPage": true,
      "startCursor": "abc6",
       "endCursor": "abc10"
    }
}}}

Same thing happens when paginating backwards but with hasNextPage. When I ask for the ...(last: 5, before: "abc16") then hasNextPage comes back as false.

I think it's a bug on the connection_from_list_slice method in this file. And here is a fragment of the code where I think the problem is:

def connection_from_list_slice(list_slice, args=None, connection_type=None,
                               edge_type=None, pageinfo_type=None,
                               slice_start=0, list_length=0, list_slice_length=None):
    # ...

    first_edge_cursor = edges[0].cursor if edges else None
    last_edge_cursor = edges[-1].cursor if edges else None
    lower_bound = after_offset + 1 if after else 0
    upper_bound = before_offset if before else list_length

    return connection_type(
        edges=edges,
        page_info=pageinfo_type(
            start_cursor=first_edge_cursor,
            end_cursor=last_edge_cursor,
            has_previous_page=isinstance(last, int) and start_offset > lower_bound,
            has_next_page=isinstance(first, int) and end_offset < upper_bound
        )
    )

    # ...

[LexFrench]

I'm seeing the same issue as well even when not using the graphene-django plugin.

[chrisbrantley]

I have this problem as well. According to the spec it seems like this is expected behavior but for the life of me I can't understand why.

[syrusakbary]

I'm closing the issue here, as is more related on the spec than this implementation. Feel free to open an issue in the GraphQL relay spec: https://github.com/facebook/relay

[janosroden]

Hi @syrusakbary, coud you please reopen this issue because the spec changed on 12 Sep 2017?

Current spec

[jkimbo]

Hi @daironmichel . We're currently going through old issues that appear to have gone stale (ie. not updated in about the last 3 months) to try and clean up the issue tracker. If this is still important to you please comment and we'll re-open this.

Thanks!

[janosroden]

Hi, I'd like to keep this issue open if you don't mind. Thanks!

[djsmedes]

Though I find this fairly annoying, I do understand why the Relay spec allows this - it can apparently reduce server load, and the client doesn't need the server to tell it the missing info. When you're paginating, the client knows there is a previous page if you just clicked "next"; you must have clicked "next" from somewhere. Implementation details:

• Maintain local state for hasNext and hasPrev with 3 possible values, true, false, and null in addition to the results of your GraphQL query
• Have some kind of function that first uses hasX local state if it's non-null, otherwise checking the result of the GraphQL query
• When you trigger your GraphQL query to get the next page, set local hasPrev to true (we know there's a previous, since you must have clicked "next" from somewhere) and hasNext to null (so that our function above uses the result of the query, which must contain hasNextPage according to the Relay spec). When you query the previous page, set hasPrev to null and hasNext to true.

Here's a concrete example, using Vue and Vue-Apollo:

export default {
  apollo: {
    myQuery: {
      query: gql`
        query myPaginatedQuery(
          $first: Int
          $last: Int
          $after: String
          $before: String
        ) {
          myQuery(
            first: $first
            after: $after
            last: $last
            before: $before
          ) {
            edges {
              node {
                uuid
                name
              }
            }
            pageInfo {
              hasNextPage
              hasPreviousPage
              startCursor
              endCursor
            }
          }
        }
      `,
      variables() {
        return {
          first: this.itemsPerPage,
          after: null,
        };
      },
    },
  },
  data() {
    return {
      itemsPerPage: 10,
      hasNextPage: null,
      hasPrevPage: null,
    };
  },
  computed: {
    // use these to determine whether there is actually a previous or next page
    canNextPage() {
      if (this.hasNextPage !== null) {
        return this.hasNextPage;
      } else if (this.myQuery) {
        return this.myQuery.pageInfo.hasNextPage;
      } else {
        return false;
      }
    },
    canPrevPage() {
      if (this.hasPrevPage !== null) {
        return this.hasPrevPage;
      } else if (this.myQuery) {
        return this.myQuery.pageInfo.hasPreviousPage;
      } else {
        return false;
      }
    },
  },
  methods: {
    async showMore(previous = false) {
      await this.$apollo.queries.myQuery.fetchMore({
        variables: previous
          ? {
              last: this.itemsPerPage,
              before: this.combatantSet.pageInfo.startCursor,
            }
          : {
              first: this.itemsPerPage,
              after: this.combatantSet.pageInfo.endCursor,
            },
        updateQuery: (priorResult, { fetchMoreResult }) => fetchMoreResult,
      });
      if (previous) {
        this.hasPrevPage = null;
        this.hasNextPage = true;
      } else {
        this.hasPrevPage = true;
        this.hasNextPage = null;
      }
    },
  },
};

[tony]

Related: https://github.com/graphql-python/graphql-relay-py/pull/14, https://github.com/facebook/relay/pull/2079

[tony]

@jkimbo Can you keep this open? I don't believe this is fixed.

[kkinder]

I'm also a little flummoxed. The spec reads:

HasPreviousPage(allEdges, before, after, first, last)

1. If last is set: a. Let edges be the result of calling ApplyCursorsToEdges(allEdges, before, after). b. If edges contains more than last elements return true, otherwise false.
2. If after is set: a. If the server can efficiently determine that elements exist prior to after, return true.
3. Return false.

In this instance, after is set and there's really no reason the server can't efficiently determine that a prior page exists. At the very least, it should be an option on one's Connection class to spend the extra few cycles to look backwards in the index.

I think for a lot of purposes, it would make sense for hasPreviousPage to report on whether the server has a previous page, and to my reading of the spec, while that's not required, it is allowed.

[ghost]

I have this problem as well. Is there any workaround or solution to this issue yet?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[wkoot]

I think this works as intended and is not a bug (the naming is just unclear); the idea is that the server can efficiently determine if there's a previous page. Efficient in this sense apparently means 'without executing additional queries' and 'without maintaining state'.

So in that sense If the server can efficiently determine that elements exist prior to after, return true. would for instance be possible when you're iterating over rows of which the pks are strictly sequential, without gaps and with a known start id. Then the server can infer previous pages by inspecting the pks in the current page.

For instance, iterating through 1, 2, 3, 4, 5, 6 with a page size of 2 would be as follows:

• [1, 2,] 3, 4, 5, 6 => hasNextPage:true, hasPreviousPage:false
• 1, 2, [3, 4,] 5, 6 => hasNextPage:true, hasPreviousPage:true
• 1, 2, 3, 4, [5, 6] => hasNextPage:false, hasPreviousPage:true

If there's gaps in the list, e.g. 3, 8, 9, 15, 16, 20, the server cannot and will return false:

• [3, 8,] 9, 15, 16, 20 => hasNextPage:true, hasPreviousPage:false
• 3, 8, [9, 15,] 16, 20 => hasNextPage:true, hasPreviousPage:false
• 3, 8, 9, 15, [16, 20] => hasNextPage:false, hasPreviousPage:false

By maintaining state in the frontend as suggested above, you can avoid relying on solely on what the backend is reporting. I agree that it's counterintuitive, though :-)

[xtream1101]

I came across this issue because I was running into the same problem. I ended up solving this by creating a custom connection class that I add to all my DjangoObjectType classes

import base64

class CustomRelayConnection(graphene.relay.Connection):

    class Meta:
        abstract = True

    def resolve_page_info(self, info, **args):
        # Needed to have custom has next/prev page values

        if hasattr(self, "iterable") and hasattr(self, "edges"):
            # Next page logic
            has_next_page = False
            count = self.iterable.count()
            last_cursor_index = 0
            if self.edges:
                last_cursor_index = int(
                    base64.b64decode(self.edges[-1].cursor)
                    .decode("utf-8")
                    .split(":")[1]
                )
            if last_cursor_index + 1 < count:
                has_next_page = True

            # Previous page logic
            has_previous_page = False
            first_cursor_index = 0
            if self.edges:
                first_cursor_index = int(
                    base64.b64decode(self.edges[0].cursor).decode("utf-8").split(":")[1]
                )
            if first_cursor_index != 0:
                has_previous_page = True

            return graphene.relay.PageInfo(
                has_next_page=has_next_page,
                has_previous_page=has_previous_page,
                # These values are fine and can reuse
                start_cursor=self.page_info.start_cursor,
                end_cursor=self.page_info.end_cursor,
            )
        return None

Then on each of my DjangoObjectType classes:

class ThingOneType(DjangoObjectType):
    class Meta:
        # all your usual bits stay the same, but add connection_class
        connection_class = CustomRelayConnection

Hopefully this helps anyone else that comes across this issue.

[WojciechMigda]

The simplest and most high-level workaround to this issue is to simply issue one more query. Using pageInfo.startCursor from last pagination response you should ask for preceding cursor value:

query previousCursor($before: String!) {
  myQuery(last: 1, before: $before) {
    pageInfo {
      endCursor
    }
  }
}

using pageInfo.startCursor as "before" value in query's variables. If resulting pageInfo.endCursor is not null then there is a previous page (note: this describes forward pagination).