Changelog fixes (#73)

* fix: fetch github releases if package name in tag_name * fix: fetching releases with differring tag_names * refactor: use NPM_API_URL constant * feature: extract breaking changes from CHANGELOG.md or CHANGES.md * fix: fetching releases with differring tag_names part 2 * refactor: dont crash if version is not in releases * refactor: log message * change success message * feature: handle more changelog md formats * dont mark as OK PRs with @types updates --------- Co-authored-by: Lauris Skraucis <[email protected]>
getadaptly · Aug 15, 2023 · 5363432 · 5363432
1 parent 6a437b6
commit 5363432
Show file tree

Hide file tree

Showing 10 changed files with 244 additions and 49 deletions.
diff --git a/source/events/issue_handler/actions/created/commands/go/index.ts b/source/events/issue_handler/actions/created/commands/go/index.ts
@@ -32,7 +32,7 @@ export const go = async (payload: IssueCommentEvent, installationId: number, oct
 
     // await setupRepositoryLocally(payload, installationId, octokit);
 
-    const breakingChangesReports = await getBreakingChangesReports(updatedDependencies);
+    const breakingChangesReports = await getBreakingChangesReports(updatedDependencies, octokit);
 
     Logger.info(`Prepared breaking changes reports`, {
         repository: payload.repository.full_name,

diff --git a/.../issue_handler/actions/created/commands/go/source/breaking-changes/findBreakingChanges.ts b/.../issue_handler/actions/created/commands/go/source/breaking-changes/findBreakingChanges.ts
@@ -11,6 +11,7 @@ import { getMessageContent } from '@adaptly/services/openai/utils/getMessageCont
 import { tokenCount } from '@adaptly/services/openai/utils/tokenCount';
 import { GPT35_MODEL, GPT4_MODEL, MAX_NUM_TOKENS_8K } from '@adaptly/services/openai/client';
 import { getConversationContents } from '@adaptly/services/openai/utils/getConversationContents';
+import { Octokit } from '@octokit/core';
 
 export type BreakingChanges = {
     cursorVersion: string;
@@ -71,18 +72,28 @@ const breakingChangesDoubleCheckPrompt = `Please review your analysis, paying cl
 - For each breaking change found, double check that is fits the provided definition of "breaking change". If it doesn't, remove it from the list.
 - For each entry, confirm that the titles and descriptions are clear, concise, and would provide software engineers with an immediate understanding of the impact to their code and how to mitigate it.`;
 
-export async function findBreakingChanges(dependencyUpdate: DependencyUpdate): Promise<BreakingChanges> {
+export async function findBreakingChanges(dependencyUpdate: DependencyUpdate, octokit: Octokit): Promise<BreakingChanges> {
     let cursorVersion: string | undefined = moveCursorVersion(dependencyUpdate);
 
     if (dependencyUpdate.dependencyName.startsWith('@types/')) {
         return {
-            cursorVersion: dependencyUpdate.targetVersion,
-            changes: []
+            cursorVersion: dependencyUpdate.cursorVersion,
+            changes: [
+                {
+                    title: 'Adaptly ignores Type updates',
+                    description: 'Types have no clear source of change logs so Adaptly does not check Type updates'
+                }
+            ]
         };
     }
 
     while (cursorVersion) {
-        const breakingChanges = await extractBreakingChanges(dependencyUpdate.dependencyName, cursorVersion, dependencyUpdate.dependencyRepoUrl);
+        const breakingChanges = await extractBreakingChanges(
+            dependencyUpdate.dependencyName,
+            cursorVersion,
+            dependencyUpdate.dependencyRepoUrl,
+            octokit
+        );
 
         if (breakingChanges.length) {
             return {
@@ -100,15 +111,20 @@ export async function findBreakingChanges(dependencyUpdate: DependencyUpdate): P
     };
 }
 
-async function extractBreakingChanges(packageName: string, cursorVersion: string, dependecyRepoUrl: string): Promise<BreakingChange[]> {
+async function extractBreakingChanges(
+    packageName: string,
+    cursorVersion: string,
+    dependecyRepoUrl: string,
+    octokit: Octokit
+): Promise<BreakingChange[]> {
     const breakingChangesConversation: ChatCompletionRequestMessage[] = [];
 
     breakingChangesConversation.push({
         role: RoleSystem,
         content: `${breakingChangesPrompt}`
     });
 
-    const changelog = await getChangelog(dependecyRepoUrl, cursorVersion);
+    const changelog = await getChangelog(dependecyRepoUrl, cursorVersion, packageName, octokit);
 
     breakingChangesConversation.push({
         role: RoleUser,

diff --git a/source/events/issue_handler/actions/created/commands/go/source/breaking-changes/index.ts b/source/events/issue_handler/actions/created/commands/go/source/breaking-changes/index.ts
@@ -2,17 +2,18 @@ import { IssueCommentEvent } from '@octokit/webhooks-types';
 import { BreakingChange, findBreakingChanges } from './findBreakingChanges';
 import Logger from '@adaptly/logging/logger';
 import { DependencyUpdate } from '../pr-dependencies/getDependenciesUpdated';
+import { Octokit } from '@octokit/core';
 
 export type BreakingChangesReport = {
     dependencyUpdate: DependencyUpdate;
     breakingChanges: BreakingChange[];
 };
 
-export async function getBreakingChangesReports(updatedDependencies: DependencyUpdate[]): Promise<BreakingChangesReport[]> {
+export async function getBreakingChangesReports(updatedDependencies: DependencyUpdate[], octokit: Octokit): Promise<BreakingChangesReport[]> {
     const reports: BreakingChangesReport[] = [];
 
     for (const update of updatedDependencies) {
-        const breakingChanges = await findBreakingChanges(update);
+        const breakingChanges = await findBreakingChanges(update, octokit);
 
         reports.push({
             dependencyUpdate: {

diff --git a/source/events/issue_handler/actions/created/commands/go/source/breaking-changes/report.ts b/source/events/issue_handler/actions/created/commands/go/source/breaking-changes/report.ts
@@ -71,6 +71,8 @@ async function getBreakingChangesMessage(dependencyUpdate: DependencyUpdate, bre
         message += '\n\n</details>\n';
     } else {
         message = `:white_check_mark:&nbsp;&nbsp;No breaking changes found.\n\n`;
+        const releaseUrl = await getReleaseUrl(dependencyUpdate.dependencyRepoUrl, dependencyUpdate.cursorVersion);
+        message += `Package: [${dependencyUpdate.dependencyName}](${dependencyUpdate.dependencyUrl})\nVersion: [${dependencyUpdate.cursorVersion}](${releaseUrl})\n\n`;
     }
     message += getProgressMessage(dependencyUpdate);
 

diff --git a/...ssue_handler/actions/created/commands/go/source/pr-dependencies/getDependenciesUpdated.ts b/...ssue_handler/actions/created/commands/go/source/pr-dependencies/getDependenciesUpdated.ts
@@ -56,20 +56,17 @@ export async function getDependenciesUpdated(
 
                 const dependencyRepoUrl = await parser.getDependencyRepoUrl(dependency);
                 const dependencyUrl = parser.getDependencyUrl(dependency);
-                const versionsRange = await getVersionsRange(
+                const { versionsRange, prefix } = await getVersionsRange(
                     dependency,
                     dependencyRepoUrl,
                     dependenciesBase[dependency],
                     dependenciesHead[dependency],
                     octokit
                 );
 
-                const isVersionPrefixed = versionsRange.some((version) => version.startsWith('v'));
-
-                const currentVersionFormatted = isVersionPrefixed ? formatVersion(currentVersion) : currentVersion;
-                const cursorVersionFormatted = isVersionPrefixed ? formatVersion(cursorVersion) : cursorVersion;
-                const targetVersionFormatted = isVersionPrefixed ? formatVersion(targetVersion) : targetVersion;
-                const versionsRangeFormatted = isVersionPrefixed ? formatVersions(versionsRange) : versionsRange;
+                const currentVersionFormatted = prefix ? formatVersion(currentVersion, prefix) : currentVersion;
+                const cursorVersionFormatted = prefix ? formatVersion(cursorVersion, prefix) : cursorVersion;
+                const targetVersionFormatted = prefix ? formatVersion(targetVersion, prefix) : targetVersion;
 
                 updatedDependencies.push({
                     dependencyName: dependency,
@@ -78,7 +75,7 @@ export async function getDependenciesUpdated(
                     currentVersion: currentVersionFormatted,
                     cursorVersion: cursorVersionFormatted,
                     targetVersion: targetVersionFormatted,
-                    intermediaryVersions: versionsRangeFormatted,
+                    intermediaryVersions: versionsRange,
                     dirName: getPackageDirectory(manifestFilename),
                     manifestFilename
                 });
@@ -89,12 +86,8 @@ export async function getDependenciesUpdated(
     return updatedDependencies;
 }
 
-function formatVersions(versions: string[]): string[] {
-    return versions.map((version) => formatVersion(version));
-}
-
-function formatVersion(version: string): string {
-    return version.startsWith('v') ? version : `v${version}`;
+function formatVersion(version: string, prefix: string): string {
+    return version.startsWith(prefix) ? version : `${prefix}${version}`;
 }
 
 function getPackageDirectory(manifestFilename: string): string {

diff --git a/source/routes/canMergePR.ts b/source/routes/canMergePR.ts
@@ -61,7 +61,7 @@ type Verdict = {
 
 async function getVerdict(repoNameWithOwner: string, prNumber: number, octokit: Octokit): Promise<Verdict> {
     const updatedDependencies = await getPackagesDependenciesUpdated(repoNameWithOwner, prNumber, octokit);
-    const breakingChangesReports = await getBreakingChangesReports(updatedDependencies);
+    const breakingChangesReports = await getBreakingChangesReports(updatedDependencies, octokit);
 
     const reachedTargetVersion = breakingChangesReports.every(
         (report) => report.dependencyUpdate.cursorVersion === report.dependencyUpdate.targetVersion

diff --git a/source/services/adaptly/changelogHunter.ts b/source/services/adaptly/changelogHunter.ts
@@ -6,13 +6,51 @@ import Logger, { getMessage } from '@adaptly/logging/logger';
 import { GITHUB_API_URL } from '@adaptly/consts';
 import { ErrorHandler, GithubError } from '@adaptly/errors/types';
 import { ADAPTLY_ERRORS } from '@adaptly/errors';
+import { getFileContent } from '../github/contents/getContentFile';
+import { Octokit } from '@octokit/core';
+import { extractVersionChanges } from './extractVersionChanges';
 
-export const getChangelog = async (githubRepoUrl: string, targetVersion: string): Promise<string> => {
+export const getChangelog = async (githubRepoUrl: string, targetVersion: string, packageName: string, octokit: Octokit): Promise<string> => {
     const accessToken = getEnv('GITHUB_ACCESS_TOKEN');
 
-    const releaseNotes = await getReleaseNotes(githubRepoUrl, accessToken, targetVersion);
+    // note(Lauris): We need to try every possible way to fetch release notes because even if some of the version is
+    // prefixed with, for example, 'v', then it might be that some other is not. If all versions are prefixed, then they will come in already
+    // formatted with the prefix, so the first try below will succeed.
+    try {
+        const releaseNotes = await getReleaseNotes(githubRepoUrl, accessToken, targetVersion);
+        return releaseNotes;
+    } catch (error) {
+        Logger.info(`getChangelog: Could not fetch release notes for version: ${targetVersion}`);
+    }
+
+    try {
+        Logger.info('getChangelog: Trying to fetch with v prefix');
+        const releaseNotes = await getReleaseNotesWithPrefix(githubRepoUrl, accessToken, targetVersion, 'v');
+        return releaseNotes;
+    } catch (error) {
+        Logger.info(`getChangelog: Could not fetch release notes for version with v prefix v${targetVersion}`);
+    }
+
+    try {
+        Logger.info('getChangelog: Trying to fetch with package name prefix');
+        const releaseNotes = await getReleaseNotesWithPrefix(githubRepoUrl, accessToken, targetVersion, `${packageName}@`);
+        return releaseNotes;
+    } catch (error) {
+        Logger.info(`getChangelog: Could not fetch release notes for version with package name prefix ${packageName}@${targetVersion}`);
+    }
 
-    return releaseNotes;
+    try {
+        Logger.info('getChangelog: Trying to fetch changelog file');
+        const releaseNotes = await getChangelogMd(githubRepoUrl, targetVersion, octokit);
+        return releaseNotes;
+    } catch (error) {
+        Logger.info(`getChangelog: Could not fetch ${packageName} changelog file`);
+    }
+
+    // note(Lauris): above should cover 99% of cases and if there is an edge case ignore it for now.
+    // instead of crashing.
+    Logger.info('getChangelog: could not retrieve changelog. Returning default changelog: everything looks fine');
+    return 'Nothing has been changed. Everything will work perfectly';
 };
 
 const getReleaseNotes = async (githubRepoUrl: string, accessToken: string, targetVersion: string): Promise<string> => {
@@ -27,6 +65,55 @@ const getReleaseNotes = async (githubRepoUrl: string, accessToken: string, targe
     return response.data.body;
 };
 
+const getReleaseNotesWithPrefix = async (githubRepoUrl: string, accessToken: string, targetVersion: string, prefix: string): Promise<string> => {
+    Logger.info(`Looking for release notes in ${githubRepoUrl} for ${targetVersion}`);
+
+    const { repoOwner, repoName } = getRepoOwnerAndName(githubRepoUrl);
+
+    const releaseUrl = `${GITHUB_API_URL}/repos/${repoOwner}/${repoName}/releases/tags/${prefix}${targetVersion}`;
+
+    const response: AxiosResponse = await fetchReleaseNotes(releaseUrl, accessToken);
+
+    return response.data.body;
+};
+
+async function getChangelogMd(githubRepoUrl: string, targetVersion: string, octokit: Octokit): Promise<string> {
+    const { repoOwner, repoName } = getRepoOwnerAndName(githubRepoUrl);
+
+    try {
+        const content = await getFileContent(`${repoOwner}/${repoName}`, 'CHANGELOG.md', octokit);
+        const versionChanges = extractVersionChanges(content, targetVersion);
+        return versionChanges;
+    } catch (error) {
+        Logger.info("Couldn't find CHANGELOG.md");
+    }
+
+    try {
+        const content = await getFileContent(`${repoOwner}/${repoName}`, 'changelog.md', octokit);
+        const versionChanges = extractVersionChanges(content, targetVersion);
+        return versionChanges;
+    } catch (error) {
+        Logger.info("Couldn't find changelog.md");
+    }
+
+    try {
+        const content = await getFileContent(`${repoOwner}/${repoName}`, 'CHANGES.md', octokit);
+        const versionChanges = extractVersionChanges(content, targetVersion);
+        return versionChanges;
+    } catch (error) {
+        Logger.info("Couldn't find CHANGES.md");
+    }
+
+    try {
+        const content = await getFileContent(`${repoOwner}/${repoName}`, 'changes.md', octokit);
+        const versionChanges = extractVersionChanges(content, targetVersion);
+        return versionChanges;
+    } catch (error) {
+        Logger.info("Couldn't find changes.md");
+        throw error;
+    }
+}
+
 export function getRepoOwnerAndName(githubRepoUrl: string): { repoOwner: string; repoName: string } {
     const parsedUrl = new URL(githubRepoUrl);
     const pathParts = parsedUrl.pathname.split('/').filter((part) => part.length > 0);

diff --git a/source/services/adaptly/extractVersionChanges.test.ts b/source/services/adaptly/extractVersionChanges.test.ts
@@ -0,0 +1,48 @@
+import { extractVersionChanges } from './extractVersionChanges';
+
+describe('extractVersionChanges', () => {
+    const changelog = `
+    # Changelog
+    ## 1.0.44
+    Added a set_timeout method to the Client class in the Python SDK
+
+    ## 1.0.43
+    Added a setTimeout method
+    to the Configuration class in the PHP SDK.
+
+    ## 1.0.42
+  `;
+
+    it('should return the correct changes for a version that exists', () => {
+        const result = extractVersionChanges(changelog, '1.0.44');
+        expect(result).toBe('Added a set_timeout method to the Client class in the Python SDK');
+    });
+
+    it('should return the changes spread over multiple lines', () => {
+        const result = extractVersionChanges(changelog, '1.0.43');
+        expect(result).toBe('Added a setTimeout method\nto the Configuration class in the PHP SDK.');
+    });
+
+    it('should return the default message for a version without changes', () => {
+        const result = extractVersionChanges(changelog, '1.0.42');
+        expect(result).toBe('Nothing has been changed. Everything will work perfectly');
+    });
+
+    it('should return the default message for a version that does not exist', () => {
+        const result = extractVersionChanges(changelog, '1.0.45');
+        expect(result).toBe('Nothing has been changed. Everything will work perfectly');
+    });
+
+    it('should return the correct changes for a version that exists', () => {
+        const changelog2 = `
+          ## Version 6.1.0 (2019-08-19)
+          football
+      
+          ## Version 6.2.0 (2019-28-19)
+          basketball
+        `;
+
+        const result = extractVersionChanges(changelog2, '6.1.0');
+        expect(result).toBe('football');
+    });
+});
diff --git a/source/services/adaptly/extractVersionChanges.ts b/source/services/adaptly/extractVersionChanges.ts
@@ -0,0 +1,25 @@
+export function extractVersionChanges(changelog: string, version: string): string {
+    // Split the changelog into lines
+    const lines = changelog.split('\n');
+
+    // Find the line index of the specified version
+    const versionIndex = lines.findIndex((line) => line.trim().startsWith('#') && line.includes(version));
+
+    // If the version is not found, return the default message
+    if (versionIndex === -1) {
+        return 'Nothing has been changed. Everything will work perfectly';
+    }
+
+    // Find the index of the next header line, or use the end of the file if there's no next header
+    const nextHeaderIndex = lines.slice(versionIndex + 1).findIndex((line) => line.trim().startsWith('#'));
+    const endIndex = nextHeaderIndex !== -1 ? versionIndex + nextHeaderIndex + 1 : lines.length;
+
+    // Extract the lines between the version header and the next header (or the end of the file), trimming whitespace from each line
+    const changes = lines
+        .slice(versionIndex + 1, endIndex)
+        .map((line) => line.trim())
+        .join('\n')
+        .trim();
+
+    return changes || 'Nothing has been changed. Everything will work perfectly';
+}