diff --git a/src/flux/syncback-task-api-request.es6 b/src/flux/syncback-task-api-request.es6 new file mode 100644 index 000000000..473d24d1b --- /dev/null +++ b/src/flux/syncback-task-api-request.es6 @@ -0,0 +1,48 @@ +import Actions from './actions' +import NylasAPIRequest from './nylas-api-request' + + +/** + * This API request is meant to be used for requests that create a + * SyncbackRequest inside K2. + * When the initial http request succeeds, this means that the task was created, + * but we cant tell if the task actually succeeded or failed until some time in + * the future when its processed inside K2's sync loop. + * + * A SyncbackTaskAPIRequest will only resolve until the underlying K2 syncback + * request has actually succeeded, or reject when it fails, by listening to + * deltas for ProviderSyncbackRequests + */ +class SyncbackTaskAPIRequest { + + constructor({api, options}) { + this._request = new NylasAPIRequest({api, options}) + this._onSyncbackRequestCreated = options.onSyncbackRequestCreated || (() => {}) + } + + run() { + return new Promise(async (resolve, reject) => { + try { + const syncbackRequest = await this._request.run() + await this._onSyncbackRequestCreated(syncbackRequest) + const syncbackRequestId = syncbackRequest.id + const unsubscribe = Actions.didReceiveSyncbackRequestDeltas + .listen((deltas) => { + const failed = deltas.failed.find(d => d.objectId === syncbackRequestId) + const succeeded = deltas.succeeded.find(d => d.objectId === syncbackRequestId) + if (failed) { + unsubscribe() + reject(failed.attributes.error) + } else if (succeeded) { + unsubscribe() + resolve(syncbackRequest) + } + }) + } catch (err) { + reject(err) + } + }) + } +} + +export default SyncbackTaskAPIRequest diff --git a/src/flux/tasks/change-labels-task.es6 b/src/flux/tasks/change-labels-task.es6 index d6f217597..7738d5240 100644 --- a/src/flux/tasks/change-labels-task.es6 +++ b/src/flux/tasks/change-labels-task.es6 @@ -93,7 +93,7 @@ export default class ChangeLabelsTask extends ChangeMailTask { // In Gmail all threads /must/ belong to either All Mail, Trash and Spam, and // they are mutually exclusive, so we need to make sure that any add/remove // label operation still guarantees that constraint - _ensureAndUpdateLabels = (account, existingLabelsToAdd, existingLabelsToRemove = {}) => { + _ensureAndUpdateLabels(account, existingLabelsToAdd, existingLabelsToRemove = {}) { const labelsToAdd = existingLabelsToAdd; let labelsToRemove = existingLabelsToRemove; diff --git a/src/flux/tasks/change-mail-task.es6 b/src/flux/tasks/change-mail-task.es6 index 36e4cf33c..24f369049 100644 --- a/src/flux/tasks/change-mail-task.es6 +++ b/src/flux/tasks/change-mail-task.es6 @@ -4,7 +4,7 @@ import Task from './task'; import Thread from '../models/thread'; import Message from '../models/message'; import NylasAPI from '../nylas-api'; -import NylasAPIRequest from '../nylas-api-request'; +import SyncbackTaskAPIRequest from '../syncback-task-api-request'; import DatabaseStore from '../stores/database-store'; import {APIError} from '../errors'; @@ -226,7 +226,8 @@ export default class ChangeMailTask extends Task { } performRemote() { - return this._performRequests(this.objectClass(), this.objectArray()).then(() => { + return this._performRequests(this.objectClass(), this.objectArray()) + .then(() => { this._ensureLocksRemoved(); return Promise.resolve(Task.Status.Success); }) @@ -251,7 +252,7 @@ export default class ChangeMailTask extends Task { const endpoint = (klass === Thread) ? 'threads' : 'messages'; - return new NylasAPIRequest({ + return new SyncbackTaskAPIRequest({ api: NylasAPI, options: { path: `/${endpoint}/${model.id}`, @@ -264,7 +265,8 @@ export default class ChangeMailTask extends Task { return body; }, }, - }).run() + }) + .run() .catch((err) => { if (err instanceof APIError && err.statusCode === 404) { return Promise.resolve(); diff --git a/src/flux/tasks/syncback-category-task.es6 b/src/flux/tasks/syncback-category-task.es6 index ab69e502c..4ed2d98ce 100644 --- a/src/flux/tasks/syncback-category-task.es6 +++ b/src/flux/tasks/syncback-category-task.es6 @@ -1,12 +1,10 @@ import DatabaseStore from '../stores/database-store'; import AccountStore from '../stores/account-store'; import Task from './task'; -import Actions from '../actions'; import NylasAPI from '../nylas-api'; -import NylasAPIRequest from '../nylas-api-request'; +import SyncbackTaskAPIRequest from '../syncback-task-api-request'; import {APIError} from '../errors'; - export default class SyncbackCategoryTask extends Task { constructor({category, displayName} = {}) { @@ -48,69 +46,50 @@ export default class SyncbackCategoryTask extends Task { const account = AccountStore.accountForId(accountId); const collection = account.usesLabels() ? "labels" : "folders"; - const isUpdate = serverId != null - const method = isUpdate ? "PUT" : "POST"; + const method = serverId ? "PUT" : "POST"; const path = serverId ? `/${collection}/${serverId}` : `/${collection}`; - return new Promise(async (resolve) => { - try { - const json = await new NylasAPIRequest({ - api: NylasAPI, - options: { - path, - method, - accountId, - body: { - display_name: displayName, - }, - // returnsModel must be false because we want to update the - // existing model rather than returning a new model. - returnsModel: false, - }, - }).run() - // TODO sorry - // Previously, when we sent the request to create a folder or label to our api, - // we would immediately get back a serverId because it was created optimistically - // in the back end— given that K2 is strictly non-optimistic, we won’t have a serverId - // until some undetermined time in the future, and we need to somehow reference - // the object that /was/ optimistically created in N1 to update the ui when - // we do get the server id. - // Pre-assigning the id from N1 is the most simple solution to get thing working - // correctly right now, but we’ll need to revisit this in the future for - // other types of objects (drafts, contacts, events), and revisit how we - // will manage optimistic updates in N1 when we merge the 2 codebases - // with K2 (given that K2 was designed to be non-optimisitc). - this.category.serverId = json.props.objectId || null - if (!this.category.serverId) { - throw new Error('SyncbackRequest for creating category did not return a serverId!') - } - await DatabaseStore.inTransaction((t) => - t.persistModel(this.category) - ); - const unsubscribe = Actions.didReceiveSyncbackRequestDeltas.listen(async (deltas) => { - const failed = deltas.failed.find(d => d.attributes.props.objectId === this.category.serverId) - const succeeded = deltas.succeeded.find(d => d.attributes.props.objectId === this.category.serverId) - if (failed) { - unsubscribe() - this._isReverting = true - await this.performLocal() - resolve(Task.Status.Failed); - } else if (succeeded) { - unsubscribe() - resolve(Task.Status.Success) + return new SyncbackTaskAPIRequest({ + api: NylasAPI, + options: { + path, + method, + accountId, + body: { + display_name: displayName, + }, + // returnsModel must be false because we want to update the + // existing model rather than returning a new model. + returnsModel: false, + onSyncbackRequestCreated: (json) => { + // TODO + // Previously, when we sent the request to create a folder or label to our api, + // we would immediately get back a serverId because it was created optimistically + // in the back end. Given that K2 is strictly non-optimistic, we won’t have a serverId + // until some undetermined time in the future. + // + // For now, the simplest solution is for the created syncback request to + // include the id of the category that will eventually be created + // (given that ids are generated deterministically). + // We’ll need to revisit this in the future for other types of objects + // (drafts, contacts, events), and revisit how we will manage optimistic + // updates in N1 when we merge the 2 codebases with K2 + this.category.serverId = json.props.objectId || null + if (!this.category.serverId) { + throw new Error('SyncbackRequest for creating category did not return a serverId!') } - }) - } catch (err) { - if (err instanceof APIError) { - if (!NylasAPI.PermanentErrorCodes.includes(err.statusCode)) { - resolve(Task.Status.Retry); - } else { - this._isReverting = true - await this.performLocal() - resolve(Task.Status.Failed); - } - } + return DatabaseStore.inTransaction(t => t.persistModel(this.category)) + }, + }, + }) + .run() + .thenReturn(Task.Status.Success) + .catch(APIError, (err) => { + if (!NylasAPI.PermanentErrorCodes.includes(err.statusCode)) { + return Promise.resolve(Task.Status.Retry); } + this._isReverting = true; + return this.performLocal().thenReturn(Task.Status.Failed); }) } } diff --git a/src/pro b/src/pro index 38a60b30a..d951de161 160000 --- a/src/pro +++ b/src/pro @@ -1 +1 @@ -Subproject commit 38a60b30a0c20a2874007f2208785763b9a7df6a +Subproject commit d951de1611a1c336dd85290f1b6ec3125df04d1e