Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 422 errors which might be do to Archive changes #806

Merged
merged 4 commits into from
Apr 6, 2025
Merged

Fix 422 errors which might be do to Archive changes #806

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8239721
undo Archive changes
decyjphr Apr 6, 2025
6dc9555
Update lib/settings.js
decyjphr Apr 6, 2025
11ed49c
Update lib/settings.js
decyjphr Apr 6, 2025
46a6dc0
rollback the object assign fix suggested by Copilot
decyjphr Apr 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 50 additions & 82 deletions lib/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ class Settings {

// remove duplicate rows in this.results
this.results = this.results.filter((thing, index, self) => {
return index === self.findIndex((t) => {
return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin
return index === self.findIndex((t) => {
return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin
})
})
})

let error = false
// Different logic
Expand Down Expand Up @@ -296,12 +296,11 @@ ${this.results.reduce((x, y) => {
}
}

async updateRepos (repo) {
async updateRepos(repo) {
this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs()
// Create a fresh copy of the base repository config
let repoConfig = this.config.repository ? Object.assign({}, this.config.repository) : {}
let repoConfig = this.config.repository
if (repoConfig) {
repoConfig = Object.assign({}, repoConfig, { name: repo.repo, org: repo.owner })
repoConfig = Object.assign(repoConfig, { name: repo.repo, org: repo.owner })
Copy link
Preview

Copilot AI Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly modifying repoConfig may introduce unintended side effects. Consider using a shallow clone (e.g., Object.assign({}, repoConfig, ...)) to avoid mutating the base configuration.

Suggested change
repoConfig = Object.assign(repoConfig, { name: repo.repo, org: repo.owner })
repoConfig = Object.assign({}, repoConfig, { name: repo.repo, org: repo.owner })

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

}

const subOrgConfig = this.getSubOrgConfig(repo.repo)
Expand All @@ -315,7 +314,7 @@ ${this.results.reduce((x, y) => {
this.log.debug(`Process normally... Not a SubOrg config change or SubOrg config was changed and this repo is part of it. ${JSON.stringify(repo)} suborg config ${JSON.stringify(this.subOrgConfigMap)}`)

if (subOrgConfig) {
let suborgRepoConfig = subOrgConfig.repository ? Object.assign({}, subOrgConfig.repository) : {}
let suborgRepoConfig = subOrgConfig.repository
if (suborgRepoConfig) {
suborgRepoConfig = Object.assign({}, suborgRepoConfig, { name: repo.repo, org: repo.owner })
repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, suborgRepoConfig)
Expand All @@ -328,45 +327,42 @@ ${this.results.reduce((x, y) => {
if (overrideRepoConfig) {
repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig)
}
const { shouldContinue, nopCommands } = await new Archive(this.nop, this.github, repo, repoConfig, this.log).sync()
if (nopCommands) this.appendToResults(nopCommands)
if (shouldContinue) {
if (repoConfig) {
try {
this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`)
const childPlugins = this.childPluginsList(repo)
const RepoPlugin = Settings.PLUGINS.repository
return new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync().then(res => {
this.appendToResults(res)
return Promise.all(
childPlugins.map(([Plugin, config]) => {
return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync()
}))
}).then(res => {
this.appendToResults(res)
})
} catch (e) {
if (this.nop) {
const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR')
this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`)
this.appendToResults([nopcommand])
// throw e
} else {
throw e
}
}
} else {
this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`)
if (repoConfig) {
try {
this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`)
const childPlugins = this.childPluginsList(repo)
return Promise.all(childPlugins.map(([Plugin, config]) => {
return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => {
this.appendToResults(res)
})
}))
const RepoPlugin = Settings.PLUGINS.repository
return new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync().then(res => {
this.appendToResults(res)
return Promise.all(
childPlugins.map(([Plugin, config]) => {
return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync()
}))
}).then(res => {
this.appendToResults(res)
})
} catch (e) {
if (this.nop) {
const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR')
this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`)
this.appendToResults([nopcommand])
// throw e
} else {
throw e
}
}
} else {
this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`)
const childPlugins = this.childPluginsList(repo)
return Promise.all(childPlugins.map(([Plugin, config]) => {
return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => {
this.appendToResults(res)
})
}))
}
}


async updateAll () {
// this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(this.github, this.repo, this.log)
// this.repoConfigs = this.repoConfigs || await this.getRepoConfigs(this.github, this.repo, this.log)
Expand Down Expand Up @@ -487,47 +483,17 @@ ${this.results.reduce((x, y) => {

async eachRepositoryRepos (github, log) {
log.debug('Fetching repositories')

const processedRepos = new Set()
const results = []

// Process existing repositories
const existingRepoResults = await github.paginate('GET /installation/repositories')
.then(repositories => {
return Promise.all(repositories.map(repository => {
if (this.isRestricted(repository.name)) {
return null
}
const { owner, name } = repository
processedRepos.add(`${owner.login}/${name}`)
return this.updateRepos({ owner: owner.login, repo: name })
}))
})

// Process missing repositories
const repoInConfigs = Object.values(this.repoConfigs)
.filter(config => config.repository?.name)
.map(config => {
return {
name: config.repository.name,
owner: config.repository.organization || this.repo.owner
return github.paginate('GET /installation/repositories').then(repositories => {
return Promise.all(repositories.map(repository => {
if (this.isRestricted(repository.name)) {
return null
}
})
const missingRepoResults = await Promise.all(
repoInConfigs
.filter(repo => !this.isRestricted(repo.name))
.filter(repo => !processedRepos.has(`${repo.owner}/${repo.name}`))
.map(repo => {
processedRepos.add(`${repo.owner}/${repo.name}`)
return this.updateRepos({ owner: repo.owner, repo: repo.name })
})
)

results
.concat(existingRepoResults || [], missingRepoResults || [])
.filter(result => result !== null)

return results
const { owner, name } = repository
return this.updateRepos({ owner: owner.login, repo: name })
})
)
})
}

/**
Expand Down Expand Up @@ -790,7 +756,7 @@ ${this.results.reduce((x, y) => {
}
)) {
delete subOrgConfigs[key]
}
}
}
}
return subOrgConfigs
Expand Down Expand Up @@ -894,6 +860,7 @@ ${this.results.reduce((x, y) => {
throw new Error(`Failed to filter repositories for property ${name}: ${error.message}`)
}
}


async getSubOrgRepositories (subOrgProperties) {
const organizationName = this.repo.owner
Expand Down Expand Up @@ -940,6 +907,7 @@ function prettify (obj) {
return JSON.stringify(obj, null, 2).replaceAll('\n', '<br>').replaceAll(' ', '&nbsp;')
}

Settings.FILE_NAME = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH)
Settings.FILE_PATH = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH)
Settings.SUB_ORG_PATTERN = new Glob(`${CONFIG_PATH}/suborgs/*.yml`)
Settings.REPO_PATTERN = new Glob(`${CONFIG_PATH}/repos/*.yml`)
Expand Down