refactored based on reviewer feedback to use filesToDelete array

Signed-off-by: Phred <fearphage@gmail.com>

plugins/scaffolder-backend-module-github/report.api.md

@@ -446,7 +446,7 @@ export const createPublishGithubPullRequestAction: (
     branchName: string;
     title: string;
     description: string;
-    deletionMarker?: string | undefined;
+    filesToDelete?: string[] | undefined;
     targetBranchName?: string | undefined;
     draft?: boolean | undefined;
     sourcePath?: string | undefined;

plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts

@@ -24,6 +24,7 @@ import {
   TemplateAction,
 } from '@backstage/plugin-scaffolder-node';
 import fs from 'fs-extra';
+import path from 'node:path';
 import { createPublishGithubPullRequestAction } from './githubPullRequest';
 import { createMockDirectory } from '@backstage/backend-test-utils';
 import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils';
@@ -304,16 +305,13 @@ describe('createPublishGithubPullRequestAction', () => {
     });
   });
 
-  describe('with deletionMarker', () => {
-    const deletionMarker =
-      'if-you-find-a-file-whose-contents-matches-this-delete-it';
-
+  describe('with filesToDelete', () => {
     let input: GithubPullRequestActionInput;
     let ctx: ActionContext<GithubPullRequestActionInput, any, any>;
 
     beforeEach(() => {
       input = {
-        deletionMarker,
+        filesToDelete: ['changed-file-to-delete.txt', 'delete-me-too.md'],
         repoUrl: 'github.com?owner=myorg&repo=myrepo',
         title: 'Create my new app',
         branchName: 'new-app',
@@ -323,6 +321,7 @@ describe('createPublishGithubPullRequestAction', () => {
       mockDir.setContent({
         [workspacePath]: {
           'catpants.md': 'cat + pants',
+          'changed-file-to-delete.txt': 'file is changed and deleted',
           'foobar.txt': 'Hello there!',
         },
       });
@@ -330,7 +329,7 @@ describe('createPublishGithubPullRequestAction', () => {
       ctx = createMockActionContext({ input, workspacePath });
     });
 
-    it('should create a pull request when no files match the marker', async () => {
+    it('should delete named files', async () => {
       await instance.handler(ctx);
 
       expect(fakeClient.createPullRequest).toHaveBeenCalledWith({
@@ -353,25 +352,37 @@ describe('createPublishGithubPullRequestAction', () => {
                 encoding: 'base64',
                 mode: '100644',
               },
+              'changed-file-to-delete.txt': DELETE_FILE,
+              'delete-me-too.md': DELETE_FILE,
             },
           },
         ],
       });
     });
 
-    describe('when files are marked for deletion', () => {
+    describe('with targetPath', () => {
+      const targetPath = `target-path-${Date.now()}`;
+
       beforeEach(() => {
+        Object.assign(input, {
+          filesToDelete: [
+            path.posix.join('nested', 'catpants.md'),
+            path.posix.join('nested', 'delete-me.too'),
+          ],
+          targetPath,
+        });
+
         mockDir.setContent({
           [workspacePath]: {
-            'foo.txt': 'Hello there!',
-            'im-here-to-be-deleted': deletionMarker,
-            'baz.txt': 'baz text',
-            'delete-me-too': deletionMarker,
+            'catpants.md': 'cat + pants',
+            'foobar.txt': 'Hello there!',
+            [path.posix.join('nested', 'catpants.md')]: 'delete me',
+            [path.posix.join('nested', 'delete-me.too')]: 'delete me too',
           },
         });
       });
 
-      it('should delete marked files', async () => {
+      it('should delete named files', async () => {
         await instance.handler(ctx);
 
         expect(fakeClient.createPullRequest).toHaveBeenCalledWith({
@@ -384,18 +395,20 @@ describe('createPublishGithubPullRequestAction', () => {
             {
               commit: input.title,
               files: {
-                'foo.txt': {
+                [path.posix.join(targetPath, 'catpants.md')]: {
+                  content: Buffer.from('cat + pants').toString('base64'),
+                  encoding: 'base64',
+                  mode: '100644',
+                },
+                [path.posix.join(targetPath, 'foobar.txt')]: {
                   content: Buffer.from('Hello there!').toString('base64'),
                   encoding: 'base64',
                   mode: '100644',
                 },
-                'im-here-to-be-deleted': DELETE_FILE,
-                'baz.txt': {
-                  content: Buffer.from('baz text').toString('base64'),
-                  encoding: 'base64',
-                  mode: '100644',
-                },
-                'delete-me-too': DELETE_FILE,
+                [path.posix.join(targetPath, 'nested', 'catpants.md')]:
+                  DELETE_FILE,
+                [path.posix.join(targetPath, 'nested', 'delete-me.too')]:
+                  DELETE_FILE,
               },
             },
           ],

plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts

@@ -146,13 +146,10 @@ export const createPublishGithubPullRequestAction = (
           z.string({
             description: 'The name for the branch',
           }),
-        deletionMarker: z =>
+        filesToDelete: z =>
           z
-            .string({
-              description: 'Contents of files that will be deleted',
-            })
-            .min(33, {
-              message: 'deletion marker must be at least 33 characters long',
+            .array(z.string(), {
+              description: 'List of files that will be deleted',
             })
             .optional(),
         targetBranchName: z =>
@@ -281,7 +278,7 @@ export const createPublishGithubPullRequestAction = (
       const {
         repoUrl,
         branchName,
-        deletionMarker,
+        filesToDelete,
         targetBranchName,
         title,
         description,
@@ -336,34 +333,38 @@ export const createPublishGithubPullRequestAction = (
         file: SerializedFile,
       ): 'utf-8' | 'base64' => (file.symlink ? 'utf-8' : 'base64');
 
-      const encodedDeletionMarker =
-        deletionMarker && Buffer.from(deletionMarker).toString('base64');
       const files = Object.fromEntries(
-        directoryContents.map(file => {
-          const content = file.content.toString(determineFileEncoding(file));
-
-          return [
-            targetPath ? path.posix.join(targetPath, file.path) : file.path,
-            content === encodedDeletionMarker
-              ? DELETE_FILE
-              : {
-                  // See the properties of tree items
-                  // in https://docs.github.com/en/rest/reference/git#trees
-                  mode: determineFileMode(file),
-                  // Always use base64 encoding where possible to avoid doubling a binary file in size
-                  // due to interpreting a binary file as utf-8 and sending github
-                  // the utf-8 encoded content. Symlinks are kept as utf-8 to avoid them
-                  // being formatted as a series of scrambled characters
-                  //
-                  // For example, the original gradle-wrapper.jar is 57.8k in https://github.com/kennethzfeng/pull-request-test/pull/5/files.
-                  // Its size could be doubled to 98.3K (See https://github.com/kennethzfeng/pull-request-test/pull/4/files)
-                  encoding: determineFileEncoding(file),
-                  content,
-                },
-          ];
-        }),
+        directoryContents.map(file => [
+          targetPath ? path.posix.join(targetPath, file.path) : file.path,
+          {
+            // See the properties of tree items
+            // in https://docs.github.com/en/rest/reference/git#trees
+            mode: determineFileMode(file),
+            // Always use base64 encoding where possible to avoid doubling a binary file in size
+            // due to interpreting a binary file as utf-8 and sending github
+            // the utf-8 encoded content. Symlinks are kept as utf-8 to avoid them
+            // being formatted as a series of scrambled characters
+            //
+            // For example, the original gradle-wrapper.jar is 57.8k in https://github.com/kennethzfeng/pull-request-test/pull/5/files.
+            // Its size could be doubled to 98.3K (See https://github.com/kennethzfeng/pull-request-test/pull/4/files)
+            encoding: determineFileEncoding(file),
+            content: file.content.toString(determineFileEncoding(file)),
+          },
+        ]),
       );
 
+      if (filesToDelete) {
+        Object.assign(
+          files,
+          Object.fromEntries(
+            filesToDelete.map(filePath => [
+              targetPath ? path.posix.join(targetPath, filePath) : filePath,
+              DELETE_FILE,
+            ]),
+          ),
+        );
+      }
+
       // If this is a dry run, log and return
       if (ctx.isDryRun) {
         ctx.logger.info(`Performing dry run of creating pull request`);