Very basic implementation of undo/redo where redo data is built in JS

packages/client-app/spec/stores/undo-redo-store-spec.es6

@@ -1,20 +1,16 @@
 import {Actions, Task, UndoRedoStore} from 'nylas-exports'
 
 class Undoable extends Task {
-  canBeUndone() {
-    return true
-  }
-
-  createIdenticalTask() {
-    const t = new Undoable()
-    t.id = this.id
-    return t
+  constructor() {
+    super();
+    this.canBeUndone = true;
   }
 }
 
 class PermanentTask extends Task {
-  canBeUndone() {
-    return false
+  constructor() {
+    super();
+    this.canBeUndone = false;
   }
 }
 
@@ -73,7 +69,7 @@ describe("UndoRedoStore", function undoRedoStoreSpec() {
   it("doesn't refresh redo if our task is itself a redo task", () => {
     UndoRedoStore._redo = [[this.t1, this.t2], [this.t3]]
     const tr = new Undoable()
-    tr.isRedoTask = true
+    tr.source = 'redo'
     Actions.queueTask(tr)
     expect(UndoRedoStore._redo).toEqual([[this.t1, this.t2], [this.t3]])
     expect(UndoRedoStore._undo).toEqual([[tr]])
@@ -112,8 +108,8 @@ describe("UndoRedoStore", function undoRedoStoreSpec() {
     UndoRedoStore._undo = [[this.t3], [this.t1, this.t2]]
     UndoRedoStore.undo()
     UndoRedoStore.redo()
-    expect(Actions.queueTasks.calls[0].args[0][0].isRedoTask).toBe(true)
-    expect(Actions.queueTasks.calls[0].args[0][1].isRedoTask).toBe(true)
+    expect(Actions.queueTasks.calls[0].args[0][0].source).toEqual('redo')
+    expect(Actions.queueTasks.calls[0].args[0][1].source).toEqual('redo')
   });
 
   it("correctly follows the undo redo sequence of events", () => {

packages/client-app/spec/tasks/change-mail-task-spec.coffee

@@ -80,18 +80,18 @@ xdescribe "ChangeMailTask", ->
       expect(clone.threads).toEqual([@threadA.id, @threadB.id])
 
   describe "createUndoTask", ->
-    it "should return a task initialized with _isUndoTask and _restoreValues", ->
+    it "should return a task initialized with isUndo and _restoreValues", ->
       task = new ChangeMailTask()
       task.messages = [@threadAMesage1, @threadAMesage2]
       task._restoreValues = {'A': 'bla'}
       undo = task.createUndoTask()
       expect(undo.messages).toEqual([@threadAMesage1.id, @threadAMesage2.id])
       expect(undo._restoreValues).toBe(task._restoreValues)
-      expect(undo._isUndoTask).toBe(true)
+      expect(undo.isUndo).toBe(true)
 
     it "should throw if you try to make an undo task of an undo task", ->
       task = new ChangeMailTask()
-      task._isUndoTask = true
+      task.isUndo = true
       expect( -> task.createUndoTask()).toThrow()
 
     it "should throw if _restoreValues are not availble", ->

packages/client-app/spec/tasks/syncback-model-task-spec.es6

@@ -199,11 +199,11 @@ xdescribe('SyncbackModelTask', function syncbackModelTask() {
 
   describe("undo/redo", () => {
     it("cant be undone", () => {
-      expect(this.task.canBeUndone()).toBe(false)
+      expect(this.task.canBeUndone).toBe(false)
     });
 
     it("isn't an undo task", () => {
-      expect(this.task.isUndo()).toBe(false)
+      expect(this.task.isUndo).toBe(false)
     });
   });
 });

packages/client-app/src/flux/attributes/attribute-collection.es6

@@ -32,7 +32,7 @@ Section: Database
 export default class AttributeCollection extends Attribute {
   constructor({modelKey, jsonKey, itemClass, joinOnField, joinQueryableBy, queryable}) {
     super({modelKey, jsonKey, queryable});
-    this.ItemClass = this.itemClass = itemClass;
+    this.itemClass = itemClass;
     this.joinOnField = joinOnField;
     this.joinQueryableBy = joinQueryableBy || [];
   }
@@ -47,22 +47,24 @@ export default class AttributeCollection extends Attribute {
     }
 
     return vals.map((val) => {
-      if (this.ItemClass && !(val instanceof this.ItemClass)) {
-        throw new Error(`AttributeCollection::toJSON: Value \`${val}\` in ${this.modelKey} is not an ${this.ItemClass.name}`);
+      if (this.itemClass && !(val instanceof this.itemClass)) {
+        throw new Error(`AttributeCollection::toJSON: Value \`${val}\` in ${this.modelKey} is not an ${this.itemClass.name}`);
       }
       return (val.toJSON !== undefined) ? val.toJSON() : val;
     });
   }
 
   fromJSON(json) {
+    const Klass = this.itemClass;
+
     if (!json || !(json instanceof Array)) {
       return [];
     }
     return json.map((objJSON) => {
-      if (!objJSON || !this.ItemClass || objJSON instanceof this.ItemClass) {
+      if (!objJSON || !Klass || objJSON instanceof Klass) {
         return objJSON;
       }
-      return new this.ItemClass(objJSON);
+      return new Klass(objJSON);
     });
   }
 

packages/client-app/src/flux/attributes/attribute-object.es6

@@ -8,7 +8,7 @@ Section: Database
 export default class AttributeObject extends Attribute {
   constructor({modelKey, jsonKey, itemClass, queryable}) {
     super({modelKey, jsonKey, queryable});
-    this.ItemClass = itemClass;
+    this.itemClass = itemClass;
   }
 
   toJSON(val) {
@@ -16,11 +16,12 @@ export default class AttributeObject extends Attribute {
   }
 
   fromJSON(val) {
-    if (!val || (this.ItemClass && val instanceof this.ItemClass)) {
+    const Klass = this.itemClass;
+    if (!val || (Klass && val instanceof Klass)) {
       return val;
     }
-    if (this.ItemClass) {
-      return new this.ItemClass(val);
+    if (Klass) {
+      return new Klass(val);
     }
     if (val.__cls) {
       return Utils.convertToModel(val);

packages/client-app/src/flux/stores/undo-redo-store.es6

@@ -1,8 +1,8 @@
-import _ from 'underscore';
 import NylasStore from 'nylas-store';
-
 import Actions from '../actions';
 
+const TASK_SOURCE_REDO = 'redo';
+
 class UndoRedoStore extends NylasStore {
 
   constructor() {
@@ -19,16 +19,15 @@ class UndoRedoStore extends NylasStore {
     NylasEnv.commands.add(document.body, {'core:redo': this.redo });
   }
 
-  _onQueue = (taskArg) => {
-    if (!taskArg) { return; }
-    let tasks = taskArg;
-    if (!(tasks instanceof Array)) { tasks = [tasks]; }
-    if (tasks.length <= 0) { return; }
-    const undoable = _.every(tasks, t => t.canBeUndone());
-    const isRedoTask = _.every(tasks, t => t.isRedoTask);
+  _onQueue = (taskOrTasks) => {
+    const tasks = taskOrTasks instanceof Array ? taskOrTasks : [taskOrTasks];
+    if (tasks.length === 0) { return; }
 
-    if (undoable) {
-      if (!isRedoTask) { this._redo = []; }
+    const isUndoableAndNotUndo = tasks.every(t => t.canBeUndone && !t.isUndo);
+    const isRedo = tasks.every(t => t.source === TASK_SOURCE_REDO);
+
+    if (isUndoableAndNotUndo) {
+      if (!isRedo) { this._redo = []; }
       this._undo.push(tasks);
       this._mostRecentTasks = tasks;
       this.trigger();
@@ -43,12 +42,12 @@ class UndoRedoStore extends NylasStore {
     this.trigger();
 
     for (const task of topTasks) {
-      Actions.queueTask(task.createUndoTask());
+      Actions.queueTask(task.createUndoTask())
     }
 
     const redoTasks = topTasks.map((t) => {
       const redoTask = t.createIdenticalTask();
-      redoTask.isRedoTask = true;
+      redoTask.source = TASK_SOURCE_REDO;
       return redoTask;
     });
     this._redo.push(redoTasks);

packages/client-app/src/flux/tasks/change-folder-task.es6

@@ -16,6 +16,10 @@ import Folder from '../models/folder';
 export default class ChangeFolderTask extends ChangeMailTask {
 
   static attributes = Object.assign({}, ChangeMailTask.attributes, {
+    previousFolder: Attributes.Object({
+      modelKey: 'folder',
+      itemClass: Folder,
+    }),
     folder: Attributes.Object({
       modelKey: 'folder',
       itemClass: Folder,
@@ -23,7 +27,17 @@ export default class ChangeFolderTask extends ChangeMailTask {
   });
 
   constructor(data = {}) {
+    if (data.threads) {
+      data.threads = data.threads.filter(t => t.folder.id !== data.folder.id);
+    }
+    if (data.messages) {
+      data.messages = data.messages.filter(m => m.folder.id !== data.folder.id);
+    }
+
     super(data);
+
+    // grab the folder we'll revert to in case of undo/redo
+    this.previousFolder = this.previousFolder || [].concat(data.threads, data.messages).pop().folder;
   }
 
   label() {
@@ -65,4 +79,12 @@ export default class ChangeFolderTask extends ChangeMailTask {
   _isArchive() {
     return this.folder.name === "archive" || this.folder.name === "all"
   }
+
+  createUndoTask() {
+    const task = super.createUndoTask();
+    const {folder, previousFolder} = task;
+    task.folder = previousFolder;
+    task.previousFolder = folder;
+    return task;
+  }
 }

packages/client-app/src/flux/tasks/change-labels-task.es6

@@ -15,16 +15,19 @@ export default class ChangeLabelsTask extends ChangeMailTask {
   static attributes = Object.assign({}, ChangeMailTask.attributes, {
     labelsToAdd: Attributes.Collection({
       modelKey: 'labelsToAdd',
-      ItemClass: Label,
+      itemClass: Label,
     }),
     labelsToRemove: Attributes.Collection({
       modelKey: 'labelsToRemove',
-      ItemClass: Label,
+      itemClass: Label,
     }),
   });
 
-  constructor(options = {}) {
-    super(options);
+  constructor(data = {}) {
+    if (data.messages) {
+      throw new Error("ChangeLabelsTask: Changing individual message labels is unsupported");
+    }
+    super(data);
   }
 
   label() {
@@ -83,4 +86,12 @@ export default class ChangeLabelsTask extends ChangeMailTask {
     }
     super.validate();
   }
+
+  createUndoTask() {
+    const task = super.createUndoTask();
+    const {labelsToAdd, labelsToRemove} = task;
+    task.labelsToAdd = labelsToRemove;
+    task.labelsToRemove = labelsToAdd;
+    return task;
+  }
 }

packages/client-app/src/flux/tasks/change-mail-task.es6

@@ -9,19 +9,10 @@ Subclasses implement {ChangeMailTask::changesToModel} and
 {ChangeMailTask::requestBodyForModel} to define the specific transforms
 they provide, and override {ChangeMailTask::performLocal} to perform
 additional consistency checks.
-
-ChangeMailTask aims to be fast and efficient. It does not write changes to
-the database or make API requests for models that are unmodified by
-{ChangeMailTask::changesToModel}
-
-ChangeMailTask stores the previous values of all models it changes into
-this._restoreValues and handles undo/redo. When undoing, it restores previous
-values and calls {ChangeMailTask::requestBodyForModel} to make undo API
-requests. It does not call {ChangeMailTask::changesToModel}.
 */
 export default class ChangeMailTask extends Task {
 
-  static attributes = Object.assign({}, ChangeMailTask.attributes, {
+  static attributes = Object.assign({}, Task.attributes, {
     taskDescription: Attributes.String({
       modelKey: 'taskDescription',
     }),
@@ -31,47 +22,36 @@ export default class ChangeMailTask extends Task {
     messageIds: Attributes.Collection({
       modelKey: 'messageIds',
     }),
+    canBeUndone: Attributes.Boolean({
+      modelKey: 'canBeUndone',
+    }),
+    isUndo: Attributes.Boolean({
+      modelKey: 'isUndo',
+    }),
   });
 
-  constructor({threads, thread, messages, message, ...rest} = {}) {
+  constructor({threads = [], messages = [], ...rest} = {}) {
     super(rest);
 
-    const t = threads || [];
-    if (thread) {
-      t.push(thread);
-    }
-    const m = messages || [];
-    if (message) {
-      m.push(message);
-    }
-
     // we actually only keep a small bit of data now
-    this.threadIds = t.map(i => i.id);
-    this.messageIds = m.map(i => i.id);
-    this.accountId = (t[0] || m[0] || {}).accountId;
+    this.threadIds = this.threadIds || threads.map(i => i.id);
+    this.messageIds = this.messageIds || messages.map(i => i.id);
+    this.accountId = this.accountId || (threads[0] || messages[0] || {}).accountId;
+
+    if (this.canBeUndone === undefined) {
+      this.canBeUndone = true;
+    }
   }
 
   // Task lifecycle
 
-  canBeUndone() {
-    return true;
-  }
-
-  isUndo() {
-    return this._isUndoTask === true;
-  }
-
   createUndoTask() {
-    if (this._isUndoTask) {
+    if (this.isUndo) {
       throw new Error("ChangeMailTask::createUndoTask Cannot create an undo task from an undo task.");
     }
-    if (!this._restoreValues) {
-      throw new Error("ChangeMailTask::createUndoTask Cannot undo a task which has not finished performLocal yet.");
-    }
 
     const task = this.createIdenticalTask();
-    task._restoreValues = this._restoreValues;
-    task._isUndoTask = true;
+    task.isUndo = true;
     return task;
   }
 

packages/client-app/src/flux/tasks/change-starred-task.es6

@@ -14,9 +14,14 @@ export default class ChangeStarredTask extends ChangeMailTask {
     }),
   });
 
-  constructor({starred, ...rest} = {}) {
-    super(rest);
-    this.starred = starred;
+  constructor(data = {}) {
+    if (data.threads) {
+      data.threads = data.threads.filter(t => t.starred !== data.starred);
+    }
+    if (data.messages) {
+      data.messages = data.messages.filter(m => m.starred !== data.starred);
+    }
+    super(data);
   }
 
   label() {
@@ -27,7 +32,7 @@ export default class ChangeStarredTask extends ChangeMailTask {
     const count = this.threadIds.length;
     const type = count > 1 ? "threads" : "thread";
 
-    if (this._isUndoTask) {
+    if (this.isUndo) {
       return `Undoing changes to ${count} ${type}`
     }
 
@@ -45,6 +50,12 @@ export default class ChangeStarredTask extends ChangeMailTask {
     super.validate();
   }
 
+  createUndoTask() {
+    const task = super.createUndoTask();
+    task.starred = !this.starred;
+    return task;
+  }
+
   recordUserEvent() {
     if (this.source === "Mail Rules") {
       return
@@ -54,7 +65,7 @@ export default class ChangeStarredTask extends ChangeMailTask {
       source: this.source,
       numThreads: this.threadIds.length,
       description: this.description(),
-      isUndo: this._isUndoTask,
+      isUndo: this.isUndo,
     })
   }
 }

packages/client-app/src/flux/tasks/change-unread-task.es6

@@ -9,18 +9,19 @@ import ChangeMailTask from './change-mail-task';
 export default class ChangeUnreadTask extends ChangeMailTask {
 
   static attributes = Object.assign({}, ChangeMailTask.attributes, {
-    starred: Attributes.Boolean({
+    unread: Attributes.Boolean({
       modelKey: 'unread',
     }),
-    _canBeUndone: Attributes.Boolean({
-      modelKey: '_canBeUndone',
-    }),
   });
 
-  constructor({unread, canBeUndone, ...rest} = {}) {
-    super(rest);
-    this.unread = unread;
-    this._canBeUndone = canBeUndone;
+  constructor(data = {}) {
+    if (data.threads) {
+      data.threads = data.threads.filter(t => t.unread !== data.unread);
+    }
+    if (data.messages) {
+      data.messages = data.messages.filter(m => m.unread !== data.unread);
+    }
+    super(data);
   }
 
   label() {
@@ -31,7 +32,7 @@ export default class ChangeUnreadTask extends ChangeMailTask {
     const count = this.threadIds.length;
     const type = count > 1 ? 'threads' : 'thread';
 
-    if (this._isUndoTask) {
+    if (this.isUndo) {
       return `Undoing changes to ${count} ${type}`;
     }
 
@@ -42,10 +43,9 @@ export default class ChangeUnreadTask extends ChangeMailTask {
     return `Marked as ${newState}`;
   }
 
-  canBeUndone() {
-    if (this._canBeUndone == null) {
-      return super.canBeUndone()
-    }
-    return this._canBeUndone
+  createUndoTask() {
+    const task = super.createUndoTask();
+    task.unread = !this.unread;
+    return task;
   }
 }

packages/client-app/src/flux/tasks/destroy-model-task.es6

@@ -25,7 +25,4 @@ export default class DestroyModelTask extends Task {
 
   validate() {
   }
-
-  canBeUndone() { return false }
-
 }

packages/client-app/src/flux/tasks/syncback-model-task.es6

@@ -21,8 +21,4 @@ export default class SyncbackModelTask extends Task {
     this.validateRequiredFields(["clientId"])
     return Promise.resolve()
   }
-
-  canBeUndone() { return false }
-
-  isUndo() { return false }
 }

packages/client-app/src/flux/tasks/task.es6

@@ -6,15 +6,14 @@ import {generateTempId} from '../models/utils';
 import {PermanentErrorCodes} from '../nylas-api';
 import {APIError} from '../errors';
 
-const TaskStatus = {
-  Retry: "RETRY",
-  Success: "SUCCESS",
-  Continue: "CONTINUE",
-  Failed: "FAILED",
+const Status = {
+  Local: "local",
+  Remote: "remote",
+  Complete: "complete",
 };
 
 export default class Task extends Model {
-  static Status = TaskStatus;
+  static Status = Status;
   static SubclassesUseModelTable = Task;
 
   static attributes = Object.assign({}, Model.attributes, {
@@ -44,8 +43,8 @@ export default class Task extends Model {
   // On construction, all Tasks instances are given a unique `id`.
   constructor(data) {
     super(data);
+    this.status = this.status || Status.Local;
     this.id = this.id || generateTempId();
-    this.accountId = null;
   }
 
   // Public: Override to raise exceptions if your task is missing required
@@ -54,23 +53,6 @@ export default class Task extends Model {
 
   }
 
-  // Public: It's up to you to determine how you want to indicate whether
-  // or not you have an instance of an "Undo Task". We commonly use a
-  // simple instance variable boolean flag.
-  //
-  // Returns `true` (is an Undo Task) or `false` (is not an Undo Task)
-  isUndo() {
-    return false;
-  }
-
-  // Public: Determines whether or not this task can be undone via the
-  // {UndoRedoStore}
-  //
-  // Returns `true` (can be undone) or `false` (can't be undone)
-  canBeUndone() {
-    return false;
-  }
-
   // Public: Return from `createIdenticalTask` and set a flag so your
   // `performLocal` and `performRemote` methods know that this is an undo
   // task.
@@ -82,7 +64,9 @@ export default class Task extends Model {
   createIdenticalTask() {
     const json = this.toJSON();
     delete json.status;
-    return (new this.constructor()).fromJSON(json);
+    delete json.version;
+    delete json.id;
+    return new this.constructor(json);
   }
 
   // Public: code to run if (someone tries to dequeue your task while it is)
@@ -107,19 +91,6 @@ export default class Task extends Model {
     return 1;
   }
 
-  // Private: Allows for serialization of tasks
-  toJSON() {
-    return Object.assign(super.toJSON(), this);
-  }
-
-  // Private: Allows for deserialization of tasks
-  fromJSON(json) {
-    for (const key of Object.keys(json)) {
-      this[key] = json[key];
-    }
-    return this;
-  }
-
   onError(err) {
     // noop
   }