fix: add missing nop logic for custom properties plugin (#738)

PendaGTP · decyjphr · web-flow · commit 8503a9460d9b · 2025-03-21T09:44:12.000-04:00
Co-authored-by: Yadhav Jayaraman &lt;57544838+decyjphr@users.noreply.github.com&gt;
diff --git a/lib/plugins/custom_properties.js b/lib/plugins/custom_properties.js
@@ -1,25 +1,47 @@
 const Diffable = require('./diffable')
+const NopCommand = require('../nopcommand')
 
 module.exports = class CustomProperties extends Diffable {
   constructor (...args) {
     super(...args)
 
     if (this.entries) {
-      // Force all names to lowercase to avoid comparison issues.
-      this.entries.forEach(prop => {
-        prop.name = prop.name.toLowerCase()
-      })
+      this.normalizeEntries()
     }
   }
 
+  // Force all names to lowercase to avoid comparison issues.
+  normalizeEntries () {
+    this.entries = this.entries.map(({ name, value }) => ({
+      name: name.toLowerCase(),
+      value
+    }))
+  }
+
   async find () {
-    const data = await this.github.request('GET /repos/:org/:repo/properties/values', {
-      org: this.repo.owner,
-      repo: this.repo.repo
-    })
+    const { owner, repo } = this.repo
+    const repoFullName = `${owner}/${repo}`
+
+    this.log.debug(`Getting all custom properties for the repo ${repoFullName}`)
+
+    const customProperties = await this.github.paginate(
+      this.github.repos.getCustomPropertiesValues,
+      {
+        owner,
+        repo,
+        per_page: 100
+      }
+    )
+    this.log.debug(`Found ${customProperties.length} custom properties`)
+    return this.normalize(customProperties)
+  }
 
-    const properties = data.data.map(d => { return { name: d.property_name, value: d.value } })
-    return properties
+  // Force all names to lowercase to avoid comparison issues.
+  normalize (properties) {
+    return properties.map(({ property_name: propertyName, value }) => ({
+      name: propertyName.toLowerCase(),
+      value
+    }))
   }
 
   comparator (existing, attrs) {
@@ -30,38 +52,47 @@ module.exports = class CustomProperties extends Diffable {
     return attrs.value !== existing.value
   }
 
-  async update (existing, attrs) {
-    await this.github.request('PATCH /repos/:org/:repo/properties/values', {
-      org: this.repo.owner,
-      repo: this.repo.repo,
-      properties: [{
-        property_name: attrs.name,
-        value: attrs.value
-      }]
-    })
+  async update ({ name }, { value }) {
+    return this.modifyProperty('Update', { name, value })
+  }
+
+  async add ({ name, value }) {
+    return this.modifyProperty('Create', { name, value })
   }
 
-  async add (attrs) {
-    await this.github.request('PATCH /repos/:org/:repo/properties/values', {
-      org: this.repo.owner,
-      repo: this.repo.repo,
+  // Custom Properties on repository does not support deletion, so we set the value to null
+  async remove ({ name }) {
+    return this.modifyProperty('Delete', { name, value: null })
+  }
+
+  async modifyProperty (operation, { name, value }) {
+    const { owner, repo } = this.repo
+    const repoFullName = `${owner}/${repo}`
+
+    const params = {
+      owner,
+      repo,
       properties: [{
-        property_name: attrs.name,
-        value: attrs.value
+        property_name: name,
+        value
       }]
-    })
-  }
+    }
+
+    if (this.nop) {
+      return new NopCommand(
+        this.constructor.name,
+        this.repo,
+        this.github.repos.createOrUpdateCustomPropertiesValues.endpoint(params),
+        `${operation} Custom Property`
+      )
+    }
 
-  async remove (existing) {
-    await this.github.request('PATCH /repos/:org/:repo/properties/values', {
-      org: this.repo.owner,
-      repo: this.repo.repo,
-      properties: [
-        {
-          property_name: existing.name,
-          value: null
-        }
-      ]
-    })
+    try {
+      this.log.debug(`${operation} Custom Property "${name}" for the repo ${repoFullName}`)
+      await this.github.repos.createOrUpdateCustomPropertiesValues(params)
+      this.log.debug(`Successfully ${operation.toLowerCase()}d Custom Property "${name}" for the repo ${repoFullName}`)
+    } catch (e) {
+      this.logError(`Error during ${operation} Custom Property "${name}" for the repo ${repoFullName}: ${e.message || e}`)
+    }
   }
 }
diff --git a/test/unit/lib/plugins/custom_properties.test.js b/test/unit/lib/plugins/custom_properties.test.js
@@ -1,127 +1,142 @@
 const CustomProperties = require('../../../../lib/plugins/custom_properties')
 
 describe('CustomProperties', () => {
+  const nop = false
   let github
   let log
 
+  const owner = 'test-owner'
+  const repo = 'test-repo'
+
   function configure (config) {
-    const nop = false
-    const errors = []
-    return new CustomProperties(nop, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors)
+    return new CustomProperties(nop, github, { owner, repo }, config, log, [])
   }
 
   beforeEach(() => {
     github = {
-      request: jest.fn()
-      //     .mockResolvedValue({
-      //     data: [
-      //         { property_name: 'test', value: 'test' }
-      //     ]
-      // })
+      paginate: jest.fn(),
+      repos: {
+        getCustomPropertiesValues: jest.fn(),
+        createOrUpdateCustomPropertiesValues: jest.fn()
+      }
     }
+
     log = { debug: jest.fn(), error: console.error }
   })
 
-  describe('sync', () => {
-    it('syncs custom properties', async () => {
-      const plugin = configure([
-        { name: 'test', value: 'test' }
-      ])
+  describe('Custom Properties plugin', () => {
+    it('should normalize entries when be instantiated', () => {
+      const plugin = configure([{ name: 'Test', value: 'test' }])
+      expect(plugin.entries).toEqual([{ name: 'test', value: 'test' }])
+    })
 
-      github.request.mockResolvedValue({
-        data: [
-          { property_name: 'test', value: 'test' }
-        ]
-      })
+    it('should fetch and normalize custom properties successfully', async () => {
+      const mockResponse = [
+        { property_name: 'Test1', value: 'value1' },
+        { property_name: 'Test2', value: 'value2' }
+      ]
 
-      return plugin.sync().then(() => {
-        expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test'
-        })
-      })
+      github.paginate.mockResolvedValue(mockResponse)
+
+      const plugin = configure()
+      const result = await plugin.find()
+
+      expect(github.paginate).toHaveBeenCalledWith(
+        github.repos.getCustomPropertiesValues,
+        {
+          owner,
+          repo,
+          per_page: 100
+        }
+      )
+
+      expect(result).toEqual([
+        { name: 'test1', value: 'value1' },
+        { name: 'test2', value: 'value2' }
+      ])
     })
-  })
-  describe('sync', () => {
-    it('add custom properties', async () => {
+
+    it('should sync', async () => {
+      const mockResponse = [
+        { property_name: 'no-change', value: 'no-change' },
+        { property_name: 'new-value', value: '' },
+        { property_name: 'update-value', value: 'update-value' },
+        { property_name: 'delete-value', value: 'update-value' }
+      ]
+
+      github.paginate.mockResolvedValue(mockResponse)
+
       const plugin = configure([
-        { name: 'test', value: 'test' }
+        { name: 'no-change', value: 'no-change' },
+        { name: 'new-value', value: 'new-value' },
+        { name: 'update-value', value: 'new-value' },
+        { name: 'delete-value', value: null }
       ])
 
-      github.request.mockResolvedValue({
-        data: []
-      })
-
       return plugin.sync().then(() => {
-        expect(github.request).toHaveBeenNthCalledWith(1, 'GET /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test'
-        })
-        expect(github.request).toHaveBeenNthCalledWith(2, 'PATCH /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test',
+        expect(github.paginate).toHaveBeenCalledWith(
+          github.repos.getCustomPropertiesValues,
+          {
+            owner,
+            repo,
+            per_page: 100
+          }
+        )
+        expect(github.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalledWith({
+          owner,
+          repo,
           properties: [
             {
-              property_name: 'test',
-              value: 'test'
+              property_name: 'no-change',
+              value: 'no-change'
             }
           ]
         })
-      })
-    })
-  })
-  describe('sync', () => {
-    it('remove custom properties', async () => {
-      const plugin = configure([])
-
-      github.request.mockResolvedValue({
-        data: [{ property_name: 'test', value: 'test' }]
-      })
-
-      return plugin.sync().then(() => {
-        expect(github.request).toHaveBeenNthCalledWith(1, 'GET /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test'
-        })
-        expect(github.request).toHaveBeenNthCalledWith(2, 'PATCH /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test',
+        expect(github.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith({
+          owner,
+          repo,
           properties: [
             {
-              property_name: 'test',
-              value: null
+              property_name: 'new-value',
+              value: 'new-value'
             }
           ]
         })
-      })
-    })
-  })
-  describe('sync', () => {
-    it('update custom properties', async () => {
-      const plugin = configure([
-        { name: 'test', value: 'foobar' }
-      ])
-
-      github.request.mockResolvedValue({
-        data: [{ property_name: 'test', value: 'test' }]
-      })
-
-      return plugin.sync().then(() => {
-        expect(github.request).toHaveBeenNthCalledWith(1, 'GET /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test'
+        expect(github.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith({
+          owner,
+          repo,
+          properties: [
+            {
+              property_name: 'update-value',
+              value: 'new-value'
+            }
+          ]
         })
-        expect(github.request).toHaveBeenNthCalledWith(2, 'PATCH /repos/:org/:repo/properties/values', {
-          org: 'bkeepers',
-          repo: 'test',
+        expect(github.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith({
+          owner,
+          repo,
           properties: [
             {
-              property_name: 'test',
-              value: 'foobar'
+              property_name: 'delete-value',
+              value: null
             }
           ]
         })
       })
+
+      // const plugin = configure([{ name: 'Test', value: 'test' }])
+      // await plugin.update({ name: 'test', value: 'old' }, { name: 'test', value: 'test' })
+
+      // expect(github.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith({
+      //   owner,
+      //   repo,
+      //   properties: [
+      //     {
+      //       property_name: 'test',
+      //       value: 'test'
+      //     }
+      //   ]
+      // })
     })
   })
 })
