Skip to content

Commit 594f3c7

Browse files
decyjphrCopilot
decyjphr
and
Copilot
authored
Handle multiple config changes in a PR or Push event and process them as a batch (#888)
* handle multiple changes as a batch * Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * depup files in a push * Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * moved the dedup logic * Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 2d244e4 commit 594f3c7

File tree

2 files changed

+56
-41
lines changed

2 files changed

+56
-41
lines changed

‎index.js‎

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
4040
}
4141
}
4242

43-
async function syncSubOrgSettings (nop, context, suborg, repo = context.repo(), ref) {
43+
async function syncSettings (nop, context, repo = context.repo(), ref) {
4444
try {
4545
deploymentConfig = await loadYamlFileSystem()
4646
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
4747
const configManager = new ConfigManager(context, ref)
4848
const runtimeConfig = await configManager.loadGlobalSettingsYaml()
4949
const config = Object.assign({}, deploymentConfig, runtimeConfig)
5050
robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`)
51-
return Settings.syncSubOrgs(nop, context, suborg, repo, config, ref)
51+
return Settings.sync(nop, context, repo, config, ref)
5252
} catch (e) {
5353
if (nop) {
5454
let filename = env.SETTINGS_FILE_PATH
@@ -65,25 +65,25 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
6565
}
6666
}
6767

68-
async function syncSettings (nop, context, repo = context.repo(), ref) {
68+
async function syncSelectedSettings (nop, context, repos, subOrgs, ref) {
6969
try {
7070
deploymentConfig = await loadYamlFileSystem()
7171
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
7272
const configManager = new ConfigManager(context, ref)
7373
const runtimeConfig = await configManager.loadGlobalSettingsYaml()
7474
const config = Object.assign({}, deploymentConfig, runtimeConfig)
7575
robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`)
76-
return Settings.sync(nop, context, repo, config, ref)
76+
return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config, ref)
7777
} catch (e) {
7878
if (nop) {
7979
let filename = env.SETTINGS_FILE_PATH
8080
if (!deploymentConfig) {
8181
filename = env.DEPLOYMENT_CONFIG_FILE_PATH
8282
deploymentConfig = {}
8383
}
84-
const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
84+
const nopcommand = new NopCommand(filename, context.repo(), null, e, 'ERROR')
8585
robot.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`)
86-
Settings.handleError(nop, context, repo, deploymentConfig, ref, nopcommand)
86+
Settings.handleError(nop, context, context.repo(), deploymentConfig, ref, nopcommand)
8787
} else {
8888
throw e
8989
}
@@ -263,18 +263,17 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
263263
return syncAllSettings(false, context)
264264
}
265265

266-
const repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner)
267-
if (repoChanges.length > 0) {
268-
return Promise.all(repoChanges.map(repo => {
269-
return syncSettings(false, context, repo)
270-
}))
271-
}
266+
let repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner)
272267

273-
const changes = getAllChangedSubOrgConfigs(payload)
274-
if (changes.length) {
275-
return Promise.all(changes.map(suborg => {
276-
return syncSubOrgSettings(false, context, suborg)
277-
}))
268+
let subOrgChanges = getAllChangedSubOrgConfigs(payload)
269+
repoChanges = repoChanges.filter((r, i, arr) => arr.findIndex(item => item.repo === r.repo) === i)
270+
271+
subOrgChanges = subOrgChanges.filter((s, i, arr) => arr.findIndex(item => item.repo === s.repo) === i)
272+
robot.log.debug(`deduped repos ${JSON.stringify(repoChanges)}`)
273+
robot.log.debug(`deduped subOrgs ${JSON.stringify(subOrgChanges)}`)
274+
275+
if (repoChanges.length > 0 || subOrgChanges.length > 0) {
276+
return syncSelectedSettings(false, context, repoChanges, subOrgChanges)
278277
}
279278

280279
robot.log.debug(`No changes in '${Settings.FILE_PATH}' detected, returning...`)
@@ -572,15 +571,10 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
572571
robot.log.debug(`Updating check run ${JSON.stringify(params)}`)
573572
await context.octokit.checks.update(params)
574573

575-
// guarding against null value from upstream libary that is
576-
// causing a 404 and the check to stall
577-
// from issue: https://github.com/github/safe-settings/issues/185#issuecomment-1075240374
578-
if (check_suite.before === '0000000000000000000000000000000000000000') {
579-
check_suite.before = check_suite.pull_requests[0].base.sha
580-
}
581-
params = Object.assign(context.repo(), { basehead: `${check_suite.before}...${check_suite.after}` })
582-
const changes = await context.octokit.repos.compareCommitsWithBasehead(params)
583-
const files = changes.data.files.map(f => { return f.filename })
574+
params = Object.assign(context.repo(), { pull_number: pull_request.number })
575+
576+
const changes = await context.octokit.pulls.listFiles(params)
577+
const files = changes.data.map(f => { return f.filename })
584578

585579
const settingsModified = files.includes(Settings.FILE_PATH)
586580

@@ -590,17 +584,10 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
590584
}
591585

592586
const repoChanges = getChangedRepoConfigName(files, context.repo().owner)
593-
if (repoChanges.length > 0) {
594-
return Promise.all(repoChanges.map(repo => {
595-
return syncSettings(true, context, repo, pull_request.head.ref)
596-
}))
597-
}
598-
599587
const subOrgChanges = getChangedSubOrgConfigName(files)
600-
if (subOrgChanges.length) {
601-
return Promise.all(subOrgChanges.map(suborg => {
602-
return syncSubOrgSettings(true, context, suborg, context.repo(), pull_request.head.ref)
603-
}))
588+
589+
if (repoChanges.length > 0 || subOrgChanges.length > 0) {
590+
return syncSelectedSettings(true, context, repoChanges, subOrgChanges, pull_request.head.ref)
604591
}
605592

606593
// if no safe-settings changes detected, send a success to the check run

‎lib/settings.js‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,31 @@ class Settings {
4242
}
4343
}
4444

45+
static async syncSelectedRepos (nop, context, repos, subOrgs, config, ref) {
46+
const settings = new Settings(nop, context, context.repo(), config, ref)
47+
48+
try {
49+
for (const repo of repos) {
50+
settings.repo = repo
51+
await settings.loadConfigs(repo)
52+
if (settings.isRestricted(repo.repo)) {
53+
continue
54+
}
55+
await settings.updateRepos(repo)
56+
}
57+
for (const suborg of subOrgs) {
58+
settings.subOrgConfigMap = [suborg]
59+
settings.suborgChange = !!suborg
60+
await settings.loadConfigs()
61+
await settings.updateAll()
62+
}
63+
await settings.handleResults()
64+
} catch (error) {
65+
settings.logError(error.message)
66+
await settings.handleResults()
67+
}
68+
}
69+
4570
static async sync (nop, context, repo, config, ref) {
4671
const settings = new Settings(nop, context, repo, config, ref)
4772
try {
@@ -506,17 +531,20 @@ ${this.results.reduce((x, y) => {
506531
log.debug('Fetching repositories')
507532
return github.paginate('GET /installation/repositories').then(repositories => {
508533
return Promise.all(repositories.map(repository => {
509-
if (this.isRestricted(repository.name)) {
510-
return null
511-
}
512-
513534
const { owner, name } = repository
514-
return this.updateRepos({ owner: owner.login, repo: name })
535+
return this.checkAndProcessRepo(owner.login, name)
515536
})
516537
)
517538
})
518539
}
519540

541+
async checkAndProcessRepo (owner, name) {
542+
if (this.isRestricted(name)) {
543+
return null
544+
}
545+
return this.updateRepos({ owner, repo: name })
546+
}
547+
520548
/**
521549
* Loads a file from GitHub
522550
*

0 commit comments

Comments
 (0)