Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected URL of the Nylas Wunderlist plugin in README #3585

Closed
wants to merge 5,201 commits into from

Conversation

miguelrs
Copy link

@miguelrs miguelrs commented Jun 6, 2017

Also added (new v2) to indicate a new version has just been released, which hopefully improves the functionality a lot.

jstejada and others added 30 commits March 13, 2017 22:42
Summary:
This commit makes it so `resetEmailCache` works as expected, i.e. it
removes all databases, without forcing the user to re sign-in to their
accounts or NylasID

Previously, this method removed the database without removing the
accounts, left users in an un-authed state that was hard to recover
from. This was fixed in D4212 which makes sure that when we get a new
identity, sync and deltas are restarted

However, resetEmailCache would still force you to log in to yoru NylasID
because it was deleted from the database. However, if we reuse the
command `application:relaunch-to-initial-windows` instead of manually
deleting the database, we can relaunc the app while preserving the users
NylasID session, so they don't have to sign back in manually.

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4215
Summary:
Use electron's `powerMonitor` module to detect when the computer resumes
from sleep, and restart the sync loop when that happens in order to
sync the inbox immediately, in case we received any new mail events
while the computer was asleep

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4216
Summary:
On windows running `git rev-parse --short HEAD` does in fact now give you
9 characters instead of 7 like it does on Mac. This will ensure that
builds get uploaded to the same folder and help ensure we don't post a
version that doesn't exist on the release page.

Test Plan: Manual

Reviewers: juan, halla, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4217
Summary: Minor copy change via Sachin

Test Plan: manual

Reviewers: juan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4218
Summary:
This is going to be a diff way shorter than the previous one! Basically, it adds a new endpoint, `/blobs` to our API to store send later attachments. When a user schedules a draft to be sent, we send all attachments to this endpoint. Separately, we store the rest of the message as metadata.

When it's time to send the message, we fetch the attachments from S3, fetch the metadata and merge them together to get a message we can send.

Test Plan: Tested manually. Will make a final QA pass before landing.

Reviewers: juan, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4196
Summary: We need to upload the nupkg for the Windows autoupdater to work

Test Plan: manual

Reviewers: juan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4219
Summary:
The ignore list was very old. It included several dozen MB of docs_src and
other crap in our builds

Test Plan: manual

Reviewers: halla, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4213
Summary:
We don't need to spend the time running script/bootstrap in our cloud-*
folders!

Test Plan: manual

Reviewers: spang, juan, halla

Reviewed By: juan, halla

Differential Revision: https://phab.nylas.com/D4214
Summary:
We were using a version that was ~9 months old and a lot of development has
happened since.

v3 is not compatible w/v2, but it looks like we aren't using any of the
features that had breaking changes:

    http://nodemailer.com/about/migrate/

I chose not to switch to the new built-in OAuth2 token refresh support
because we already have a mechanism for refreshing oauth tokens and
adding a different implementation specifically for SMTP would introduce
more opportunities for bugs.

Since mailcomposer is no longer a dependency of nodemailer, I added this
dependency as well.

Test Plan: manual - sent a message, sent a message w/an attachment

Reviewers: evan, khamidou, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4201
…y received

Summary:
We don't want to bump threads to the top of the inbox when a user sends a
reply. We originally used `!isSent` to prevent this, but that was removed in
a diff that made sure messages showed up in the inbox when users send emails
to themselves. In order to implement both of these cases properly, this diff
introduces `isReceived` and uses that to determine whether lastReceivedDate
should be updated. Addresses T7991.

Also changes the order of some `or` statements, so that we actually check that
the variable exists before comparing against it.

Test Plan: manual

Reviewers: evan, juan, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4226
Summary:
Previously, after creating a new folder, the UI would indicate that the new
folder had children, even though it didn't. This was caused by duplicate models
in our `MutableQueryResultSet` for the user's categories. Basically, we would
sync the server version of the folder before the `SyncbackTask` for the new
folder returned its `serverId`. Without the `serverId`, the synced version of
the folder couldn't yet be tied to the optimistic folder, so a second row was
created in the database. This second row is removed when the `syncbackTask`
does return the `serverId`, because we persist the optimistic folder with a
`REPLACE INTO` query. (This deletes other rows with the same id.) However,
since this was done inside a `persist` change with the `serverId` and no
`unpersist` was ever recorded for the `clientId`, our `MutableQueryResultSet`
never removed the `clientId` model.

To address this, this diff adds a check in `updateModel` to see if the
`serverId` is being added. If it is, and both the `serverId` and `clientId`
exist in the `_ids` list, we remove the `clientId`.

The children indicator does still briefly show up while there are still two
separate rows for that folder in the database. If we want to get rid of this
completely, we would have to ensure that we do not sync the folder before the
`syncbackTask` returns the `serverId`. However, this would probably be pretty
involved, and for not much gain. This fix is much simpler and reduces most of
the issue.

Test Plan: manual

Reviewers: juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4228
This reverts commit e798e50.

Reason for revert: breaks Send Later. Needs more testing.
Summary: see title. also convert to es6

Test Plan: manual

Reviewers: evan, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4225
for ei-prod only.  I set up the ei EB env as prod level, but
have been working on launch changed in master branch. Easy enough to undo,
but I didn't wan't to mess with prod branch until done because
the launch scripts affect all the n1cloud-related environments.
Summary:
When querying transactions for the delta stream, if no cursor is provided,
we should default to 0. Otherwise this will generate a query like:
```
WHERE `transaction`.`id` > ‘null’
```
Which is obviously wrong

Test Plan: manual

Reviewers: evan, halla, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4242
Summary:
Currently, when we auth an account for the first time in Nylas Mail (or
we blow away the database), the app is going to request transactions
since cursor `null` from the /delta/streaming endpoint and from the local-sync
delta observable, instead of requesting transactions since cursor `0`

This is due to a subtle bug with the use of default values when
destructuring an object. Our coded did the following:
```
const {cursor = 0} = this._state
```
Which at a glance seems correct. However, this will only work as
expected if `this._state` has the following shape:
```
{cursor: undefined}
```
And unfortunately, our `this._state` looked like this when authing an
account for the first time:
```
{cursor: null}
```

Which would make `cursor === null` instead of `0`.
This is because when using default values, null is considered an
intentional argument/value, as opposed to not passing any argument/value
(which will mean that the argument is undefined).

This was a regression introduced in d60a23c and 8bc2ec5

Test Plan: manual, will add regression test in upcoming diff

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4243
jstejada and others added 28 commits April 13, 2017 12:29
…work

Summary: See title

Test Plan: manual

Reviewers: spang, khamidou, halla

Reviewed By: khamidou, halla

Differential Revision: https://phab.nylas.com/D4421
Summary:
This was showing up in Sentry. Check to see if the recipient has an email
before calling toLowerCase.

Test Plan: yolo

Reviewers: halla, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4423
Summary:
By nullifying the key inside `workersByAccountId` we would attempt to
access accountIds that no longer existed in other parts of the code by
using Object.keys()

This makes it so we correctly completely remove the record from the map

Test Plan: manual

Reviewers: halla, spang, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4426
Summary:
After https://github.com/nylas/nylas-mail-all/commit/008cb4c, the shape of
account.connectionSettings changed, which means that ids for accounts
will also change, given that they are based on the connection settings (see
Account.hash()).

This is fine in most cases, except for accounts that were running on a
version of Nylas Mail before 008cb4c, then upgraded to a version
after 008cb4c, and then re-authed their account.
In this scenario, re-authing the account will create a second account with
a different `id` but with the same email address, along with an extra
sequelize database, and will start a second SyncWorker for the same
account. App-side, the `AccountStore` will correctly overwrite the first account,
so users would correctly see just 1 account in the sidebar. However, given
that 2 SyncWorkers would be running for the same account,
message ids would collide in edgehill.db and this would cause
threads to disappear from the ui as if they were being deleted.

To fix this, we need to do 2 things:

- Upon app start, we remove any duplicate accounts that might have been created due to this bug before starting any sync workers
- If we detect that we are going to create a duplicate account upon auth success, we delete the old account first. This will effectively cause sync to restart for the account.

Test Plan:
Verified problem: Checked out a commit before 008cb4c, authed an account, checked out master, re-authed account, verified that duplicate accounts are created.

Then test 2 scenarios:
- With duplicate accounts present, checked out this commit, verified that duplicate account would be removed and sync would function normally.
- With an account authed on a build before 008cb4c, checked out this commit, re-authed the account, and verified that duplicate account wouldn't be created

Reviewers: mark, halla, khamidou, spang

Reviewed By: spang

Subscribers: tomasz

Differential Revision: https://phab.nylas.com/D4425
Summary:
Which was arguably already a code smell being inside the
DatabaseStore

Test Plan: manual

Reviewers: mark, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4430
Summary:
This commit addressess issue T8135, which prevents the app from
starting.

This was happening because when 3111c16 landed, we added database
access from the main (backend) electron process to be able to read the
identity now stored in the database: nylas@3111c166#diff-1efa26fa0ae1603366b2c0033d971028R44
However, we omitted to add any error handling, so if the database failed to open
due to a database malformed error (which it does:
https://sentry.io/nylas/nylas-mail/?query=is%3Aunresolved+release%3A2.0.14+malformed&statsPeriod=14d), the app will just fail to start,
given that this happens during the initialization of the main process.
Additionally, the fact that we had no error handling increased the error
reports for malformed errors given that we would never handle them, so
every-time we opened the app we would report the same error

This commit adds the same error handling we have in the DatabaseStore
and moves the code around so it's available both in the main and
renderer processes.

After this commit, if the database fails to open during main process
initialization, due to malformed errors or others, we will correctly
inform the user that the database is corrupted, rebuild it, and restart
the app.

Test Plan:
manually throw errors during setup, verify that we handle them
correctly

Reviewers: mark, spang, evan, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D4431
Summary: see title

Test Plan: manual

Reviewers: halla, mark, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4432
Summary:
Given that this error is unrecoverable it shouldn't be retryable.
Additionally, this commit improves error handling for this error in a
few ways:

- Don't delete all databases, just edgehill.db which is the one we know is corrupted. (Use `handleDatabaseUnrecoverableError` instead of `Actions.resetEmailCache`)
- Message the user about the database being reset so the app doesn't restart without notice, and make sure that this message is only displayed once by moving it to the main process

Test Plan: manual

Reviewers: mark, spang, evan, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D4433
Summary:
We weren't passing benchmarkMode through in the default window options,
so the worker window (or any other spawned window) would think it wasn't in
benchmark mode.

Test Plan:
Run locally, verify that the worker window thinks it's in
benchmark mode

Reviewers: halla, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4427
…t files

Summary:
Previously we stored message bodies uncompressed inside two different
databases. This was bad for a few reasons:

* Duplicate data in multiple places is an obvious waste of disk space
* Uncompressed data also made the disk footprint bigger than it might
  otherwise be
* Storing these large message bodies in the database made the db file
  larger than it otherwise could have been, increasing the size of
  tables and slowing down queries.

This diff adds support for storing message bodies outside of the
database in compressed flat files. It changes the use of the body column
in the K2 database and the MessageBody table in the edgehill database to
contain a blob of JSON that contains a path to the file on disk. We
use the new format in an incremental fashion without having to perform an
actual database migration by first checking if the body matches our expected
JSON format and treating it appropriately if it doesn't. Both databases refer
to the same file on disk, thus deduplicating the messages bodies. We also
transparently support gzipping the message bodies stored on disk when we
read from/write to the files. The real world space savings depends on the
compressibility of the messages, but we've seen up to ~60% improvement in disk
space usage for certain inboxes. Typical savings are closer to 20%. Also, by
storing messages as separate files on disk we can potentially integrate with
Spotlight search at some point in the future.

Test Plan:
Run locally, make sure that upgrade to this doesn't hose things,
look at size of DB, read emails

Reviewers: evan, spang, halla, juan

Reviewed By: halla, juan

Differential Revision: https://phab.nylas.com/D4422
Summary: see title

Test Plan: manual

Reviewers: spang, evan, mark, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4445
Summary: see title

Test Plan: manual

Reviewers: evan, spang, mark, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4446
Summary: see title

Test Plan: manual

Reviewers: mark

Differential Revision: https://phab.nylas.com/D4447
Also added `(new v2)` to indicate a new version has just been released, which hopefully improves the functionality a lot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants