Summary:
It always drives me crazy when folks use "setup" (no space) as a verb,
because the verb version has a space in it.
Also remove the word "Setup" from the displayName for custom IMAP accounts
because it doesn't make sense for how we use this field, e.g.
a8f36b88d9/packages/client-app/internal_packages/onboarding/lib/page-account-onboarding-success.jsx (L27)
will display "Sucessfully connected to IMAP / SMTP Setup!" with the old
wording when you successfully auth a custom IMAP account.
Test Plan: manual
Reviewers: evan, juan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4199
Summary:
Dunno if I'm the only one having this problem, but I spent at least an
hour over the last two days wondering why hapi just sits there and hangs
in the Joi validation step when I copy-paste calls to /auth from the
developer bar to my terminal. It turns out that if you fail to send the
correct Content-Type when sending a JSON payload, Joi just hangs
forever. (WTF!)
Test Plan: manual
Reviewers: halla, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4204
Summary:
Previously, n1 cloud auth errors were stuffed into an account's
syncState. The problem with that is that the sync loop manages that
state, and as long as the sync loop is running it will set that state to
running. However, it might be the case that the sync loop is running but
we can't connect to n1Cloud, so even though we would set the `syncState`
to `n1_cloud_auth_failed`, the sync loop would just set it back to
'running', and the user wouldn't see the error notification indicating
that it can't connect to n1Cloud
This commit makes it so we keep tracj of the auth failure state for
n1Cloud in a separate field, and makes sure that the error notification
component shows that error.
Test Plan: manual
Reviewers: mark, spang, evan, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D4172
Summary:
This commit adds a new perf metric for inline composer times.
Additionally, it consolidates the timer logic for all other types of draft
creation (mailto links, dropping a file in the app dock icon, etc).
Test Plan: manual
Reviewers: halla, evan, mark
Reviewed By: evan, mark
Differential Revision: https://phab.nylas.com/D4186
Summary:
This commit consolidates the `DeltaConnectionStatusStore` and the
`DeltaConnectionStore` which kept track of very similar state and made
sense to be the same store (as per feedback in D4118#77647)
Given that this state needs to be available app-wide for plugins to
query the status of delta connections, `internal_packages/deltas` was
removed (given that it only activated that store), in favor of having the
unified store inside `src/flux/stores` and available via `nylas-exports`
The `deltas` package also contained some contacts-ranking code, which is
no longer in use until we restore that fetaure, so I created a
`internal_packages/contact-rankings` which contains this unused code for
now.
Test Plan:
manually open, close, end delta connections, verify that I'm getting
correct results. unit tests to come
Reviewers: halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4145
Summary:
This DRY's up code that use these status codes
Depends on D4130
Test Plan: manual
Reviewers: spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4140
Summary:
Previously if we got back a huge number of unknown UIDs from IMAP search
we would try to sync all of them at once. This could lead to hanging the sync
loop trying to download tons of messages. This diff limits the UIDs we're
willing to sync per task to 500 and splits each task up into chunks of 25
messages so that we don't try to download all of them at once. If we need
to sync more than 500 uids then at the end of the syncback task it will
queue another task to run the next time the sync loop rolls back around.
Test Plan:
Run locally, verify that we gracefully handle various situations
including cancelling during the syncback task, cancelling between syncback
tasks, huge numbers of results, etc
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4142
Summary:
This commit splits apart the `AccountDeltaConnection` class, which was
in charge of listening to both cloud /and/ local deltas by way of an
artificial interface, `DeltaStreamingInMemoryConnection`.
Splitting this into 2 modules with separate responsibilities will hopefully
make this code easier to reason about and reduce some cruft and unnecessary
indirection.
Specifically, this commit makes it so:
- `DeltaConnectionManager` is only in charge of starting and ending `DeltaStreamingConnection`s, which are solely in charge of listening to deltas from the cloud api
- `LocalSyncDeltaEmitter` no longer unnecessarily emits events for the `deltas` package to listen to but rather directly processes and saves those deltas from the K2 db to edgehill.db
- `LocalSyncDeltaEmitter` is also in charge of keeping track of the latest received cursor, under its own JSONBlob key in edgehill.db. This migrates localSync cursors saved under the old key.
- `LocalSyncDeltaEmitter` is now instantiated and managed from within the `SyncProcessManager` as opposed to the `SyncWorker`. Apart from removing extra state from the `SyncWorker`, this removes dependencies on the client-app environment from the sync-worker.
- `DeltaStreamingInMemoryConnection` and `AccountDeltaConnection` are now gone
(Sorry for the big diff! This one was a little hard to split up without landing something broken)
Depends on D4121
Test Plan: manual + unit tests planned in upcoming diff
Reviewers: halla, mark, evan, spang
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4122
Summary:
This is to make it available to all plugins in the app
Depends on D4120
Test Plan: manual
Reviewers: halla, mark, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4121
Summary:
This commit completely refactors `DeltaStreamingConnection`, notably
introducing the following changes:
- Right now, `AccountDeltaConnection` establishes both delta connections to the cloud api and to the `client-sync` database (K2). This class is meant to disapper in favor of splitting into two different classes meant for syncing with the n1Cloud api and the local database. Specifically, `DeltaStreamingConnection`'s only responsibility is now to connect to the n1Cloud API and establish an http streaming connection for metadata deltas, etc. This class no longer unecessarily inherits from `NylasLongConnection`, which removes a lot of unecessary callback indirection.
- The statuses of the n1Cloud delta streaming connections are now stored in as JSONBlobs in edgehill.db under new keys. This commit ensures that the data is correctly migrated from the old key (`NylasSyncWorker:<account_id>`).
- The `DeltaStreamingConnection` now correctly retries when closed or errors. This logic previously existed, but was removed during the K2 transition: https://github.com/nylas/nylas-mail/blob/n1-pro/internal_packages/worker-sync/lib/nylas-sync-worker.coffee#L67
- Delta connection retries now backoff using the `ExponentialBackoffScheduler`
- Attempt to restore the delta connection whenever the app comes back online
Depends on D4119
Test Plan: manual + planned unit tests in upcoming diff
Reviewers: halla, mark, evan, spang
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4120
Summary:
For legacy reasons, NylasLongConnection did not take the `onResults`
callback as an option, but rather it was a method you had to call on the
connection, which took the callback, and which you had to call after
instantiating the connection. This was an annoying and clunky interface.
This commit makes it so NylasLongConnection takes an `onResults` option
as one would expect it to, and updates any references to the old
interface
Depends on D4118
Test Plan: manual
Reviewers: halla, mark, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4119
Summary:
Turns out, NylasSyncStatusStore doesn't need to concern itself with the
status of delta connections.
This is legacy cruft from the days when this store used to provide the
online status of the app based on the delta connections. Now we have an
OnlineStatusStore, so we don't need this at all
Instead of stuffing that state in that store, this commit adds a small
DeltaConnectionStatusStore to provide an easy way to listen to changes
on the status of delta connections. The only part of the app concerned
with that state is the DeveloperBarStore in order to render the green
circles for delta connections in the worker window.
Depends on D4117
Test Plan: manual
Reviewers: mark, halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4118
Summary:
When searching using IMAP/Gmail commands we sometimes get back UIDs for
messages that we have yet to sync. Previously we would just ignore these
results, which would decrease the quality search results for quite some
time during initial sync. This diff enables us to eagerly sync the unknown
messages we get back from the provider by creating a syncback task which
interrupts the sync loop and runs a sync task for the unknown UIDs.
Test Plan: Run locally, verify that we sync unknown messages
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4101
Summary:
Gleb mentioned that we need the sample size of search events to be
bigger, so let's report all search events
Test Plan: manual
Reviewers: evan, gleb, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4077
Summary:
We no longer optimistically fetch messages when we load their bodies. We
always get the full message with bodies returned via the in memory delta
stream
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: spang, halla, juan
Differential Revision: https://phab.nylas.com/D4066
Summary:
We no longer need to use the `returnsModel` param since we get all of our
models through the in memory delta stream. There were a ton of places
unnecessarily passing `returnsModel: false` when it defaults to false in
the first place
Depends on D4057
Test Plan: manual
Reviewers: halla, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4065
Summary:
Previously we were using the raw visitors that were confined to the flux
attributes directory. We're going to add more search query backends, so this
is mostly just moving things to a new, more general place.
Test Plan:
Run locally, verify parser specs still work, verify in-app search
still works.
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4053
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
Temporarily reverting this commit for 1.0.30 release because it is causing
performance issues: https://phab.nylas.com/T7910
This reverts commit 5e35d39eb2.
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
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
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
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
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
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
Summary:
We did this to see if we should clear the index, which no longer makes
sense because the index never exceeds our threshold. Additionally, none
of the other search indexes do this. This was causing us to spend a ton
of time scanning the ThreadSearch table at startup.
Test Plan: Run locally, make sure things are fine
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4007
Summary: See title
Test Plan: Depends on D3990
Reviewers: spang, halla, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3991
Summary:
See title
Depends on D3989
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D3990
Summary:
This commit adds new actions, `applyCategoryToThreads` and `removeCategoryFromThreads`
to be proxied and measured through the ThreadListActionsStore. These are called when
labels are added or removed via the category picker or by removing using
the label icon.
For now, we are only interested in timing actions that remove threads
from the inbox.
Test Plan: manual + reenabled unit tests for category-picker
Reviewers: spang, evan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3989
Summary:
This commit adds a new action, `moveThreadsToPerspective` to be proxied and
measured through the ThreadListActionsStore. This action is called when
a thread or group of threads is dragged and dropped into an item in the
sidebar.
For now, we are only interested in timing actions that remove threads
from the inbox
Depends on D3984
Test Plan: manual
Reviewers: halla, evan, spang
Reviewed By: evan, spang
Differential Revision: https://phab.nylas.com/D3985
Summary:
This commit adds a new action, `removeThreadsFromView` to be proxied and
measured through the ThreadListActionsStore
This action can encompass many different actions, e.g.:
- unstarring in starred view
- changing unread in unread view
- Moving to inbox from trash
- archiving a search result (which won't actually remove it from the thread-list)
However, for now, we are only interested in timing actions that remove threads
from the inbox
Depends on D3983
Test Plan: manual
Reviewers: halla, spang, evan
Reviewed By: spang, evan
Differential Revision: https://phab.nylas.com/D3984
Summary:
This commit makes so it we report perf metrics for archive actions.
To achieve this, I added a new `ThreadListActionsStore` which serves as
a proxy for thread actions, which allow us to time them.
The new store is in charge of listening to thread list actions, creating and
queueing the appropriate tasks for any given action, and timing and
reporting action times to our MetricsReporter.
This commit only times archiving actions, and subsequent diffs will time
other relevant thread list actions.
Test Plan: manual
Reviewers: halla, spang, evan
Reviewed By: spang, evan
Differential Revision: https://phab.nylas.com/D3983
Summary:
This global module wasn't really related to performance, but rather with
timing things across different processes in the app. I believe this name
is more appropriate.
Test Plan: I can still use NylasEnv.timer (instead of NylasEnv.perf)
Reviewers: spang, evan
Reviewed By: spang, evan
Differential Revision: https://phab.nylas.com/D3972
Summary:
This removes client-sync deltas from the developerbar delta list.
We ONLY show cloud deltas now.
The connection between client-sync is no longer a network delta stream,
it's a direct function call. It makes no sense to show its status.
This now shows a single dot representing the state of the cloud delta
stream.
Test Plan: Manually connect and disconnect local cloud API and see icon change
Reviewers: halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3977
