From b0810e3d19ed169f33440e6ccb843fba1e61186a Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 14:51:29 +0200 Subject: [PATCH 01/15] Add deleteRef method to API Signed-off-by: Erik Engervall --- .../src/api/GitReleaseClient.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/plugins/git-release-manager/src/api/GitReleaseClient.ts b/plugins/git-release-manager/src/api/GitReleaseClient.ts index 948c77b47a..996b8bfdd3 100644 --- a/plugins/git-release-manager/src/api/GitReleaseClient.ts +++ b/plugins/git-release-manager/src/api/GitReleaseClient.ts @@ -293,6 +293,19 @@ export class GitReleaseClient implements GitReleaseApi { }; }; + deleteRef: GitReleaseApi['deleteRef'] = async ({ owner, repo, ref }) => { + const { octokit } = await this.getOctokit(); + const createRefResponse = await octokit.git.deleteRef({ + owner, + repo, + ref, + }); + + return { + success: createRefResponse.status === 204, + }; + }; + getComparison: GitReleaseApi['getComparison'] = async ({ owner, repo, @@ -651,6 +664,14 @@ export interface GitReleaseApi { }; }>; + deleteRef: ( + args: { + ref: string; + } & OwnerRepo, + ) => Promise<{ + success: boolean; + }>; + getComparison: ( args: { base: string; From 638bf560fa19e8a9daaa2ce82743ef3163709616 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 14:55:27 +0200 Subject: [PATCH 02/15] Add typings for catch errors Signed-off-by: Erik Engervall --- .../CreateReleaseCandidate/hooks/useCreateReleaseCandidate.ts | 2 +- .../src/features/PromoteRc/hooks/usePromoteRc.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/git-release-manager/src/features/CreateReleaseCandidate/hooks/useCreateReleaseCandidate.ts b/plugins/git-release-manager/src/features/CreateReleaseCandidate/hooks/useCreateReleaseCandidate.ts index 1173d6fbbe..4030d9370f 100644 --- a/plugins/git-release-manager/src/features/CreateReleaseCandidate/hooks/useCreateReleaseCandidate.ts +++ b/plugins/git-release-manager/src/features/CreateReleaseCandidate/hooks/useCreateReleaseCandidate.ts @@ -279,7 +279,7 @@ export function useCreateReleaseCandidate({ previousTag: latestRelease?.tagName, }); } catch (error) { - asyncCatcher(error); + asyncCatcher(error as Error); } addStepToResponseSteps({ diff --git a/plugins/git-release-manager/src/features/PromoteRc/hooks/usePromoteRc.ts b/plugins/git-release-manager/src/features/PromoteRc/hooks/usePromoteRc.ts index 232bc0b744..7ebc4fed54 100644 --- a/plugins/git-release-manager/src/features/PromoteRc/hooks/usePromoteRc.ts +++ b/plugins/git-release-manager/src/features/PromoteRc/hooks/usePromoteRc.ts @@ -182,7 +182,7 @@ export function usePromoteRc({ updatedTagUrl: promotedReleaseRes.value.htmlUrl, }); } catch (error) { - asyncCatcher(error); + asyncCatcher(error as Error); } addStepToResponseSteps({ From 1f03560b0a2c86c93821f999efb43c6ee5f7d4ef Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 14:56:27 +0200 Subject: [PATCH 03/15] Add patch prep code to avoid broken states upon merge conflict errors Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.ts | 85 ++--- .../src/features/Patch/hooks/usePatchPrep.ts | 335 ++++++++++++++++++ 2 files changed, 380 insertions(+), 40 deletions(-) create mode 100644 plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts index 338fa2a2d1..e331215793 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts @@ -15,7 +15,7 @@ */ import { useEffect, useState } from 'react'; -import { useAsync, useAsyncFn } from 'react-use'; +import { useAsync } from 'react-use'; import { GetLatestReleaseResult, GetRecentCommitsResultSingle, @@ -32,9 +32,9 @@ import { gitReleaseManagerApiRef } from '../../../api/serviceApiRef'; import { Project } from '../../../contexts/ProjectContext'; import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; import { TAG_OBJECT_MESSAGE } from '../../../constants/constants'; -import { useResponseSteps } from '../../../hooks/useResponseSteps'; import { useUserContext } from '../../../contexts/UserContext'; import { useApi } from '@backstage/core-plugin-api'; +import { usePatchPreparation } from './usePatchPrep'; export interface UsePatch { bumpedTag: string; @@ -54,38 +54,53 @@ export function usePatch({ }: UsePatch): CardHook { const pluginApiClient = useApi(gitReleaseManagerApiRef); const { user } = useUserContext(); - const { responseSteps, addStepToResponseSteps, asyncCatcher, abortIfError } = - useResponseSteps(); const releaseBranchName = latestRelease.targetCommitish; + const { + run, + runInvoked, + lastCallRes, + abortIfError, + addStepToResponseSteps, + asyncCatcher, + responseSteps, + TOTAL_PATCH_PREP_STEPS, + selectedPatchCommit, + } = usePatchPreparation({ + bumpedTag, + releaseBranchName, + project, + tagParts, + }); + /** * (1) Here is the branch we want to cherry-pick to: * > branch = GET /repos/$owner/$repo/branches/$branchName * > branchSha = branch.commit.sha * > branchTree = branch.commit.commit.tree.sha */ - const [releaseBranchRes, run] = useAsyncFn( - async (selectedPatchCommit: GetRecentCommitsResultSingle) => { - const { branch: releaseBranch } = await pluginApiClient - .getBranch({ - owner: project.owner, - repo: project.repo, - branch: releaseBranchName, - }) - .catch(asyncCatcher); + const releaseBranchRes = useAsync(async () => { + abortIfError(lastCallRes.error); + if (!lastCallRes.value) return undefined; - addStepToResponseSteps({ - message: `Fetched release branch "${releaseBranch.name}"`, - link: releaseBranch.links.html, - }); + const { branch: releaseBranch } = await pluginApiClient + .getBranch({ + owner: project.owner, + repo: project.repo, + branch: releaseBranchName, + }) + .catch(asyncCatcher); - return { - releaseBranch, - selectedPatchCommit, - }; - }, - ); + addStepToResponseSteps({ + message: `Fetched release branch "${releaseBranch.name}"`, + link: releaseBranch.links.html, + }); + + return { + releaseBranch, + }; + }, [lastCallRes.value, lastCallRes.error]); /** * (2) Create a temporary commit on the branch, which extends as a sibling of @@ -102,9 +117,7 @@ export function usePatch({ owner: project.owner, repo: project.repo, message: `Temporary commit for patch ${tagParts.patch}`, - parents: [ - releaseBranchRes.value.selectedPatchCommit.firstParentSha ?? '', - ], + parents: [selectedPatchCommit.firstParentSha ?? ''], tree: releaseBranchRes.value.releaseBranch.commit.commit.tree.sha, }) .catch(asyncCatcher); @@ -159,7 +172,7 @@ export function usePatch({ owner: project.owner, repo: project.repo, base: releaseBranchName, - head: releaseBranchRes.value.selectedPatchCommit.sha, + head: selectedPatchCommit.sha, }) .catch(asyncCatcher); @@ -184,7 +197,6 @@ export function usePatch({ if (!mergeRes.value || !releaseBranchRes.value) return undefined; const releaseBranchSha = releaseBranchRes.value.releaseBranch.commit.sha; - const selectedPatchCommit = releaseBranchRes.value.selectedPatchCommit; const { commit: cherryPickCommit } = await pluginApiClient.createCommit({ owner: project.owner, @@ -299,8 +311,6 @@ export function usePatch({ abortIfError(createdReferenceRes.error); if (!createdReferenceRes.value || !releaseBranchRes.value) return undefined; - const selectedPatchCommit = releaseBranchRes.value.selectedPatchCommit; - const { release } = await pluginApiClient .updateRelease({ owner: project.owner, @@ -343,16 +353,15 @@ ${selectedPatchCommit.commit.message}`, project, tagParts, }, - patchCommitMessage: - releaseBranchRes.value.selectedPatchCommit.commit.message, - patchCommitUrl: releaseBranchRes.value.selectedPatchCommit.htmlUrl, + patchCommitMessage: selectedPatchCommit.commit.message, + patchCommitUrl: selectedPatchCommit.htmlUrl, patchedTag: updatedReleaseRes.value.tagName, previousTag: latestRelease.tagName, updatedReleaseName: updatedReleaseRes.value.name, updatedReleaseUrl: updatedReleaseRes.value.htmlUrl, }); } catch (error) { - asyncCatcher(error); + asyncCatcher(error as Error); } addStepToResponseSteps({ @@ -361,7 +370,7 @@ ${selectedPatchCommit.commit.message}`, }); }, [updatedReleaseRes.value, updatedReleaseRes.error]); - const TOTAL_STEPS = 9 + (!!onSuccess ? 1 : 0); + const TOTAL_STEPS = 9 + (!!onSuccess ? 1 : 0) + TOTAL_PATCH_PREP_STEPS; const [progress, setProgress] = useState(0); useEffect(() => { setProgress((responseSteps.length / TOTAL_STEPS) * 100); @@ -371,10 +380,6 @@ ${selectedPatchCommit.commit.message}`, progress, responseSteps, run, - runInvoked: Boolean( - releaseBranchRes.loading || - releaseBranchRes.value || - releaseBranchRes.error, - ), + runInvoked, }; } diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts new file mode 100644 index 0000000000..a302c3fae8 --- /dev/null +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts @@ -0,0 +1,335 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { useAsync, useAsyncFn } from 'react-use'; +import { useApi } from '@backstage/core-plugin-api'; + +import { GetRecentCommitsResultSingle } from '../../../api/GitReleaseClient'; +import { CalverTagParts } from '../../../helpers/tagParts/getCalverTagParts'; +import { getPatchCommitSuffix } from '../helpers/getPatchCommitSuffix'; +import { gitReleaseManagerApiRef } from '../../../api/serviceApiRef'; +import { GitReleaseManagerError } from '../../../errors/GitReleaseManagerError'; +import { Project } from '../../../contexts/ProjectContext'; +import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; +import { useResponseSteps } from '../../../hooks/useResponseSteps'; + +export interface UsePatchPreparation { + bumpedTag: string; + releaseBranchName: string; + project: Project; + tagParts: NonNullable; +} + +// Inspiration: https://stackoverflow.com/questions/53859199/how-to-cherry-pick-through-githubs-api +export function usePatchPreparation({ + bumpedTag, + releaseBranchName, + project, + tagParts, +}: UsePatchPreparation) { + const pluginApiClient = useApi(gitReleaseManagerApiRef); + const { responseSteps, addStepToResponseSteps, asyncCatcher, abortIfError } = + useResponseSteps(); + + const tempPatchBranchName = `${releaseBranchName}-temp-patch-prep`; + + /** + * (1) Get the release branch's most recent commit + */ + const [latestCommitOnReleaseBranchRes, run] = useAsyncFn( + async (selectedPatchCommit: GetRecentCommitsResultSingle) => { + const { commit: latestCommit } = await pluginApiClient + .getCommit({ + owner: project.owner, + repo: project.repo, + ref: releaseBranchName, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: `[patch prep] Fetched latest commit from "${releaseBranchName}"`, + secondaryMessage: `with message "${latestCommit.commit.message}"`, + link: latestCommit.htmlUrl, + }); + + return { + latestCommit, + selectedPatchCommit, + }; + }, + ); + + /** + * (2) Create temporary patch branch based on release branch's most recent sha + * to test if programmatic cherry pick patching is possible + */ + const createTempPatchBranchRes = useAsync(async () => { + abortIfError(latestCommitOnReleaseBranchRes.error); + if (!latestCommitOnReleaseBranchRes.value) return undefined; + + const { reference: createdReleaseBranch } = await pluginApiClient + .createRef({ + owner: project.owner, + repo: project.repo, + sha: latestCommitOnReleaseBranchRes.value.latestCommit.sha, + ref: `refs/heads/${tempPatchBranchName}`, + }) + .catch(error => { + if (error?.body?.message === 'Reference already exists') { + throw new GitReleaseManagerError( + `Branch "${tempPatchBranchName}" already exists: .../tree/${tempPatchBranchName}`, + ); + } + throw error; + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: '[patch prep] Created Temp Patch Branch', + secondaryMessage: `with ref "${createdReleaseBranch.ref}"`, + }); + + return { + ...createdReleaseBranch, + selectedPatchCommit: + latestCommitOnReleaseBranchRes.value.selectedPatchCommit, + }; + }, [ + latestCommitOnReleaseBranchRes.value, + latestCommitOnReleaseBranchRes.error, + ]); + + /** + * (3) Here is the branch we want to cherry-pick to: + * > branch = GET /repos/$owner/$repo/branches/$branchName + * > branchSha = branch.commit.sha + * > branchTree = branch.commit.commit.tree.sha + */ + const tempPatchBranchRes = useAsync(async () => { + abortIfError(createTempPatchBranchRes.error); + if (!createTempPatchBranchRes.value) return undefined; + + const { branch: releaseBranch } = await pluginApiClient + .getBranch({ + owner: project.owner, + repo: project.repo, + branch: tempPatchBranchName, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: `[patch prep] Fetched release branch "${releaseBranch.name}"`, + link: releaseBranch.links.html, + }); + + return { + releaseBranch, + selectedPatchCommit: createTempPatchBranchRes.value.selectedPatchCommit, + }; + }, [createTempPatchBranchRes.value, createTempPatchBranchRes.error]); + + /** + * (4) Create a temporary commit on the branch, which extends as a sibling of + * the commit we want but contains the current tree of the target branch: + * > parentSha = commit.parents.head // first parent -- there should only be one + * > tempCommit = POST /repos/$owner/$repo/git/commits { "message": "temp", "tree": branchTree, "parents": [parentSha] } + */ + const tempCommitRes = useAsync(async () => { + abortIfError(tempPatchBranchRes.error); + if (!tempPatchBranchRes.value) return undefined; + + const { commit: tempCommit } = await pluginApiClient + .createCommit({ + owner: project.owner, + repo: project.repo, + message: `[patch prep] Temporary commit for patch ${tagParts.patch}`, + parents: [ + tempPatchBranchRes.value.selectedPatchCommit.firstParentSha ?? '', + ], + tree: tempPatchBranchRes.value.releaseBranch.commit.commit.tree.sha, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: '[patch prep] Created temporary commit', + secondaryMessage: `with message "${tempCommit.message}"`, + }); + + return { + ...tempCommit, + }; + }, [tempPatchBranchRes.value, tempPatchBranchRes.error]); + + /** + * (5) Now temporarily force the branch over to that commit: + * > PATCH /repos/$owner/$repo/git/refs/heads/$refName { sha = tempCommit.sha, force = true } + */ + const forceBranchRes = useAsync(async () => { + abortIfError(tempCommitRes.error); + if (!tempCommitRes.value) return undefined; + + await pluginApiClient + .updateRef({ + owner: project.owner, + repo: project.repo, + sha: tempCommitRes.value.sha, + ref: `heads/${tempPatchBranchName}`, + force: true, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: `[patch prep] Forced branch "${tempPatchBranchName}" to temporary commit "${tempCommitRes.value.sha}"`, + }); + + return { + trigger: 'next step 🚀 ', + }; + }, [tempCommitRes.value, tempCommitRes.error]); + + /** + * (6) Merge the commit we want into this mess: + * > merge = POST /repos/$owner/$repo/merges { "base": branchName, "head": commit.sha } + */ + const mergeRes = useAsync(async () => { + abortIfError(forceBranchRes.error); + if (!forceBranchRes.value || !tempPatchBranchRes.value) return undefined; + + const { merge } = await pluginApiClient + .merge({ + owner: project.owner, + repo: project.repo, + base: tempPatchBranchName, + head: tempPatchBranchRes.value.selectedPatchCommit.sha, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: `[patch prep] Merged temporary commit into "${tempPatchBranchName}"`, + secondaryMessage: `with message "${merge.commit.message}"`, + link: merge.htmlUrl, + }); + + return { + ...merge, + }; + }, [forceBranchRes.value, forceBranchRes.error]); + + /** + * (7) Now that we know what the tree should be, create the cherry-pick commit. + * Note that branchSha is the original from up at the top. + * > cherry = POST /repos/$owner/$repo/git/commits { "message": "looks good!", "tree": mergeTree, "parents": [branchSha] } + */ + const cherryPickRes = useAsync(async () => { + abortIfError(mergeRes.error); + if (!mergeRes.value || !tempPatchBranchRes.value) return undefined; + + const releaseBranchSha = tempPatchBranchRes.value.releaseBranch.commit.sha; + const selectedPatchCommit = tempPatchBranchRes.value.selectedPatchCommit; + + const { commit: cherryPickCommit } = await pluginApiClient.createCommit({ + owner: project.owner, + repo: project.repo, + message: `[patch prep] [patch ${bumpedTag}] ${ + selectedPatchCommit.commit.message + } + + ${getPatchCommitSuffix({ + commitSha: selectedPatchCommit.sha, + })}`, + parents: [releaseBranchSha], + tree: mergeRes.value.commit.tree.sha, + }); + + addStepToResponseSteps({ + message: `[patch prep] Cherry-picked patch commit to "${releaseBranchSha}"`, + secondaryMessage: `with message "${cherryPickCommit.message}"`, + }); + + return { + ...cherryPickCommit, + }; + }, [mergeRes.value, mergeRes.error]); + + /** + * (8) Replace the temp commit with the real commit: + * > PATCH /repos/$owner/$repo/git/refs/heads/$refName { sha = cherry.sha, force = true } + */ + const updatedRefRes = useAsync(async () => { + abortIfError(cherryPickRes.error); + if (!cherryPickRes.value) return undefined; + + const { reference: updatedReference } = await pluginApiClient + .updateRef({ + owner: project.owner, + repo: project.repo, + ref: `heads/${tempPatchBranchName}`, + sha: cherryPickRes.value.sha, + force: true, + }) + .catch(asyncCatcher); + + addStepToResponseSteps({ + message: `[patch prep] Updated reference "${updatedReference.ref}"`, + }); + + return { + ...updatedReference, + }; + }, [cherryPickRes.value, cherryPickRes.error]); + + /** + * (9) Delete temp patch branch + */ + const deleteTempPatchBranchRes = useAsync(async () => { + abortIfError(cherryPickRes.error); + if (!cherryPickRes.value) return undefined; + + const { success: deletedReferenceSuccess } = + await pluginApiClient.deleteRef({ + owner: project.owner, + repo: project.repo, + ref: `heads/${tempPatchBranchName}`, + }); + + addStepToResponseSteps({ + message: `[patch prep] Deleted temporary patch prep branch "${tempPatchBranchName}"`, + }); + + return { + deletedReferenceSuccess, + }; + }, [updatedRefRes.value, updatedRefRes.error]); + + const TOTAL_PATCH_PREP_STEPS = 9; + + return { + TOTAL_PATCH_PREP_STEPS, + run, + runInvoked: Boolean( + deleteTempPatchBranchRes.loading || + deleteTempPatchBranchRes.value || + deleteTempPatchBranchRes.error, + ), + lastCallRes: deleteTempPatchBranchRes, + responseSteps, + addStepToResponseSteps, + asyncCatcher, + abortIfError, + selectedPatchCommit: latestCommitOnReleaseBranchRes.value + ?.selectedPatchCommit as any, + }; +} From d3e2b80637b34d79361700785e946c43fbd4ba0b Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 15:27:46 +0200 Subject: [PATCH 04/15] Add description to Patch body Signed-off-by: Erik Engervall --- .../src/features/Patch/PatchBody.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx index 272363380f..71aaa7e1f7 100644 --- a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx +++ b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx @@ -141,6 +141,15 @@ export const PatchBody = ({ )} + + + Patches the release branch, creates a new tag and updates the Git + release. Before patching the release branch, a validation sequence + will run to ensure there's no merge conflicts. Manual patching is + recommended should the validation fail. + + + Date: Wed, 6 Oct 2021 15:28:24 +0200 Subject: [PATCH 05/15] Improve readability of responseSteps Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.ts | 4 +- ...Prep.ts => usePatchValidationSequence.tsx} | 110 ++++++++++++++---- ...eResponseSteps.ts => useResponseSteps.tsx} | 17 +-- .../git-release-manager/src/types/types.ts | 2 +- 4 files changed, 98 insertions(+), 35 deletions(-) rename plugins/git-release-manager/src/features/Patch/hooks/{usePatchPrep.ts => usePatchValidationSequence.tsx} (78%) rename plugins/git-release-manager/src/hooks/{useResponseSteps.ts => useResponseSteps.tsx} (84%) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts index e331215793..df80339108 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts @@ -34,7 +34,7 @@ import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; import { TAG_OBJECT_MESSAGE } from '../../../constants/constants'; import { useUserContext } from '../../../contexts/UserContext'; import { useApi } from '@backstage/core-plugin-api'; -import { usePatchPreparation } from './usePatchPrep'; +import { usePatchValidationSequence } from './usePatchValidationSequence'; export interface UsePatch { bumpedTag: string; @@ -67,7 +67,7 @@ export function usePatch({ responseSteps, TOTAL_PATCH_PREP_STEPS, selectedPatchCommit, - } = usePatchPreparation({ + } = usePatchValidationSequence({ bumpedTag, releaseBranchName, project, diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx similarity index 78% rename from plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts rename to plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx index a302c3fae8..7778fffe42 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatchPrep.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx @@ -14,8 +14,9 @@ * limitations under the License. */ -import { useAsync, useAsyncFn } from 'react-use'; import { useApi } from '@backstage/core-plugin-api'; +import { useAsync, useAsyncFn } from 'react-use'; +import React from 'react'; import { GetRecentCommitsResultSingle } from '../../../api/GitReleaseClient'; import { CalverTagParts } from '../../../helpers/tagParts/getCalverTagParts'; @@ -26,31 +27,49 @@ import { Project } from '../../../contexts/ProjectContext'; import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; import { useResponseSteps } from '../../../hooks/useResponseSteps'; -export interface UsePatchPreparation { +export interface UsePatchValidationSequence { bumpedTag: string; releaseBranchName: string; project: Project; tagParts: NonNullable; } +const PatchValidationMessage = ({ message }: { message: string }) => ( + <> + [Patch validation] {message} + +); + // Inspiration: https://stackoverflow.com/questions/53859199/how-to-cherry-pick-through-githubs-api -export function usePatchPreparation({ +export function usePatchValidationSequence({ bumpedTag, releaseBranchName, project, tagParts, -}: UsePatchPreparation) { +}: UsePatchValidationSequence) { const pluginApiClient = useApi(gitReleaseManagerApiRef); const { responseSteps, addStepToResponseSteps, asyncCatcher, abortIfError } = useResponseSteps(); - const tempPatchBranchName = `${releaseBranchName}-temp-patch-prep`; + const tempPatchBranchName = `${releaseBranchName}-grm-temp-patch-prep`; /** * (1) Get the release branch's most recent commit */ const [latestCommitOnReleaseBranchRes, run] = useAsyncFn( async (selectedPatchCommit: GetRecentCommitsResultSingle) => { + try { + await pluginApiClient.deleteRef({ + owner: project.owner, + repo: project.repo, + ref: `heads/${tempPatchBranchName}`, + }); + } catch (error: any) { + if (error.message !== 'Reference does not exist') { + throw error; + } + } + const { commit: latestCommit } = await pluginApiClient .getCommit({ owner: project.owner, @@ -60,9 +79,11 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: `[patch prep] Fetched latest commit from "${releaseBranchName}"`, - secondaryMessage: `with message "${latestCommit.commit.message}"`, - link: latestCommit.htmlUrl, + message: ( + + ), }); return { @@ -98,8 +119,11 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: '[patch prep] Created Temp Patch Branch', - secondaryMessage: `with ref "${createdReleaseBranch.ref}"`, + message: ( + + ), }); return { @@ -131,8 +155,11 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: `[patch prep] Fetched release branch "${releaseBranch.name}"`, - link: releaseBranch.links.html, + message: ( + + ), }); return { @@ -155,7 +182,7 @@ export function usePatchPreparation({ .createCommit({ owner: project.owner, repo: project.repo, - message: `[patch prep] Temporary commit for patch ${tagParts.patch}`, + message: `[Patch validation] Temporary commit for patch ${tagParts.patch}`, parents: [ tempPatchBranchRes.value.selectedPatchCommit.firstParentSha ?? '', ], @@ -164,8 +191,7 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: '[patch prep] Created temporary commit', - secondaryMessage: `with message "${tempCommit.message}"`, + message: , }); return { @@ -192,7 +218,11 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: `[patch prep] Forced branch "${tempPatchBranchName}" to temporary commit "${tempCommitRes.value.sha}"`, + message: ( + + ), }); return { @@ -215,12 +245,33 @@ export function usePatchPreparation({ base: tempPatchBranchName, head: tempPatchBranchRes.value.selectedPatchCommit.sha, }) + .catch(async error => { + if (error?.message === 'Merge conflict') { + try { + await pluginApiClient.deleteRef({ + owner: project.owner, + repo: project.repo, + ref: `heads/${tempPatchBranchName}`, + }); + } catch (_error) { + // swallow + } + + throw new GitReleaseManagerError( + 'Patching failed due to merge conflict. Will attempt to delete temporary patch validation branch. Manual patching is recommended.', + ); + } + + throw error; + }) .catch(asyncCatcher); addStepToResponseSteps({ - message: `[patch prep] Merged temporary commit into "${tempPatchBranchName}"`, - secondaryMessage: `with message "${merge.commit.message}"`, - link: merge.htmlUrl, + message: ( + + ), }); return { @@ -243,7 +294,7 @@ export function usePatchPreparation({ const { commit: cherryPickCommit } = await pluginApiClient.createCommit({ owner: project.owner, repo: project.repo, - message: `[patch prep] [patch ${bumpedTag}] ${ + message: `[Patch validation] [patch ${bumpedTag}] ${ selectedPatchCommit.commit.message } @@ -255,8 +306,11 @@ export function usePatchPreparation({ }); addStepToResponseSteps({ - message: `[patch prep] Cherry-picked patch commit to "${releaseBranchSha}"`, - secondaryMessage: `with message "${cherryPickCommit.message}"`, + message: ( + + ), }); return { @@ -283,7 +337,11 @@ export function usePatchPreparation({ .catch(asyncCatcher); addStepToResponseSteps({ - message: `[patch prep] Updated reference "${updatedReference.ref}"`, + message: ( + + ), }); return { @@ -306,7 +364,11 @@ export function usePatchPreparation({ }); addStepToResponseSteps({ - message: `[patch prep] Deleted temporary patch prep branch "${tempPatchBranchName}"`, + message: ( + + ), }); return { diff --git a/plugins/git-release-manager/src/hooks/useResponseSteps.ts b/plugins/git-release-manager/src/hooks/useResponseSteps.tsx similarity index 84% rename from plugins/git-release-manager/src/hooks/useResponseSteps.ts rename to plugins/git-release-manager/src/hooks/useResponseSteps.tsx index d39996e2f3..3c44a34a0d 100644 --- a/plugins/git-release-manager/src/hooks/useResponseSteps.ts +++ b/plugins/git-release-manager/src/hooks/useResponseSteps.tsx @@ -14,15 +14,10 @@ * limitations under the License. */ -import { useState } from 'react'; +import React, { useState } from 'react'; import { ResponseStep } from '../types/types'; -const RESPONSE_STEP_FAILURE_ABORT: ResponseStep = { - message: 'Skipped due to error in previous step', - icon: 'failure', -}; - export function useResponseSteps() { const [responseSteps, setResponseSteps] = useState([]); @@ -32,7 +27,14 @@ export function useResponseSteps() { const asyncCatcher = (error: Error): never => { const responseStepError: ResponseStep = { - message: 'Something went wrong 🔥', + message: ( + + Something went wrong{' '} + + 🔥 + + + ), secondaryMessage: `Error message: ${ error?.message ? error.message : 'unknown' }`, @@ -45,7 +47,6 @@ export function useResponseSteps() { const abortIfError = (error?: Error) => { if (error) { - addStepToResponseSteps(RESPONSE_STEP_FAILURE_ABORT); throw error; } }; diff --git a/plugins/git-release-manager/src/types/types.ts b/plugins/git-release-manager/src/types/types.ts index 39418fa9d3..600a293deb 100644 --- a/plugins/git-release-manager/src/types/types.ts +++ b/plugins/git-release-manager/src/types/types.ts @@ -53,7 +53,7 @@ export interface PatchOnSuccessArgs { } export interface ResponseStep { - message: string | React.ReactNode; + message: React.ReactNode; secondaryMessage?: string | React.ReactNode; link?: string; icon?: 'success' | 'failure'; From 95a25a3f27bccf36cdfb58d8a72dfd6149cd6c0e Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 15:29:18 +0200 Subject: [PATCH 06/15] Make temp patch name more specific Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatchValidationSequence.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx b/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx index 7778fffe42..0ab0deee73 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx @@ -51,7 +51,7 @@ export function usePatchValidationSequence({ const { responseSteps, addStepToResponseSteps, asyncCatcher, abortIfError } = useResponseSteps(); - const tempPatchBranchName = `${releaseBranchName}-grm-temp-patch-prep`; + const tempPatchBranchName = `${releaseBranchName}-backstage-grm-patch-validation`; /** * (1) Get the release branch's most recent commit From a6f86fae1a0e11dd51124c6354bf57d422c15ce9 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:37:30 +0200 Subject: [PATCH 07/15] Update snapshots Signed-off-by: Erik Engervall --- .../src/api/GitReleaseClient.test.ts | 1 + .../src/hooks/useResponseSteps.test.ts | 45 +++++++++---------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/plugins/git-release-manager/src/api/GitReleaseClient.test.ts b/plugins/git-release-manager/src/api/GitReleaseClient.test.ts index 9125cb6110..418da485a2 100644 --- a/plugins/git-release-manager/src/api/GitReleaseClient.test.ts +++ b/plugins/git-release-manager/src/api/GitReleaseClient.test.ts @@ -37,6 +37,7 @@ describe('GitReleaseClient', () => { "createRef": [Function], "createRelease": [Function], "createTagObject": [Function], + "deleteRef": [Function], "getAllReleases": [Function], "getAllTags": [Function], "getBranch": [Function], diff --git a/plugins/git-release-manager/src/hooks/useResponseSteps.test.ts b/plugins/git-release-manager/src/hooks/useResponseSteps.test.ts index 29ef7b7aae..5daa8b47ad 100644 --- a/plugins/git-release-manager/src/hooks/useResponseSteps.test.ts +++ b/plugins/git-release-manager/src/hooks/useResponseSteps.test.ts @@ -72,7 +72,16 @@ describe('useResponseSteps', () => { Array [ Object { "icon": "failure", - "message": "Something went wrong 🔥", + "message": + Something went wrong + + + 🔥 + + , "secondaryMessage": "Error message: :(", }, ] @@ -96,7 +105,16 @@ describe('useResponseSteps', () => { Array [ Object { "icon": "failure", - "message": "Something went wrong 🔥", + "message": + Something went wrong + + + 🔥 + + , "secondaryMessage": "Error message: unknown", }, ] @@ -105,29 +123,6 @@ describe('useResponseSteps', () => { }); describe('abortIfError', () => { - it('should throw if Error and add a failure step', async () => { - const { result } = renderHook(() => useResponseSteps()); - - expect(result.current.responseSteps).toMatchInlineSnapshot(`Array []`); - - act(() => { - try { - result.current.abortIfError(new Error('Das kaboom')); - } catch (error) { - // - } - }); - - expect(result.current.responseSteps).toMatchInlineSnapshot(` - Array [ - Object { - "icon": "failure", - "message": "Skipped due to error in previous step", - }, - ] - `); - }); - it('should do nothing if not Error', async () => { const { result } = renderHook(() => useResponseSteps()); From 820d901c2e13b0f026d01debdfd6046d8ee4f32b Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:41:08 +0200 Subject: [PATCH 08/15] Update usePatch tests Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.test.ts | 53 +++++++++++++++++-- .../src/test-helpers/mock-api-client.ts | 4 ++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts index 41eb936d59..6677a0bbe5 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts @@ -39,7 +39,7 @@ jest.mock('../../../contexts/UserContext', () => ({ describe('patch', () => { beforeEach(jest.clearAllMocks); - it('should return the expected responseSteps and progress', async () => { + it('should return the expected responseSteps (including patch validation sequence) and progress', async () => { const { result } = renderHook(() => usePatch({ bumpedTag: mockBumpedTag, @@ -54,10 +54,10 @@ describe('patch', () => { }); expect(result.error).toEqual(undefined); - expect(result.current.responseSteps).toHaveLength(9); + expect(result.current.responseSteps).toHaveLength(18); }); - it('should return the expected responseSteps and progress (with onSuccess)', async () => { + it('should return the expected responseSteps (including patch validation sequence) and progress (with onSuccess)', async () => { const { result } = renderHook(() => usePatch({ bumpedTag: mockBumpedTag, @@ -73,11 +73,56 @@ describe('patch', () => { }); expect(result.error).toEqual(undefined); - expect(result.current.responseSteps).toHaveLength(10); + expect(result.current.responseSteps).toHaveLength(19); expect(result.current).toMatchInlineSnapshot(` Object { "progress": 100, "responseSteps": Array [ + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, + Object { + "message": , + }, Object { "link": "https://mock_branch_links_html", "message": "Fetched release branch \\"rc/1.2.3\\"", diff --git a/plugins/git-release-manager/src/test-helpers/mock-api-client.ts b/plugins/git-release-manager/src/test-helpers/mock-api-client.ts index b2e39d6f7c..bbfe8f7636 100644 --- a/plugins/git-release-manager/src/test-helpers/mock-api-client.ts +++ b/plugins/git-release-manager/src/test-helpers/mock-api-client.ts @@ -87,6 +87,10 @@ export const mockApiClient: GitReleaseApi = { }, })), + deleteRef: jest.fn(async () => ({ + success: true, + })), + createRelease: jest.fn(async () => ({ release: { name: 'mock_createRelease_name', From 1ed2f1af42e38ef8749bc145d20a53dfa0c10c54 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:47:40 +0200 Subject: [PATCH 09/15] Update naming from patch validation to patch dry run Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.ts | 18 +++---- ...idationSequence.tsx => usePatchDryRun.tsx} | 51 +++++++++---------- 2 files changed, 34 insertions(+), 35 deletions(-) rename plugins/git-release-manager/src/features/Patch/hooks/{usePatchValidationSequence.tsx => usePatchDryRun.tsx} (90%) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts index df80339108..188ffbca2e 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.ts @@ -14,27 +14,27 @@ * limitations under the License. */ -import { useEffect, useState } from 'react'; import { useAsync } from 'react-use'; -import { - GetLatestReleaseResult, - GetRecentCommitsResultSingle, -} from '../../../api/GitReleaseClient'; +import { useEffect, useState } from 'react'; -import { CalverTagParts } from '../../../helpers/tagParts/getCalverTagParts'; import { CardHook, ComponentConfig, PatchOnSuccessArgs, } from '../../../types/types'; +import { + GetLatestReleaseResult, + GetRecentCommitsResultSingle, +} from '../../../api/GitReleaseClient'; +import { CalverTagParts } from '../../../helpers/tagParts/getCalverTagParts'; import { getPatchCommitSuffix } from '../helpers/getPatchCommitSuffix'; import { gitReleaseManagerApiRef } from '../../../api/serviceApiRef'; import { Project } from '../../../contexts/ProjectContext'; import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; import { TAG_OBJECT_MESSAGE } from '../../../constants/constants'; -import { useUserContext } from '../../../contexts/UserContext'; import { useApi } from '@backstage/core-plugin-api'; -import { usePatchValidationSequence } from './usePatchValidationSequence'; +import { usePatchDryRun } from './usePatchDryRun'; +import { useUserContext } from '../../../contexts/UserContext'; export interface UsePatch { bumpedTag: string; @@ -67,7 +67,7 @@ export function usePatch({ responseSteps, TOTAL_PATCH_PREP_STEPS, selectedPatchCommit, - } = usePatchValidationSequence({ + } = usePatchDryRun({ bumpedTag, releaseBranchName, project, diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx b/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx similarity index 90% rename from plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx rename to plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx index 0ab0deee73..5227399970 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatchValidationSequence.tsx +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx @@ -18,40 +18,40 @@ import { useApi } from '@backstage/core-plugin-api'; import { useAsync, useAsyncFn } from 'react-use'; import React from 'react'; -import { GetRecentCommitsResultSingle } from '../../../api/GitReleaseClient'; import { CalverTagParts } from '../../../helpers/tagParts/getCalverTagParts'; import { getPatchCommitSuffix } from '../helpers/getPatchCommitSuffix'; +import { GetRecentCommitsResultSingle } from '../../../api/GitReleaseClient'; import { gitReleaseManagerApiRef } from '../../../api/serviceApiRef'; import { GitReleaseManagerError } from '../../../errors/GitReleaseManagerError'; import { Project } from '../../../contexts/ProjectContext'; import { SemverTagParts } from '../../../helpers/tagParts/getSemverTagParts'; import { useResponseSteps } from '../../../hooks/useResponseSteps'; -export interface UsePatchValidationSequence { +export interface UsePatchDryRun { bumpedTag: string; releaseBranchName: string; project: Project; tagParts: NonNullable; } -const PatchValidationMessage = ({ message }: { message: string }) => ( +const PatchDryRunMessage = ({ message }: { message: string }) => ( <> - [Patch validation] {message} + [Patch dry run] {message} ); // Inspiration: https://stackoverflow.com/questions/53859199/how-to-cherry-pick-through-githubs-api -export function usePatchValidationSequence({ +export function usePatchDryRun({ bumpedTag, releaseBranchName, project, tagParts, -}: UsePatchValidationSequence) { +}: UsePatchDryRun) { const pluginApiClient = useApi(gitReleaseManagerApiRef); const { responseSteps, addStepToResponseSteps, asyncCatcher, abortIfError } = useResponseSteps(); - const tempPatchBranchName = `${releaseBranchName}-backstage-grm-patch-validation`; + const tempPatchBranchName = `${releaseBranchName}-backstage-grm-patch-dry-run`; /** * (1) Get the release branch's most recent commit @@ -80,7 +80,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), @@ -120,8 +120,8 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), }); @@ -156,7 +156,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), @@ -182,7 +182,7 @@ export function usePatchValidationSequence({ .createCommit({ owner: project.owner, repo: project.repo, - message: `[Patch validation] Temporary commit for patch ${tagParts.patch}`, + message: `Temporary commit for patch ${tagParts.patch}`, parents: [ tempPatchBranchRes.value.selectedPatchCommit.firstParentSha ?? '', ], @@ -191,7 +191,7 @@ export function usePatchValidationSequence({ .catch(asyncCatcher); addStepToResponseSteps({ - message: , + message: , }); return { @@ -219,7 +219,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), @@ -258,7 +258,7 @@ export function usePatchValidationSequence({ } throw new GitReleaseManagerError( - 'Patching failed due to merge conflict. Will attempt to delete temporary patch validation branch. Manual patching is recommended.', + 'Patching failed due to merge conflict. Will attempt to delete temporary patch dry run branch. Manual patching is recommended.', ); } @@ -268,7 +268,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), @@ -289,25 +289,24 @@ export function usePatchValidationSequence({ if (!mergeRes.value || !tempPatchBranchRes.value) return undefined; const releaseBranchSha = tempPatchBranchRes.value.releaseBranch.commit.sha; - const selectedPatchCommit = tempPatchBranchRes.value.selectedPatchCommit; + const { + commit: { message }, + sha: commitSha, + } = tempPatchBranchRes.value.selectedPatchCommit; const { commit: cherryPickCommit } = await pluginApiClient.createCommit({ owner: project.owner, repo: project.repo, - message: `[Patch validation] [patch ${bumpedTag}] ${ - selectedPatchCommit.commit.message - } + message: `[patch (dry run) ${bumpedTag}] ${message} - ${getPatchCommitSuffix({ - commitSha: selectedPatchCommit.sha, - })}`, + ${getPatchCommitSuffix({ commitSha })}`, parents: [releaseBranchSha], tree: mergeRes.value.commit.tree.sha, }); addStepToResponseSteps({ message: ( - ), @@ -338,7 +337,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), @@ -365,7 +364,7 @@ export function usePatchValidationSequence({ addStepToResponseSteps({ message: ( - ), From 8fcee9d1de40d46380a5c3d5e100199cc8122029 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:49:22 +0200 Subject: [PATCH 10/15] Update PatchBody copy to use dry runn instead of validation Signed-off-by: Erik Engervall --- .../git-release-manager/src/features/Patch/PatchBody.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx index 71aaa7e1f7..bf2f91580b 100644 --- a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx +++ b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx @@ -144,9 +144,9 @@ export const PatchBody = ({ Patches the release branch, creates a new tag and updates the Git - release. Before patching the release branch, a validation sequence - will run to ensure there's no merge conflicts. Manual patching is - recommended should the validation fail. + release. A dry run on a temporary branch will run prioer to patching + the release branch to ensure there's no merge conflicts. Manual + patching is recommended should the dry run fail. From b311fe253f34a46e383d476d4ce6cf6144efa15f Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:50:58 +0200 Subject: [PATCH 11/15] Update copy Signed-off-by: Erik Engervall --- plugins/git-release-manager/src/features/Patch/PatchBody.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx index bf2f91580b..2a2692bb22 100644 --- a/plugins/git-release-manager/src/features/Patch/PatchBody.tsx +++ b/plugins/git-release-manager/src/features/Patch/PatchBody.tsx @@ -144,7 +144,7 @@ export const PatchBody = ({ Patches the release branch, creates a new tag and updates the Git - release. A dry run on a temporary branch will run prioer to patching + release. A dry run on a temporary branch will run prior to patching the release branch to ensure there's no merge conflicts. Manual patching is recommended should the dry run fail. From 507d38cfbe62ebd25cb6daa6b7420fba3625a3b0 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:52:07 +0200 Subject: [PATCH 12/15] Update test copy Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts index 6677a0bbe5..e73b1ca236 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts @@ -39,7 +39,7 @@ jest.mock('../../../contexts/UserContext', () => ({ describe('patch', () => { beforeEach(jest.clearAllMocks); - it('should return the expected responseSteps (including patch validation sequence) and progress', async () => { + it('should return the expected responseSteps (including patch dry run) and progress', async () => { const { result } = renderHook(() => usePatch({ bumpedTag: mockBumpedTag, @@ -57,7 +57,7 @@ describe('patch', () => { expect(result.current.responseSteps).toHaveLength(18); }); - it('should return the expected responseSteps (including patch validation sequence) and progress (with onSuccess)', async () => { + it('should return the expected responseSteps (including patch dry run) and progress (with onSuccess)', async () => { const { result } = renderHook(() => usePatch({ bumpedTag: mockBumpedTag, From f3fe86a51423053c2c882ef12cdfe2cff63d9515 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:53:12 +0200 Subject: [PATCH 13/15] Update test Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatch.test.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts index e73b1ca236..f3b94c2476 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatch.test.ts @@ -79,48 +79,48 @@ describe('patch', () => { "progress": 100, "responseSteps": Array [ Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { - "message": , }, Object { From 6c318336b423ed183840d0890e8d6695203f0302 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Wed, 6 Oct 2021 16:58:00 +0200 Subject: [PATCH 14/15] Add changeset Signed-off-by: Erik Engervall --- .changeset/sour-terms-end.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/sour-terms-end.md diff --git a/.changeset/sour-terms-end.md b/.changeset/sour-terms-end.md new file mode 100644 index 0000000000..fd20fa13c8 --- /dev/null +++ b/.changeset/sour-terms-end.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-git-release-manager': minor +--- + +Errors caused while patching can leave the release branch in a broken state. Most commonly caused due to merge errors. + +This has been solved by introducing a dry run prior to patching the release branch. The dry run will attempt to cherry pick the selected patch commit onto a temporary branch created off of the release branch. If it succeeds, the temporary branch is deleted and the patch is applied on the release branch From c78560440c42f708dac3f71f9f0745ca4ce9ae32 Mon Sep 17 00:00:00 2001 From: Erik Engervall Date: Thu, 7 Oct 2021 13:24:26 +0200 Subject: [PATCH 15/15] deleteRef should wait for updatedRefRes, not cherryPickRes Signed-off-by: Erik Engervall --- .../src/features/Patch/hooks/usePatchDryRun.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx b/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx index 5227399970..de3a67e874 100644 --- a/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx +++ b/plugins/git-release-manager/src/features/Patch/hooks/usePatchDryRun.tsx @@ -352,8 +352,8 @@ export function usePatchDryRun({ * (9) Delete temp patch branch */ const deleteTempPatchBranchRes = useAsync(async () => { - abortIfError(cherryPickRes.error); - if (!cherryPickRes.value) return undefined; + abortIfError(updatedRefRes.error); + if (!updatedRefRes.value) return undefined; const { success: deletedReferenceSuccess } = await pluginApiClient.deleteRef({