Mailspring/spec-nylas/tasks/syncback-draft-spec.coffee
Ben Gotow 45bb16561f feat(offline-mode, undo-redo): Tasks handle network errors better and retry, undo/redo based on tasks
Summary:
This diff does a couple things:

- Undo redo with a new undo/redo store that maintains it's own queue of undo/redo tasks. This queue is separate from the TaskQueue because not all tasks should be considered for undo history! Right now just the AddRemoveTagsTask is undoable.

- NylasAPI.makeRequest now returns a promise which resolves with the result or rejects with an error. For things that still need them, there's still `success` and `error` callbacks. I also added `started:(req) ->` which allows you to get the underlying request.

- Aborting a NylasAPI request now makes it call it's error callback / promise reject.

- You can now run code after perform local has completed using this syntax:

```
    task = new AddRemoveTagsTask(focused, ['archive'], ['inbox'])
    task.waitForPerformLocal().then ->
      Actions.setFocus(collection: 'thread', item: nextFocus)
      Actions.setCursorPosition(collection: 'thread', item: nextKeyboard)
    Actions.queueTask(task)
```

- In specs, you can now use `advanceClock` to get through a Promise.then/catch/finally. Turns out it was using something low level and not using setTimeout(0).

- The TaskQueue uses promises better and defers a lot of the complexity around queueState for performLocal/performRemote to a task subclass called APITask. APITask implements "perform" and breaks it into "performLocal" and "performRemote".

- All tasks either resolve or reject. They're always removed from the queue, unless they resolve with Task.Status.Retry, which means they internally did a .catch (err) => Promise.resolve(Task.Status.Retry) and they want to be run again later.

- API tasks retry until they succeed or receive a NylasAPI.PermanentErrorCode (400,404,500), in which case they revert and finish.

- The AddRemoveTags Task can now take more than one thread! This is super cool because you can undo/redo a bulk action and also because we'll probably have a bulk tag modification API endpoint soon.

Getting undo / redo working revealed that the thread versioning system we built isn't working because the server was incrementing things by more than 1 at a time. Now we count the number of unresolved "optimistic" changes we've made to a given model, and only accept the server's version of it once the number of optimistic changes is back at zero.

Known Issues:

- AddRemoveTagsTasks aren't dependent on each other, so if you (undo/redo x lots) and then come back online, all the tasks try to add / remove all the tags at the same time. To fix this we can either allow the tasks to be merged together into a minimal set or make them block on each other.

- When Offline, you still get errors in the console for GET requests. Need to catch these and display an offline status bar.

- The metadata tasks haven't been updated yet to the new API. Wanted to get it reviewed first!

Test Plan: All the tests still pass!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1694
2015-07-07 13:38:53 -04:00

124 lines
4.7 KiB
CoffeeScript

_ = require 'underscore'
{generateTempId, isTempId} = require '../../src/flux/models/utils'
NylasAPI = require '../../src/flux/nylas-api'
Task = require '../../src/flux/tasks/task'
Actions = require '../../src/flux/actions'
Message = require '../../src/flux/models/message'
Contact = require '../../src/flux/models/contact'
{APIError} = require '../../src/flux/errors'
DatabaseStore = require '../../src/flux/stores/database-store'
TaskQueue = require '../../src/flux/stores/task-queue'
SyncbackDraftTask = require '../../src/flux/tasks/syncback-draft'
inboxError =
message: "No draft with public id bvn4aydxuyqlbmzowh4wraysg",
type: "invalid_request_error"
testError = (opts) ->
new APIError
error:null
response:{statusCode: 404}
body:inboxError
requestOptions: opts
testData =
to: new Contact(name: "Ben Gotow", email: "ben@nylas.com")
from: new Contact(name: "Evan Morikawa", email: "evan@nylas.com")
date: new Date
draft: true
subject: "Test"
namespaceId: "abc123"
localDraft = new Message _.extend {}, testData, {id: "local-id"}
remoteDraft = new Message _.extend {}, testData, {id: "remoteid1234"}
describe "SyncbackDraftTask", ->
beforeEach ->
spyOn(DatabaseStore, "findByLocalId").andCallFake (klass, localId) ->
if localId is "localDraftId" then Promise.resolve(localDraft)
else if localId is "remoteDraftId" then Promise.resolve(remoteDraft)
else if localId is "missingDraftId" then Promise.resolve()
spyOn(DatabaseStore, "persistModel").andCallFake ->
Promise.resolve()
spyOn(DatabaseStore, "swapModel").andCallFake ->
Promise.resolve()
describe "performRemote", ->
beforeEach ->
spyOn(NylasAPI, 'makeRequest').andCallFake (opts) ->
Promise.resolve(remoteDraft.toJSON())
it "does nothing if no draft can be found in the db", ->
task = new SyncbackDraftTask("missingDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(NylasAPI.makeRequest).not.toHaveBeenCalled()
it "should start an API request with the Message JSON", ->
task = new SyncbackDraftTask("localDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(NylasAPI.makeRequest).toHaveBeenCalled()
reqBody = NylasAPI.makeRequest.mostRecentCall.args[0].body
expect(reqBody.subject).toEqual testData.subject
it "should do a PUT when the draft has already been saved", ->
task = new SyncbackDraftTask("remoteDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(NylasAPI.makeRequest).toHaveBeenCalled()
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.path).toBe("/n/abc123/drafts/remoteid1234")
expect(options.method).toBe('PUT')
it "should do a POST when the draft is unsaved", ->
task = new SyncbackDraftTask("localDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(NylasAPI.makeRequest).toHaveBeenCalled()
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.path).toBe("/n/abc123/drafts")
expect(options.method).toBe('POST')
it "should pass returnsModel:false so that the draft can be manually removed/added to the database, accounting for its ID change", ->
task = new SyncbackDraftTask("localDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(NylasAPI.makeRequest).toHaveBeenCalled()
options = NylasAPI.makeRequest.mostRecentCall.args[0]
expect(options.returnsModel).toBe(false)
it "should swap the ids if we got a new one from the DB", ->
task = new SyncbackDraftTask("localDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(DatabaseStore.swapModel).toHaveBeenCalled()
expect(DatabaseStore.persistModel).not.toHaveBeenCalled()
it "should not swap the ids if we're using a persisted one", ->
task = new SyncbackDraftTask("remoteDraftId")
waitsForPromise =>
task.performRemote().then ->
expect(DatabaseStore.swapModel).not.toHaveBeenCalled()
expect(DatabaseStore.persistModel).toHaveBeenCalled()
describe "When the api throws a 404 error", ->
beforeEach ->
spyOn(NylasAPI, "makeRequest").andCallFake (opts) ->
Promise.reject(testError(opts))
it "resets the id", ->
task = new SyncbackDraftTask("remoteDraftId")
taskStatus = null
task.performRemote().then (status) => taskStatus = status
waitsFor ->
DatabaseStore.swapModel.calls.length > 0
runs ->
newDraft = DatabaseStore.swapModel.mostRecentCall.args[0].newModel
expect(isTempId(newDraft.id)).toBe true
expect(taskStatus).toBe(Task.Status.Retry)