Summary: More files that don't need linting
Test Plan: ... run the build
Reviewers: evan
Reviewed By: evan
Subscribers: juan, evan
Differential Revision: https://phab.nylas.com/D3599
Summary:
- Make creating/renaming categories non-optimistic (this, along with the
submodule commit D3593, prevents sub-categories with emtpy titles)
- Use SyncbackTaskAPIRequest in DestroyCategoryTask
- Don't get upset that renaming a category doesn't return a server id
Test Plan: local
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3594
Summary: Second attempt at not linting the error logger and tiny lint cleanup
Test Plan: run the build
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3591
Summary:
- Handle `categories` if it exists, because clone() will call fromJSON()
on the results of toJSON(), which populates `categories` instead of
`folder` or `labels`
- Remove the else-if for `folder` and `labels`, because both can co-exist.
Also concatenate the results from these two, rathering than overwriting.
Submodule commit is D3581, these two diffs together help make sure the
inflated messages in N1 have the right categories.
Test Plan: tested locally
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3582
Summary: Skipping linting for the error logger
Test Plan: Run the build
Reviewers: juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3583
refactor(send): split delivery from sent folder stuffing
Summary:
See explanation in https://phab.nylas.com/D3577
Depends on D3577
Test Plan: manual
Reviewers: jackie, juan, halla
Reviewed By: juan, halla
Differential Revision: https://phab.nylas.com/D3578
Summary:
We weren't removing the inbox category in the search perspective, so
things weren't actually being archived.
Test Plan: Run locally
Reviewers: juan, evan
Reviewed By: evan
Maniphest Tasks: T7389
Differential Revision: https://phab.nylas.com/D3575
Summary:
We want users to know that their account is still syncing if they're
looking for a message and end up at the bottom of a thread list.
This diff changes some of the wording around the messaging when
sync isn't complete and also adds a footer at the bottom of the
syncing thread lists. Additionally, we added a "Show Progress"
link that will expand the sync details in the lower left.
Addresses T7354
Test Plan: tested locally
Reviewers: juan, evan, jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3551
Summary: We weren't doing this which was causing us to deref undefined later.
Test Plan: Run locally
Reviewers: juan, evan
Maniphest Tasks: T7355
Differential Revision: https://phab.nylas.com/D3561
Summary:
This replaces the API delta stream with a direct in-memory one
Depends on D3548
Test Plan: manual
Reviewers: halla, jackie
Reviewed By: halla, jackie
Subscribers: juan
Differential Revision: https://phab.nylas.com/D3549
Summary: Also move calculation to sync store, rename stuff a little bit
Test Plan: Manual :(
Reviewers: evan, jackie, halla
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3547
Summary:
Depends on D3544 (K2 diff)
This commit ensures that auth notifications are showed when the
underlying sync worker fails and are cleared when an account is
successfully reconnected
To achieve this, we manually keep track and update syncStates where
appropriate via `Actions.updateAccount`, given that we have access to
N1's version of the account directly from local-sync.
Initially I was considering account delta stream to the cloud-api and the local-api, but that
just complicated things more than it helped.
This commit also fixes a bug with refreshing the gmail token in which we
we were only attempting a token refresh upon restarting the app
This addresses: T7346, T7305, T7335
Test Plan: Manual
Reviewers: halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3545
Summary:
I believe I discovered why our tests intermittently fail, why those
failures cause a cascade of failures, and how to fix it.
The bug is subtle and it's helpful to get a quick refresher on how various
parts of our testing system work:
First a deeper dive into how Promises work:
1. Upon creating a new Promise, the "executor" block (the one that gets
passed `resolve` and `reject`), gets synchronously run.
2. You eventually call `resolve()`.
3. The minute you call `resolve()`, Bluebird's `async.js` queues up
whatever's downstream (the next `then` block)
4. The queue gets processed on every "tick".
5. Once the "tick" happens, our queue processes and downstream `then`
blocks get run.
6. If any more Promises come in before the "tick", they get added to the
queue. Nothing gets processed until the "tick" happens.
The important takeaway here is this "tick" in step 4. This "tick" is the
core of what makes Promises asynchronous. By default, Bluebird in our
Node-like environment uses `setImmediate` as the underlying
implementation.
Our test environment is different. We do NOT use `setImmediate` in our test
environment.
We use Bluebird's `Promise.setScheduler` API to implement our own "tick". This
gives us much greater control over when Promises advance. Node's `setImmediate`
puts you at the whim of the underlying event loop.
Before today, our test "tick" implementation used `setTimeout` and
`advanceClock` triggered by `process.nextTick`.
Let me also quickly explain `setTimeout` in our test environment.
We globally override `setTimeout` in our test environment to not be based on
actual time at all. There are places in our code where we wait for several
hundred miliseconds, or have timeouts places. Instead of "sleeping" some amount
of time and hoping for the best, we gain absolute control over "time". "Time"
is just an integer value that only moves forward when you manually call
`advanceClock()` and pass it a certain number of "milliseconds".
At the beginning of each test, we reset time back to zero, we record
setTimeouts that come in, and as advanceClock gets called, we know if we need
to run any of the setTimeout callbacks.
Back to the Promise "tick" implementation. Before today, our testing "tick"
implementation relied our our stubbed `setTimeout` and our control of time.
This almost always works fine. Unfortunately tests would sometimes
intermittently fail, and furthermore cause a cascade of failures down the road.
We've been plauged with this for as long as I can remember. I think I finally
found how all of this comes together to cause these intermittent failures and
how to fix it.
The issue arises from a test like the one in query-subscription-pool-spec. We
have tests here (and subtly in other yet unknown places) that go
something like this:
```
it("TEST A", () => {
Foo.add(new Thing())
expect(Foo._things.length).toBe(1)
})
it("TEST B", () => {
expect(true).toBe(true)
})
```
At the surface this test looks straightforward. The problem is that
`Foo.add` may down the line call something like `trigger()`, which may
have listeners setup, which might try and asynchronously do all kinds of
things (like read the fs or read/write to the database).
The biggest issue with this is that the test 'finishes' immediately after
that `expect` block and immediately moves onto the next test. If `Foo.add`
is asynchronous, by the time whatever downstream effects of `Foo.add` take
place we may be in a completely different test. Furthremore if those
downstream function errors, those errors will be raised, but Jasmine will
catch them in the wrong test sending you down a rabbit hole of dispair.
It gets worse.
At the start of each test, we reset our `setTimeout` stub time back to
zero. This is problematic when combined with the last issue.
Suppose `Foo.add` ends up queuing a downstream Promsie. Before today, that
downstream Promise used `setTimeout(0)` to trigger the `then` block.
Suppose TEST A finishes before `process.nextTick` in our custom scheduler
can call `advanceClock` and run the downstream Promise.
Once Test B starts, it will reset our `setTimeout` stub time back to zero.
`process.nextTick` comes back after Test B has started and calls
`advanceClock` like it's supposed to.
Unfortunately, because our stub time has been reset, advanceClock will NOT
find the original callback function that would have resolved `Foo.add`'s
downstream Promise!!!
This means that Bluebird is now stuck waiting for a "tick" that will never
come anymore.
Since Bluebird thinks it's waiting for a "tick", all future Promises will
get queued, but never called (see Step 6 of the Promise description
above).
This is why once one test fails, downstream ones never complete and
Jasmine times out.
The failure is intermittent because `process.nextTick` is racing agianst a
test finishing, the next one starting, and how many and how far downstream
promises are setup.
Okay. So how do we fix this? First I tried to simply not reset the time back to
zero again in our stubbed time-override. This doesn't work because it simply
exposes the diasterous consequences of downstream Promises resolving after a
test has completed. When a test completes we cleanup objects, unmount React
components. Those downstream promises and timeouts come back and throw all
kinds of errors like: "can't read property x of undefined" and "can't find a
match for component X".
The fix that works the best is to simply MAKE PROMISES FULLY SYCNRHONOUS.
Now if you look at our custom Promise Test Scheduler in time-override,
you'll see that it immediately and sychronously calls the function. This
means that all downstream promises will run BEFORE the test ends.
Note that this is designed as a safeguard. The best way to make a more
robust test is to declare that your funtion body is asynchronous. If you
call a method that has downstream effects, it's your responsibility to
wait for them to finish. I would consider the test example above a very,
very subtle bug. Unfortunately it's so subtle that it's unreasonable to
expect that we'll always catch them. Making everything in our testing
environment synchronous ensures that test setup and cleanup happen when we
intuitively expect them to.
Addendum:
The full Promise call chain looks something like this:
-> `Promise::_resolveCallback`
-> `Promise::_fulfill`
-> `Promise::_async.settlePromises`
-> `AsyncSettlePromises`
-> `Async::_queueTick`
-> CHECK `Async::_isTickUsed`
-> `Async::_schedule`
-> `TimeOverride.scheduler`
-> `setTimeout`
-> `process.nextTick`
-> `advanceClock`
-> `Async::_drainQueues`
-> `Async::_drainQueue`
-> `THEN BLOCK RUNS`
-> `Maybe more things get added to queue`
-> `Async::_reset`
-> `Async::_queueTick` works again.
Test Plan: They now work
Reviewers: halla, mark, spang, juan, jackie
Reviewed By: juan, jackie
Differential Revision: https://phab.nylas.com/D3538
Summary:
Adds a UI component to see the progress of initial sync.
(The behavior is sometimes a little strange, but I believe this is
just because of weird behavior in the NylasSyncStatusStore itself.
For instance, it sometimes never populates the `folderSyncProgress`
fields, or it never sets the progress to 1, so the bar stops at 99%)
Test Plan: tested locally
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3536
Summary:
Associated K2 diff: D3529
This commit converts multi-send from a 3 step process into a 2 step
process
The first step creates the base message and sends a message per
recipient, each with its customized message body for tracking.
The second step reconciles all sent messages, specifically removing any
sent messages created by gmail, and saving the correct message to the
sent folder
This commit also ensures that we run the send tasks immediately by
ensuring we restart the sync loop if its already running
Depends on D3529
Test Plan: Manual
Reviewers: evan, jackie, halla
Reviewed By: jackie, halla
Differential Revision: https://phab.nylas.com/D3530
Summary: Adds a new button in auth for native Office 365 support
Test Plan: manual
Reviewers: jackie, halla, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3532
Summary:
The composer wasn't popping out correctly because the session's draft
was still null. This diff switches to using sessionForClientId(),
even if the session already exists in _draftSessions, because
sessionForClientId() won't create a new session if it doesn't need
to, and will always return a promise that resolves once the session
gets its draft. This promise will resolve immediately if the session
already has a draft.
Test Plan: tested locally
Reviewers: evan, juan
Reviewed By: juan
Subscribers: juan, spang
Differential Revision: https://phab.nylas.com/D3531
