Commit graph

6 commits

Author SHA1 Message Date
Ben Gotow
041817c589 patch(save): Only save drafts when necessary, avoid sync-engine issues
Summary:
Previously, we have saved drafts back to the user's provider through the sync engine. There are a handful of very serious edge case issues we're working to solve that are creating a bad user experience. (#933, #1175, #1504, #1237)

For now, we're going to change the behavior of N1 to mitagate these issues.

- If you create a draft in N1, we will not sync it to other mail clients while you're working on it.
- If you enable send later, we'll start syncing the draft to the server as before.
- If you created the draft in another client, we'll sync the draft to the server as before.

Fix specs

Test Plan: Run specs

Reviewers: evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D2706
2016-03-10 11:03:38 -08:00
Evan Morikawa
74e21bce16 feat(tasks): add Create, Update, Destroy tasks plus spec & lint fixes
Summary:
1. **Generic CUD Tasks**: There is now a generic `CreateModelTask`,
`UpdateModelTask`, and `DestroyModelTask`. These can either be used as-is
or trivially overridden to easily update simple objects. Hopefully all of
the boilerplate rollback, error handling, and undo logic won't have to be
re-duplicated on every task. There are also tests for these tasks. We use
them to perform mutating actions on `Metadata` objects.

1. **Failing on Promise Rejects**: Turns out that if a Promise rejected
due to an error or `Promise.reject` we were ignoring it and letting tests
pass. Now, tests will Fail if any unhandled promise rejects. This
uncovered a variety of errors throughout the test suite that had to be
fixed. The most significant one was during the `theme-manager` tests when
all packages (and their stores with async DB requests) was loaded. Long
after the `theme-manager` specs finished, those DB requests were
(somtimes) silently failing.

1. **Globally stub `DatabaseStore._query`**: All tests shouldn't actually
make queries on the database. Furthremore, the `inTransaction` block
doesn't resolve at all unless `_query` is stubbed. Instead of manually
remembering to do this in every test that touches the DB, it's now mocked
in `spec_helper`. This broke a handful of tests that needed to be manually
fixed.

1. **ESLint Fixes**: Some minor fixes to the linter config to prevent
yelling about minor ES6 things and ensuring we have the correct parser.

Test Plan: new tests

Reviewers: bengotow, juan, drew

Differential Revision: https://phab.nylas.com/D2419

Remove cloudState and N1-Send-Later
2016-01-15 15:16:21 -05:00
Evan Morikawa
a267ccd0bc fix(draft): New Send Draft logic
Summary:
This is a WIP for the new send draft logic.

I'll add tests then update the diff

Test Plan: todo

Reviewers: bengotow, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D2341
2015-12-21 11:50:52 -08:00
Ben Gotow
a14a5212ac feat(transactions): Explicit (and faster) database transactions
Summary:
Until now, we've been hiding transactions beneath the surface. When you call persistModel, you're implicitly creating a transaction.
You could explicitly create them with `atomically`..., but there were several critical problems that are fixed in this diff:

- Calling persistModel / unpersistModel within a transaction could cause the DatabaseStore to trigger. This could result in other parts of the app making queries /during/
  the transaction, potentially before the COMMIT occurred and saved the changes. The new, explicit inTransaction syntax holds all changes until after COMMIT and then triggers.

- Calling atomically and then calling persistModel inside that resulted in us having to check whether a transaction was present and was gross.

- Many parts of the code ran extensive logic inside a promise chained within `atomically`:

  BAD:

```
  DatabaseStore.atomically =>
   DatabaseStore.persistModel(draft) =>
     GoMakeANetworkRequestThatReturnsAPromise
```

OVERWHELMINGLY BETTER:

```
  DatabaseStore.inTransaction (t) =>
     t.persistModel(draft)
  .then =>
    GoMakeANetworkRequestThatReturnsAPromise
```

Having explicit transactions also puts us on equal footing with Sequelize and other ORMs. Note that you /have/ to call DatabaseStore.inTransaction (t) =>. There is no other way to access the methods that let you alter the database. :-)

Other changes:
- This diff removes Message.labels and the Message-Labels table. We weren't using Message-level labels anywhere, and the table could grow very large.
- This diff changes the page size during initial sync from 250 => 200 in an effort to make transactions a bit faster.

Test Plan: Run tests!

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D2353
2015-12-17 11:46:05 -08:00
Evan Morikawa
d0212c3dd9 fix(drafts): only syncback every 30 seconds instead of 5 seconds
This will help prevent API errors from causing multiple drafts to appear
2015-10-28 19:51:46 -04:00
Evan Morikawa
137e09eb51 refactor(spec) move spec-nylas to spec 2015-10-01 21:39:44 -07:00
Renamed from spec-nylas/stores/draft-store-proxy-spec.coffee (Browse further)