500303: Refactor the Octokit work

This refactor means that the tests and production code are using the
same work. The only difference being the stubs pass a baseUrl to point
to cdp-portal-stubs, in Journey tests.

This work will now the privateKey error we had in #347

src/helpers/github/commit-github-file.js

@@ -1,5 +1,5 @@
 import { getLatestCommitSha } from '~/src/helpers/github/get-latest-commit-sha.js'
-import { graphql } from '~/src/helpers/oktokit.js'
+import { graphql } from '~/src/helpers/oktokit/oktokit.js'
 
 /**
  * @param {{owner: string, repo: string, branch: string, commitMessage: string, filePath: string, content: object, logger: import('pino').Logger}} options

src/helpers/github/delete-github-file.js

@@ -1,5 +1,5 @@
 import { getLatestCommitSha } from '~/src/helpers/github/get-latest-commit-sha.js'
-import { graphql } from '~/src/helpers/oktokit.js'
+import { graphql } from '~/src/helpers/oktokit/oktokit.js'
 import { createLogger } from '~/src/helpers/logging/logger.js'
 
 const logger = createLogger()

src/helpers/github/get-content.js

@@ -1,4 +1,4 @@
-import { octokit } from '~/src/helpers/oktokit.js'
+import { octokit } from '~/src/helpers/oktokit/oktokit.js'
 
 async function getContent(owner, repo, filePath, ref = 'main') {
   const { data } = await octokit.rest.repos.getContent({

src/helpers/github/get-latest-commit-sha.js

@@ -1,4 +1,4 @@
-import { octokit } from '~/src/helpers/oktokit.js'
+import { octokit } from '~/src/helpers/oktokit/oktokit.js'
 
 async function getLatestCommitSha(owner, repo, branch = 'main') {
   const { data } = await octokit.rest.git.getRef({

src/helpers/github/trigger-workflow.js

@@ -1,4 +1,4 @@
-import { octokit } from '~/src/helpers/oktokit.js'
+import { octokit } from '~/src/helpers/oktokit/oktokit.js'
 
 import { config } from '~/src/config/index.js'
 

src/helpers/oktokit/factory.js

@@ -0,0 +1,59 @@
+import { createAppAuth } from '@octokit/auth-app'
+
+import { removeNil } from '~/src/helpers/remove-nill.js'
+import { proxyFetch } from '~/src/helpers/proxy/proxy-fetch.js'
+
+/**
+ * @typedef {object} GitHubConfig
+ * @property {string|null} baseUrl
+ * @property {object} app
+ * @property {string} app.id
+ * @property {string} app.privateKey
+ * @property {string} app.installationId
+ */
+
+/**
+ * @typedef {import('@octokit/core').Octokit} Octokit
+ * @typedef {import('@octokit/core').Constructor} Constructor
+ * @typedef {import('@octokit/graphql').graphql} graphql
+ */
+
+/**
+ * Builds the Octokit and graphql instances to be used across the app
+ * @param {Octokit & Constructor} OctokitExtra
+ * @param {GitHubConfig} gitHubConfig
+ * @returns {{octokit: Octokit, graphql: graphql}}
+ */
+function octokitFactory(OctokitExtra, gitHubConfig) {
+  const baseUrl = gitHubConfig.baseUrl
+  const auth = {
+    appId: gitHubConfig.app.id,
+    privateKey: Buffer.from(gitHubConfig.app.privateKey, 'base64').toString(
+      'utf8'
+    ),
+    installationId: gitHubConfig.app.installationId
+  }
+
+  const octokit = new OctokitExtra(
+    removeNil({
+      authStrategy: createAppAuth,
+      auth,
+      request: { fetch: proxyFetch, baseUrl },
+      baseUrl
+    })
+  )
+
+  const graphql = octokit.graphql.defaults(
+    removeNil({
+      request: {
+        hook: octokit.auth.hook,
+        fetch: proxyFetch
+      },
+      baseUrl
+    })
+  )
+
+  return { octokit, graphql }
+}
+
+export { octokitFactory }

src/helpers/oktokit/factory.test.js

@@ -0,0 +1,94 @@
+import { createAppAuth } from '@octokit/auth-app'
+import { octokitFactory } from '~/src/helpers/oktokit/factory.js'
+import { proxyFetch } from '~/src/helpers/proxy/proxy-fetch.js'
+
+const buildConfig = (baseUrl) => ({
+  ...(baseUrl && { baseUrl }),
+  isStubbed: baseUrl !== undefined,
+  app: {
+    id: '123',
+    privateKey: 'bW9jay1wcml2YXRlLWtleQ==', // base64 encoded 'mock-private-key' string
+    installationId: '456'
+  }
+})
+
+describe('#octokitFactory', () => {
+  const mockGraphqlDefaults = jest.fn()
+  const mockOctokitExtra = jest.fn().mockReturnValue({
+    graphql: { defaults: mockGraphqlDefaults },
+    auth: { hook: 'mockAuthHook' }
+  })
+  const stubBaseUrl = 'http://cdp.127.0.0.1.sslip.io:3333'
+
+  describe('When stubbed', () => {
+    beforeEach(() => {
+      octokitFactory(mockOctokitExtra, buildConfig(stubBaseUrl))
+    })
+
+    test('Should call OctokitExtra for a stubbed octokit instance', () => {
+      expect(mockOctokitExtra).toHaveBeenCalledWith({
+        authStrategy: createAppAuth,
+        auth: {
+          appId: '123',
+          privateKey: 'mock-private-key',
+          installationId: '456'
+        },
+        request: {
+          fetch: proxyFetch,
+          baseUrl: stubBaseUrl
+        },
+        baseUrl: stubBaseUrl
+      })
+    })
+
+    test('Should call OctokitExtra for a stubbed graphql instance', () => {
+      expect(mockGraphqlDefaults).toHaveBeenCalledWith({
+        request: {
+          hook: 'mockAuthHook',
+          fetch: proxyFetch
+        },
+        baseUrl: stubBaseUrl
+      })
+    })
+  })
+
+  describe('When not stubbed', () => {
+    beforeEach(() => {
+      octokitFactory(mockOctokitExtra, buildConfig())
+    })
+
+    test('Should call OctokitExtra for a non stubbed octokit instance', () => {
+      expect(mockOctokitExtra).toHaveBeenCalledWith({
+        authStrategy: createAppAuth,
+        auth: {
+          appId: '123',
+          privateKey: 'mock-private-key',
+          installationId: '456'
+        },
+        request: {
+          fetch: proxyFetch
+        }
+      })
+    })
+
+    test('Octokit extra options should not include baseUrl', () => {
+      expect(mockOctokitExtra.mock.calls[0][0]).not.toHaveProperty('baseUrl')
+      expect(mockOctokitExtra.mock.calls[0][0].request).not.toHaveProperty(
+        'baseUrl'
+      )
+    })
+
+    test('Should call OctokitExtra for a non stubbed graphql instance', () => {
+      expect(mockGraphqlDefaults).toHaveBeenCalledWith({
+        request: {
+          hook: 'mockAuthHook',
+          fetch: proxyFetch
+        }
+      })
+    })
+
+    test('Octokit graphql defaults should not include baseUrl', () => {
+      expect(mockGraphqlDefaults.mock.calls[0][0]).not.toHaveProperty('baseUrl')
+    })
+  })
+})

src/helpers/oktokit/oktokit.js

@@ -0,0 +1,13 @@
+import { Octokit } from '@octokit/core'
+import { restEndpointMethods } from '@octokit/plugin-rest-endpoint-methods'
+import { createPullRequest } from 'octokit-plugin-create-pull-request'
+
+import { config } from '~/src/config/index.js'
+import { octokitFactory } from '~/src/helpers/oktokit/factory.js'
+
+const OctokitExtra = Octokit.plugin(restEndpointMethods, createPullRequest)
+const githubConfig = config.get('github')
+
+const { octokit, graphql } = octokitFactory(OctokitExtra, githubConfig)
+
+export { octokit, graphql }

src/helpers/remove-nill.js

@@ -0,0 +1,23 @@
+import isNil from 'lodash/isNil'
+
+/**
+ * Remove null and undefined values from an object
+ * @param {object} obj
+ * @returns {object}
+ */
+function removeNil(obj) {
+  if (Array.isArray(obj)) {
+    return obj.map(removeNil).filter((value) => !isNil(value))
+  } else if (!isNil(obj) && typeof obj === 'object') {
+    return Object.entries(obj).reduce((cleaned, [key, value]) => {
+      const clean = removeNil(value)
+      if (!isNil(clean)) {
+        cleaned[key] = clean
+      }
+      return cleaned
+    }, {})
+  }
+  return obj
+}
+
+export { removeNil }

src/helpers/remove-nill.test.js

@@ -0,0 +1,45 @@
+import { removeNil } from '~/src/helpers/remove-nill.js'
+
+describe('#removeNil', () => {
+  test('Should remove null and undefined values from a flat object', () => {
+    const input = { a: 1, b: null, c: undefined, d: 4 }
+    const expected = { a: 1, d: 4 }
+
+    expect(removeNil(input)).toEqual(expected)
+  })
+
+  test('Should remove null and undefined values from a nested object', () => {
+    const input = { a: 1, b: { c: null, d: 4, e: undefined }, f: 5 }
+    const expected = { a: 1, b: { d: 4 }, f: 5 }
+
+    expect(removeNil(input)).toEqual(expected)
+  })
+
+  test('Should remove null and undefined values from an array', () => {
+    const input = [1, null, 2, undefined, 3]
+    const expected = [1, 2, 3]
+
+    expect(removeNil(input)).toEqual(expected)
+  })
+
+  test('Should remove null and undefined values from a nested array', () => {
+    const input = [1, [null, 2, undefined], 3]
+    const expected = [1, [2], 3]
+
+    expect(removeNil(input)).toEqual(expected)
+  })
+
+  test('Should return the same value for non-object and non-array inputs', () => {
+    expect(removeNil(1)).toBe(1)
+    expect(removeNil(null)).toBeNull()
+    expect(removeNil(undefined)).toBeUndefined()
+    expect(removeNil('string')).toBe('string')
+  })
+
+  test('Should handle complex nested structures', () => {
+    const input = { a: [1, null, { b: undefined, c: 3 }], d: { e: [null, 4] } }
+    const expected = { a: [1, { c: 3 }], d: { e: [4] } }
+
+    expect(removeNil(input)).toEqual(expected)
+  })
+})