Foundry376/Mailspring
1
1
Fork 0
mirror of https://github.com/Foundry376/Mailspring.git synced 2025-03-03 19:43:04 +08:00
Code Issues Projects Releases Packages Wiki Activity

fix(reply): better coverge for reply-all participants

Browse source 
Summary:
Fixes: T3550
Fixes: T3504

Test Plan: many of them

Reviewers: dillon, bengotow

Reviewed By: dillon, bengotow

Subscribers: mg

Differential Revision: https://phab.nylas.com/D2017
This commit is contained in:
Evan Morikawa 2015-09-14 16:18:55 -04:00
parent 91ccc940a6
commit b761afe960
9 changed files with 380 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
internal_packages/message-list/lib/message-controls.cjsx
View file

@ -38,11 +38,14 @@ class MessageControls extends React.Component
image: 'ic-dropdown-forward.png'
select: @_onForward
defaultReplyType = atom.config.get('core.sending.defaultReplyType')
if @props.message.canReplyAll() and defaultReplyType is 'reply-all'
return [replyAll, reply, forward]
if @props.message.canReplyAll()
defaultReplyType = atom.config.get('core.sending.defaultReplyType')
if defaultReplyType is 'reply-all'
return [replyAll, reply, forward]
else
return [reply, replyAll, forward]
else
return [reply, replyAll, forward]
return [reply, forward]
_dropdownMenu: (items) ->
itemContent = (item) ->

7
internal_packages/message-list/lib/message-list.cjsx
View file

@ -248,8 +248,11 @@ class MessageList extends React.Component
lastMsg = _.last(_.filter((@state.messages ? []), (m) -> not m.draft))
return 'reply' unless lastMsg
if lastMsg.canReplyAll() and defaultReplyType is 'reply-all'
return 'reply-all'
if lastMsg.canReplyAll()
if defaultReplyType is 'reply-all'
return 'reply-all'
else
return 'reply'
else
return 'reply'

18
internal_packages/message-list/spec/message-list-spec.cjsx
View file

@ -33,9 +33,11 @@ MessageItemContainer = proxyquire("../lib/message-item-container", {
MessageList = proxyquire '../lib/message-list',
"./message-item-container": MessageItemContainer
# User_1 needs to be "me" so that when we calculate who we should reply
# to, it properly matches the AccountStore
user_1 = new Contact
name: "User One"
email: "user1@nylas.com"
name: TEST_ACCOUNT_NAME
email: TEST_ACCOUNT_EMAIL
user_2 = new Contact
name: "User Two"
email: "user2@nylas.com"
@ -251,7 +253,8 @@ describe "MessageList", ->
cs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "footer-reply-area")
expect(cs.length).toBe 1
it "prompts for a reply-all when there's more then one participant", ->
it "prompts for a reply-all when there's more than one participant and the default is reply-all", ->
spyOn(atom.config, "get").andReturn "reply-all"
MessageStore._items = [m5, m3]
MessageStore._thread = test_thread
MessageStore.trigger()
@ -259,6 +262,15 @@ describe "MessageList", ->
cs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "footer-reply-area")
expect(cs.length).toBe 1
it "prompts for a reply-all when there's more than one participant and the default is reply", ->
spyOn(atom.config, "get").andReturn "reply"
MessageStore._items = [m5, m3]
MessageStore._thread = test_thread
MessageStore.trigger()
expect(@messageList._replyType()).toBe "reply"
cs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "footer-reply-area")
expect(cs.length).toBe 1
it "hides the reply type if the last message is a draft", ->
MessageStore._items = [m5, m3, draftMessages[0]]
MessageStore._thread = test_thread

333
spec-nylas/models/message-spec.coffee
View file

@ -1,17 +1,28 @@
Utils = require "../../src/flux/models/utils"
Message = require "../../src/flux/models/message"
Contact = require "../../src/flux/models/contact"
contact_1 =
name: "Contact One"
email: "contact1@nylas.com"
contact_2 =
name: "Contact Two"
email: "contact2@nylas.com"
contact_3 =
name: ""
email: "contact3@nylas.com"
contact_4 =
name: "Contact Four"
email: ""
evan = new Contact
name: "Evan Morikawa"
email: "evan@nylas.com"
ben = new Contact
name: "Ben Gotow"
email: "ben@nylas.com"
team = new Contact
name: "Nylas Team"
email: "team@nylas.com"
edgehill = new Contact
name: "Edgehill"
email: "edgehill@nylas.com"
noEmail = new Contact
name: "Edgehill"
email: null
me = new Contact
name: TEST_ACCOUNT_NAME
email: TEST_ACCOUNT_EMAIL
almost_me = new Contact
name: TEST_ACCOUNT_NAME
email: "tester+12345@nylas.com"
describe "Message", ->
it "correctly aggregates participants", ->
@ -22,31 +33,303 @@ describe "Message", ->
expect(m1.participants().length).toBe 0
m2 = new Message
to: [contact_1]
to: [evan]
cc: []
bcc: []
from: [contact_2]
from: [ben]
expect(m2.participants().length).toBe 2
m3 = new Message
to: [contact_1]
cc: [contact_1]
bcc: [contact_1]
from: [contact_1]
to: [evan]
cc: [evan]
bcc: [evan]
from: [evan]
expect(m3.participants().length).toBe 1
m4 = new Message
to: [contact_1]
cc: [contact_2, contact_3, contact_4]
bcc: [contact_3]
from: [contact_3]
to: [evan]
cc: [ben, team, noEmail]
bcc: [team]
from: [team]
# because contact 4 has no email
expect(m4.participants().length).toBe 3
m5 = new Message
to: [contact_1]
to: [evan]
cc: []
bcc: [contact_3]
from: [contact_2]
bcc: [team]
from: [ben]
# because we exclude bccs
expect(m5.participants().length).toBe 2
describe "participant replies", ->
cases = [
# Basic cases
{
msg: new Message
from: [evan]
to: [me]
cc: []
bcc: []
expected:
to: [evan]
cc: []
}
{
msg: new Message
from: [evan]
to: [me]
cc: [ben]
bcc: []
expected:
to: [evan]
cc: [ben]
}
{
msg: new Message
from: [evan]
to: [ben]
cc: [me]
bcc: []
expected:
to: [evan]
cc: [ben]
}
{
msg: new Message
from: [evan]
to: [me]
cc: [ben, team, evan]
bcc: []
expected:
to: [evan]
cc: [ben, team]
}
{
msg: new Message
from: [evan]
to: [me, ben, evan, ben, ben, evan]
cc: []
bcc: []
expected:
to: [evan]
cc: [ben]
}
{
msg: new Message
from: [evan]
to: [me, ben]
cc: [team, edgehill]
bcc: [evan, me, ben]
expected:
to: [evan]
cc: [ben, team, edgehill]
}
# From me (replying to a message I just sent)
{
msg: new Message
from: [me]
to: [me]
cc: []
bcc: []
expected:
to: [me]
cc: []
}
{
msg: new Message
from: [me]
to: [ben]
cc: []
bcc: []
expected:
to: [ben]
cc: []
}
{
msg: new Message
from: [me]
to: [ben, team, ben]
cc: [edgehill]
bcc: []
expected:
to: [ben, team]
cc: [edgehill]
}
{
msg: new Message
from: [me]
to: [ben, team, ben]
cc: [edgehill]
bcc: []
expected:
to: [ben, team]
cc: [edgehill]
}
# From me in cases my similar alias is used
{
msg: new Message
from: [me]
to: [almost_me]
cc: [ben]
bcc: []
expected:
to: [almost_me]
cc: [ben]
}
{
msg: new Message
from: [me]
to: [me, almost_me, me]
cc: [ben, almost_me, me, me, ben, ben]
bcc: []
expected:
to: [me]
cc: [ben]
}
{
msg: new Message
from: [almost_me]
to: [me]
cc: [ben]
bcc: []
expected:
to: [me]
cc: [ben]
}
{
msg: new Message
from: [almost_me]
to: [almost_me]
cc: [ben]
bcc: []
expected:
to: [almost_me]
cc: [ben]
}
# Cases when I'm on email lists
{
msg: new Message
from: [evan]
to: [team]
cc: []
bcc: []
expected:
to: [evan]
cc: [team]
}
{
msg: new Message
from: [evan]
to: [team]
cc: [ben, edgehill]
bcc: []
expected:
to: [evan]
cc: [team, ben, edgehill]
}
{
msg: new Message
from: [evan]
to: [team]
cc: [me]
bcc: []
expected:
to: [evan]
cc: [team]
}
{
msg: new Message
from: [evan]
to: [team, me]
cc: [ben]
bcc: []
expected:
to: [evan]
cc: [team, ben]
}
# Cases when I'm bcc'd
{
msg: new Message
from: [evan]
to: []
cc: []
bcc: [me]
expected:
to: [evan]
cc: []
}
{
msg: new Message
from: [evan]
to: [ben]
cc: []
bcc: [me]
expected:
to: [evan]
cc: [ben]
}
{
msg: new Message
from: [evan]
to: [ben]
cc: [team, edgehill]
bcc: [me]
expected:
to: [evan]
cc: [ben, team, edgehill]
}
# Cases my similar alias is used
{
msg: new Message
from: [evan]
to: [almost_me]
cc: []
bcc: []
expected:
to: [evan]
cc: []
}
{
msg: new Message
from: [evan]
to: [ben]
cc: [almost_me]
bcc: []
expected:
to: [evan]
cc: [ben]
}
{
msg: new Message
from: [evan]
to: [ben]
cc: []
bcc: [almost_me]
expected:
to: [evan]
cc: [ben]
}
]
itString = (prefix, msg) ->
return "#{prefix} from: #{msg.from.map( (c) -> c.email).join(', ')} | to: #{msg.to.map( (c) -> c.email).join(', ')} | cc: #{msg.cc.map( (c) -> c.email).join(', ')} | bcc: #{msg.bcc.map( (c) -> c.email).join(', ')}"
it "thinks me and almost_me are equivalent", ->
expect(Utils.emailIsEquivalent(me.email, almost_me.email)).toBe true
expect(Utils.emailIsEquivalent(ben.email, me.email)).toBe false
cases.forEach ({msg, expected}) ->
it itString("Reply All:", msg), ->
expect(msg.participantsForReplyAll()).toEqual expected
it itString("Reply:", msg), ->
{to, cc} = msg.participantsForReply()
expect(to).toEqual expected.to
expect(cc).toEqual []
describe "participantsForReplyAll", ->

8
spec/spec-helper.coffee
View file

@ -108,6 +108,8 @@ Promise.setScheduler (fn) ->
# So it passes the Utils.isTempId test
window.TEST_ACCOUNT_CLIENT_ID = "local-test-account-client-id"
window.TEST_ACCOUNT_ID = "test-account-server-id"
window.TEST_ACCOUNT_EMAIL = "tester@nylas.com"
window.TEST_ACCOUNT_NAME = "Nylas Test"
beforeEach ->
atom.testOrganizationUnit = null
@ -152,16 +154,16 @@ beforeEach ->
# Log in a fake user
spyOn(AccountStore, 'current').andCallFake -> new Account
name: "Nylas Test"
name: TEST_ACCOUNT_NAME
provider: "gmail"
emailAddress: 'tester@nylas.com'
emailAddress: TEST_ACCOUNT_EMAIL
organizationUnit: atom.testOrganizationUnit
clientId: TEST_ACCOUNT_CLIENT_ID
serverId: TEST_ACCOUNT_ID
usesLabels: -> atom.testOrganizationUnit is "label"
usesFolders: -> atom.testOrganizationUnit is "folder"
me: ->
new Contact(email: 'tester@nylas.com', name: 'Ben Tester')
new Contact(email: TEST_ACCOUNT_EMAIL, name: TEST_ACCOUNT_NAME)
# reset config before each spec; don't load or save from/to `config.json`
spyOn(Config::, 'load')

3
src/flux/models/contact.coffee
View file

@ -1,4 +1,5 @@
Model = require './model'
Utils = require './utils'
Attributes = require '../attributes'
_ = require 'underscore'
@ -81,7 +82,7 @@ class Contact extends Model
# the account email, since it is case-insensitive and future-proof.
isMe: ->
AccountStore = require '../stores/account-store'
@email.toLowerCase() is AccountStore.current()?.emailAddress.toLowerCase()
Utils.emailIsEquivalent(@email, AccountStore.current()?.emailAddress)
# Returns a {String} display name.
# - "You" if the contact is the current user

25
src/flux/models/message.coffee
View file

@ -3,6 +3,7 @@ moment = require 'moment'
File = require './file'
Label = require './label'
Utils = require './utils'
Folder = require './folder'
Model = require './model'
Event = require './event'
@ -196,7 +197,8 @@ class Message extends Model
return @
canReplyAll: ->
@cc.length > 0 or @to.length > 1
{to, cc} = @participantsForReplyAll()
to.length > 1 or cc.length > 0
# Public: Returns a set of uniqued message participants by combining the
# `to`, `cc`, and `from` fields.
@ -215,22 +217,27 @@ class Message extends Model
to = []
cc = []
excluded = @from.map (c) -> Utils.toEquivalentEmailForm(c.email)
excluded.push(Utils.toEquivalentEmailForm(AccountStore.current().emailAddress))
filterCc = (cc) ->
return _.filter cc, (p) ->
!_.contains(excluded, Utils.toEquivalentEmailForm(p.email))
if @isFromMe()
to = @to
cc = @cc
cc = filterCc(@cc)
else
excluded = @from.map (c) -> c.email
excluded.push(AccountStore.current().emailAddress)
if @replyTo.length
to = @replyTo
else
to = @from
cc = [].concat(@cc, @to)
cc = _.filter cc, (p) -> !_.contains(excluded, p.email)
cc = [].concat(@to, @cc)
cc = filterCc(cc)
to = _.uniq to, (p) -> p.email.toLowerCase().trim()
cc = _.uniq cc, (p) -> p.email.toLowerCase().trim()
to = _.uniq to, (p) -> Utils.toEquivalentEmailForm(p.email)
cc = _.uniq cc, (p) -> Utils.toEquivalentEmailForm(p.email)
{to, cc}
# Public: Returns a hash with `to` and `cc` keys for authoring a new draft in
@ -247,7 +254,7 @@ class Message extends Model
else
to = @from
to = _.uniq to, (p) -> p.email.toLowerCase().trim()
to = _.uniq to, (p) -> Utils.toEquivalentEmailForm(p.email)
{to, cc}
# Public: Returns an {Array} of {File} IDs

17
src/flux/models/utils.coffee
View file

@ -206,6 +206,23 @@ Utils =
domain = _.last(email.toLowerCase().trim().split("@"))
return (Utils.commonDomains[domain] ? false)
# This looks for and removes plus-ing, it taks a VERY liberal approach
# to match an email address. We'd rather let false positives through.
toEquivalentEmailForm: (email) ->
# https://regex101.com/r/iS7kD5/1
localPart1 = /([^+]+?)[+@].*/gi.exec(email)?[1] ? ""
# https://regex101.com/r/iS7kD5/2
domainPart1 = /@(.+)/gi.exec(email)?[1] ? ""
email = "#{localPart1}#{domainPart1}".trim().toLowerCase()
return email
emailIsEquivalent: (email1="", email2="") ->
email1 = Utils.toEquivalentEmailForm(email1)
email2 = Utils.toEquivalentEmailForm(email2)
return email1 is email2
rectVisibleInRect: (r1, r2) ->
return !(r2.left > r1.right || r2.right < r1.left || r2.top > r1.bottom || r2.bottom < r1.top)

6
src/regexp-utils.coffee
View file

@ -7,7 +7,11 @@ RegExpUtils =
# It's also imporant we return a fresh copy of the RegExp every time. A
# javascript regex is stateful and multiple functions using this method
# will cause unexpected behavior!
emailRegex: -> new RegExp(/([a-z.A-Z0-9%+=_-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4})/g)
#
# See http://tools.ietf.org/html/rfc5322#section-3.4 and
# https://tools.ietf.org/html/rfc6531 and
# https://en.wikipedia.org/wiki/Email_address#Local_part
emailRegex: -> new RegExp(/([a-z.A-Z0-9%#_~$&*+;=:-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4})/g)
# https://regex101.com/r/zG7aW4/3
imageTagRegex: -> /<img\s+[^>]*src="([^"]*)"[^>]*>/g