Mailspring/spec/tasks/send-draft-spec.coffee

431 lines
18 KiB
CoffeeScript
Raw Normal View History

_ = require 'underscore'
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-18 03:46:05 +08:00
{APIError,
Actions,
DatabaseStore,
DatabaseTransaction,
Message,
2016-01-27 11:12:46 +08:00
Contact,
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-18 03:46:05 +08:00
Task,
TaskQueue,
SendDraftTask,
SyncbackDraftTask,
NylasAPI,
SoundRegistry} = require 'nylas-exports'
DBt = DatabaseTransaction.prototype
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
describe "SendDraftTask", ->
fix(tasks): don't continue if dependent task fails Summary: Fixes T4291 If I made a final edit to a pre-existing draft and sent, we'd queue a `SyncbackDraftTask` before a `SendDraftTask`. This is important because since we have a valid draft `server_id`, the `SendDraftTask` will send by server_id, not by POSTing the whole body. If the `SyncbackDraftTask` fails, then we had a very serious issue whereby the `SendDraftTask` would keep on sending. Unfortunately the server never got the latest changes and sent the wrong version of the draft. This incorrect version would show up later when the `/send` endpoint returned the message that got actually sent. The solution was to make any queued `SendDraftTask` fail if a dependent `SyncbackDraftTask` failed. This meant we needed to make the requirements for `shouldWaitForTask` stricter, and block if tasks failed. Unfortunatley there was no infrastructure in place to do this. The first change was to change `shouldWaitForTask` to `isDependentTask`. If we're going to fail when a dependent task fails, I wanted the method name to reflect this. Now, if a dependent task fails, we recursively check the dependency tree (and check for cycles) and `dequeue` anything that needed that to succeed. I chose `dequeue` as the default action because it seemed as though all current uses of `shouldWaitForTask` really should bail if their dependencies fail. It's possible you don't want your task dequeued in this dependency case. You can return the special `Task.DO_NOT_DEQUEUE_ME` constant from the `onDependentTaskError` method. When a task gets dequeued because of the reason above, the `onDependentTaskError` callback gets fired. This gives tasks like the `SendDraftTask` a chance to notify the user that it bailed. Not all tasks need to notify. The next big issue was a better way to determine if a task truely errored to the point that we need to dequeue dependencies. In the Developer Status area we were showing tasks that had errored as "Green" because we caught the error and resolved with `Task.Status.Finished`. This used to be fine since nothing life-or-death cared if a task errored or not. Now that it might cause abortions down the line, we needed a more robust method then this. For one I changed `Task.Status.Finished` to a variety of finish types including `Task.Status.Success`. The way you "error" out is to `throw` or `Promise.reject` an `Error` object from the `performRemote` method. This allows us to propagate API errors up, and acts as a safety net that can catch any malformed code or unexpected responses. The developer bar now shows a much richer set of statuses instead of a binary one, which was REALLY helpful in debugging this. We also record when a Task got dequeued because of the conditions introduced here. Once all this was working we still had an issue of sending old drafts. If after a `SyncbackDraftTask` failed, now we'd block the send and notify the users as such. However, if we tried to send again, there was a separate issue whereby we wouldn't queue another `SyncbackDraftTask` to update the server with the latest information. Since our changes were persisted to the DB, we thought we had no changes, and therefore didn't need to queue a `SyncbackDraftTask`. The fix to this is to always force the creation of a `SyncbackDraftTask` before send regardless of the state of the `DraftStoreProxy`. Test Plan: new tests. Lots of manual testing Reviewers: bengotow Reviewed By: bengotow Subscribers: mg Maniphest Tasks: T4291 Differential Revision: https://phab.nylas.com/D2156
2015-10-22 01:33:43 +08:00
describe "isDependentTask", ->
it "is not dependent on any pending SyncbackDraftTasks", ->
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
@draftA = new Message
version: '1'
clientId: 'localid-A'
serverId: '1233123AEDF1'
accountId: 'A12ADE'
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
subject: 'New Draft'
draft: true
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
to:
name: 'Dummy'
email: 'dummy@nylas.com'
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
2016-01-27 11:12:46 +08:00
@saveA = new SyncbackDraftTask(@draftA, [])
@sendA = new SendDraftTask(@draftA, [])
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
expect(@sendA.isDependentTask(@saveA)).toBe(false)
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
describe "performLocal", ->
2016-01-27 11:12:46 +08:00
it "throws an error if we we don't pass a draft", ->
badTask = new SendDraftTask()
badTask.performLocal()
.then ->
throw new Error("Shouldn't succeed")
.catch (err) ->
2016-01-27 11:12:46 +08:00
expect(err.message).toBe "SendDraftTask - must be provided a draft."
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
2016-01-27 11:12:46 +08:00
it "throws an error if we we don't pass uploads", ->
2016-01-28 16:48:45 +08:00
message = new Message()
message.uploads = null
badTask = new SendDraftTask(message)
2016-01-27 11:12:46 +08:00
badTask.performLocal()
.then ->
throw new Error("Shouldn't succeed")
.catch (err) ->
expect(err.message).toBe "SendDraftTask - must be provided an array of uploads."
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
describe "performRemote", ->
beforeEach ->
@response =
version: 2
2016-01-27 11:12:46 +08:00
id: '1233123AEDF1'
account_id: TEST_ACCOUNT_ID
from: [new Contact(email: TEST_ACCOUNT_EMAIL)]
subject: 'New Draft'
body: 'hello world'
to:
name: 'Dummy'
email: 'dummy@nylas.com'
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
options.success?(@response)
Promise.resolve(@response)
spyOn(DBt, 'unpersistModel').andReturn Promise.resolve()
spyOn(DBt, 'persistModel').andReturn Promise.resolve()
2015-10-02 10:24:06 +08:00
spyOn(SoundRegistry, "playSound")
spyOn(Actions, "postNotification")
spyOn(Actions, "sendDraftSuccess")
2016-01-27 11:12:46 +08:00
sharedTests = =>
it "makes a send request with the correct data", ->
2016-01-27 11:12:46 +08:00
waitsForPromise => @task._sendAndCreateMessage().then =>
expect(NylasAPI.makeRequest).toHaveBeenCalled()
reqArgs = NylasAPI.makeRequest.calls[0].args[0]
2016-01-27 11:12:46 +08:00
expect(reqArgs.accountId).toBe TEST_ACCOUNT_ID
expect(reqArgs.body).toEqual @draft.toJSON()
it "should pass returnsModel:false", ->
2016-01-27 11:12:46 +08:00
waitsForPromise => @task._sendAndCreateMessage().then ->
expect(NylasAPI.makeRequest.calls.length).toBe(1)
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.returnsModel).toBe(false)
it "should always send the draft body in the request body (joined attribute check)", ->
waitsForPromise =>
2016-01-27 11:12:46 +08:00
@task._sendAndCreateMessage().then =>
expect(NylasAPI.makeRequest.calls.length).toBe(1)
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.body.body).toBe('hello world')
it "should start an API request to /send", -> waitsForPromise =>
2016-01-27 11:12:46 +08:00
@task._sendAndCreateMessage().then =>
expect(NylasAPI.makeRequest.calls.length).toBe(1)
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.path).toBe("/send")
fix(drafts): Various improvements and fixes to drafts, draft state management Summary: This diff contains a few major changes: 1. Scribe is no longer used for the text editor. It's just a plain contenteditable region. The toolbar items (bold, italic, underline) still work. Scribe was causing React inconcistency issues in the following scenario: - View thread with draft, edit draft - Move to another thread - Move back to thread with draft - Move to another thread. Notice that one or more messages from thread with draft are still there. There may be a way to fix this, but I tried for hours and there are Github Issues open on it's repository asking for React compatibility, so it may be fixed soon. For now contenteditable is working great. 2. Action.saveDraft() is no longer debounced in the DraftStore. Instead, firing that action causes the save to happen immediately, and the DraftStoreProxy has a new "DraftChangeSet" class which is responsbile for batching saves as the user interacts with the ComposerView. There are a couple big wins here: - In the future, we may want to be able to call Action.saveDraft() in other situations and it should behave like a normal action. We may also want to expose the DraftStoreProxy as an easy way of backing interactive draft UI. - Previously, when you added a contact to To/CC/BCC, this happened: <input> -> Action.saveDraft -> (delay!!) -> Database -> DraftStore -> DraftStoreProxy -> View Updates Increasing the delay to something reasonable like 200msec meant there was 200msec of lag before you saw the new view state. To fix this, I created a new class called DraftChangeSet which is responsible for accumulating changes as they're made and firing Action.saveDraft. "Adding" a change to the change set also causes the Draft provided by the DraftStoreProxy to change immediately (the changes are a temporary layer on top of the database object). This means no delay while changes are being applied. There's a better explanation in the source! This diff includes a few minor fixes as well: 1. Draft.state is gone—use Message.object = draft instead 2. String model attributes should never be null 3. Pre-send checks that can cancel draft send 4. Put the entire curl history and task queue into feedback reports 5. Cache localIds for extra speed 6. Move us up to latest React Test Plan: No new tests - once we lock down this new design I'll write tests for the DraftChangeSet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1125
2015-02-04 08:24:31 +08:00
expect(options.method).toBe('POST')
it "should write the saved message to the database with the same client ID", ->
waitsForPromise =>
2016-01-27 11:12:46 +08:00
@task._sendAndCreateMessage().then =>
expect(DBt.persistModel).toHaveBeenCalled()
2016-01-27 11:12:46 +08:00
expect(DBt.persistModel.mostRecentCall.args[0].clientId).toEqual(@draft.clientId)
expect(DBt.persistModel.mostRecentCall.args[0].serverId).toEqual(@response.id)
expect(DBt.persistModel.mostRecentCall.args[0].draft).toEqual(false)
it "should notify the draft was sent", ->
waitsForPromise =>
@task.performRemote().then =>
args = Actions.sendDraftSuccess.calls[0].args[0]
expect(args.message instanceof Message).toBe(true)
expect(args.messageClientId).toBe(@draft.clientId)
it "should play a sound", ->
spyOn(NylasEnv.config, "get").andReturn true
waitsForPromise => @task.performRemote().then ->
expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds")
expect(SoundRegistry.playSound).toHaveBeenCalledWith("send")
it "shouldn't play a sound if the config is disabled", ->
spyOn(NylasEnv.config, "get").andReturn false
waitsForPromise => @task.performRemote().then ->
expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds")
expect(SoundRegistry.playSound).not.toHaveBeenCalled()
describe "when there are errors", ->
beforeEach ->
2016-01-27 11:12:46 +08:00
spyOn(Actions, 'draftSendingFailed')
jasmine.unspy(NylasAPI, "makeRequest")
it "notifies of a permanent error of misc error types", ->
## DB error
thrownError = null
spyOn(NylasEnv, "reportError")
jasmine.unspy(DBt, "persistModel")
spyOn(DBt, "persistModel").andCallFake =>
thrownError = new Error('db error')
throw thrownError
waitsForPromise =>
@task.performRemote().then (status) =>
expect(status[0]).toBe Task.Status.Failed
expect(status[1]).toBe thrownError
2016-01-27 11:12:46 +08:00
expect(Actions.draftSendingFailed).toHaveBeenCalled()
feat(error): improve error reporting. Now `NylasEnv.reportError` Summary: The goal is to let us see what plugins are throwing errors on Sentry. We are using a Sentry `tag` to identify and group plugins and their errors. Along the way, I cleaned up the error catching and reporting system. There was a lot of duplicate error logic (that wasn't always right) and some legacy Atom error handling. Now, if you catch an error that we should report (like when handling extensions), call `NylasEnv.reportError`. This used to be called `emitError` but I changed it to `reportError` to be consistent with the ErrorReporter and be a bit more indicative of what it does. In the production version, the `ErrorLogger` will forward the request to the `nylas-private-error-reporter` which will report to Sentry. The `reportError` function also now inspects the stack to determine which plugin(s) it came from. These are passed along to Sentry. I also cleaned up the `console.log` and `console.error` code. We were logging errors multiple times making the console confusing to read. Worse is that we were logging the `error` object, which would print not the stack of the actual error, but rather the stack of where the console.error was logged from. Printing `error.stack` instead shows much more accurate stack traces. See changes in the Edgehill repo here: https://github.com/nylas/edgehill/commit/8c4a86eb7ee1a06249a9ae35397e2084a09ad1dc Test Plan: Manual Reviewers: juan, bengotow Reviewed By: bengotow Differential Revision: https://phab.nylas.com/D2509
2016-02-04 07:06:52 +08:00
expect(NylasEnv.reportError).toHaveBeenCalled()
2016-01-27 11:12:46 +08:00
it "retries the task if 'Invalid message public id'", ->
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
if options.body.reply_to_message_id
err = new APIError(body: "Invalid message public id")
Promise.reject(err)
else
options.success?(@response)
Promise.resolve(@response)
@draft.replyToMessageId = "reply-123"
@draft.threadId = "thread-123"
waitsForPromise => @task._sendAndCreateMessage(@draft).then =>
expect(NylasAPI.makeRequest).toHaveBeenCalled()
expect(NylasAPI.makeRequest.callCount).toEqual 2
req1 = NylasAPI.makeRequest.calls[0].args[0]
req2 = NylasAPI.makeRequest.calls[1].args[0]
expect(req1.body.reply_to_message_id).toBe "reply-123"
expect(req1.body.thread_id).toBe "thread-123"
expect(req2.body.reply_to_message_id).toBe null
expect(req2.body.thread_id).toBe "thread-123"
it "retries the task if 'Invalid message public id'", ->
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
if options.body.reply_to_message_id
err = new APIError(body: "Invalid thread")
Promise.reject(err)
else
options.success?(@response)
Promise.resolve(@response)
@draft.replyToMessageId = "reply-123"
@draft.threadId = "thread-123"
waitsForPromise => @task._sendAndCreateMessage(@draft).then =>
expect(NylasAPI.makeRequest).toHaveBeenCalled()
expect(NylasAPI.makeRequest.callCount).toEqual 2
req1 = NylasAPI.makeRequest.calls[0].args[0]
req2 = NylasAPI.makeRequest.calls[1].args[0]
expect(req1.body.reply_to_message_id).toBe "reply-123"
expect(req1.body.thread_id).toBe "thread-123"
expect(req2.body.reply_to_message_id).toBe null
expect(req2.body.thread_id).toBe null
it "notifies of a permanent error on 500 errors", ->
thrownError = new APIError(statusCode: 500, body: "err")
spyOn(NylasEnv, "reportError")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise => @task.performRemote().then (status) =>
expect(status[0]).toBe Task.Status.Failed
expect(status[1]).toBe thrownError
2016-01-27 11:12:46 +08:00
expect(Actions.draftSendingFailed).toHaveBeenCalled()
it "notifies us and users of a permanent error on 400 errors", ->
thrownError = new APIError(statusCode: 400, body: "err")
spyOn(NylasEnv, "reportError")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise => @task.performRemote().then (status) =>
expect(status[0]).toBe Task.Status.Failed
expect(status[1]).toBe thrownError
2016-01-27 11:12:46 +08:00
expect(Actions.draftSendingFailed).toHaveBeenCalled()
it "presents helpful error messages for 402 errors (security blocked)", ->
thrownError = new APIError(statusCode: 402, body: {
"message": "Message content rejected for security reasons",
"server_error": "552 : 5.7.0 This message was blocked because its content presents a potential\n5.7.0 security issue. Please visit\n5.7.0 https://support.google.com/mail/answer/6590 to review our message\n5.7.0 content and attachment content guidelines. fk9sm21147314pad.9 - gsmtp",
"type": "api_error"
})
expectedMessage =
"""
Sorry, this message could not be sent because it was rejected by your mail provider. (Message content rejected for security reasons)
552 : 5.7.0 This message was blocked because its content presents a potential
5.7.0 security issue. Please visit
5.7.0 https://support.google.com/mail/answer/6590 to review our message
5.7.0 content and attachment content guidelines. fk9sm21147314pad.9 - gsmtp
"""
spyOn(NylasEnv, "reportError")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise => @task.performRemote().then (status) =>
expect(status[0]).toBe Task.Status.Failed
expect(status[1]).toBe thrownError
expect(Actions.draftSendingFailed).toHaveBeenCalled()
expect(Actions.draftSendingFailed.calls[0].args[0].errorMessage).toEqual(expectedMessage)
it "presents helpful error messages for 402 errors (recipient failed)", ->
thrownError = new APIError(statusCode: 402, body: {
"message": "Sending to at least one recipient failed.",
"server_error": "<<Don't know what this looks like >>",
"type": "api_error"
})
expectedMessage = "This message could not be delivered to at least one recipient. (Note: other recipients may have received this message - you should check Sent Mail before re-sending this message.)"
spyOn(NylasEnv, "reportError")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise => @task.performRemote().then (status) =>
expect(status[0]).toBe Task.Status.Failed
expect(status[1]).toBe thrownError
expect(Actions.draftSendingFailed).toHaveBeenCalled()
expect(Actions.draftSendingFailed.calls[0].args[0].errorMessage).toEqual(expectedMessage)
it "retries on timeouts", ->
thrownError = new APIError(statusCode: NylasAPI.TimeoutErrorCode, body: "err")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise => @task.performRemote().then (status) =>
expect(status).toBe Task.Status.Retry
describe "checking the promise chain halts on errors", ->
beforeEach ->
spyOn(NylasEnv, 'reportError')
2016-01-27 11:12:46 +08:00
spyOn(@task,"_sendAndCreateMessage").andCallThrough()
spyOn(@task,"_deleteRemoteDraft").andCallThrough()
2016-01-27 11:12:46 +08:00
spyOn(@task,"_onSuccess").andCallThrough()
spyOn(@task,"_onError").andCallThrough()
@expectBlockedChain = =>
2016-01-27 11:12:46 +08:00
expect(@task._sendAndCreateMessage).toHaveBeenCalled()
expect(@task._deleteRemoteDraft).not.toHaveBeenCalled()
2016-01-27 11:12:46 +08:00
expect(@task._onSuccess).not.toHaveBeenCalled()
expect(@task._onError).toHaveBeenCalled()
it "halts on 500s", ->
thrownError = new APIError(statusCode: 500, body: "err")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise =>
@task.performRemote().then (status) =>
@expectBlockedChain()
it "halts on 400s", ->
thrownError = new APIError(statusCode: 400, body: "err")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise =>
@task.performRemote().then (status) =>
@expectBlockedChain()
it "halts and retries on not permanent error codes", ->
thrownError = new APIError(statusCode: 409, body: "err")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise =>
@task.performRemote().then (status) =>
@expectBlockedChain()
it "halts on other errors", ->
thrownError = new Error("oh no")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
Promise.reject(thrownError)
waitsForPromise =>
@task.performRemote().then (status) =>
@expectBlockedChain()
it "doesn't halt on success", ->
# Don't spy reportError to make sure to fail the test on unexpected
# errors
jasmine.unspy(NylasEnv, 'reportError')
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
options.success?(@response)
Promise.resolve(@response)
waitsForPromise =>
@task.performRemote().then (status) =>
expect(status).toBe Task.Status.Success
expect(@task._sendAndCreateMessage).toHaveBeenCalled()
expect(@task._deleteRemoteDraft).toHaveBeenCalled()
expect(@task._onSuccess).toHaveBeenCalled()
expect(@task._onError).not.toHaveBeenCalled()
describe "with a new draft", ->
beforeEach ->
@draft = new Message
version: 1
2016-01-27 11:12:46 +08:00
clientId: 'client-id'
accountId: TEST_ACCOUNT_ID
from: [new Contact(email: TEST_ACCOUNT_EMAIL)]
subject: 'New Draft'
draft: true
body: 'hello world'
2016-01-28 16:48:45 +08:00
uploads: []
@task = new SendDraftTask(@draft)
@calledBody = "ERROR: The body wasn't included!"
spyOn(DatabaseStore, "findBy").andCallFake =>
Promise.resolve(@draft)
2016-01-27 11:12:46 +08:00
sharedTests()
it "can complete a full performRemote", ->
waitsForPromise =>
@task.performRemote().then (status) ->
expect(status).toBe Task.Status.Success
it "shouldn't attempt to delete a draft", ->
waitsForPromise =>
@task._deleteRemoteDraft(@draft).then =>
expect(NylasAPI.makeRequest).not.toHaveBeenCalled()
it "should locally convert the draft to a message on send", ->
waitsForPromise =>
@task.performRemote().then =>
expect(DBt.persistModel).toHaveBeenCalled()
model = DBt.persistModel.calls[0].args[0]
expect(model.clientId).toBe @draft.clientId
expect(model.serverId).toBe @response.id
expect(model.draft).toBe false
describe "with an existing persisted draft", ->
beforeEach ->
@draft = new Message
version: 1
2016-01-27 11:12:46 +08:00
clientId: 'client-id'
serverId: 'server-123'
accountId: TEST_ACCOUNT_ID
from: [new Contact(email: TEST_ACCOUNT_EMAIL)]
subject: 'New Draft'
draft: true
body: 'hello world'
to:
name: 'Dummy'
email: 'dummy@nylas.com'
2016-01-28 16:48:45 +08:00
uploads: []
@task = new SendDraftTask(@draft)
@calledBody = "ERROR: The body wasn't included!"
spyOn(DatabaseStore, "findBy").andCallFake =>
Promise.resolve(@draft)
2016-01-27 11:12:46 +08:00
sharedTests()
it "can complete a full performRemote", -> waitsForPromise =>
2016-01-27 11:12:46 +08:00
@task.performLocal()
@task.performRemote().then (status) ->
expect(status).toBe Task.Status.Success
it "should make a request to delete a draft", ->
2016-01-27 11:12:46 +08:00
@task.performLocal()
waitsForPromise =>
@task._deleteRemoteDraft(@draft).then =>
expect(NylasAPI.makeRequest).toHaveBeenCalled()
expect(NylasAPI.makeRequest.callCount).toBe 1
req = NylasAPI.makeRequest.calls[0].args[0]
expect(req.path).toBe "/drafts/#{@draft.serverId}"
expect(req.accountId).toBe TEST_ACCOUNT_ID
expect(req.method).toBe "DELETE"
expect(req.returnsModel).toBe false
2016-01-27 11:12:46 +08:00
it "should continue if the request fails", ->
jasmine.unspy(NylasAPI, "makeRequest")
spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
2016-01-27 11:12:46 +08:00
Promise.reject(new APIError(body: "Boo", statusCode: 500))
@task.performLocal()
waitsForPromise =>
@task._deleteRemoteDraft(@draft)
.then =>
expect(NylasAPI.makeRequest).toHaveBeenCalled()
expect(NylasAPI.makeRequest.callCount).toBe 1
.catch =>
throw new Error("Shouldn't fail the promise")
it "should locally convert the existing draft to a message on send", ->
2016-01-27 11:12:46 +08:00
expect(@draft.clientId).toBe @draft.clientId
expect(@draft.serverId).toBe "server-123"
2016-01-27 11:12:46 +08:00
@task.performLocal()
waitsForPromise => @task.performRemote().then =>
expect(DBt.persistModel).toHaveBeenCalled()
model = DBt.persistModel.calls[0].args[0]
2016-01-27 11:12:46 +08:00
expect(model.clientId).toBe @draft.clientId
expect(model.serverId).toBe @response.id
expect(model.draft).toBe false