mirror of
https://github.com/Foundry376/Mailspring.git
synced 2025-02-03 22:11:57 +08:00
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:
parent
5b014c3fda
commit
98ca7f15bd
8 changed files with 178 additions and 69 deletions
|
@ -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')
|
||||
|
|
|
@ -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]
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
43
src/window-bridge.coffee
Normal 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
|
Loading…
Reference in a new issue