Conversation
src/github/pullRequestManager.ts
Outdated
| pageInformation.pullRequestPage++; | ||
| addPage(await fetchPage(pageInformation.pullRequestPage)); | ||
| } else { | ||
| if (pageInformation.pullRequestPage === 0) { pageInformation.pullRequestPage = 1; } |
There was a problem hiding this comment.
should pullRequestPage be initialized to 1 above instead?
There was a problem hiding this comment.
That would cause the "continue searching in other remotes" action, which triggers a "fetchNextPage" command, to pull from page 2.
This whole thing is a bit tricky because there are three behaviours:
- Initialize: fetch the first page of the first remote that has pages
- Fetch Next: fetch the next page from this remote, or if it has no more pages, the first page from the next remote that does have pages
- Restore (new): fetch all the pages you previously have fetched
And only 2 options: fetchNextPage: true | false.
So when fetchNextPage: true we do 2, and when it's false, we do either 1 or 3. The initial state of 0 pullRequestPages fetched so far is what distinguishes 1 and 3.
Working on a better solution now...
There was a problem hiding this comment.
Just pushed a reworking of the logic
Add some comments describing the logic. Fix bug where fetching the next page of a non-all query would fetch all.
| } else { | ||
| try { | ||
| const response = await this._prManager.getPullRequests(this._type, { fetchNextPage: true }); | ||
| const response = await this._prManager.getPullRequests(this._type, { fetchNextPage: true }, this._categoryQuery); |
There was a problem hiding this comment.
This was missing previously, causing "fetch more" on "created by me" (for instance), to just load the next page of "all"
| }; | ||
| } | ||
|
|
||
| public mayHaveMorePages(): boolean { |
There was a problem hiding this comment.
Unused and not really fundamentally compatible with the fact that there are many different queries that may or may not have more pages.
RMacfarlane
left a comment
There was a problem hiding this comment.
Nice, this looks good to me.
Fixes #1036.