From ff2b225d912cb8ed59186a5237d5e676ea3e3bb4 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Tue, 22 Sep 2015 18:56:15 -0700 Subject: [PATCH] fix(thread-list-participants): Add a new failing test case + fix to the thread list participants I had to make this less functional so that the token generation function could modify data in the output as it was created. Bust :( --- internal_packages/thread-list/lib/main.cjsx | 2 +- .../lib/thread-list-participants.cjsx | 80 +++++++++---------- .../spec/thread-list-participants-spec.cjsx | 41 ++++++---- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/internal_packages/thread-list/lib/main.cjsx b/internal_packages/thread-list/lib/main.cjsx index 8c4f0a6e4..4df4aa631 100644 --- a/internal_packages/thread-list/lib/main.cjsx +++ b/internal_packages/thread-list/lib/main.cjsx @@ -1,6 +1,6 @@ _ = require 'underscore' React = require "react" -{MailViewFilter, ComponentRegistry, WorkspaceStore} = require "nylas-exports" +{ComponentRegistry, WorkspaceStore} = require "nylas-exports" {DownButton, UpButton, ThreadBulkArchiveButton, ThreadBulkStarButton, ThreadBulkToggleUnreadButton} = require "./thread-buttons" {DraftDeleteButton} = require "./draft-buttons" diff --git a/internal_packages/thread-list/lib/thread-list-participants.cjsx b/internal_packages/thread-list/lib/thread-list-participants.cjsx index eb7b15c4f..b32ee4f14 100644 --- a/internal_packages/thread-list/lib/thread-list-participants.cjsx +++ b/internal_packages/thread-list/lib/thread-list-participants.cjsx @@ -1,4 +1,5 @@ React = require 'react' +{Utils} = require 'nylas-exports' _ = require 'underscore' class ThreadListParticipants extends React.Component @@ -12,12 +13,12 @@ class ThreadListParticipants extends React.Component true render: => - items = @getParticipants() + items = @getTokens()
- {@getSpans(items)} + {@renderSpans(items)}
- getSpans: (items) => + renderSpans: (items) => spans = [] accumulated = null accumulatedUnread = false @@ -59,50 +60,47 @@ class ThreadListParticipants extends React.Component return spans - getParticipants: => - makeMetadataFilterer = (toOrFrom) -> - (msg, i, msgs) -> - isFirstMsg = i is 0 - if msg.draft - false - else if isFirstMsg - true - else # check adjacent email uniqueness - last = msgs[i - 1][toOrFrom][0] - curr = msgs[i][toOrFrom][0] - if last and curr - isUniqueEmail = last.email.toLowerCase() isnt curr.email.toLowerCase() - isUniqueName = last.name isnt curr.name - isUniqueEmail or isUniqueName + getTokensFromMetadata: => + messages = @props.thread.metadata + tokens = [] + + field = 'from' + if (messages.every (message) -> message.isFromMe()) + field = 'to' + + for message, idx in messages + if message.draft + continue + + for contact in message[field] + if tokens.length is 0 + tokens.push({ contact: contact, unread: message.unread }) + else + lastToken = tokens[tokens.length - 1] + lastContact = lastToken.contact + + sameEmail = Utils.emailIsEquivalent(lastContact.email, contact.email) + sameName = lastContact.name is contact.name + if sameEmail and sameName + lastToken.unread ||= message.unread else - return false + tokens.push({ contact: contact, unread: message.unread }) - makeMetadataMapper = (toOrFrom) -> - (msg) -> - msg[toOrFrom].map (contact) -> - { contact: contact, unread: msg.unread } + tokens - if @props.thread.metadata - shouldOnlyShowRecipients = @props.thread.metadata.every (msg) -> - msg.from[0]?.isMe() - input = @props.thread.metadata - toOrFrom = if shouldOnlyShowRecipients then "to" else "from" - filterer = makeMetadataFilterer toOrFrom - mapper = makeMetadataMapper toOrFrom + getTokensFromParticipants: => + contacts = @props.thread.participants ? [] + contacts = contacts.filter (contact) -> not contact.isMe() + contacts.map (contact) -> { contact: contact, unread: false } + + getTokens: => + if @props.thread.metadata instanceof Array + list = @getTokensFromMetadata() else - input = @props.thread.participants - return [] unless input and input instanceof Array - filterer = (contact) -> not contact.isMe() - mapper = (contact) -> { contact: contact, unread: false } - - list = _.chain(input) - .filter(filterer) - .map(mapper) - .reduce(((prevContacts, next) -> prevContacts.concat(next)), []) - .value() + list = @getTokensFromParticipants() # If no participants, we should at least add current user as sole participant - if list.length is 0 and @props.thread.participants.length > 0 + if list.length is 0 and @props.thread.participants?.length > 0 list.push({ contact: @props.thread.participants[0], unread: false }) # We only ever want to show three. Ben...Kevin... Marty diff --git a/internal_packages/thread-list/spec/thread-list-participants-spec.cjsx b/internal_packages/thread-list/spec/thread-list-participants-spec.cjsx index a20d0e645..e834f2783 100644 --- a/internal_packages/thread-list/spec/thread-list-participants-spec.cjsx +++ b/internal_packages/thread-list/spec/thread-list-participants-spec.cjsx @@ -25,7 +25,7 @@ describe "ThreadListParticipants", -> unread = ReactTestUtils.scryRenderedDOMComponentsWithClass(@participants, 'unread-true') expect(unread.length).toBe(1) - describe "getParticipants", -> + describe "getTokens", -> beforeEach -> @ben = new Contact(email: 'ben@nylas.com', name: 'ben') @evan = new Contact(email: 'evan@nylas.com', name: 'evan') @@ -105,6 +105,17 @@ describe "ThreadListParticipants", -> {spacer: true}, {contact: @michael, unread: true}, {contact: @kavya, unread: true}] + },{ + name: 'ends with two emails from the same person, second one is unread' + in: [ + new Message(unread: false, from: [@ben]), + new Message(unread: false, from: [@evan]), + new Message(unread: false, from: [@kavya]), + new Message(unread: true, from: [@kavya]), + ] + out: [{contact: @ben, unread: false}, + {contact: @evan, unread: false}, + {contact: @kavya, unread: true}] },{ name: 'three unread responses to long thread' in: [ @@ -162,13 +173,13 @@ describe "ThreadListParticipants", -> ) - expect(participants.getParticipants()).toEqual(scenario.out) + expect(participants.getTokens()).toEqual(scenario.out) # Slightly misuse jasmine to get the output we want to show - if (!_.isEqual(participants.getParticipants(), scenario.out)) + if (!_.isEqual(participants.getTokens(), scenario.out)) expect(scenario.name).toBe('correct') - describe "when getParticipants() called and current user is only sender", -> + describe "when getTokens() called and current user is only sender", -> beforeEach -> @me = AccountStore.current().me() @ben = new Contact(email: 'ben@nylas.com', name: 'ben') @@ -177,19 +188,19 @@ describe "ThreadListParticipants", -> @michael = new Contact(email: 'michael@nylas.com', name: 'michael') @kavya = new Contact(email: 'kavya@nylas.com', name: 'kavya') - getParticipants = (threadMetadata) -> + getTokens = (threadMetadata) -> thread = new Thread() thread.metadata = threadMetadata participants = ReactTestUtils.renderIntoDocument( ) - participants.getParticipants() + participants.getTokens() it "shows only recipients for emails sent from me to different recipients", -> input = [new Message(unread: false, from: [@me], to: [@ben]) new Message(unread: false, from: [@me], to: [@evan]) new Message(unread: false, from: [@me], to: [@ben])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @ben, unread: false} {contact: @evan, unread: false} {contact: @ben, unread: false}] @@ -198,7 +209,7 @@ describe "ThreadListParticipants", -> it "is case insensitive", -> input = [new Message(unread: false, from: [@me], to: [@evan]) new Message(unread: false, from: [@me], to: [@evanCapitalized])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @evan, unread: false}] expect(actualOut).toEqual expectedOut @@ -207,7 +218,7 @@ describe "ThreadListParticipants", -> new Message(unread: false, from: [@me], to: [@evan]) new Message(unread: false, from: [@me], to: [@michael]) new Message(unread: false, from: [@me], to: [@kavya])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @ben, unread: false} {spacer: true} {contact: @michael, unread: false} @@ -216,7 +227,7 @@ describe "ThreadListParticipants", -> it "shows correct recipients even if only one email", -> input = [new Message(unread: false, from: [@me], to: [@ben, @evan, @michael, @kavya])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @ben, unread: false} {spacer: true} {contact: @michael, unread: false} @@ -228,13 +239,13 @@ describe "ThreadListParticipants", -> new Message(unread: false, from: [@me], to: [@evan]) new Message(unread: false, from: [@me], to: [@evan]) new Message(unread: false, from: [@me], to: [@evan])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @evan, unread: false}] expect(actualOut).toEqual expectedOut it "shows only the recipient for one sent email", -> input = [new Message(unread: false, from: [@me], to: [@evan])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @evan, unread: false}] expect(actualOut).toEqual expectedOut @@ -243,7 +254,7 @@ describe "ThreadListParticipants", -> new Message(unread: false, from: [@me], to: [@ben]) new Message(unread: true, from: [@me], to: [@kavya]) new Message(unread: true, from: [@me], to: [@michael])] - actualOut = getParticipants input + actualOut = getTokens(input) expectedOut = [{contact: @evan, unread: false}, {spacer: true}, {contact: @kavya, unread: true}, @@ -285,8 +296,8 @@ describe "ThreadListParticipants", -> ) - expect(participants.getParticipants()).toEqual(scenario.out) + expect(participants.getTokens()).toEqual(scenario.out) # Slightly misuse jasmine to get the output we want to show - if (!_.isEqual(participants.getParticipants(), scenario.out)) + if (!_.isEqual(participants.getTokens(), scenario.out)) expect(scenario.name).toBe('correct')