Fix bug introduced to updateChangelog in #158 (#181)

## Motivation

#158 incorrectly refactored the branches in updateChangelog, resulting in a state where new changelog entries weren't added to the return value if isReleaseCandidate is false.

## Explanation

- This error is fixed by moving the logic for retrieving new entries out of the if (isReleaseCandidate) block.
- The code for retrieving new entries is also extracted into its own method: getNewChangeEntries, abstracting away details that obscured the flow of the method's core logic.
- To verify that the buggy logic is correctly restored, it's necessary to compare the current state of updateChangelog to its state before the bug was introduced. For this, see: [diff link](https://github.com/MetaMask/auto-changelog/compare/e8df1ec717f534c8fe84c46ea86a847fa5a32973..da39a58a55571cf4e8a03e28096aa12c02c75f77#diff-4228e8302e41dd1c51e813af8368efdcb40b3d9db3e3f21f417ae87275d9f389R249-R316).

## References

- Closes #180
- Followed by #188

src/update-changelog.ts

@@ -2,7 +2,7 @@ import { strict as assert } from 'assert';
 import execa from 'execa';
 
 import type Changelog from './changelog';
-import { Formatter } from './changelog';
+import { Formatter, getKnownPropertyNames } from './changelog';
 import { ChangeCategory, Version } from './constants';
 import { parseChangelog } from './parse-changelog';
 import { PackageRename } from './shared-types';
@@ -20,6 +20,9 @@ async function getMostRecentTag({
 }: {
   tagPrefixes: [string, ...string[]];
 }) {
+  // Ensure we have all tags on remote
+  await runCommand('git', ['fetch', '--tags']);
+
   let mostRecentTagCommitHash: string | null = null;
   for (const tagPrefix of tagPrefixes) {
     const revListArgs = [
@@ -157,6 +160,53 @@ async function getCommitHashesInRange(
   return await runCommand('git', revListArgs);
 }
 
+type AddNewCommitsOptions = {
+  mostRecentTag: string | null;
+  repoUrl: string;
+  loggedPrNumbers: string[];
+  projectRootDirectory?: string;
+};
+
+/**
+ * Get the list of new change entries to add to a changelog.
+ *
+ * @param options - Options.
+ * @param options.mostRecentTag - The most recent tag.
+ * @param options.repoUrl - The GitHub repository URL for the current project.
+ * @param options.loggedPrNumbers - A list of all pull request numbers included in the relevant parsed changelog.
+ * @param options.projectRootDirectory - The root project directory, used to
+ * filter results from various git commands. This path is assumed to be either
+ * absolute, or relative to the current directory. Defaults to the root of the
+ * current git repository.
+ * @returns A list of new change entries to add to the changelog, based on commits made since the last release.
+ */
+async function getNewChangeEntries({
+  mostRecentTag,
+  repoUrl,
+  loggedPrNumbers,
+  projectRootDirectory,
+}: AddNewCommitsOptions) {
+  const commitRange =
+    mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
+  const commitsHashesSinceLastRelease = await getCommitHashesInRange(
+    commitRange,
+    projectRootDirectory,
+  );
+  const commits = await getCommits(commitsHashesSinceLastRelease);
+
+  const newCommits = commits.filter(
+    ({ prNumber }) => !prNumber || !loggedPrNumbers.includes(prNumber),
+  );
+
+  return newCommits.map(({ prNumber, description }) => {
+    if (prNumber) {
+      const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
+      return `${description} ${suffix}`;
+    }
+    return description;
+  });
+}
+
 export type UpdateChangelogOptions = {
   changelogContent: string;
   currentVersion?: Version;
@@ -213,43 +263,17 @@ export async function updateChangelog({
     packageRename,
   });
 
-  // Ensure we have all tags on remote
-  await runCommand('git', ['fetch', '--tags']);
   const mostRecentTag = await getMostRecentTag({
     tagPrefixes,
   });
 
-  const commitRange =
-    mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
-  const commitsHashesSinceLastRelease = await getCommitHashesInRange(
-    commitRange,
-    projectRootDirectory,
-  );
-  const commits = await getCommits(commitsHashesSinceLastRelease);
-
-  const loggedPrNumbers = getAllLoggedPrNumbers(changelog);
-  const newCommits = commits.filter(
-    ({ prNumber }) =>
-      prNumber === undefined || !loggedPrNumbers.includes(prNumber),
-  );
-
-  const hasUnreleasedChanges =
-    Object.keys(changelog.getUnreleasedChanges()).length !== 0;
-  if (
-    newCommits.length === 0 &&
-    (!isReleaseCandidate || hasUnreleasedChanges)
-  ) {
-    return undefined;
-  }
-
   if (isReleaseCandidate) {
     if (!currentVersion) {
       throw new Error(
         `A version must be specified if 'isReleaseCandidate' is set.`,
       );
     }
-
-    if (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) {
+    if (mostRecentTag === `${tagPrefixes[0]}${currentVersion}`) {
       throw new Error(
         `Current version already has a tag ('${mostRecentTag}'), which is unexpected for a release candidate.`,
       );
@@ -264,28 +288,31 @@ export async function updateChangelog({
       changelog.addRelease({ version: currentVersion });
     }
 
-    if (hasUnreleasedChanges) {
+    const hasUnreleasedChangesToRelease =
+      getKnownPropertyNames(changelog.getUnreleasedChanges()).length > 0;
+    if (hasUnreleasedChangesToRelease) {
       changelog.migrateUnreleasedChangesToRelease(currentVersion);
     }
+  }
 
-    const newChangeEntries = newCommits.map(({ prNumber, description }) => {
-      if (prNumber) {
-        const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
-        return `${description} ${suffix}`;
-      }
-      return description;
-    });
+  const newChangeEntries = await getNewChangeEntries({
+    mostRecentTag,
+    repoUrl,
+    loggedPrNumbers: getAllLoggedPrNumbers(changelog),
+    projectRootDirectory,
+  });
 
-    for (const description of newChangeEntries.reverse()) {
-      changelog.addChange({
-        version: isReleaseCandidate ? currentVersion : undefined,
-        category: ChangeCategory.Uncategorized,
-        description,
-      });
-    }
+  for (const description of newChangeEntries.reverse()) {
+    changelog.addChange({
+      version: isReleaseCandidate ? currentVersion : undefined,
+      category: ChangeCategory.Uncategorized,
+      description,
+    });
   }
 
-  return changelog.toString();
+  const newChangelogContent = changelog.toString();
+  const isChangelogUpdated = changelogContent !== newChangelogContent;
+  return isChangelogUpdated ? newChangelogContent : undefined;
 }
 
 /**