fix(contact): fix contacts completion in popout composers

Summary:
Fixes T3568

The composer windows had the wrong cache in their `ContactStore`s. Since
it's VERY expensive to repopulate a ContactStore's cache, we now have a
`WindowBridge` that allow you to, with a Promise, invoke methods on the
main window instead.

(Still need to fix some tests)

Test Plan: Fixed tests

Reviewers: dillon, bengotow

Reviewed By: dillon, bengotow

Maniphest Tasks: T3568

Differential Revision: https://phab.nylas.com/D2045
This commit is contained in:
Evan Morikawa 2015-09-22 15:32:27 -07:00
parent 4ec059cf5b
commit b3fc080f43
8 changed files with 178 additions and 69 deletions

View file

@ -79,15 +79,15 @@ class ParticipantsTextField extends React.Component
# an array of contact objects. For each email address wrapped in
# parentheses, look for a preceding name, if one exists.
if string.length is 0
return []
return Promise.resolve([])
contacts = ContactStore.parseContactsInString(string, options)
if contacts.length > 0
return contacts
else
# If no contacts are returned, treat the entire string as a single
# (malformed) contact object.
return [new Contact(email: string, name: null)]
ContactStore.parseContactsInString(string, options).then (contacts) =>
if contacts.length > 0
return Promise.resolve(contacts)
else
# If no contacts are returned, treat the entire string as a single
# (malformed) contact object.
return [new Contact(email: string, name: null)]
_remove: (values) =>
field = @props.field
@ -101,41 +101,43 @@ class ParticipantsTextField extends React.Component
_edit: (token, replacementString) =>
field = @props.field
tokenIndex = @props.participants[field].indexOf(token)
replacements = @_tokensForString(replacementString)
updates = {}
updates[field] = [].concat(@props.participants[field])
updates[field].splice(tokenIndex, 1, replacements...)
@props.change(updates)
@_tokensForString(replacementString).then (replacemehts) =>
updates = {}
updates[field] = [].concat(@props.participants[field])
updates[field].splice(tokenIndex, 1, replacements...)
@props.change(updates)
_add: (values, options={}) =>
# If the input is a string, parse out email addresses and build
# an array of contact objects. For each email address wrapped in
# parentheses, look for a preceding name, if one exists.
if _.isString(values)
values = @_tokensForString(values, options)
tokensPromise = @_tokensForString(values, options)
else
tokensPromise = Promise.resolve(values)
# Safety check: remove anything from the incoming values that isn't
# a Contact. We should never receive anything else in the values array.
values = _.filter values, (value) -> value instanceof Contact
tokensPromise.then (tokens) =>
# Safety check: remove anything from the incoming tokens that isn't
# a Contact. We should never receive anything else in the tokens array.
tokens = _.filter tokens, (value) -> value instanceof Contact
updates = {}
for field in Object.keys(@props.participants)
updates[field] = [].concat(@props.participants[field])
for value in values
# first remove the participant from all the fields. This ensures
# that drag and drop isn't "drag and copy." and you can't have the
# same recipient in multiple places.
updates = {}
for field in Object.keys(@props.participants)
updates[field] = _.reject updates[field], (p) ->
p.email is value.email
updates[field] = [].concat(@props.participants[field])
# add the participant to field
updates[@props.field] = _.union(updates[@props.field], [value])
for token in tokens
# first remove the participant from all the fields. This ensures
# that drag and drop isn't "drag and copy." and you can't have the
# same recipient in multiple places.
for field in Object.keys(@props.participants)
updates[field] = _.reject updates[field], (p) ->
p.email is token.email
@props.change(updates)
""
# add the participant to field
updates[@props.field] = _.union(updates[@props.field], [token])
@props.change(updates)
return ""
_showContextMenu: (participant) =>
remote = require('remote')

View file

@ -35,6 +35,7 @@ participant5 = new Contact
describe 'ParticipantsTextField', ->
beforeEach ->
spyOn(atom, "isMainWindow").andReturn true
@propChange = jasmine.createSpy('change')
@fieldName = 'to'
@ -55,16 +56,27 @@ describe 'ParticipantsTextField', ->
@renderedInput = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(@renderedField, 'input'))
@expectInputToYield = (input, expected) ->
ReactTestUtils.Simulate.change(@renderedInput, {target: {value: input}})
ReactTestUtils.Simulate.keyDown(@renderedInput, {key: 'Enter', keyCode: 9})
reviver = (k,v) ->
return undefined if k in ["id", "client_id", "server_id", "object"]
return v
found = @propChange.mostRecentCall.args[0]
found = JSON.parse(JSON.stringify(found), reviver)
expected = JSON.parse(JSON.stringify(expected), reviver)
expect(found).toEqual(expected)
runs =>
ReactTestUtils.Simulate.change(@renderedInput, {target: {value: input}})
advanceClock(100)
ReactTestUtils.Simulate.keyDown(@renderedInput, {key: 'Enter', keyCode: 9})
waitsFor =>
@propChange.calls.length > 0
runs =>
found = @propChange.mostRecentCall.args[0]
found = JSON.parse(JSON.stringify(found), reviver)
expected = JSON.parse(JSON.stringify(expected), reviver)
expect(found).toEqual(expected)
# This advance clock needs to be here because our waitsFor latch
# catches the first time that propChange gets called. More stuff
# may happen after this and we need to advance the clock to
# "clear" all of that. If we don't do this it throws errors about
# `setState` being called on unmounted components :(
advanceClock(100)
it 'renders into the document', ->
expect(ReactTestUtils.isCompositeComponentWithType @renderedField, ParticipantsTextField).toBe(true)
@ -86,9 +98,13 @@ describe 'ParticipantsTextField', ->
bcc: []
it "should use the name of an existing contact in the ContactStore if possible", ->
spyOn(ContactStore, 'searchContacts').andCallFake (val, options) ->
return [participant3] if val is participant3.email
return []
spyOn(ContactStore, 'searchContacts').andCallFake (val, options={}) ->
if options.noPromise
return [participant3] if val is participant3.email
return []
else
return Promise.resolve([participant3]) if val is participant3.email
return Promise.resolve([])
@expectInputToYield participant3.email,
to: [participant1, participant2, participant3]
@ -96,9 +112,13 @@ describe 'ParticipantsTextField', ->
bcc: []
it "should not allow the same contact to appear multiple times", ->
spyOn(ContactStore, 'searchContacts').andCallFake (val, options) ->
return [participant2] if val is participant2.email
return []
spyOn(ContactStore, 'searchContacts').andCallFake (val, options={}) ->
if options.noPromise
return [participant2] if val is participant2.email
return []
else
return Promise.resolve([participant2]) if val is participant2.email
return Promise.resolve([])
@expectInputToYield participant2.email,
to: [participant1, participant2]

View file

@ -48,9 +48,10 @@ SearchSuggestionStore = Reflux.createStore
@trigger(@)
return
@_contactResults = ContactStore.searchContacts(val, limit:10)
@_rebuildThreadResults()
@_compileSuggestions()
ContactStore.searchContacts(val, limit:10).then (results) =>
@_contactResults = results
@_rebuildThreadResults()
@_compileSuggestions()
_rebuildThreadResults: ->
{key, val} = @queryKeyAndVal()

View file

@ -385,6 +385,23 @@ class Application
ipc.on 'login-successful', (event) =>
@_loginSuccessful()
ipc.on 'run-in-window', (event, params) =>
@_sourceWindows ?= {}
sourceWindow = BrowserWindow.fromWebContents(event.sender)
@_sourceWindows[params.taskId] = sourceWindow
if params.window is "work"
targetWindow = @windowManager.workWindow()
else if params.window is "main"
targetWindow = @windowManager.mainWindow()
else throw new Error("We don't support running in that window")
return if not targetWindow or not targetWindow.browserWindow.webContents
targetWindow.browserWindow.webContents.send('run-in-window', params)
ipc.on 'remote-run-results', (event, params) =>
sourceWindow = @_sourceWindows[params.taskId]
sourceWindow.webContents.send('remote-run-results', params)
delete @_sourceWindows[params.taskId]
# Public: Executes the given command.
#
# If it isn't handled globally, delegate to the currently focused window.

View file

@ -4,8 +4,7 @@ _ = require 'underscore'
{CompositeDisposable} = require 'event-kit'
{Utils,
Contact,
RegExpUtils,
ContactStore} = require 'nylas-exports'
RegExpUtils} = require 'nylas-exports'
RetinaImg = require './retina-img'
class SizeToFitInput extends React.Component
@ -260,6 +259,9 @@ class TokenizingTextField extends React.Component
completions: []
selectedTokenKey: null
componentDidMount: -> @_mounted = true
componentWillUnmount: -> @_mounted = false
render: =>
{Menu} = require 'nylas-component-kit'
@ -503,7 +505,7 @@ class TokenizingTextField extends React.Component
@setState completions: filterTokens(tokensOrPromise)
else if tokensOrPromise instanceof Promise
tokensOrPromise.then (tokens) =>
return unless @isMounted()
return unless @_mounted
@setState completions: filterTokens(tokens)
else
console.warn "onRequestCompletions returned an invalid type. It must return an Array of tokens or a Promise that resolves to an array of tokens"

View file

@ -14,6 +14,8 @@ NylasAPI = require '../nylas-api'
{Listener, Publisher} = require '../modules/reflux-coffee'
CoffeeHelpers = require '../coffee-helpers'
WindowBridge = require '../../window-bridge'
###
The JSONCache class exposes a simple API for maintaining a local cache of data
in a JSON file that needs to be refreshed periodically. Using JSONCache is a good
@ -135,16 +137,17 @@ Section: Stores
class ContactStore extends NylasStore
constructor: ->
@_contactCache = []
@_accountId = null
if atom.isMainWindow() or atom.inSpecMode()
@_contactCache = []
@_accountId = null
@_rankingsCache = new RankingsJSONCache()
@listenTo DatabaseStore, @_onDatabaseChanged
@listenTo AccountStore, @_onAccountChanged
@listenTo @_rankingsCache, @_sortContactsCacheWithRankings
@_rankingsCache = new RankingsJSONCache()
@listenTo DatabaseStore, @_onDatabaseChanged
@listenTo AccountStore, @_onAccountChanged
@listenTo @_rankingsCache, @_sortContactsCacheWithRankings
@_accountId = AccountStore.current()?.id
@_refreshCache()
@_accountId = AccountStore.current()?.id
@_refreshCache()
# Public: Search the user's contact list for the given search term.
# This method compares the `search` string against each Contact's
@ -157,8 +160,19 @@ class ContactStore extends NylasStore
#
# Returns an {Array} of matching {Contact} models
#
searchContacts: (search, {limit}={}) =>
return [] if not search or search.length is 0
searchContacts: (search, options={}) =>
{limit, noPromise} = options
if not atom.isMainWindow()
if noPromise
throw new Error("We search Contacts in the Main window, which makes it impossible for this to be a noPromise method from this window")
# Returns a promise that resolves to the value of searchContacts
return WindowBridge.runInMainWindow("ContactStore", "searchContacts", [search, options])
if not search or search.length is 0
if noPromise
return []
else
return Promise.resolve([])
limit ?= 5
limit = Math.max(limit, 0)
@ -191,7 +205,10 @@ class ContactStore extends NylasStore
if matches.length is limit
break
matches
if noPromise
return matches
else
return Promise.resolve(matches)
# Public: Returns true if the contact provided is a {Contact} instance and
# contains a properly formatted email address.
@ -200,7 +217,11 @@ class ContactStore extends NylasStore
return false unless contact instanceof Contact
return contact.email and RegExpUtils.emailRegex().test(contact.email)
parseContactsInString: (contactString, {skipNameLookup}={}) =>
parseContactsInString: (contactString, options={}) =>
{skipNameLookup} = options
if not atom.isMainWindow()
# Returns a promise that resolves to the value of searchContacts
return WindowBridge.runInMainWindow("ContactStore", "parseContactsInString", [contactString, options])
detected = []
emailRegex = RegExpUtils.emailRegex()
lastMatchEnd = 0
@ -222,7 +243,7 @@ class ContactStore extends NylasStore
if (not name or name.length is 0) and not skipNameLookup
# Look to see if we can find a name for this email address in the ContactStore.
# Otherwise, just populate the name with the email address.
existing = @searchContacts(email, {limit:1})[0]
existing = @searchContacts(email, {limit:1, noPromise: true})[0]
if existing and existing.name
name = existing.name
else
@ -241,7 +262,7 @@ class ContactStore extends NylasStore
detected.push(new Contact({email, name}))
detected
return Promise.resolve(detected)
__refreshCache: =>
return unless @_accountId

View file

@ -435,12 +435,15 @@ class DraftStore
pristine: true
accountId: account.id
contacts = {}
for attr in ['to', 'cc', 'bcc']
if query[attr]
draft[attr] = ContactStore.parseContactsInString(query[attr])
contacts[attr] = ContactStore.parseContactsInString(query[attr])
@_finalizeAndPersistNewMessage(draft).then ({draftClientId}) =>
@_onPopoutDraftClientId(draftClientId)
Promise.props(contacts).then (contacts) =>
draft = _.extend(draft, contacts)
@_finalizeAndPersistNewMessage(draft).then ({draftClientId}) =>
@_onPopoutDraftClientId(draftClientId)
_onDestroyDraft: (draftClientId) =>
session = @_draftSessions[draftClientId]
@ -498,4 +501,4 @@ class DraftStore
detail: errorMessage
}
module.exports = new DraftStore()
module.exports = new DraftStore()

43
src/window-bridge.coffee Normal file
View file

@ -0,0 +1,43 @@
_ = require 'underscore'
ipc = require 'ipc'
Utils = require './flux/models/utils'
class WindowBridge
constructor: ->
@_tasks = {}
ipc.on("remote-run-results", @_onResults)
ipc.on("run-in-window", @_onRunInWindow)
runInWindow: (window, objectName, methodName, args) ->
taskId = Utils.generateTempId()
new Promise (resolve, reject) =>
@_tasks[taskId] = {resolve, reject}
args = Utils.serializeRegisteredObjects(args)
params = {window, objectName, methodName, args, taskId}
ipc.send("run-in-window", params)
runInMainWindow: (args...) ->
@runInWindow("main", args...)
runInWorkWindow: ->
@runInWindow("work", args...)
_onResults: ({returnValue, taskId}={}) =>
returnValue = Utils.deserializeRegisteredObjects(returnValue)
@_tasks[taskId].resolve(returnValue)
delete @_tasks[taskId]
_onRunInWindow: ({objectName, methodName, args, taskId}={}) =>
args = Utils.deserializeRegisteredObjects(args)
exports = require 'nylas-exports'
result = exports[objectName][methodName].apply(null, args)
if _.isFunction(result.then)
result.then (returnValue) ->
returnValue = Utils.serializeRegisteredObjects(returnValue)
ipc.send('remote-run-results', {returnValue, taskId})
else
returnValue = result
returnValue = Utils.serializeRegisteredObjects(returnValue)
ipc.send('remote-run-results', {returnValue, taskId})
module.exports = new WindowBridge