`show-open-prs-of-forks` includes unrelated PRs #2934
fregante posted onGitHub
- Visit one of your (source) repos where you opened PRs
- Fork it
- Open it
- Notice: your existing PRs are counted in this feature
In my case, I have this dummy fork https://github.com/fregante/refined-github but all of PRs are directly on the source (https://github.com/sindresorhus/refined-github)
<img width="364" alt="" src="https://user-images.githubusercontent.com/1402241/77570114-91196b00-6ecb-11ea-8762-a48694535e69.png">
cc @loilo
Fair point. I'll take care of it. 🙂
Oof. From what I can tell, GitHub has no search filter in place to limit results to a certain head repo.
This creates two problems:
- We can't determine the exact number of open PRs without fetching a lot more data from the API than we do now. (At least I think we can't. There may be a possibility to utilize the GraphQL API in a way which can filter for certain nested node values. But since I have no considerable knowledge of GraphQL, I can't tell whether or not this is possible.)
- We also can't reliably link to a list of open PRs since that link is entirely composed of issue search filters.
I'm not sure how to handle this.
We could solve part of it by checking if the user has write access not only to the forked repo but also to the source repo. This should be solvable in one API request and could be cached for a long time.
- If the user has no write access to the source repo, we should be fine keeping the current process in place.
- If the user has write access to the source repo (as in your reproduction setup), we may consider fetching individual pull requests from the API and filtering them by head repo on the client side. We then could forego linking to a list of PRs and just tell the user about the fact that there are open PRs.
What do you think @fregante?
Here: https://developer.github.com/v4/explorer/ We'd have to fetch all your PR info, not just the first one, but it's not a lot of data.
query {
search(
first: 100,
type: ISSUE,
query: "repo:sindresorhus/refined-github is:pr is:open author:fregante"
) {
nodes {
... on PullRequest {
baseRepository {
nameWithOwner
}
number
}
}
}
}
{
"data": {
"search": {
"nodes": [
{
"baseRepository": {
"nameWithOwner": "sindresorhus/refined-github"
},
"number": 2925
}
]
}
}
}
Then something like:
const prs = search.nodes.filter(pr => pr.baseRepository.nameWithOwner === getRepoURL());
Yes, I had tried it out in the explorer, that's basically how I imagined it to be implemented. The "lot" of data was more in comparison with before, and it grows linearly with number of PRs.
I think this would be a reasonable implementation. In the (probably a bit absurd) case that somebody has > 100 open PRs, I'd bail out of counting.
How'd you deal with the link problem?
I don’t expect anyone to have a considerable amount of PRs open from a fork, they’re distant outliers.
As for the link, there’s no direct fix, but:
- I’d expect people to open PRs from the fork OR the source, not at the same time (generally)
- if they do have mixed repos, our
pr-branches
feature will help them understand which is which
Alternatively, I think that the search accepts searching by number and logical operators. If that works, we could link to the search Not possibleissue:2 OR issue:30 OR issue:40
Doesn't seem to be possible though. 🙁
If some one wants to send a PR I tested https://github.com/sindresorhus/refined-github/issues/2934#issuecomment-604146813 and it looks fine to me. (I changed it to headRepository because I think that is correct) https://github.com/yakov116/refined-github/commit/b698cf3f3e8f2f5d38e815f4eedbd2cb9aa47888
@@ -17,24 +17,28 @@ const countPRs = cache.function(async (forkedRepo: string): Promise<[number, num
// This allows to link to the PR directly if only one is found
const {search} = await api.v4(`
search(
- first: 1,
+ first: 100,
type: ISSUE,
query: "repo:${forkedRepo} is:pr is:open author:${getUsername()}"
) {
- issueCount
nodes {
... on PullRequest {
number
+ headRepository {
+ nameWithOwner
+ }
}
}
}
`);
- if (search.issueCount === 1) {
- return [1, search.nodes[0].number];
+ const prs = search.nodes.filter((pr: AnyObject) => pr.headRepository.nameWithOwner.toLowerCase() === getRepoURL());
+
+ if (prs.length === 1) {
+ return [1, prs[0].number];
}
- return [search.issueCount];
+ return [prs.length];
}, {
maxAge: 1 / 2, // Stale after 12 hours
staleWhileRevalidate: 2,