Commit graph

855 commits

Author SHA1 Message Date
Halla Moore
953a8e438e [*] Run the isomorphic-core specs as part of the client test suite
Summary:
Convert the isomorphic-core specs to Jasmine 1 and symlink them into
client-app/internal_packages so they are run as part of the client
test suite.

Test Plan: Ran the suite with fdescribes in iso-core to ensure they were being called properly

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D4055
2017-03-01 11:07:48 -08:00
Christine Spang
8c30697241 [client-sync] Enable logging in prod builds
Summary:
Disabling verbose logging in production builds just makes it harder to help our
users help us fix their problems.

Test Plan: run app without --dev

Reviewers: mark, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4070
2017-03-01 08:11:00 -08:00
Juan Tejada
b6a1fc5234 [client-app] Fix measurement of thread-list action metrics
Summary:
We detect when a thread is removed from the thread-list by listening to
`Actions.threadListDidUpdate`. However, we were not firing the action at
the correct moment.

Before this commit, we fired the action on the root `ThreadList`'s
`componentDidUpdate`, however did this not actually fire when the
child list actually updated/removed threads from the list.

This sort of used to work because before 396a027bcb
the root ThreadList component re-rendered all the time, so it fired the
action, but after initial sync, we would never actually report any
thread-list action metrics at all

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4051
2017-02-28 14:07:52 -08:00
Juan Tejada
d3381b7446 [client-app] Update changelog 2017-02-28 13:19:34 -08:00
Christine Spang
26d247bd97 [client-app] Actually fix send+archive
The previous fix only worked by coincidence. It turns out that we don't have
the right data to associate draftClientId with ChangeMailTask on task
creation, so we have to either look up this data inside the dependency
function or make the dependency check looser.

This check gets run all the time, so we chose to make the check looser
rather than putting a db call in here and changing the interface to support
an async version of isDependentOnTask().

This patch also makes it so that the fix works properly when Undo Send
is enabled.

Fixes T7889
2017-02-28 12:38:14 -08:00
Christine Spang
1d90f7f770 [client-app] Fix ChangeMailTask dependency on SendDraftTask
Because ChangeMailTask only depended on EnsureMessageInSentFolder task,
if a user sent a message & then archived it before the send completed,
the ChangeMailTask could run in between the SendDraftTask and
EnsureMessageInSentFolderTask and fail because it's
EnsureMessageInSentFolderTask that detects the new UID for the message.

Fixes T7889
2017-02-28 12:12:39 -08:00
Christine Spang
5d440c6b93 [client-sync] Fix wording in comment 2017-02-28 12:12:39 -08:00
Christine Spang
6e1dabadbb [client-sync] Add check for syncWorker to SyncbackTaskRunner 2017-02-28 12:12:39 -08:00
Juan Tejada
3f726d4d0b [client-app] Update changelog 2017-02-28 12:12:10 -08:00
Juan Tejada
19d504524b bump(version) 1.0.30 2017-02-28 11:48:28 -08:00
Juan Tejada
bb76da14cc [client-app] Fix EnsureMessageInFolderTask - no need to get acct from imap connection 2017-02-28 11:26:58 -08:00
Juan Tejada
6cf9e487b0 [iso-core] Fix typo in IMAPBox 2017-02-28 11:16:37 -08:00
Evan Morikawa
9fd2f097b0 [client-sync] add assertion about Reference not cleaned up properly 2017-02-28 11:09:17 -08:00
Juan Tejada
de0e30d8dd Revert "[client-app] Limit search to focused perspective"
Temporarily reverting this commit for 1.0.30 release.
See https://phab.nylas.com/T7910 for details
This reverts commit da6bc473f8.
2017-02-28 11:07:44 -08:00
Evan Morikawa
2126060d27 [iso-core] imap-connection properly resolves this 2017-02-28 10:57:29 -08:00
Evan Morikawa
875fac3227 [iso-core] imap-box.js -> imap-box.es6 2017-02-28 10:43:49 -08:00
Juan Tejada
0294db95cf Revert "[client-app] Properly re-index threads when their data has changed"
Temporarily reverting this commit for 1.0.30 release because it is causing
performance issues: https://phab.nylas.com/T7910

This reverts commit 5e35d39eb2.
2017-02-28 10:35:58 -08:00
Juan Tejada
5714a23501 [client-app] Remove contact support btn from red box
Summary: See title

Test Plan: manual

Reviewers: evan, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4049
2017-02-28 10:27:58 -08:00
Juan Tejada
396a027bcb [client-app] Prevent unnecessary re-rendering of thread-list
Summary:
This commit adds a `shouldComponentUpdate` to thread-list.cjsx so that
the thread list doesn't unnecessarily /try/ to re-render when state or
props haven't actually changed.

I noticed this because the thread list was constantly calling `render`
even though it didn't render any changes to the DOM. This was caused
because it listens to `NylasSyncStatusStore` which constantly triggers,
even though the piece of state the list is interested in rarely changes,
causing unnecessary calls to the `render` loop.

Test Plan: manual

Reviewers: halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4047
2017-02-28 10:04:31 -08:00
Juan Tejada
1e7541a1ee [iso-core] Loosen checks for detecting retryable errors + add new one
Summary: see title

Test Plan: manual

Reviewers: evan, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4046
2017-02-28 10:00:12 -08:00
Juan Tejada
8ade5d6486 [iso-core] (imap-P3) Fixup IMAPConnection and IMAPBox
Summary:
This commit is an attempt to cleanup duplicated code and crufty code
inside IMAPConnection and IMAPBox

Specifically:
- It replaces `_createConnectionPromise` with a more aptly named (imo) `_withPreparedConnection` helper, which provides the user a node-imap connection that will correctly time out and handle `error` and `end` events. Most of these changes are just changing existing code to use the new interface.
- Adds a subtle change to how we handle `end` and `error` events on the connections. Previously, we manually called `this.end()` on `error`, but not on `end`. From what I could gather from the old comment documenting `_createConnectionPromise`, we should /also/ call `this.end()` on `end` because node-imap doesn't clean up correctly and can leave the connection hanging (taking care not to introduce a recursion loop by `end`ing on `end`). Additionally, it no longer listens to the events via `once` but via `on`, which should be okay given that the listeners get cleared at the end.

This /might/ fix some instances of sync freezing up (T7837).

Depends on D4035

Test Plan: manual -- this really needs some unit tests 😢

Reviewers: spang, halla, mark, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4036
2017-02-28 09:58:10 -08:00
Juan Tejada
18095cac3e [iso-core] (imap-P2) Actually fix constant socket timeout errors 😢
Summary:
Remember how we made it so imap socket timeouts would increment up to 10m if
we constantly got timeout errors? Remember how we told everyone in
https://github.com/nylas/nylas-mail/issues/3232 that we'd fixed their problems, but they weren't actually fixed?

Well, we weren't /actually/ applying the correct timeout value. 😢

Depends on D4033

Test Plan: manual

Reviewers: spang, halla, mark, evan

Reviewed By: mark, evan

Differential Revision: https://phab.nylas.com/D4035
2017-02-28 09:41:21 -08:00
Juan Tejada
a2e0b81493 [iso-core] (imap-P1) 🎨 Fixup settings resolution in IMAPConnection
Summary:
The way it was set up was a little smelly.
- `this._settings` were /not/ the actual settings. Now, internally we have access to `this._resolvedSettings`
- `this.resolvedSettings` was being set kind of out of nowhere (gmail-oauth-helpers needs it, hence adding a getter instead)

Test Plan: manual

Reviewers: spang, mark, evan, halla

Reviewed By: mark, evan, halla

Subscribers: khamidou

Differential Revision: https://phab.nylas.com/D4033
2017-02-28 09:21:09 -08:00
Juan Tejada
72613baf5d [client-app] Fix selecting and creating signatures in preferences
Summary:
This commit fixes 2 issues with signatures in the preferences:
- Creating a signature (via any of the create buttons) would create 2 signatures
- Trying to select accounts to associate with a signature in the preferences would not work (the account would not be selected)

This was a regression introduced in e638e94084 (diff-4f38fd25aa24b48a309354be643165d3R26)

3111c16 made it so we attempt to `activate` any Stores that are registered with
the StoreRegistry. When adding stores to `nylas-exports`, they are
automatically registered in the StoreRegistry. Given that the
`SignatureStore` is in `nylas-exports`, and consequently is registered in
the StoreRegistry, it would be `activate`d upon window boot.

However, we were also manually activating it inside `internal_packages/composer-signatures/lib/main.es6`.
This caused it to register listeners for every action **twice**. For
this reason, 2 signatures would be created when trying to create 1, and
an account would be immediately unselected after being selected int he
signatures dropdown in preferences.

(Other changes in this are just stylistic)

Test Plan: manual

Reviewers: spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4043
2017-02-28 09:14:28 -08:00
Juan Tejada
de9484cc52 [client-sync] Fix missing pass in of syncWorker in EnsureMessageInSentFolder 2017-02-26 10:36:31 -08:00
Mark Hahnenberg
6bd6242398 [client-sync] Use IMAPConnectionPool in client sync worker
Summary: See title

Test Plan: Run locally, make sure sync still works

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D3998
2017-02-24 13:09:32 -08:00
Christine Spang
e991349708 [client-sync] Fix missing pass in of syncWorker to FetchNewMessagesInFolder 2017-02-24 12:49:37 -08:00
Evan Morikawa
f23a96c481 [client-sync] limit attributes returned for messages
Summary:
I did a grep for `Message.find.*` to see if there were any other places
we could limit the attributes returned

There were.

This used to be INSANELY wasteful to return all bodies when we only needed
the ids.

Test Plan: Manually bootup app and ensure search still works

Reviewers: mark, juan, spang

Reviewed By: mark, juan, spang

Differential Revision: https://phab.nylas.com/D4042
2017-02-24 11:04:18 -08:00
Evan Morikawa
c7bdb5804a [client-sync] fix unbouded query
Summary:
Fixes T7783

This unbounded query, plus a yet-to-be-diagnosed bug causing large numbers
of messages with no folderIds, led to frequent crashes of the worker
window at the end of every sync loop.

This took a fair amount to diagnose. Notes on that are here:
https://paper.dropbox.com/doc/Diagnosing-Crash-X329VTxevrqtIMESBtHNV

Huge thanks to @mark.

Test Plan: Reboot app with patch, no more crashes!

Reviewers: spang, halla, mark, juan

Reviewed By: mark, juan

Subscribers: mark

Maniphest Tasks: T7783

Differential Revision: https://phab.nylas.com/D4039
2017-02-24 10:58:21 -08:00
Christine Spang
15eac9c9db [client-sync] Parse comma-separated References properly
This goes against the spec, but we've seen emails which use comma-separated
References instead of space-separated references. This could cause threading
to break if the message with comma-separated references was a reply and
we hadn't yet synced the message directly in the In-Reply-To header.
2017-02-23 18:58:13 -08:00
Juan Tejada
5e35d39eb2 [client-app] Properly re-index threads when their data has changed
Summary:
When we detect that threads in the database have changed, if they've already
been indexed, mark them as unindexed so that the indexer picks them up and
re-indexes them with the updated data

Test Plan: manual

Reviewers: evan, spang, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4023
2017-02-23 17:52:09 -08:00
Juan Tejada
93836f7973 [iso-core] Fix imap error detection/coercion (again)
Summary:
If for whatever reason we passed an error with a `source` property
we weren't accounting for in `convertIMAPError`, we would return an
`undefined` error. Bad!

Instead, just return the original error

Test Plan: manual

Reviewers: spang, halla, evan, mark

Reviewed By: evan, mark

Differential Revision: https://phab.nylas.com/D4037
2017-02-23 17:46:15 -08:00
Juan Tejada
e6b35497b8 [client-app] When reporting search metrics, clip all times to a range
Summary:
When reporting different times to mixpanel other than `actionTimeMs` via
recordPerfMetric, we also want to clip those times to a range to have
good data in mixpanel

This commit adds an extra option to recordPerfMetrics to be able to clip
data other than the default `actionTimeMs`, and uses the new option to
report search metrics

Test Plan: manual

Reviewers: spang, evan, halla, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4038
2017-02-23 17:42:27 -08:00
Juan Tejada
f1ed46d144 [client-app] Sample metrics reporting of actions we're keeping track of
Summary:
I'm making these up based on gut feeling, so feel free to completely change
these.

Test Plan: manual

Reviewers: spang, mark, evan

Reviewed By: mark, evan

Differential Revision: https://phab.nylas.com/D4026
2017-02-23 17:40:22 -08:00
Juan Tejada
49586e2e06 [client-app] Fix passing in nylasId and accountId to report metrics
Summary: This was really broken

Test Plan: manual

Reviewers: halla, mark, evan

Reviewed By: mark, evan

Differential Revision: https://phab.nylas.com/D4021
2017-02-23 16:35:33 -08:00
Juan Tejada
ad1a192b53 [client-app] Report source when reporting open times for accts window
Summary: See title

Test Plan: manual

Reviewers: mark, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4020
2017-02-23 16:17:50 -08:00
Juan Tejada
aeddc92e8e [client-app] Measure and report search perf metrics
Summary: See title

Test Plan: manual

Reviewers: evan, spang, halla, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4019
2017-02-23 16:02:22 -08:00
Juan Tejada
7cd5128db4 [client-app] Measure and report app boot time
Summary: See title

Test Plan: manual

Reviewers: spang, mark, evan, halla

Reviewed By: mark, evan, halla

Differential Revision: https://phab.nylas.com/D4018
2017-02-23 16:01:07 -08:00
Christine Spang
49103367c1 [client-sync] Update comment about Gmail inbox prioritization 2017-02-23 15:36:35 -08:00
Christine Spang
4a1dde8830 [client-sync] Deduplicate UID downloads across sync loop interruptions
Summary:
Since we only persist updates to fetchedmin/fetchedmax at the end of a
batch, and a batch can contain many messages, if the sync loop is
getting interrupted often we will download the same messages over and
over again and not make much progress in downloading the message
backlog. This patch keeps a set of already downloaded messages in memory
for each batch and skips downloading UIDs we've processed in interrupted
sync loops.

Messages may still be redownloaded across app restarts.

Fixes T7798

Test Plan: manual

Reviewers: juan, mark

Reviewed By: juan, mark

Maniphest Tasks: T7798

Differential Revision: https://phab.nylas.com/D4040
2017-02-23 15:23:28 -08:00
Mark Hahnenberg
da6bc473f8 [client-app] Limit search to focused perspective
Summary:
Previously we would always search all mail. Now, if the user has focused
a particular folder we will limit our search to that folder. The inbox
is an exception--it will always search all mail unless the user
explicitly uses an "in:" clause.

Test Plan: Run locally, verify that searching folders returns the correct results.

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4032
2017-02-23 13:36:45 -08:00
Juan Tejada
95766e990a [cloud-api] DONT Timeout streaming API connections every 15 minutes anymore
This is a temporary commit to prevent connections from being closed
while we release logic to re-establish delta connections after they are closed.

Our infrastructure should be able to support current usage of delta
connections after our fixes to offline notification and infrastructure

This reverts commit 78f67d4a76.
2017-02-23 11:56:31 -08:00
Juan Tejada
18bf86769d [client-app] Update changelog 2017-02-23 10:52:14 -08:00
Juan Tejada
dced0aa99e [client-sync] 🎨 Remove unecessary use of PromiseUtils
Given that this packages lives inside the NylasMail app, it has access
to Bluebird Promises, which already define a .each method.

Using PromiseUtils here is unecessary
2017-02-22 18:30:34 -08:00
Juan Tejada
b96300f000 [iso-core] Fix imap error detection!
Summary:
We were only detecting and classifying IMAP errors as retryable and non
retryable in a few places, but errors thrown in any of our imap
operations were not being properly passed through our `convertImapErrors` :(

This was broken, oh so broken

Also loosen condition to detect System Errors

Test Plan: manual

Reviewers: spang, mark, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4022
2017-02-22 17:51:56 -08:00
Juan Tejada
feda238d9c [client-app] Measure and record times for opening account windows
Summary: See title

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4017
2017-02-22 17:09:42 -08:00
Juan Tejada
ecf3709669 [client-app] Measure and report composer window open times
Summary: To Mixpanel and Honeycomb.

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4016
2017-02-22 17:08:31 -08:00
Juan Tejada
b819889b36 [client-app] Measure and report task performLocal times
Summary: See title

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4015
2017-02-22 16:47:15 -08:00
Juan Tejada
ae9aede300 [client-app] Measure and report sending times
Summary: See title

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4014
2017-02-22 16:45:08 -08:00
Juan Tejada
4c57fcfab4 [client-app] Measure and report time to select and load a thread
Summary: See title

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4013
2017-02-22 16:39:34 -08:00