fix(rulesets): account for details like rules not being included in…

… the list response

for #732

lib/plugins/rulesets.js

@@ -6,23 +6,29 @@ export default class Rulesets extends Diffable {
   async find () {
     const { data: rulesets } = await this.github.repos.getRepoRulesets(this.repo)
 
-    return rulesets
+    const expandedRulesetsData = await Promise.all(
+      rulesets.map(({ id }) => this.github.repos.getRepoRuleset({ ...this.repo, ruleset_id: id }))
+    )
+
+    return expandedRulesetsData.map(({ data }) => data)
   }
 
   comparator (existing, attrs) {
     return existing.name === attrs.name
   }
 
   changed (existing, attrs) {
-    return !deepEqual(existing, attrs)
+    const { id, ...existingAttrs } = existing
+
+    return !deepEqual(existingAttrs, attrs)
   }
 
   update (existing, attrs) {
-    return this.github.repos.updateRepoRuleset({ ...this.repo, ruleset_id: existing.ruleset_id, ...attrs })
+    return this.github.repos.updateRepoRuleset({ ...this.repo, ruleset_id: existing.id, ...attrs })
   }
 
   remove (existing) {
-    return this.github.repos.deleteRepoRuleset({ ...this.repo, ruleset_id: existing.ruleset_id })
+    return this.github.repos.deleteRepoRuleset({ ...this.repo, ruleset_id: existing.id })
   }
 
   async add (attrs) {

test/integration/features/step_definitions/rulesets-steps.js

@@ -11,6 +11,7 @@ import any from '@travi/any'
 
 const rulesetId = any.integer()
 const rulesetName = any.word()
+const existingRules = any.listOf(any.simpleObject)
 
 Given('no rulesets are defined for the repository', async function () {
   this.server.use(
@@ -21,9 +22,15 @@ Given('no rulesets are defined for the repository', async function () {
 })
 
 Given('a ruleset exists for the repository', async function () {
+  const rulesetSubset = { name: rulesetName }
+
   this.server.use(
     http.get(`https://api.github.com/repos/${repository.owner.name}/${repository.name}/rulesets`, ({ request }) =>
-      HttpResponse.json([{ ruleset_id: rulesetId, name: rulesetName, enforcement: 'evaluate' }])
+      HttpResponse.json([{ id: rulesetId, ...rulesetSubset }])
+    ),
+    http.get(
+      `https://api.github.com/repos/${repository.owner.name}/${repository.name}/rulesets/${rulesetId}`,
+      ({ request }) => HttpResponse.json({ id: rulesetId, ...rulesetSubset, rules: existingRules })
     )
   )
 })
@@ -50,26 +57,20 @@ Given('a ruleset is defined in the config', async function () {
 })
 
 Given('the ruleset is modified in the config', async function () {
-  this.ruleset = { name: rulesetName, enforcement: 'active' }
+  const additionalRule = any.simpleObject()
+  this.updatedRuleset = { name: rulesetName, rules: [...existingRules, additionalRule] }
 
   this.server.use(
     http.get(
       `https://api.github.com/repos/${repository.owner.name}/${repository.name}/contents/${encodeURIComponent(
         settings.FILE_NAME
       )}`,
-      ({ request }) =>
-        HttpResponse.arrayBuffer(
-          Buffer.from(
-            dump({
-              rulesets: [{ ruleset_id: rulesetId, ...this.ruleset }]
-            })
-          )
-        )
+      ({ request }) => HttpResponse.arrayBuffer(Buffer.from(dump({ rulesets: [this.updatedRuleset] })))
     ),
     http.put(
       `https://api.github.com/repos/${repository.owner.name}/${repository.name}/rulesets/${rulesetId}`,
       async ({ request }) => {
-        this.updatedRuleset = await request.json()
+        this.rulesetUpdate = await request.json()
 
         return new HttpResponse(null, { status: StatusCodes.OK })
       }
@@ -101,7 +102,7 @@ Then('the ruleset is enabled for the repository', async function () {
 })
 
 Then('the ruleset is updated', async function () {
-  assert.deepEqual(this.updatedRuleset, this.ruleset)
+  assert.deepEqual(this.rulesetUpdate, this.updatedRuleset)
 })
 
 Then('the ruleset is deleted', async function () {

test/unit/lib/plugins/rulesets.test.js

@@ -18,27 +18,43 @@ describe('rulesets', () => {
       repos: {
         createRepoRuleset: jest.fn(),
         deleteRepoRuleset: jest.fn(),
+        getRepoRuleset: jest.fn(),
         getRepoRulesets: jest.fn(),
         updateRepoRuleset: jest.fn()
       }
     }
   })
 
   it('should sync rulesets', async () => {
-    const updatedValue = any.word()
+    const additionalRule = any.simpleObject()
     const existingRulesetId = any.integer()
+    const secondExistingRulesetId = any.integer()
     const removedRulesetId = any.integer()
-    const existingRuleset = { name: any.word(), foo: updatedValue }
+    const existingRuleset = { name: any.word(), rules: [any.simpleObject()] }
+    const secondExistingRuleset = { name: any.word(), rules: [any.simpleObject()] }
     const newRuleset = { name: any.word() }
-    const plugin = configure(github, owner, repo, [newRuleset, existingRuleset])
+    const plugin = configure(github, owner, repo, [
+      newRuleset,
+      { ...existingRuleset, rules: [...existingRuleset.rules, additionalRule] },
+      secondExistingRuleset
+    ])
+    const existingRulesets = [
+      { id: existingRulesetId, ...existingRuleset },
+      { id: secondExistingRulesetId, ...secondExistingRuleset },
+      { id: removedRulesetId, name: any.word() }
+    ]
     when(github.repos.getRepoRulesets)
       .calledWith({ owner, repo })
       .mockResolvedValue({
-        data: [
-          { ruleset_id: existingRulesetId, ...existingRuleset, foo: any.word() },
-          { ruleset_id: removedRulesetId, name: any.word() }
-        ]
+        data: existingRulesets.map(
+          ({ rules, conditions, bypass_actors: bypassActors, ...rulesetListProperties }) => rulesetListProperties
+        )
       })
+    existingRulesets.forEach(ruleset => {
+      when(github.repos.getRepoRuleset)
+        .calledWith({ owner, repo, ruleset_id: ruleset.id })
+        .mockResolvedValue({ data: ruleset })
+    })
 
     await plugin.sync()
 
@@ -48,7 +64,13 @@ describe('rulesets', () => {
       repo,
       ruleset_id: existingRulesetId,
       ...existingRuleset,
-      foo: updatedValue
+      rules: [...existingRuleset.rules, additionalRule]
+    })
+    expect(github.repos.updateRepoRuleset).not.toHaveBeenCalledWith({
+      owner,
+      repo,
+      ruleset_id: secondExistingRulesetId,
+      ...secondExistingRuleset
     })
     expect(github.repos.deleteRepoRuleset).toHaveBeenCalledWith({ owner, repo, ruleset_id: removedRulesetId })
   })