Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix : Commit statuses from forks are not included #62

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions utils/buildQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ const pullRequestQuery = (name, owner, pr, sha) => `
commits(last: 1) {
nodes {
commit {
oid
author {
user {
url
}
}
statusCheckRollup {
state
contexts(last: 100) {
Expand Down Expand Up @@ -70,6 +76,13 @@ const commitStatusQuery = (name, owner, sha) => `
... on Commit {
statusCheckRollup {
state
commit {
author {
user {
url
}
}
}
contexts(last: 100) {
totalCount
pageInfo {
Expand Down
2 changes: 1 addition & 1 deletion utils/models/pullRequestStatus.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
REVIEW_APPROVED: '✔ This PR has been approved',
REVIEW_DISAPPROVED: 'ℹ There are requested changes on this PR - merging is blocked',
REVIEW_REQUIRED: 'ℹ This PR requires a review - merging is blocked',
STATUS_FAILURE: '⚠ Some status checks are not successful',
STATUS_FAILURE: '⚠ Some status checks were not successful',
STATUS_PENDING: 'ℹ Some status checks are pending',
BYPASSABLE: 'ℹ This PR requires a review before merging, but as an ADMIN with merge rights you can bypass that restriction',
};
83 changes: 83 additions & 0 deletions utils/runQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,52 @@
const { graphql } = require('@octokit/graphql');
const buildQuery = require('./buildQuery');

const getCommitChecks = async (name, owner, sha, token) => {
const q = `
repository(name: "${name}", owner: "${owner}") {
commit: object(expression: "${sha}") {
... on Commit {
statusCheckRollup {
state
contexts(last: 100) {
totalCount
pageInfo {
endCursor
hasNextPage
}
nodes {
__typename
... on CheckRun {
status
name
conclusion
}
... on StatusContext {
state
context
description
}
}
}
}
}
}
}
`;

try {
const response = await graphql(`{${q}}`, {
headers: {
authorization: `token ${token}`,
},
});
return response.repository.commit?.statusCheckRollup?.contexts?.nodes;
} catch (error) {
console.log(error);
return null;
}
};

module.exports = async function runQuery({ commit, repo, pr, sha, token }) {
const [owner, name] = repo.split('/');
let response = null;
Expand All @@ -12,6 +58,43 @@ module.exports = async function runQuery({ commit, repo, pr, sha, token }) {
authorization: `token ${token}`,
},
});
const { search, repository } = response;

const doNodes = async ({ commit }) =>{
const {
author: {
user: { url },
},
oid,
} = commit;
// get the commit checks for the commit that ran on the fork
await getCommitChecks(
name,
url.split('/').slice(-1).shift(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this also be url.slice(url.lastIndexOf('/'))?

oid,
token
).then((commitChecks) => {
const prchecks = commit.statusCheckRollup?.contexts.nodes ?? [];
const allChecks = prchecks.concat(commitChecks || []);
// modify the values only if checks are found
if (allChecks.length > 0) {
commit.statusCheckRollup.contexts.nodes = allChecks;
commit.statusCheckRollup.contexts.totalCount = allChecks.length;
Comment on lines +81 to +82
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way we could avoid mutation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How'd we proceed without mutating ? are you suggesting creating a new object ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, exactly

}

});
};

await search.edges.reduce(async (prev, edge) => {
await prev;
return edge.node.commits.nodes.reduce(
async (prev, node) => {
await prev;
return doNodes(node);
},
Promise.resolve()
);
});
} catch (err) {
throw err.message;
}
Expand Down