Skip to content

Commit acb8a2f

Browse files
Stop filtering out PRs that were also backported to previous versions (#57)
* Stop filtering out PRs that were also backported to previous versions * remove step for selecting unreleased version numbers * remove filterPrsForVersion * cleanup semver parsing * cleanup debugging code * cleanup manualLabel * use reducer * use actual version in tooltip * move to utils. adjust type * add unit tests --------- Co-authored-by: Brad White <[email protected]>
1 parent d1ab661 commit acb8a2f

File tree

11 files changed

+153
-230
lines changed

11 files changed

+153
-230
lines changed

src/common/github-service.ts

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,25 +43,6 @@ interface ExtractDeployedShaParams extends ServerlessGitOpsParams {
4343
gitOpsSha: string;
4444
}
4545

46-
const SEMVER_REGEX = /^v(\d+)\.(\d+)\.(\d+)$/;
47-
48-
function filterPrsForVersion(
49-
prs: PrItem[],
50-
version: string,
51-
ignoredVersionLabels: readonly string[] = []
52-
): PrItem[] {
53-
return prs.filter((pr) => {
54-
const prVersions = pr.labels
55-
.filter((label) => label.name?.match(SEMVER_REGEX))
56-
.filter((label) => label.name && !ignoredVersionLabels.includes(label.name))
57-
.map((label) => semver.clean(label.name ?? '') ?? '');
58-
// Check if there is any version label below the one we are looking for
59-
// which would mean this PR has already been released (and blogged about)
60-
// in an earlier dev documentation blog post.
61-
return !prVersions.some((verLabel) => semver.lt(verLabel, version));
62-
});
63-
}
64-
6546
export class GitHubService {
6647
private octokit: Octokit;
6748
private repoId: number | undefined;
@@ -231,15 +212,13 @@ export class GitHubService {
231212
const options = this.octokit.search.issuesAndPullRequests.endpoint.merge({
232213
q: `repo:${GITHUB_OWNER}/${this.repoName} label:release_note:plugin_api_changes label:${version}`,
233214
});
234-
const items = await this.octokit.paginate<PrItem>(options);
235-
return filterPrsForVersion(items, version);
215+
return await this.octokit.paginate<PrItem>(options);
236216
}
237217

238218
public async getPrsForVersion(
239219
version: string,
240220
excludedLabels: readonly string[] = [],
241-
includedLabels: readonly string[] = [],
242-
ignoredVersionLabels: readonly string[] = []
221+
includedLabels: readonly string[] = []
243222
): Promise<Observable<Progress<PrItem>>> {
244223
const semVer = semver.parse(version);
245224

@@ -281,7 +260,7 @@ export class GitHubService {
281260
(async () => {
282261
const items: PrItem[] = [];
283262
for await (const response of this.octokit.paginate.iterator<PrItem>(options)) {
284-
items.push(...filterPrsForVersion(response.data, version, ignoredVersionLabels));
263+
items.push(...response.data);
285264
if (response.headers.link) {
286265
const links = parseLinkHeader(response.headers.link);
287266
if (links?.last?.page) {

src/common/pr-utils/extracting.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { PrItem } from '../github-service';
2-
import { normalizeTitle, findReleaseNote, extractReleaseNotes } from './extracting';
2+
import {
3+
normalizeTitle,
4+
findReleaseNote,
5+
extractReleaseNotes,
6+
hasDuplicatePatchLabels,
7+
} from './extracting';
38

49
describe('extraction tools', () => {
510
describe('normalizeTitle', () => {
@@ -142,4 +147,50 @@ Next paragraph
142147
expect(actual.title).toBe('Adds cool feature in *TSVB*');
143148
});
144149
});
150+
151+
describe('hasDuplicatePatchLabels', () => {
152+
it('should return false for invalid or missing target versions', () => {
153+
const labels = [{ name: '8.12.0' }, { name: '8.12.1' }];
154+
155+
expect(hasDuplicatePatchLabels(labels, undefined)).toBe(false);
156+
expect(hasDuplicatePatchLabels(labels, 'invalid-version')).toBe(false);
157+
});
158+
159+
it('should return false when there are no matching labels', () => {
160+
expect(hasDuplicatePatchLabels([{ name: '7.11.0' }, { name: '8.11.1' }], '8.12.0')).toBe(
161+
false
162+
);
163+
});
164+
165+
it('should return false when there is only one matching label', () => {
166+
expect(hasDuplicatePatchLabels([{ name: '8.12.0' }, { name: '7.11.1' }], '8.12.0')).toBe(
167+
false
168+
);
169+
});
170+
171+
it('should return true when there are multiple matching patch labels', () => {
172+
expect(hasDuplicatePatchLabels([{ name: '8.12.5' }, { name: '8.12.9' }], '8.12.0')).toBe(
173+
true
174+
);
175+
});
176+
177+
it('should handle invalid and undefined label names correctly', () => {
178+
expect(
179+
hasDuplicatePatchLabels(
180+
[
181+
{ name: '8.12.0' },
182+
{ name: 'bug' },
183+
{ name: '8.12.1' },
184+
{ name: 'feature' },
185+
{ name: undefined },
186+
],
187+
'8.12.0'
188+
)
189+
).toBe(true);
190+
});
191+
192+
it('should handle empty array of labels correctly', () => {
193+
expect(hasDuplicatePatchLabels([], '8.12.0')).toBe(false);
194+
});
195+
});
145196
});

src/common/pr-utils/extracting.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Config } from '../../config';
22
import { PrItem } from '../github-service';
3+
import semver from 'semver';
34

45
export type NormalizeOptions = Config['areas'][number]['options'];
56

@@ -117,3 +118,35 @@ export function extractReleaseNotes(
117118
originalTitle: pr.title,
118119
};
119120
}
121+
122+
/**
123+
* Checks if a PR has multiple patch version labels for the same major.minor version.
124+
* This indicates the PR may have been documented in multiple patch releases.
125+
*/
126+
export function hasDuplicatePatchLabels(
127+
prLabels: PrItem['labels'],
128+
targetVersion: string | undefined
129+
): boolean {
130+
const targetSemVer = semver.parse(targetVersion);
131+
if (!targetSemVer) {
132+
return false;
133+
}
134+
135+
// Find all version labels that match the same major.minor as target
136+
const sameMajorMinor = prLabels.reduce<number>((acc, { name }) => {
137+
const ver = semver.parse(name ?? '');
138+
139+
if (ver === null) {
140+
return acc;
141+
}
142+
143+
// tilde matches exactly on major.minor but allows for any patch version
144+
if (semver.intersects(`~${ver.version}`, `~${targetSemVer.version}`)) {
145+
acc++;
146+
}
147+
148+
return acc;
149+
}, 0);
150+
151+
return sameMajorMinor >= 2;
152+
}

src/common/pr-utils/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
export * from './grouping';
2-
export { extractReleaseNotes } from './extracting';
2+
export { extractReleaseNotes, hasDuplicatePatchLabels } from './extracting';
33
export type { NormalizeOptions, ReleaseNoteDetails } from './extracting';

src/common/pr.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
import { EuiLink, EuiIconTip } from '@elastic/eui';
22
import { FC, memo } from 'react';
33
import { PrItem } from './github-service';
4-
import { extractReleaseNotes, NormalizeOptions, ReleaseNoteDetails } from './pr-utils';
4+
import {
5+
extractReleaseNotes,
6+
NormalizeOptions,
7+
ReleaseNoteDetails,
8+
hasDuplicatePatchLabels,
9+
} from './pr-utils';
510

611
interface PrProps {
712
pr: PrItem;
813
showAuthor?: boolean;
914
showTransformedTitle?: boolean;
1015
normalizeOptions?: NormalizeOptions;
16+
version?: string;
1117
}
1218

1319
export const Pr: FC<PrProps> = memo(
14-
({ pr, showAuthor, showTransformedTitle, normalizeOptions }) => {
20+
({ pr, showAuthor, showTransformedTitle, normalizeOptions, version }) => {
1521
const title: ReleaseNoteDetails = showTransformedTitle
1622
? extractReleaseNotes(pr, normalizeOptions)
1723
: { type: 'title', title: pr.title };
24+
const hasDuplicates = hasDuplicatePatchLabels(pr.labels, version);
25+
const majorMinorVersion =
26+
version?.substring(0, version?.lastIndexOf('.')) ?? 'this major and minor version';
27+
1828
return (
1929
<>
2030
{title.title} (
@@ -27,6 +37,14 @@ export const Pr: FC<PrProps> = memo(
2737
</>
2838
)}
2939
){' '}
40+
{hasDuplicates && (
41+
<EuiIconTip
42+
color="warning"
43+
type="alert"
44+
size="m"
45+
content={`This PR has multiple patch version labels for ${majorMinorVersion} and may have already been documented in a previous patch release.`}
46+
/>
47+
)}
3048
{title.type === 'releaseNoteTitle' && (
3149
<EuiIconTip
3250
color="secondary"

src/pages/release-notes/components/grouped-pr-list.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ interface Props {
77
groupedPrs: { [group: string]: PrItem[] };
88
groups: Config['areas'];
99
keyPrefix: string;
10+
version: string;
1011
}
1112

12-
export const GroupedPrList: FC<Props> = memo(({ groupedPrs, groups, keyPrefix }) => {
13+
export const GroupedPrList: FC<Props> = memo(({ groupedPrs, groups, keyPrefix, version }) => {
1314
const sortedGroups = useMemo(
1415
() => [...groups].sort((a, b) => a.title.localeCompare(b.title)),
1516
[groups]
@@ -37,7 +38,12 @@ export const GroupedPrList: FC<Props> = memo(({ groupedPrs, groups, keyPrefix })
3738
<ul>
3839
{prs.map((pr) => (
3940
<li key={pr.id}>
40-
<Pr pr={pr} showTransformedTitle={true} normalizeOptions={group.options} />
41+
<Pr
42+
pr={pr}
43+
showTransformedTitle={true}
44+
normalizeOptions={group.options}
45+
version={version}
46+
/>
4147
</li>
4248
))}
4349
</ul>

src/pages/release-notes/components/uncategorized-pr.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { setConfig, useActiveConfig } from '../../../config';
1717

1818
interface UncategorizedPrProps {
1919
pr: PrItem;
20+
version: string;
2021
}
2122

2223
const LabelBadge: FC<{ label: Label }> = memo(({ label }) => {
@@ -83,7 +84,7 @@ const LabelBadge: FC<{ label: Label }> = memo(({ label }) => {
8384
);
8485
});
8586

86-
export const UncategorizedPr: FC<UncategorizedPrProps> = memo(({ pr }) => {
87+
export const UncategorizedPr: FC<UncategorizedPrProps> = memo(({ pr, version }) => {
8788
// We only want to show non version non release_note labels in the UI
8889
const filteredLables = useMemo(
8990
() =>
@@ -95,7 +96,7 @@ export const UncategorizedPr: FC<UncategorizedPrProps> = memo(({ pr }) => {
9596
return (
9697
<EuiSplitPanel.Outer>
9798
<EuiSplitPanel.Inner paddingSize="s">
98-
<Pr pr={pr} showAuthor={true} />
99+
<Pr pr={pr} showAuthor={true} version={version} />
99100
</EuiSplitPanel.Inner>
100101
<EuiSplitPanel.Inner paddingSize="s" color="subdued">
101102
{filteredLables.length > 0 && (

src/pages/release-notes/page.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@ import { ReleaseNotesWizard } from './wizard';
44

55
export const ReleaseNotesPage: FC = () => {
66
const [selectedVersion, setSelectedVersion] = useState<string>();
7-
const [ignoredVersions, setIgnoredVersions] = useState<string[]>([]);
87
const [selectedServerlessSHAs, setSelectedServerlessSHAs] = useState<Set<string>>(new Set());
98

10-
const onVersionChange = useCallback((version: string, ignoreVersions: string[] = []) => {
9+
const onVersionChange = useCallback((version: string) => {
1110
setSelectedVersion(version);
12-
setIgnoredVersions(ignoreVersions);
1311
}, []);
1412

1513
return (
@@ -24,7 +22,6 @@ export const ReleaseNotesPage: FC = () => {
2422
{selectedVersion && (
2523
<ReleaseNotes
2624
version={selectedVersion}
27-
ignoredPriorReleases={ignoredVersions}
2825
selectedServerlessSHAs={selectedServerlessSHAs}
2926
onVersionChange={() => setSelectedVersion(undefined)}
3027
/>

src/pages/release-notes/prepare-release-notes.tsx

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import { GroupedPrList, UncategorizedPr } from './components';
77

88
interface Props {
99
prs: PrItem[];
10+
version: string;
1011
}
1112

12-
export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
13+
export const PrepareReleaseNotes: FC<Props> = ({ prs, version }) => {
1314
const config = useActiveConfig();
1415
const groupedPrs = useMemo(() => groupPrs(prs), [prs]);
1516

@@ -49,7 +50,7 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
4950
<ul>
5051
{groupedPrs.missingLabel.map((pr) => (
5152
<li key={pr.id}>
52-
<Pr pr={pr} showAuthor={true} />
53+
<Pr pr={pr} showAuthor={true} version={version} />
5354
</li>
5455
))}
5556
</ul>
@@ -73,7 +74,7 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
7374
<EuiSpacer size="m" />
7475
{unknownPrs.map((pr) => (
7576
<React.Fragment key={pr.id}>
76-
<UncategorizedPr pr={pr} />
77+
<UncategorizedPr pr={pr} version={version} />
7778
<EuiSpacer size="s" />
7879
</React.Fragment>
7980
))}
@@ -87,7 +88,7 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
8788
<ul>
8889
{groupedPrs.breaking.map((pr) => (
8990
<li key={`breaking-${pr.id}`}>
90-
<Pr pr={pr} showTransformedTitle={true} />
91+
<Pr pr={pr} showTransformedTitle={true} version={version} />
9192
</li>
9293
))}
9394
</ul>
@@ -101,7 +102,7 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
101102
<ul>
102103
{groupedPrs.deprecation.map((pr) => (
103104
<li key={`deprecation-${pr.id}`}>
104-
<Pr pr={pr} showTransformedTitle={true} />
105+
<Pr pr={pr} showTransformedTitle={true} version={version} />
105106
</li>
106107
))}
107108
</ul>
@@ -112,7 +113,12 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
112113
<h2>
113114
Features (<EuiCode>release_note:feature</EuiCode>)
114115
</h2>
115-
<GroupedPrList groupedPrs={featurePrs} groups={config.areas} keyPrefix="features" />
116+
<GroupedPrList
117+
groupedPrs={featurePrs}
118+
groups={config.areas}
119+
keyPrefix="features"
120+
version={version}
121+
/>
116122
</>
117123
)}
118124
{Object.keys(enhancementPrs).length > 0 && (
@@ -124,6 +130,7 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
124130
groupedPrs={enhancementPrs}
125131
groups={config.areas}
126132
keyPrefix="enhancements"
133+
version={version}
127134
/>
128135
</>
129136
)}
@@ -132,7 +139,12 @@ export const PrepareReleaseNotes: FC<Props> = ({ prs }) => {
132139
<h2>
133140
Fixes (<EuiCode>release_note:fix</EuiCode>)
134141
</h2>
135-
<GroupedPrList groupedPrs={fixesPr} groups={config.areas} keyPrefix="fixes" />
142+
<GroupedPrList
143+
groupedPrs={fixesPr}
144+
groups={config.areas}
145+
keyPrefix="fixes"
146+
version={version}
147+
/>
136148
</>
137149
)}
138150
</EuiText>

0 commit comments

Comments
 (0)