From f25def77d4f846da4907077a05c4cee34fdabc19 Mon Sep 17 00:00:00 2001 From: PendaGTP <38917281+PendaGTP@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:44:19 +0100 Subject: [PATCH] fix: add missing nop logic for custom properties plugin --- lib/plugins/custom_properties.js | 107 ++++++---- .../lib/plugins/custom_properties.test.js | 185 ++++++++++-------- 2 files changed, 169 insertions(+), 123 deletions(-) diff --git a/lib/plugins/custom_properties.js b/lib/plugins/custom_properties.js index cb0adf7d..6b1f3ab3 100644 --- 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 index f1148837..429544f8 100644 --- 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' + // } + // ] + // }) }) }) })