Merge pull request #105 from jwsloan/merge-by-name

feat: Add mergeByName

README.md

@@ -7,7 +7,7 @@ This GitHub App syncs repository settings defined in `.github/settings.yml` to G
 1. **[Install the app](https://github.com/apps/settings)**.
 2. Create a `.github/settings.yml` file in your repository. Changes to this file on the default branch will be synced to GitHub.
 
-All settings are optional.
+All top-level settings are optional. Some plugins do have required fields.
 
 ```yaml
 # These settings are synced to GitHub by https://probot.github.io/apps/settings/
@@ -127,6 +127,15 @@ branches:
         teams: []
 ```
 
+### Inheritance
+
+This app uses [probot-config](https://github.com/probot/probot-config). This means you can inherit settings from another repo, and only override what you want to change.
+
+Individual settings in the arrays listed under `labels`, `teams` (once it is supported) and `branches` will be merged with the base repo if the `name` of an element in the array matches the `name` of an element in the corresponding array in the base repo. A possible future enhancement would be to make that work for the other settings arrays based on `username`, or `title`. This is not currently supported. 
+
+To further clarify: Inheritance within the Protected Branches plugin allows you to override specific settings per branch. For example, your `.github` repo may set default protection on the `master` branch. You can then include `master` in your `branches` array, and only override the `required_approving_review_count`.
+Alternatively, you might only have a branch like `develop` in your `branches` array, and would still get `master` protection from your base repo.
+
 **WARNING:** Note that this app inherently _escalates anyone with `push` permissions to the **admin** role_, since they can push config settings to the `master` branch, which will be synced. In a future, we may add restrictions to allow changes to the config file to be merged only by specific people/teams, or those with **admin** access _(via a combination of protected branches, required statuses, and branch restrictions)_. Until then, use caution when merging PRs and adding collaborators.
 
 Until restrictions are added in this app, one way to preserve admin/push permissions is to utilize the [GitHub CodeOwners feature](https://help.github.com/articles/about-codeowners/) to set one or more administrative users as the code owner of the `.github/settings.yml` file, and turn on "require code owner review" for the master branch. This does have the side effect of requiring code owner review for the entire branch, but helps preserve permission levels.

index.js

@@ -1,11 +1,12 @@
 const getConfig = require('probot-config')
+const mergeArrayByName = require('./lib/mergeArrayByName')
 
 module.exports = (robot, _, Settings = require('./lib/settings')) => {
   robot.on('push', async context => {
     const payload = context.payload
     const defaultBranch = payload.ref === 'refs/heads/' + payload.repository.default_branch
 
-    const config = await getConfig(context, 'settings.yml')
+    const config = await getConfig(context, 'settings.yml', {}, { arrayMerge: mergeArrayByName })
 
     const settingsModified = payload.commits.find(commit => {
       return commit.added.includes(Settings.FILE_NAME) ||

lib/mergeArrayByName.js

@@ -0,0 +1,28 @@
+// https://github.com/KyleAMathews/deepmerge#arraymerge
+
+const merge = require('deepmerge')
+
+function findMatchingIndex (sourceItem, target) {
+  if (sourceItem.hasOwnProperty('name')) {
+    return target
+      .filter(targetItem => targetItem.hasOwnProperty('name'))
+      .findIndex(targetItem => sourceItem.name === targetItem.name)
+  }
+}
+
+function mergeByName (target, source, options) {
+  const destination = target.slice()
+
+  source.forEach(sourceItem => {
+    const matchingIndex = findMatchingIndex(sourceItem, target)
+    if (matchingIndex > -1) {
+      destination[matchingIndex] = merge(target[matchingIndex], sourceItem, options)
+    } else {
+      destination.push(sourceItem)
+    }
+  })
+
+  return destination
+}
+
+module.exports = mergeByName

package.json

@@ -12,13 +12,14 @@
   "author": "Brandon Keepers",
   "license": "ISC",
   "dependencies": {
-    "js-yaml": "^3.7.0",
-    "probot": "^7.0.1",
-    "probot-config": "^0.2.0"
+    "deepmerge": "^2.2.1",
+    "js-yaml": "^3.12.0",
+    "probot": "^7.4.0",
+    "probot-config": "^1.0.0"
   },
   "devDependencies": {
-    "jest": "^22.0.0",
-    "smee-client": "^1.0.1",
+    "jest": "^22.4.4",
+    "smee-client": "^1.0.2",
     "standard": "^12.0.0"
   },
   "standard": {

test/index.test.js

@@ -14,7 +14,7 @@ describe('plugin', () => {
     app.auth = () => Promise.resolve(github)
 
     event = {
-      event: 'push',
+      name: 'push',
       payload: JSON.parse(JSON.stringify(require('./fixtures/events/push.settings.json')))
     }
     sync = jest.fn()

test/lib/mergeArrayByName.test.js

@@ -0,0 +1,76 @@
+const branchArrayMerge = require('../../lib/mergeArrayByName')
+const YAML = require('js-yaml')
+
+describe('mergeArrayByName', () => {
+  it('works', () => {
+    const target = [{
+      name: 'master',
+      shouldChange: 'did not change',
+      shouldKeep: 'kept'
+    }, {
+      name: 'develop'
+    }]
+
+    const source = [{
+      name: 'master',
+      shouldChange: 'did change'
+    }, {
+      name: 'added'
+    }]
+
+    const merged = branchArrayMerge(target, source)
+
+    expect(merged).toEqual([{
+      name: 'master',
+      shouldChange: 'did change',
+      shouldKeep: 'kept'
+    }, {
+      name: 'develop'
+    }, {
+      name: 'added'
+    }])
+  })
+
+  it('works in a realistic scenario', () => {
+    const target = YAML.safeLoad(`
+  branches:
+    - name: master
+      protection:
+        required_pull_request_reviews:
+          required_approving_review_count: 1
+          dismiss_stale_reviews: false
+          require_code_owner_reviews: true
+          dismissal_restrictions: {}
+        required_status_checks:
+          strict: true
+          contexts: []
+        enforce_admins: false
+        restrictions:
+  `)
+
+    const source = YAML.safeLoad(`
+  branches:
+    - name: master
+      protection:
+        required_pull_request_reviews:
+          required_approving_review_count: 2
+    `)
+
+    const expected = [{
+      name: 'master',
+      protection:
+       { required_pull_request_reviews:
+          { required_approving_review_count: 2,
+            dismiss_stale_reviews: false,
+            require_code_owner_reviews: true,
+            dismissal_restrictions: {} },
+       required_status_checks: { strict: true, contexts: [] },
+       enforce_admins: false,
+       restrictions: null }
+    }]
+
+    const merged = branchArrayMerge(target.branches, source.branches)
+
+    expect(merged).toEqual(expected)
+  })
+})