Enhance app installations plugin: add subject handling and improve repo selection logic

Author: decyjphr

lib/nopcommand.js

@@ -1,8 +1,22 @@
 class NopCommand {
-  constructor (pluginName, repo, endpoint, action, type = 'INFO') {
+  constructor (pluginName, repo, endpoint, action, type = 'INFO', subject = null) {
+    // Ergonomic overload: allow passing the subject in the `type` position so
+    // callers can supply a subject while omitting the (default 'INFO') type,
+    // e.g. new NopCommand(plugin, repo, endpoint, action, { name, type }).
+    if (type !== null && typeof type === 'object') {
+      subject = type
+      type = 'INFO'
+    }
     this.type = type
     this.plugin = pluginName
     this.repo = repo.repo
+    // Optional presentation subject. Some plugins (e.g. app_installations)
+    // operate on a non-repo entity — the "subject" of the change is a GitHub
+    // App, not a repository. `subject` overrides the repo as the heading in the
+    // PR-comment/check-run report, while `repo` is still used for grouping and
+    // repo counts. Defaults to the repo so existing plugins are unaffected.
+    this.subject = (subject && subject.name) || repo.repo
+    this.subjectType = (subject && subject.type) || 'repo'
     this.endpoint = endpoint ? endpoint.url : ''
     this.body = endpoint ? endpoint.body : ''
     // check if action is a string

lib/plugins/appInstallations.js

@@ -152,7 +152,7 @@ class AppInstallations {
           additions: ['(all repositories)'],
           modifications: null,
           deletions: null
-        }))
+        }, { name: appSlug, type: 'app' }))
         return results
       }
       await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'all')
@@ -181,7 +181,7 @@ class AppInstallations {
           additions: [...desiredNames],
           modifications: null,
           deletions: ['(all repositories)']
-        }))
+        }, { name: appSlug, type: 'app' }))
         return results
       }
       await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'selected', [...desiredNames])
@@ -209,7 +209,7 @@ class AppInstallations {
         additions: toAdd.length > 0 ? toAdd : null,
         modifications: null,
         deletions: toRemove.length > 0 ? toRemove : null
-      }))
+      }, { name: appSlug, type: 'app' }))
       return results
     }
 
@@ -265,7 +265,8 @@ class AppInstallations {
             additions: ['(all repositories)'],
             modifications: null,
             deletions: null
-          }
+          },
+          { name: app_slug, type: 'app' }
         ))
         return results
       }
@@ -288,7 +289,8 @@ class AppInstallations {
           additions,
           modifications: null,
           deletions
-        }
+        },
+        { name: app_slug, type: 'app' }
       ))
       return results
     }

lib/settings.js

@@ -170,14 +170,19 @@ function filterActionByChangedNames (action, changedNames) {
 
 function buildChangeSections (changes, baseConfig, config) {
   return Object.keys(changes).map(plugin => {
+    const isAppInstallations = plugin === 'app_installations'
     const repoSections = []
     Object.keys(changes[plugin]).forEach(repo => {
       const targetMap = new Map()
       changes[plugin][repo].forEach(action => {
-        targetsForAction(plugin, repo, action, baseConfig, config).forEach(target => {
+        const actionTargets = isAppInstallations
+          ? appInstallationTargets(action)
+          : targetsForAction(plugin, repo, action, baseConfig, config)
+        actionTargets.forEach(target => {
           if (!targetMap.has(target.target)) {
             targetMap.set(target.target, {
               target: target.target,
+              flat: target.flat === true,
               rows: []
             })
           }
@@ -194,11 +199,17 @@ function buildChangeSections (changes, baseConfig, config) {
     const changeCount = filteredRepoSections.reduce((count, repoSection) => {
       return count + repoSection.targets.reduce((targetCount, target) => targetCount + target.rows.length, 0)
     }, 0)
-    const targetCount = filteredRepoSections.reduce((count, repoSection) => count + repoSection.targets.length, 0)
+    // For flat targets (app_installations) each row is a distinct change; for
+    // regular targets each target counts as one changed setting.
+    const targetCount = filteredRepoSections.reduce((count, repoSection) => {
+      return count + repoSection.targets.reduce((tc, target) => tc + (target.flat ? target.rows.length : 1), 0)
+    }, 0)
     const repoCount = filteredRepoSections.length
+    const subjectSingular = isAppInstallations ? 'app' : 'repo'
+    const subjectPlural = isAppInstallations ? 'apps' : 'repos'
     const targetSingular = plugin.toLowerCase() === 'rulesets' ? 'policy' : 'setting'
     const targetPlural = plugin.toLowerCase() === 'rulesets' ? 'policies' : 'settings'
-    const impactSummary = `${repoCount} ${pluralize(repoCount, 'repo', 'repos')}, ${targetCount} ${pluralize(targetCount, targetSingular, targetPlural)} changed`
+    const impactSummary = `${repoCount} ${pluralize(repoCount, subjectSingular, subjectPlural)}, ${targetCount} ${pluralize(targetCount, targetSingular, targetPlural)} changed`
     return {
       plugin,
       repoSections: filteredRepoSections,
@@ -211,10 +222,40 @@ function buildChangeSections (changes, baseConfig, config) {
   }).filter(section => section.repoSections.length > 0)
 }
 
+// app_installations changes are presented with the GitHub App as the subject
+// (heading) and a flat list of repositories added/removed (or a toggle to
+// "all"). Returns a single flat target whose rows carry a `label` per change.
+function appInstallationTargets (action) {
+  if (!action || typeof action === 'string') {
+    return [{ target: '', flat: true, rows: action ? [{ change: 'Info', label: action }] : [] }]
+  }
+  const toList = value => {
+    if (value === null || value === undefined) return []
+    const arr = Array.isArray(value) ? value : [value]
+    return arr
+      .filter(entry => !isDeepEmpty(entry))
+      .map(entry => (typeof entry === 'string' ? entry : (getEntryIdentityValue(entry) || JSON.stringify(entry))))
+  }
+  const rows = []
+  toList(action.additions).forEach(label => rows.push({ change: 'Added', label }))
+  toList(action.modifications).forEach(label => rows.push({ change: 'Modified', label }))
+  toList(action.deletions).forEach(label => rows.push({ change: 'Deleted', label }))
+  if (rows.length === 0 && action.msg) rows.push({ change: 'Info', label: action.msg })
+  return [{ target: '', flat: true, rows }]
+}
+
 function renderChangeSections (changeSections) {
   return changeSections.map(section => {
     const repoBlocks = section.repoSections.map(repoSection => {
       const targetBlocks = repoSection.targets.map(target => {
+        if (target.flat) {
+          return target.rows.map(row => {
+            const marker = changeMarker(row.change)
+            return row.change === 'Info'
+              ? `- ${marker} ${markdownText(row.label)}`
+              : `- ${marker} ${markdownInlineCode(row.label)}`
+          }).join('\n')
+        }
         return `- ${markdownInlineCode(target.target)}\n${renderFieldChangeList(target.rows, '  ')}`
       })
       return `**${markdownText(displayRepoName(repoSection.repo))}**\n${targetBlocks.join('\n')}`
@@ -225,9 +266,13 @@ function renderChangeSections (changeSections) {
 }
 
 function affectedRepoCount (changeSections) {
-  return new Set(changeSections.flatMap(section => {
-    return section.repoSections.map(repoSection => displayRepoName(repoSection.repo))
-  })).size
+  return new Set(changeSections
+    // app_installations sections are keyed by app subject, not repositories,
+    // so they must not inflate the "repos affected" count.
+    .filter(section => section.plugin !== 'app_installations')
+    .flatMap(section => {
+      return section.repoSections.map(repoSection => displayRepoName(repoSection.repo))
+    })).size
 }
 
 function displayRepoName (repo) {
@@ -1103,10 +1148,14 @@ class Settings {
           if (!stats.changes[res.plugin]) {
             stats.changes[res.plugin] = {}
           }
-          if (!stats.changes[res.plugin][res.repo]) {
-            stats.changes[res.plugin][res.repo] = []
+          // Group by the result's subject (defaults to the repo). Plugins that
+          // act on a non-repo entity — e.g. app_installations, whose subject is
+          // a GitHub App — group under that subject instead of the (org) repo.
+          const subject = res.subject || res.repo
+          if (!stats.changes[res.plugin][subject]) {
+            stats.changes[res.plugin][subject] = []
           }
-          stats.changes[res.plugin][res.repo].push(res.action)
+          stats.changes[res.plugin][subject].push(res.action)
         }
       }
     })
@@ -2185,6 +2234,11 @@ class Settings {
       if (section !== 'repositories' && section !== 'repository') {
         // Ignore any config that is not a plugin
         if (section in Settings.PLUGINS) {
+          // app_installations is not a per-repo Diffable plugin; it operates at
+          // the org level on app installations and is reconciled separately by
+          // syncAppInstallations(). Skip it here so the per-repo pipeline does
+          // not try to call the (non-existent) sync() on it.
+          if (section === 'app_installations') continue
           this.log.debug(`Found section ${section} in the config. Creating plugin...`)
           const Plugin = Settings.PLUGINS[section]
           // Include sectionName as 3rd element so callers can thread the