fix(modelify): Five new specs and fixes for Sentry 2973, others

Fixes T3613 as well.

Pushing directly to master because it's mostly specs and resolves an "Unbreak Now!" ticket

spec-nylas/stores/database-store-spec.coffee

@@ -2,6 +2,7 @@ _ = require 'underscore'
 ipc = require 'ipc'
 
 Label = require '../../src/flux/models/label'
+Thread = require '../../src/flux/models/thread'
 TestModel = require '../fixtures/db-test-model'
 ModelQuery = require '../../src/flux/models/query'
 DatabaseStore = require '../../src/flux/stores/database-store'
@@ -49,6 +50,65 @@ describe "DatabaseStore", ->
       q = DatabaseStore.findAll(TestModel, testMatchers)
       expect(q.sql()).toBe("SELECT `TestModel`.`data` FROM `TestModel`  WHERE `TestModel`.`id` = 'b'  ")
 
+  describe "modelify", ->
+    beforeEach ->
+      @models = [
+        new Thread(clientId: 'local-A'),
+        new Thread(clientId: 'local-B'),
+        new Thread(clientId: 'local-C'),
+        new Thread(clientId: 'local-D', serverId: 'SERVER:D'),
+        new Thread(clientId: 'local-E', serverId: 'SERVER:E'),
+        new Thread(clientId: 'local-F', serverId: 'SERVER:F'),
+        new Thread(clientId: 'local-G', serverId: 'SERVER:G')
+      ]
+      # Actually returns correct sets for queries, since matchers can evaluate
+      # themselves against models in memory
+      spyOn(DatabaseStore, 'run').andCallFake (query) =>
+        results = []
+        for model in @models
+          found = _.every query._matchers, (matcher) ->
+            matcher.evaluate(model)
+          results.push(model) if found
+        Promise.resolve(results)
+
+    describe "when given an array or input that is not an array", ->
+      it "resolves immediately with an empty array", ->
+        waitsForPromise =>
+          DatabaseStore.modelify(Thread, null).then (output) =>
+            expect(output).toEqual([])
+
+    describe "when given an array of mixed IDs, clientIDs, and models", ->
+      it "resolves with an array of models", ->
+        input = ['SERVER:F', 'local-B', 'local-C', 'SERVER:D', @models[6]]
+        expectedOutput = [@models[5], @models[1], @models[2], @models[3], @models[6]]
+        waitsForPromise =>
+          DatabaseStore.modelify(Thread, input).then (output) =>
+            expect(output).toEqual(expectedOutput)
+
+    describe "when the input is only IDs", ->
+      it "resolves with an array of models", ->
+        input = ['SERVER:D', 'SERVER:F', 'SERVER:G']
+        expectedOutput = [@models[3], @models[5], @models[6]]
+        waitsForPromise =>
+          DatabaseStore.modelify(Thread, input).then (output) =>
+            expect(output).toEqual(expectedOutput)
+
+    describe "when the input is only clientIDs", ->
+      it "resolves with an array of models", ->
+        input = ['local-A', 'local-B', 'local-C', 'local-D']
+        expectedOutput = [@models[0], @models[1], @models[2], @models[3]]
+        waitsForPromise =>
+          DatabaseStore.modelify(Thread, input).then (output) =>
+            expect(output).toEqual(expectedOutput)
+
+    describe "when the input is all models", ->
+      it "resolves with an array of models", ->
+        input = [@models[0], @models[1], @models[2], @models[3]]
+        expectedOutput = [@models[0], @models[1], @models[2], @models[3]]
+        waitsForPromise =>
+          DatabaseStore.modelify(Thread, input).then (output) =>
+            expect(output).toEqual(expectedOutput)
+
   describe "count", ->
     it "should pass the provided predicates on to the ModelQuery", ->
       DatabaseStore.findAll(TestModel, testMatchers)

src/flux/stores/database-store.coffee

@@ -352,33 +352,35 @@ class DatabaseStore extends NylasStore
         else
           continue
       else if _.isString(item)
-        ids.push(item)
+        if Utils.isTempId(item)
+          clientIds.push(item)
+        else
+          ids.push(item)
       else
         throw new Error("modelify: Not sure how to convert #{item} into a #{klass.name}")
 
-    if ids.length is 0
+    if ids.length is 0 and clientIds.length is 0
       return Promise.resolve(arr)
 
-    whereId = =>
-      klass.attributes.id.in(ids)
+    queries =
+      modelsFromIds: []
+      modelsFromClientIds: []
 
-    whereClientId = =>
-      klass.attributes.clientId.in(clientIds)
-
-    queries = {}
-    queries.modelsFromIds = @findAll(klass).where(whereId) if ids.length
-    queries.modelsFromClientIds = @findAll(klass).where(whereClientId) if clientIds.length
+    if ids.length
+      queries.modelsFromIds = @findAll(klass).where(klass.attributes.id.in(ids))
+    if clientIds.length
+      queries.modelsFromClientIds = @findAll(klass).where(klass.attributes.clientId.in(clientIds))
 
     Promise.props(queries).then ({modelsFromIds, modelsFromClientIds}) =>
-      modelsById = {}
-      modelsById[model.id] = model for model in modelsFromIds
-      modelsById[model.id] = model for model in modelsFromClientIds
+      modelsByString = {}
+      modelsByString[model.id] = model for model in modelsFromIds
+      modelsByString[model.clientId] = model for model in modelsFromClientIds
 
       arr = arr.map (item) ->
         if item instanceof klass
           return item
         else
-          return modelsById[item]
+          return modelsByString[item]
 
       return Promise.resolve(arr)