diff --git a/lib/unfurls/comment.js b/lib/unfurls/comment.js index 5a69188df..cf7ab6d39 100644 --- a/lib/unfurls/comment.js +++ b/lib/unfurls/comment.js @@ -4,11 +4,11 @@ module.exports = async (params, github, unfurlType) => { const { owner, repo, number, id, } = params; - const issue = (await github.issues.get({ owner, repo, number })).data; + const issue = (await github.issues.get({ owner, repo, issue_number: number })).data; const comment = (await github.issues.getComment({ owner, repo, - id, + comment_id: id, headers: { accept: 'application/vnd.github.html+json' }, })).data; const repository = (await github.repos.get({ owner, repo })).data; diff --git a/lib/unfurls/index.js b/lib/unfurls/index.js index a61b61fdf..9139423f3 100644 --- a/lib/unfurls/index.js +++ b/lib/unfurls/index.js @@ -60,6 +60,7 @@ async function linkShared(req, res) { return; } if (err.name === 'ResourceNotFound') { + req.log.debug(err, 'Resource not found during unfurl.'); return; } if (err.name === 'UnfurlsAreDisabled') { diff --git a/test/fixtures/comment-rust.json b/test/fixtures/comment-rust.json new file mode 100644 index 000000000..d7089ec64 --- /dev/null +++ b/test/fixtures/comment-rust.json @@ -0,0 +1,31 @@ +{ + "url": "https://api.github.com/repos/rust-lang/rust/issues/comments/526118914", + "html_url": "https://github.com/rust-lang/rust/issues/63997#issuecomment-526118914", + "issue_url": "https://api.github.com/repos/rust-lang/rust/issues/63997", + "id": 526118914, + "node_id": "MDEyOklzc3VlQ29tbWVudDUyNjExODkxNA==", + "user": { + "login": "gnzlbg", + "id": 904614, + "node_id": "MDQ6VXNlcjkwNDYxNA==", + "avatar_url": "https://avatars0.githubusercontent.com/u/904614?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/gnzlbg", + "html_url": "https://github.com/gnzlbg", + "followers_url": "https://api.github.com/users/gnzlbg/followers", + "following_url": "https://api.github.com/users/gnzlbg/following{/other_user}", + "gists_url": "https://api.github.com/users/gnzlbg/gists{/gist_id}", + "starred_url": "https://api.github.com/users/gnzlbg/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/gnzlbg/subscriptions", + "organizations_url": "https://api.github.com/users/gnzlbg/orgs", + "repos_url": "https://api.github.com/users/gnzlbg/repos", + "events_url": "https://api.github.com/users/gnzlbg/events{/privacy}", + "received_events_url": "https://api.github.com/users/gnzlbg/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2019-08-29T10:07:04Z", + "updated_at": "2019-08-29T10:14:44Z", + "author_association": "CONTRIBUTOR", + "body": "I think that, at the very least, this should work:\r\n\r\n```rust\r\nconst fn foo() {}\r\nconst FOO: const fn() = foo;\r\nconst fn bar() { FOO() }\r\nconst fn baz(x: const fn()) { x() }\r\nconst fn bazz() { baz(FOO) }\r\n```\r\n\r\nFor this to work:\r\n\r\n* `const` must be part of `fn` types (just like `unsafe`, the `extern \"ABI\"`, etc.)\r\n* we should allow calling `const fn` types from const fn\r\n\r\nCurrently, `const fn`s already coerce to `fn`s, so `const fn` types should too:\r\n\r\n```rust\r\nconst fn foo() {}\r\nlet x: const fn() = foo;\r\nlet y: fn() = x; // OK: const fn => fn coercion\r\n```\r\n\r\nI don't see any problems with supporting this. The RFC mentions some issues, but I don't see anything against just supporting this restricted subset.\r\n\r\nThis subset would be **super** useful. For example, you could do:\r\n\r\n```rust\r\nstruct Foo(T);\r\ntrait Bar { const F: const fn(Self) -> Self; }\r\n\r\nimpl Foo {\r\n const fn new(x: T) -> Self { Foo(::F(x)) }\r\n}\r\n\r\nconst fn map_i32(x: i32) -> i32 { x * 2 }\r\nimpl Bar for i32 { const F: const fn(Self) -> Self = map_i32; } \r\nconst fn map_u32(x: i32) -> i32 { x * 3 }\r\nimpl Bar for u32 { const F: const fn(Self) -> Self = map_u32; } \r\n```\r\n\r\nwhich is a quite awesome work around for the lack of `const` trait methods, but much simpler since dynamic dispatch isn't an issue, as opposed to: \r\n\r\n```rust\r\ntrait Bar { const fn map(self) -> Self; } \r\nimpl Bar for i32 { ... }\r\nimpl Bar for u32 { ... }\r\n// or const impl Bar for i32 { ... {\r\n```\r\n\r\nThis is also a way to avoid having to use `if`/`match` etc. in `const fn`s, since you can create a trait with a const, and just dispatch on it to achieve \"conditional\" control-flow at least at compile-time." +} diff --git a/test/fixtures/index.js b/test/fixtures/index.js index 222fb3a26..8949853ff 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -7,11 +7,14 @@ module.exports = { installation: require('./installation'), pull: require('./pull'), comment: require('./comment'), + commentRust: require('./comment-rust'), release: require('./release'), contents: require('./contents'), user: require('./user'), org: require('./org'), repo: require('./repo'), + repoRust: require('./repo-rust'), + repoHub: require('./repo-hub'), atomRepo: require('./atom-repo'), kubernetesRepo: require('./kubernetes-repo'), combinedStatus: require('./combined_status_some_passing'), diff --git a/test/fixtures/repo-hub.json b/test/fixtures/repo-hub.json new file mode 100644 index 000000000..030a6299e --- /dev/null +++ b/test/fixtures/repo-hub.json @@ -0,0 +1,122 @@ +{ + "id": 401025, + "node_id": "MDEwOlJlcG9zaXRvcnk0MDEwMjU=", + "name": "hub", + "full_name": "github/hub", + "private": false, + "owner": { + "login": "github", + "id": 9919, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjk5MTk=", + "avatar_url": "https://avatars1.githubusercontent.com/u/9919?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/github", + "html_url": "https://github.com/github", + "followers_url": "https://api.github.com/users/github/followers", + "following_url": "https://api.github.com/users/github/following{/other_user}", + "gists_url": "https://api.github.com/users/github/gists{/gist_id}", + "starred_url": "https://api.github.com/users/github/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/github/subscriptions", + "organizations_url": "https://api.github.com/users/github/orgs", + "repos_url": "https://api.github.com/users/github/repos", + "events_url": "https://api.github.com/users/github/events{/privacy}", + "received_events_url": "https://api.github.com/users/github/received_events", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/github/hub", + "description": "A command-line tool that makes git easier to use with GitHub.", + "fork": false, + "url": "https://api.github.com/repos/github/hub", + "forks_url": "https://api.github.com/repos/github/hub/forks", + "keys_url": "https://api.github.com/repos/github/hub/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/github/hub/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/github/hub/teams", + "hooks_url": "https://api.github.com/repos/github/hub/hooks", + "issue_events_url": "https://api.github.com/repos/github/hub/issues/events{/number}", + "events_url": "https://api.github.com/repos/github/hub/events", + "assignees_url": "https://api.github.com/repos/github/hub/assignees{/user}", + "branches_url": "https://api.github.com/repos/github/hub/branches{/branch}", + "tags_url": "https://api.github.com/repos/github/hub/tags", + "blobs_url": "https://api.github.com/repos/github/hub/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/github/hub/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/github/hub/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/github/hub/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/github/hub/statuses/{sha}", + "languages_url": "https://api.github.com/repos/github/hub/languages", + "stargazers_url": "https://api.github.com/repos/github/hub/stargazers", + "contributors_url": "https://api.github.com/repos/github/hub/contributors", + "subscribers_url": "https://api.github.com/repos/github/hub/subscribers", + "subscription_url": "https://api.github.com/repos/github/hub/subscription", + "commits_url": "https://api.github.com/repos/github/hub/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/github/hub/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/github/hub/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/github/hub/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/github/hub/contents/{+path}", + "compare_url": "https://api.github.com/repos/github/hub/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/github/hub/merges", + "archive_url": "https://api.github.com/repos/github/hub/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/github/hub/downloads", + "issues_url": "https://api.github.com/repos/github/hub/issues{/number}", + "pulls_url": "https://api.github.com/repos/github/hub/pulls{/number}", + "milestones_url": "https://api.github.com/repos/github/hub/milestones{/number}", + "notifications_url": "https://api.github.com/repos/github/hub/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/github/hub/labels{/name}", + "releases_url": "https://api.github.com/repos/github/hub/releases{/id}", + "deployments_url": "https://api.github.com/repos/github/hub/deployments", + "created_at": "2009-12-05T22:15:25Z", + "updated_at": "2019-08-30T08:57:14Z", + "pushed_at": "2019-08-27T10:56:22Z", + "git_url": "git://github.com/github/hub.git", + "ssh_url": "git@github.com:github/hub.git", + "clone_url": "https://github.com/github/hub.git", + "svn_url": "https://github.com/github/hub", + "homepage": "https://hub.github.com/", + "size": 6319, + "stargazers_count": 17223, + "watchers_count": 17223, + "language": "Go", + "has_issues": true, + "has_projects": true, + "has_downloads": false, + "has_wiki": false, + "has_pages": true, + "forks_count": 1758, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 201, + "license": { + "key": "mit", + "name": "MIT License", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit", + "node_id": "MDc6TGljZW5zZTEz" + }, + "forks": 1758, + "open_issues": 201, + "watchers": 17223, + "default_branch": "master", + "organization": { + "login": "github", + "id": 9919, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjk5MTk=", + "avatar_url": "https://avatars1.githubusercontent.com/u/9919?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/github", + "html_url": "https://github.com/github", + "followers_url": "https://api.github.com/users/github/followers", + "following_url": "https://api.github.com/users/github/following{/other_user}", + "gists_url": "https://api.github.com/users/github/gists{/gist_id}", + "starred_url": "https://api.github.com/users/github/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/github/subscriptions", + "organizations_url": "https://api.github.com/users/github/orgs", + "repos_url": "https://api.github.com/users/github/repos", + "events_url": "https://api.github.com/users/github/events{/privacy}", + "received_events_url": "https://api.github.com/users/github/received_events", + "type": "Organization", + "site_admin": false + }, + "network_count": 1758, + "subscribers_count": 335 +} diff --git a/test/fixtures/repo-rust.json b/test/fixtures/repo-rust.json new file mode 100644 index 000000000..f67598435 --- /dev/null +++ b/test/fixtures/repo-rust.json @@ -0,0 +1,122 @@ +{ + "id": 724712, + "node_id": "MDEwOlJlcG9zaXRvcnk3MjQ3MTI=", + "name": "rust", + "full_name": "rust-lang/rust", + "private": false, + "owner": { + "login": "rust-lang", + "id": 5430905, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjU0MzA5MDU=", + "avatar_url": "https://avatars1.githubusercontent.com/u/5430905?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/rust-lang", + "html_url": "https://github.com/rust-lang", + "followers_url": "https://api.github.com/users/rust-lang/followers", + "following_url": "https://api.github.com/users/rust-lang/following{/other_user}", + "gists_url": "https://api.github.com/users/rust-lang/gists{/gist_id}", + "starred_url": "https://api.github.com/users/rust-lang/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/rust-lang/subscriptions", + "organizations_url": "https://api.github.com/users/rust-lang/orgs", + "repos_url": "https://api.github.com/users/rust-lang/repos", + "events_url": "https://api.github.com/users/rust-lang/events{/privacy}", + "received_events_url": "https://api.github.com/users/rust-lang/received_events", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/rust-lang/rust", + "description": "Empowering everyone to build reliable and efficient software.", + "fork": false, + "url": "https://api.github.com/repos/rust-lang/rust", + "forks_url": "https://api.github.com/repos/rust-lang/rust/forks", + "keys_url": "https://api.github.com/repos/rust-lang/rust/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/rust-lang/rust/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/rust-lang/rust/teams", + "hooks_url": "https://api.github.com/repos/rust-lang/rust/hooks", + "issue_events_url": "https://api.github.com/repos/rust-lang/rust/issues/events{/number}", + "events_url": "https://api.github.com/repos/rust-lang/rust/events", + "assignees_url": "https://api.github.com/repos/rust-lang/rust/assignees{/user}", + "branches_url": "https://api.github.com/repos/rust-lang/rust/branches{/branch}", + "tags_url": "https://api.github.com/repos/rust-lang/rust/tags", + "blobs_url": "https://api.github.com/repos/rust-lang/rust/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/rust-lang/rust/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/rust-lang/rust/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/rust-lang/rust/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/rust-lang/rust/statuses/{sha}", + "languages_url": "https://api.github.com/repos/rust-lang/rust/languages", + "stargazers_url": "https://api.github.com/repos/rust-lang/rust/stargazers", + "contributors_url": "https://api.github.com/repos/rust-lang/rust/contributors", + "subscribers_url": "https://api.github.com/repos/rust-lang/rust/subscribers", + "subscription_url": "https://api.github.com/repos/rust-lang/rust/subscription", + "commits_url": "https://api.github.com/repos/rust-lang/rust/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/rust-lang/rust/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/rust-lang/rust/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/rust-lang/rust/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/rust-lang/rust/contents/{+path}", + "compare_url": "https://api.github.com/repos/rust-lang/rust/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/rust-lang/rust/merges", + "archive_url": "https://api.github.com/repos/rust-lang/rust/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/rust-lang/rust/downloads", + "issues_url": "https://api.github.com/repos/rust-lang/rust/issues{/number}", + "pulls_url": "https://api.github.com/repos/rust-lang/rust/pulls{/number}", + "milestones_url": "https://api.github.com/repos/rust-lang/rust/milestones{/number}", + "notifications_url": "https://api.github.com/repos/rust-lang/rust/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/rust-lang/rust/labels{/name}", + "releases_url": "https://api.github.com/repos/rust-lang/rust/releases{/id}", + "deployments_url": "https://api.github.com/repos/rust-lang/rust/deployments", + "created_at": "2010-06-16T20:39:03Z", + "updated_at": "2019-08-30T09:39:24Z", + "pushed_at": "2019-08-30T09:31:46Z", + "git_url": "git://github.com/rust-lang/rust.git", + "ssh_url": "git@github.com:rust-lang/rust.git", + "clone_url": "https://github.com/rust-lang/rust.git", + "svn_url": "https://github.com/rust-lang/rust", + "homepage": "https://www.rust-lang.org", + "size": 455465, + "stargazers_count": 38763, + "watchers_count": 38763, + "language": "Rust", + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": false, + "has_pages": false, + "forks_count": 6026, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 5180, + "license": { + "key": "other", + "name": "Other", + "spdx_id": "NOASSERTION", + "url": null, + "node_id": "MDc6TGljZW5zZTA=" + }, + "forks": 6026, + "open_issues": 5180, + "watchers": 38763, + "default_branch": "master", + "organization": { + "login": "rust-lang", + "id": 5430905, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjU0MzA5MDU=", + "avatar_url": "https://avatars1.githubusercontent.com/u/5430905?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/rust-lang", + "html_url": "https://github.com/rust-lang", + "followers_url": "https://api.github.com/users/rust-lang/followers", + "following_url": "https://api.github.com/users/rust-lang/following{/other_user}", + "gists_url": "https://api.github.com/users/rust-lang/gists{/gist_id}", + "starred_url": "https://api.github.com/users/rust-lang/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/rust-lang/subscriptions", + "organizations_url": "https://api.github.com/users/rust-lang/orgs", + "repos_url": "https://api.github.com/users/rust-lang/repos", + "events_url": "https://api.github.com/users/rust-lang/events{/privacy}", + "received_events_url": "https://api.github.com/users/rust-lang/received_events", + "type": "Organization", + "site_admin": false + }, + "network_count": 6026, + "subscribers_count": 1420 +} diff --git a/test/integration/__snapshots__/unfurl.test.js.snap b/test/integration/__snapshots__/unfurl.test.js.snap index 454660fe6..686c1c108 100644 --- a/test/integration/__snapshots__/unfurl.test.js.snap +++ b/test/integration/__snapshots__/unfurl.test.js.snap @@ -643,6 +643,21 @@ Array [ ] `; +exports[`Integration: unfurls public unfurls issue comments 1`] = ` +Array [ + "unfurl-eligibility#T000A:C74M:https://github.com/rust-lang/rust/issues/63997#issuecomment-526118914", +] +`; + +exports[`Integration: unfurls public unfurls issue comments 2`] = ` +Array [ + Array [ + "unfurl-eligibility#T000A:C74M:https://github.com/rust-lang/rust/issues/63997#issuecomment-526118914", + true, + ], +] +`; + exports[`Integration: unfurls public unfurls only unfurls link first time a link is shared 1`] = ` Object { "channel": "C74M", diff --git a/test/integration/unfurl.test.js b/test/integration/unfurl.test.js index f31f35968..37cc3defc 100644 --- a/test/integration/unfurl.test.js +++ b/test/integration/unfurl.test.js @@ -53,6 +53,47 @@ describe('Integration: unfurls', () => { .expect(200); }); + test('issue comments', async () => { + // nock the repo for the isPrivate check + nock('https://api.github.com').get('/repos/rust-lang/rust') + .reply(200, { ...fixtures.repoRust }); + + // nock the repo's issue for the fetching comments + nock('https://api.github.com').get('/repos/rust-lang/rust/issues/63997') + .reply(200, {}); + + // nock fetching the actual comment + nock('https://api.github.com') + .get('/repos/rust-lang/rust/issues/comments/526118914') + .reply(200, { ...fixtures.commentRust }); + + // nock the repo again, becasue we re-fetch it in the issueComment :/ + nock('https://api.github.com').get('/repos/rust-lang/rust') + .reply(200, { ...fixtures.repo_rust }); + + nock('https://slack.com') + .post('/api/chat.unfurl') + .reply(200, { ok: true }); + + + const issueCommentEvent = { + ...fixtures.slack.link_shared().event, + links: [ + { + url: 'https://github.com/rust-lang/rust/issues/63997#issuecomment-526118914', + domain: 'github.com', + }, + ], + }; + + const response = await request(probot.server).post('/slack/events').send({ + ...fixtures.slack.link_shared(), + event: issueCommentEvent, + }); + expect(response.status).toEqual(200); + }); + + test('only unfurls link first time a link is shared', async () => { nock('https://api.github.com').get('/repos/bkeepers/dotenv').times(2).reply( 200,