Include created and removed files for diffForFile, JSONDiffForFile, JSONPatchForFile

changelog.md

@@ -2,6 +2,8 @@
 
 ### Master
 
+- Updates `diffForFile`, `JSONPatchForFile`, and `JSONDiffForFile` to include created and removed files - #368 - bdotdub
+
 ### 2.0.0-alpha.14
 
 - Adds a blank project generated in travis 8 to test no-babel or TS integration - orta

source/platforms/github/GitHubGit.ts

@@ -67,21 +67,20 @@ export default async function gitDSLForGitHub(api: GitHubAPI): Promise<GitDSL> {
     const baseFile = await api.fileContents(filename, pr.base.repo.full_name, pr.base.sha)
     const headFile = await api.fileContents(filename, pr.head.repo.full_name, pr.head.sha)
 
-    if (baseFile && headFile) {
-      // Parse JSON
-      const baseJSON = JSON.parse(baseFile)
-      const headJSON = JSON.parse(headFile)
-      // Tiny bit of hand-waving here around the types. JSONPatchOperation is
-      // a simpler version of all operations inside the rfc6902 d.ts. Users
-      // of danger wont care that much, so I'm smudging the classes slightly
-      // to be ones we can add to the hosted docs.
-      return {
-        before: baseJSON,
-        after: headJSON,
-        diff: jsonDiff.createPatch(baseJSON, headJSON) as JSONPatchOperation[],
-      }
+    // Parse JSON. `fileContents` returns empty string for files that are
+    // missing in one of the refs, ie. when the file is created or deleted.
+    const baseJSON = baseFile === "" ? {} : JSON.parse(baseFile)
+    const headJSON = headFile === "" ? {} : JSON.parse(headFile)
+
+    // Tiny bit of hand-waving here around the types. JSONPatchOperation is
+    // a simpler version of all operations inside the rfc6902 d.ts. Users
+    // of danger wont care that much, so I'm smudging the classes slightly
+    // to be ones we can add to the hosted docs.
+    return {
+      before: baseFile === "" ? null : baseJSON,
+      after: headFile === "" ? null : headJSON,
+      diff: jsonDiff.createPatch(baseJSON, headJSON) as JSONPatchOperation[],
     }
-    return null
   }
 
   /**
@@ -107,25 +106,36 @@ export default async function gitDSLForGitHub(api: GitHubAPI): Promise<GitDSL> {
       const backAStepPath = pathSteps.length <= 2 ? path : pathSteps.slice(0, pathSteps.length - 1).join("/")
 
       const diff: any = {
-        after: jsonpointer.get(after, backAStepPath),
-        before: jsonpointer.get(before, backAStepPath),
+        after: jsonpointer.get(after, backAStepPath) || null,
+        before: jsonpointer.get(before, backAStepPath) || null,
+      }
+
+      const emptyValueOfCounterpart = (other: any) => {
+        if (Array.isArray(other)) {
+          return []
+        } else if (isobject(diff.after)) {
+          return {}
+        }
+        return null
       }
 
+      const beforeValue = diff.before || emptyValueOfCounterpart(diff.after)
+      const afterValue = diff.after || emptyValueOfCounterpart(diff.before)
+
       // If they both are arrays, add some extra metadata about what was
       // added or removed. This makes it really easy to act on specific
       // changes to JSON DSLs
 
-      if (Array.isArray(diff.after) && Array.isArray(diff.before)) {
-        const arrayBefore = diff.before as any[]
-        const arrayAfter = diff.after as any[]
+      if (Array.isArray(afterValue) && Array.isArray(beforeValue)) {
+        const arrayBefore = beforeValue as any[]
+        const arrayAfter = afterValue as any[]
 
         diff.added = arrayAfter.filter(o => !includes(arrayBefore, o))
         diff.removed = arrayBefore.filter(o => !includes(arrayAfter, o))
-
         // Do the same, but for keys inside an object if they both are objects.
-      } else if (isobject(diff.after) && isobject(diff.before)) {
-        const beforeKeys = keys(diff.before) as string[]
-        const afterKeys = keys(diff.after) as string[]
+      } else if (isobject(afterValue) && isobject(beforeValue)) {
+        const beforeKeys = keys(beforeValue) as string[]
+        const afterKeys = keys(afterValue) as string[]
         diff.added = afterKeys.filter(o => !includes(beforeKeys, o))
         diff.removed = beforeKeys.filter(o => !includes(afterKeys, o))
       }
@@ -147,7 +157,7 @@ export default async function gitDSLForGitHub(api: GitHubAPI): Promise<GitDSL> {
   const diffForFile = async (filename: string) => {
     // We already have access to the diff, so see if the file is in there
     // if it's not return an empty diff
-    const structuredDiff = modifiedDiffs.find((diff: any) => diff.from === filename || diff.to === filename)
+    const structuredDiff = fileDiffs.find((diff: any) => diff.from === filename || diff.to === filename)
     if (!structuredDiff) {
       return null
     }

source/platforms/github/_tests/_github_git.test.ts

@@ -140,12 +140,62 @@ describe("the dangerfile gitDSL", async () => {
   })
 
   describe("JSONPatchForFile", () => {
-    it("returns a null for files not in the modified_files", async () => {
+    it("returns a null for files not in the change list", async () => {
       const gitDSL = await github.getPlatformGitRepresentation()
       const empty = await gitDSL.JSONPatchForFile("fuhqmahgads.json")
       expect(empty).toEqual(null)
     })
 
+    it("handles showing a patch for a created file", async () => {
+      const before = ""
+
+      const after = {
+        a: "o, world",
+        b: 3,
+        c: ["one", "two", "three", "four"],
+      }
+
+      github.api.fileContents = async (path, repo, ref) => {
+        const obj = ref === masterSHA ? before : after
+        return obj === "" ? "" : JSON.stringify(obj)
+      }
+
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const empty = await gitDSL.JSONPatchForFile("data/schema.json")
+      expect(empty).toEqual({
+        before: null,
+        after: after,
+        diff: [
+          { op: "add", path: "/a", value: "o, world" },
+          { op: "add", path: "/b", value: 3 },
+          { op: "add", path: "/c", value: ["one", "two", "three", "four"] },
+        ],
+      })
+    })
+
+    it("handles showing a patch for a deleted file", async () => {
+      const before = {
+        a: "o, world",
+        b: 3,
+        c: ["one", "two", "three", "four"],
+      }
+
+      const after = ""
+
+      github.api.fileContents = async (path, repo, ref) => {
+        const obj = ref === masterSHA ? before : after
+        return obj === "" ? "" : JSON.stringify(obj)
+      }
+
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const empty = await gitDSL.JSONPatchForFile("data/schema.json")
+      expect(empty).toEqual({
+        before: before,
+        after: null,
+        diff: [{ op: "remove", path: "/a" }, { op: "remove", path: "/b" }, { op: "remove", path: "/c" }],
+      })
+    })
+
     it("handles showing a patch for two different diff files", async () => {
       const before = {
         a: "Hello, world",
@@ -179,12 +229,70 @@ describe("the dangerfile gitDSL", async () => {
   })
 
   describe("JSONDiffForFile", () => {
-    it("returns an empty object for files not in the modified_files", async () => {
+    it("returns an empty object for files not in the change list", async () => {
       const gitDSL = await github.getPlatformGitRepresentation()
       const empty = await gitDSL.JSONDiffForFile("fuhqmahgads.json")
       expect(empty).toEqual({})
     })
 
+    it("handles showing a patch for a created file", async () => {
+      github.api.fileContents = async (path, repo, ref) => {
+        const after = {
+          a: "o, world",
+          b: 3,
+          c: ["one", "two", "three", "four"],
+          d: ["one", "two"],
+          e: ["five", "one", "three"],
+        }
+
+        const obj = ref === masterSHA ? {} : after
+        return JSON.stringify(obj)
+      }
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const empty = await gitDSL.JSONDiffForFile("data/schema.json")
+      expect(empty).toEqual({
+        a: { after: "o, world", before: null },
+        b: { after: 3, before: null },
+        c: {
+          added: ["one", "two", "three", "four"],
+          after: ["one", "two", "three", "four"],
+          before: null,
+          removed: [],
+        },
+        d: { added: ["one", "two"], after: ["one", "two"], before: null, removed: [] },
+        e: { added: ["five", "one", "three"], after: ["five", "one", "three"], before: null, removed: [] },
+      })
+    })
+
+    it("handles showing a patch for a deleted file", async () => {
+      github.api.fileContents = async (path, repo, ref) => {
+        const before = {
+          a: "o, world",
+          b: 3,
+          c: ["one", "two", "three", "four"],
+          d: ["one", "two"],
+          e: ["five", "one", "three"],
+        }
+
+        const obj = ref === masterSHA ? before : {}
+        return JSON.stringify(obj)
+      }
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const empty = await gitDSL.JSONDiffForFile("data/schema.json")
+      expect(empty).toEqual({
+        a: { after: null, before: "o, world" },
+        b: { after: null, before: 3 },
+        c: {
+          added: [],
+          after: null,
+          before: ["one", "two", "three", "four"],
+          removed: ["one", "two", "three", "four"],
+        },
+        d: { added: [], after: null, before: ["one", "two"], removed: ["one", "two"] },
+        e: { added: [], after: null, before: ["five", "one", "three"], removed: ["five", "one", "three"] },
+      })
+    })
+
     it("handles showing a patch for two different diff files", async () => {
       github.api.fileContents = async (path, repo, ref) => {
         const before = {
@@ -278,4 +386,84 @@ describe("the dangerfile gitDSL", async () => {
       })
     })
   })
+
+  describe("textDiffForFile", () => {
+    it("returns a null for files not in the change list", async () => {
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const empty = await gitDSL.diffForFile("fuhqmahgads.json")
+      expect(empty).toEqual(null)
+    })
+
+    it("returns a diff for created files", async () => {
+      const before = ""
+      const after =
+        "/* @flow */\n" +
+        "'use strict'\n" +
+        "import Relay from 'react-relay'\n" +
+        "import React from 'react'\n" +
+        "import { View, StyleSheet } from 'react-native'\n" +
+        "import Biography from './biography'\n" +
+        "import RelatedArtists from '../related_artists'\n" +
+        "import Separator from '../separator'\n" +
+        "class About extends React.Component\n"
+
+      github.api.fileContents = async (path, repo, ref) => {
+        return ref === masterSHA ? before : after
+      }
+
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const diff = await gitDSL.diffForFile("lib/components/gene/about.js")
+
+      expect(diff.before).toEqual("")
+      expect(diff.after).toMatch(/class About extends React.Component/)
+      expect(diff.diff).toMatch(/class About extends React.Component/)
+    })
+
+    it("returns a diff for deleted files", async () => {
+      const before =
+        "'use strict'\n" +
+        "import Relay from 'react-relay'\n" +
+        "import React from 'react'\n" +
+        "import { StyleSheet, View, Dimensions } from 'react-native'\n\n" +
+        "class RelatedArtists extends React.Component"
+
+      const after = ""
+
+      github.api.fileContents = async (path, repo, ref) => {
+        return ref === masterSHA ? before : after
+      }
+
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const diff = await gitDSL.diffForFile("lib/components/artist/related_artists/index.js")
+
+      expect(diff.before).toMatch(/class RelatedArtists extends React.Component/)
+      expect(diff.after).toEqual("")
+      expect(diff.diff).toMatch(/class RelatedArtists extends React.Component/)
+    })
+
+    it("returns a diff for modified files", async () => {
+      const before =
+        `- [dev] Updates Flow to 0.32 - orta\n` +
+        `- [dev] Updates React to 0.34 - orta\n` +
+        `- [dev] Turns on "keychain sharing" to fix a keychain bug in sim - orta`
+
+      const after = `- [dev] Updates Flow to 0.32 - orta
+        - [dev] Updates React to 0.34 - orta
+        - [dev] Turns on "keychain sharing" to fix a keychain bug in sim - orta
+        - GeneVC now shows about information, and trending artists - orta`
+
+      github.api.fileContents = async (path, repo, ref) => {
+        return ref === masterSHA ? before : after
+      }
+
+      const gitDSL = await github.getPlatformGitRepresentation()
+      const diff = await gitDSL.diffForFile("CHANGELOG.md")
+
+      expect(diff.before).toEqual(before)
+      expect(diff.after).toEqual(after)
+      expect(diff.added).toEqual("+- GeneVC now shows about information, and trending artists - orta")
+      expect(diff.removed).toEqual("")
+      expect(diff.diff).toMatch(/GeneVC now shows about information, and trending artists/)
+    })
+  })
 })