Skip to content
Open
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
6 changes: 6 additions & 0 deletions src/__snapshots__/markdown-renderer.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ exports[`MarkdownRenderer > renderContributionList > renders a list of contribut
* [#42](http://github.com/42) My cool PR ([@hzoo](http://hzoo.com))"
`;

exports[`MarkdownRenderer > renderContributorList > renders a list mixing a fallback (unfetchable) contributor with a full contributor 1`] = `
"#### Committers: 2
- @deleted-user
- Tobias Bieniek (@Turbo87)"
`;

exports[`MarkdownRenderer > renderContributorList > renders a list of GitHub users 1`] = `
"#### Committers: 2
- Tobias Bieniek ([@Turbo87](https://github.com/Turbo87))
Expand Down
191 changes: 191 additions & 0 deletions src/changelog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,5 +278,196 @@ describe('Changelog', () => {
},
]);
});
it('falls back gracefully when getUserData throws for a bot account (403)', async () => {
fetch.__setMockResponses({
'https://api.github.com/users/test-user-1': {
body: {
login: 'test-user-1',
html_url: 'https://github.com/test-user-1',
name: 'Test User 1',
},
},

// Simulate 403 - token lacks scope to fetch this bot's profile
'https://api.github.com/users/github-actions': {
status: 403,
statusText: 'Forbidden',
ok: false,
body: {
message: 'Resource not accessible by integration',
},
},
});

const changelog = new Changelog({ ignoreCommitters: [] });

const testCommits = [
{
commitSHA: 'a0000001',
githubIssue: {
user: {
login: 'test-user-1',
html_url: 'https://github.com/test-user-1',
},
},
},
{
commitSHA: 'a0000002',
githubIssue: {
user: {
login: 'github-actions',
html_url: 'https://github.com/apps/github-actions',
},
},
},
];

const committers = await changelog.getCommitters(testCommits);

expect(committers).toHaveLength(2);

expect(committers[0]).toEqual({
login: 'test-user-1',
html_url: 'https://github.com/test-user-1',
name: 'Test User 1',
});

// Falls back to login-only — no name, no type, html_url preserved from PR data
expect(committers[1]).toEqual({
login: 'github-actions',
html_url: 'https://github.com/apps/github-actions',
});
});
it('ignoreCommitters still works when mixed with failing getUserData', async () => {
fetch.__setMockResponses({
'https://api.github.com/users/real-user': {
body: {
login: 'real-user',
html_url: 'https://github.com/real-user',
name: 'Real User',
},
},
});

// ignored-bot is in ignoreCommitters: getUserData should never be called for it
const changelog = new Changelog({
ignoreCommitters: ['ignored-bot'],
});

const testCommits = [
{
commitSHA: 'a0000001',
githubIssue: {
user: {
login: 'real-user',
html_url: 'https://github.com/real-user',
},
},
},
{
commitSHA: 'a0000002',
githubIssue: {
user: {
login: 'ignored-bot',
html_url: 'https://github.com/ignored-bot',
},
},
},
];

const committers = await changelog.getCommitters(testCommits);

// ignored-bot skipped before getUserData is even attempted
expect(committers).toHaveLength(1);

expect(committers[0]).toEqual({
login: 'real-user',
html_url: 'https://github.com/real-user',
name: 'Real User',
});
});
it('falls back with empty html_url when PR user data has none', async () => {
fetch.__setMockResponses({});

// ignored-bot is in ignoreCommitters: getUserData should never be called for it
const changelog = new Changelog({
ignoreCommitters: [],
});

const testCommits = [
{
commitSHA: 'a0000001',
githubIssue: {
user: {
login: 'restricted-bot',
},
},
},
];

const committers = await changelog.getCommitters(testCommits);

// ignored-bot skipped before getUserData is even attempted
expect(committers).toHaveLength(1);

expect(committers[0]).toEqual({
login: 'restricted-bot',
html_url: '',
});
});
it('falls back with gracefully when getUserData throws for a deleted user', async () => {
fetch.__setMockResponses({
'https://api.github.com/users/test-user1': {
body: {
login: 'test-user1',
html_url: 'https://github.com/test-user1',
name: 'Test User 1',
},
},
// Simulate 404 - account was deleted aftr the pr was merged
'https://api.github.com/users/deleted-user': {
status: 404,
statusText: 'Not Found',
ok: false,
body: { message: 'Not Found' },
},
});
const changelog = new Changelog({ ignoreCommitters: [] });
const testCommits = [
{
commitSHA: 'a0000001',
githubIssue: {
user: {
login: 'test-user1',
html_url: 'https://github.com/test-user1',
},
},
},
{
commitSHA: 'a0000002',
githubIssue: {
user: {
login: 'deleted-user',
html_url: 'https://github.com/deleted-user',
},
},
},
];

const committers = await changelog.getCommitters(testCommits);

expect(committers).toHaveLength(2);

expect(committers[0]).toEqual({
login: 'test-user1',
html_url: 'https://github.com/test-user1',
name: 'Test User 1',
});
// Deleted account - falls back to login-only, html-url from PR data, no name
expect(committers[1]).toEqual({
login: 'deleted-user',
html_url: 'https://github.com/deleted-user',
});
});
});
});
12 changes: 11 additions & 1 deletion src/changelog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,17 @@ export default class Changelog {
const shouldKeepCommiter = login && !this.ignoreCommitter(login);

if (login && shouldKeepCommiter && !committers[login]) {
committers[login] = this.sanitizeCommitter(await this.github.getUserData(user));
try {
committers[login] = this.sanitizeCommitter(await this.github.getUserData(user));
} catch {
// getUserData can fail for various reasons (e.g. 403 for restricted bot accounts or 404 for deleted accounts).
// Fall back to a minimal entry using just the login
// so the changelog still generates rather than failing completely.
committers[login] = {
login,
html_url: user.html_url || '',
} as GitHubContributor;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`createMarkdown with an unfetchable committer > still generates a readable changelog when a bot committer is unfetchable 1`] = `
"
## Unreleased (2099-01-01)

#### :house: Maintenance
* [#2](https://github.com/embroider-build/github-changelog/pull/2) chore: bump dependencies ([@dependabot[bot]](https://github.com/apps/dependabot))

#### Committers: 2
- Real User ([@real-user](https://github.com/real-user))
- [@dependabot[bot]](https://github.com/apps/dependabot)"
`;
71 changes: 71 additions & 0 deletions src/functional/markdown-unfetchable-comitter.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { vi, describe, beforeEach, afterEach, it, expect } from 'vitest';

import * as git from '../git';
import Changelog from '../changelog';
import * as fetch from '../fetch';

vi.mock('../../src/progress-bar');
vi.mock('../changelog');
vi.mock('../../src/github-api');
vi.mock('../git');
vi.mock('../fetch');

describe('createMarkdown with an unfetchable committer', () => {
beforeEach(() => {
fetch.__resetMockResponses();

git.changedPaths.mockImplementation(() => []);
git.lastTag.mockImplementation(() => 'v1.0.0');
git.listTagNames.mockImplementation(() => ['v1.0.0']);
git.listCommits.mockImplementation(() => [
{
sha: 'a0000002',
refName: '',
summary: 'Merge pull request #2 from bump-deps',
date: '2024-01-02',
},
{
sha: 'a0000001',
refName: '',
summary: 'Merge pull request #1 from feature',
date: '2024-01-01',
},
]);

fetch.__setMockResponses({
'https://api.github.com/repos/embroider-build/github-changelog/issues/1': {
body: {
number: 1,
title: 'feat: add new feature',
labels: [{ name: 'New Feature' }],
pull_request: { html_url: 'https://github.com/embroider-build/github-changelog/pull/1' },
user: { login: 'real-user', html_url: 'https://github.com/real-user' },
},
},
'https://api.github.com/repos/embroider-build/github-changelog/issues/2': {
body: {
number: 2,
title: 'chore: bump dependencies',
labels: [{ name: 'Type: Maintenance' }],
pull_request: { html_url: 'https://github.com/embroider-build/github-changelog/pull/2' },
user: { login: 'dependabot[bot]', html_url: 'https://github.com/apps/dependabot' },
},
},
'https://api.github.com/users/real-user': {
body: { login: 'real-user', type: 'User', html_url: 'https://github.com/real-user', name: 'Real User' },
},
});
});

afterEach(() => {
vi.resetAllMocks();
});

it('still generates a readable changelog when a bot committer is unfetchable', async () => {
const changelog = new Changelog({ ignoreCommitters: [] });

const markdown = await changelog.createMarkdown();

expect(markdown).toMatchSnapshot();
});
});
38 changes: 37 additions & 1 deletion src/markdown-renderer.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { GithubAppInfo } from './github-api.js';
import type { GithubAppInfo, GitHubContributor } from './github-api.js';
import { CommitInfo, Release } from './interfaces.js';
import MarkdownRenderer from './markdown-renderer.js';

Expand Down Expand Up @@ -136,6 +136,24 @@ describe('MarkdownRenderer', () => {

expect(result).toMatchSnapshot();
});

it('renders a list mixing a fallback (unfetchable) contributor with a full contributor', () => {
const fallbackContributor = {
login: 'deleted-user',
html_url: '',
} as GitHubContributor;

const fullContributor = {
login: 'Turbo87',
name: 'Tobias Bieniek',
type: 'User',
html_url: '',
} as GitHubContributor;

const result = renderer().renderContributorList([fallbackContributor, fullContributor]);

expect(result).toMatchSnapshot();
});
});

describe('renderContributor', () => {
Expand Down Expand Up @@ -171,6 +189,24 @@ describe('MarkdownRenderer', () => {

expect(result).toEqual('Copilot [Bot] ([@copilot-swe-agent](https://github.com/apps/copilot-swe-agent))');
});

it(`renders a fallback contributor that has an html_url`, () => {
const result = renderer().renderContributor({
login: 'github-actions',
html_url: 'https://github.com/apps/github-actions',
} as GitHubContributor);

expect(result).toEqual('[@github-actions](https://github.com/apps/github-actions)');
});

it(`renders a fallback contributor with no html_url as a plain mention`, () => {
const result = renderer().renderContributor({
login: 'deleted-user',
html_url: '',
} as GitHubContributor);

expect(result).toEqual('@deleted-user');
});
});

describe('groupByCategory', () => {
Expand Down
4 changes: 3 additions & 1 deletion src/markdown-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ export default class MarkdownRenderer {

public renderContributor(contributor: GitHubContributor): string {
const userName = (contributor as GithubAppInfo).slug ?? (contributor as GithubUserInfo).login;
const userNameAndLink = `[@${userName}](${contributor.html_url})`;
// html_url can be empty when getCommiters falls back for unfetchable committers (e.g. deleted users, restricted users).
// Render a plain @login metion instead of a markdown link with an empty href.
const userNameAndLink = contributor.html_url ? `[@${userName}](${contributor.html_url})` : `@${userName}`;

if (contributor.name) {
const name = contributor.name + (!('type' in contributor) ? ' [Bot]' : '');
Expand Down