From 0efdec5fd54b31e9bfaf457f01f30d78444bbd0a Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Tue, 19 May 2015 15:59:37 -0700 Subject: [PATCH] fix(initial-sync): Make initial sync more robust, show progress, retry on failure Summary: Rename ActivityBar => DeveloperBar Expose sync workers and make them observable New activity sidebar that replaces momentary notifications Updated specs Test Plan: Run new specs! Reviewers: evan Reviewed By: evan Maniphest Tasks: T1131 Differential Revision: https://phab.nylas.com/D1521 --- docs/PackageOverview.md | 2 +- .../.gitignore | 0 .../lib/developer-bar-curl-item.cjsx} | 6 +- .../lib/developer-bar-long-poll-item.cjsx} | 6 +- .../lib/developer-bar-store.coffee} | 4 +- .../lib/developer-bar-task.cjsx} | 6 +- .../lib/developer-bar.cjsx} | 44 ++--- .../lib/main.cjsx | 6 +- .../package.json | 4 +- .../stylesheets/developer-bar.less} | 2 +- .../notifications/lib/activity-sidebar.cjsx | 122 +++++++++++++ internal_packages/notifications/lib/main.cjsx | 8 +- .../notifications/lib/notifications.cjsx | 35 ---- .../stylesheets/notifications.less | 79 ++++++-- .../thread-list/lib/thread-list-store.coffee | 1 - spec-nylas/nylas-sync-worker-spec.coffee | 170 +++++++++++++----- spec-nylas/tasks/send-draft-spec.coffee | 4 - spec/spec-helper.coffee | 2 + src/flux/nylas-api.coffee | 16 +- src/flux/nylas-sync-worker.coffee | 76 +++++--- src/flux/stores/task-queue.coffee | 3 + src/flux/tasks/add-remove-tags.coffee | 3 +- src/flux/tasks/file-upload-task.coffee | 3 +- src/flux/tasks/send-draft.coffee | 5 +- 24 files changed, 436 insertions(+), 171 deletions(-) rename internal_packages/{inbox-activity-bar => developer-bar}/.gitignore (100%) rename internal_packages/{inbox-activity-bar/lib/activity-bar-curl-item.cjsx => developer-bar/lib/developer-bar-curl-item.cjsx} (86%) rename internal_packages/{inbox-activity-bar/lib/activity-bar-long-poll-item.cjsx => developer-bar/lib/developer-bar-long-poll-item.cjsx} (87%) rename internal_packages/{inbox-activity-bar/lib/activity-bar-store.coffee => developer-bar/lib/developer-bar-store.coffee} (96%) rename internal_packages/{inbox-activity-bar/lib/activity-bar-task.cjsx => developer-bar/lib/developer-bar-task.cjsx} (93%) rename internal_packages/{inbox-activity-bar/lib/activity-bar.cjsx => developer-bar/lib/developer-bar.cjsx} (84%) rename internal_packages/{inbox-activity-bar => developer-bar}/lib/main.cjsx (61%) rename internal_packages/{inbox-activity-bar => developer-bar}/package.json (61%) rename internal_packages/{inbox-activity-bar/stylesheets/activity-bar.less => developer-bar/stylesheets/developer-bar.less} (99%) create mode 100644 internal_packages/notifications/lib/activity-sidebar.cjsx delete mode 100644 internal_packages/notifications/lib/notifications.cjsx diff --git a/docs/PackageOverview.md b/docs/PackageOverview.md index 26a040008..0ba7de8ec 100644 --- a/docs/PackageOverview.md +++ b/docs/PackageOverview.md @@ -68,7 +68,7 @@ module.exports = # watching any files, holding external resources, providing commands or # subscribing to events, release them here. deactivate: -> - ComponentRegistry.unregister('TranslateButton') + ComponentRegistry.unregister(TranslateButton) ``` diff --git a/internal_packages/inbox-activity-bar/.gitignore b/internal_packages/developer-bar/.gitignore similarity index 100% rename from internal_packages/inbox-activity-bar/.gitignore rename to internal_packages/developer-bar/.gitignore diff --git a/internal_packages/inbox-activity-bar/lib/activity-bar-curl-item.cjsx b/internal_packages/developer-bar/lib/developer-bar-curl-item.cjsx similarity index 86% rename from internal_packages/inbox-activity-bar/lib/activity-bar-curl-item.cjsx rename to internal_packages/developer-bar/lib/developer-bar-curl-item.cjsx index c91f8fa64..f1c90d369 100644 --- a/internal_packages/inbox-activity-bar/lib/activity-bar-curl-item.cjsx +++ b/internal_packages/developer-bar/lib/developer-bar-curl-item.cjsx @@ -1,7 +1,7 @@ React = require 'react/addons' -class ActivityBarCurlItem extends React.Component - @displayName: 'ActivityBarCurlItem' +class DeveloperBarCurlItem extends React.Component + @displayName: 'DeveloperBarCurlItem' render: =>
@@ -29,4 +29,4 @@ class ActivityBarCurlItem extends React.Component shell.openItem(curlFile) -module.exports = ActivityBarCurlItem +module.exports = DeveloperBarCurlItem diff --git a/internal_packages/inbox-activity-bar/lib/activity-bar-long-poll-item.cjsx b/internal_packages/developer-bar/lib/developer-bar-long-poll-item.cjsx similarity index 87% rename from internal_packages/inbox-activity-bar/lib/activity-bar-long-poll-item.cjsx rename to internal_packages/developer-bar/lib/developer-bar-long-poll-item.cjsx index 3bed72ad3..1f3047ebe 100644 --- a/internal_packages/inbox-activity-bar/lib/activity-bar-long-poll-item.cjsx +++ b/internal_packages/developer-bar/lib/developer-bar-long-poll-item.cjsx @@ -2,8 +2,8 @@ React = require 'react/addons' moment = require 'moment' {Utils} = require 'nylas-exports' -class ActivityBarLongPollItem extends React.Component - @displayName: 'ActivityBarLongPollItem' +class DeveloperBarLongPollItem extends React.Component + @displayName: 'DeveloperBarLongPollItem' constructor: (@props) -> @state = expanded: false @@ -33,4 +33,4 @@ class ActivityBarLongPollItem extends React.Component -module.exports = ActivityBarLongPollItem +module.exports = DeveloperBarLongPollItem diff --git a/internal_packages/inbox-activity-bar/lib/activity-bar-store.coffee b/internal_packages/developer-bar/lib/developer-bar-store.coffee similarity index 96% rename from internal_packages/inbox-activity-bar/lib/activity-bar-store.coffee rename to internal_packages/developer-bar/lib/developer-bar-store.coffee index 40f950c91..ad2ee5a07 100644 --- a/internal_packages/inbox-activity-bar/lib/activity-bar-store.coffee +++ b/internal_packages/developer-bar/lib/developer-bar-store.coffee @@ -5,7 +5,7 @@ _ = require 'underscore-plus' curlItemId = 0 -ActivityBarStore = Reflux.createStore +DeveloperBarStore = Reflux.createStore init: -> @_setStoreDefaults() @_registerListeners() @@ -86,4 +86,4 @@ ActivityBarStore = Reflux.createStore @triggerThrottled(@) -module.exports = ActivityBarStore +module.exports = DeveloperBarStore diff --git a/internal_packages/inbox-activity-bar/lib/activity-bar-task.cjsx b/internal_packages/developer-bar/lib/developer-bar-task.cjsx similarity index 93% rename from internal_packages/inbox-activity-bar/lib/activity-bar-task.cjsx rename to internal_packages/developer-bar/lib/developer-bar-task.cjsx index 26a08c693..fb184801c 100644 --- a/internal_packages/inbox-activity-bar/lib/activity-bar-task.cjsx +++ b/internal_packages/developer-bar/lib/developer-bar-task.cjsx @@ -3,8 +3,8 @@ classNames = require 'classnames' _ = require 'underscore-plus' {Utils} = require 'nylas-exports' -class ActivityBarTask extends React.Component - @displayName: 'ActivityBarTask' +class DeveloperBarTask extends React.Component + @displayName: 'DeveloperBarTask' constructor: (@props) -> @state = expanded: false @@ -52,4 +52,4 @@ class ActivityBarTask extends React.Component "task-success": qs.performedLocal and qs.performedRemote -module.exports = ActivityBarTask +module.exports = DeveloperBarTask diff --git a/internal_packages/inbox-activity-bar/lib/activity-bar.cjsx b/internal_packages/developer-bar/lib/developer-bar.cjsx similarity index 84% rename from internal_packages/inbox-activity-bar/lib/activity-bar.cjsx rename to internal_packages/developer-bar/lib/developer-bar.cjsx index 1c0c59553..f416f9bd7 100644 --- a/internal_packages/inbox-activity-bar/lib/activity-bar.cjsx +++ b/internal_packages/developer-bar/lib/developer-bar.cjsx @@ -9,28 +9,28 @@ React = require 'react/addons' Message} = require 'nylas-exports' {ResizableRegion} = require 'nylas-component-kit' -ActivityBarStore = require './activity-bar-store' -ActivityBarTask = require './activity-bar-task' -ActivityBarCurlItem = require './activity-bar-curl-item' -ActivityBarLongPollItem = require './activity-bar-long-poll-item' +DeveloperBarStore = require './developer-bar-store' +DeveloperBarTask = require './developer-bar-task' +DeveloperBarCurlItem = require './developer-bar-curl-item' +DeveloperBarLongPollItem = require './developer-bar-long-poll-item' -ActivityBarClosedHeight = 30 +DeveloperBarClosedHeight = 30 -class ActivityBar extends React.Component - @displayName: "ActivityBar" +class DeveloperBar extends React.Component + @displayName: "DeveloperBar" @containerRequired: false constructor: (@props) -> @state = _.extend @_getStateFromStores(), - height: ActivityBarClosedHeight + height: DeveloperBarClosedHeight section: 'curl' filter: '' componentDidMount: => ipc.on 'report-issue', => @_onFeedback() @taskQueueUnsubscribe = TaskQueue.listen @_onChange - @activityStoreUnsubscribe = ActivityBarStore.listen @_onChange + @activityStoreUnsubscribe = DeveloperBarStore.listen @_onChange componentWillUnmount: => @taskQueueUnsubscribe() if @taskQueueUnsubscribe @@ -39,9 +39,9 @@ class ActivityBar extends React.Component render: => return
unless @state.visible -
{@_caret()} @@ -76,7 +76,7 @@ class ActivityBar extends React.Component _caret: => - if @state.height > ActivityBarClosedHeight + if @state.height > DeveloperBarClosedHeight else @@ -90,26 +90,26 @@ class ActivityBar extends React.Component if @state.section == 'curl' itemDivs = @state.curlHistory.filter(matchingFilter).map (item) -> - + expandedDiv =
{itemDivs}
else if @state.section == 'long-polling' itemDivs = @state.longPollHistory.filter(matchingFilter).map (item) -> - + expandedDiv =
{itemDivs}
else if @state.section == 'queue' queue = @state.queue.filter(matchingFilter) queueDivs = for i in [@state.queue.length - 1..0] by -1 task = @state.queue[i] - queueCompleted = @state.completed.filter(matchingFilter) queueCompletedDivs = for i in [@state.completed.length - 1..0] by -1 task = @state.completed[i] - @@ -140,7 +140,7 @@ class ActivityBar extends React.Component _onHide: => @setState - height: ActivityBarClosedHeight + height: DeveloperBarClosedHeight _onShow: => @setState(height: 200) if @state.height < 100 @@ -200,12 +200,12 @@ class ActivityBar extends React.Component Actions.composePopoutDraft(localId) _getStateFromStores: => - visible: ActivityBarStore.visible() + visible: DeveloperBarStore.visible() queue: TaskQueue._queue completed: TaskQueue._completed - curlHistory: ActivityBarStore.curlHistory() - longPollHistory: ActivityBarStore.longPollHistory() - longPollState: ActivityBarStore.longPollState() + curlHistory: DeveloperBarStore.curlHistory() + longPollHistory: DeveloperBarStore.longPollHistory() + longPollState: DeveloperBarStore.longPollState() -module.exports = ActivityBar +module.exports = DeveloperBar diff --git a/internal_packages/inbox-activity-bar/lib/main.cjsx b/internal_packages/developer-bar/lib/main.cjsx similarity index 61% rename from internal_packages/inbox-activity-bar/lib/main.cjsx rename to internal_packages/developer-bar/lib/main.cjsx index c6e09f20a..4e07c06c2 100644 --- a/internal_packages/inbox-activity-bar/lib/main.cjsx +++ b/internal_packages/developer-bar/lib/main.cjsx @@ -1,13 +1,13 @@ React = require 'react' {ComponentRegistry, WorkspaceStore} = require 'nylas-exports' -ActivityBar = require './activity-bar' +DeveloperBar = require './developer-bar' module.exports = item: null activate: (@state={}) -> - ComponentRegistry.register ActivityBar, + ComponentRegistry.register DeveloperBar, location: WorkspaceStore.Sheet.Global.Footer deactivate: -> - ComponentRegistry.unregister ActivityBar + ComponentRegistry.unregister DeveloperBar diff --git a/internal_packages/inbox-activity-bar/package.json b/internal_packages/developer-bar/package.json similarity index 61% rename from internal_packages/inbox-activity-bar/package.json rename to internal_packages/developer-bar/package.json index 47cdc7aa7..ddab3cd7b 100755 --- a/internal_packages/inbox-activity-bar/package.json +++ b/internal_packages/developer-bar/package.json @@ -1,8 +1,8 @@ { - "name": "inbox-activity-bar", + "name": "developer-bar", "version": "0.1.0", "main": "./lib/main", - "description": "Activity bar at the very bottom of the window", + "description": "Developer bar at the very bottom of the window", "license": "Proprietary", "private": true, "engines": { diff --git a/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less b/internal_packages/developer-bar/stylesheets/developer-bar.less similarity index 99% rename from internal_packages/inbox-activity-bar/stylesheets/activity-bar.less rename to internal_packages/developer-bar/stylesheets/developer-bar.less index aa12e7036..13f1423c6 100755 --- a/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less +++ b/internal_packages/developer-bar/stylesheets/developer-bar.less @@ -1,6 +1,6 @@ @import "ui-variables"; -.activity-bar { +.developer-bar { -webkit-font-smoothing: auto; background-color: rgba(80,80,80,1); border-top:1px solid rgba(0,0,0,0.7); diff --git a/internal_packages/notifications/lib/activity-sidebar.cjsx b/internal_packages/notifications/lib/activity-sidebar.cjsx new file mode 100644 index 000000000..b37e9d877 --- /dev/null +++ b/internal_packages/notifications/lib/activity-sidebar.cjsx @@ -0,0 +1,122 @@ +React = require 'react' +_ = require 'underscore-plus' +classNames = require 'classnames' +NotificationStore = require './notifications-store' +{Actions, + TaskQueue, + NamespaceStore, + NylasAPI} = require 'nylas-exports' +{TimeoutTransitionGroup} = require 'nylas-component-kit' + +class ActivitySidebar extends React.Component + @displayName: 'ActivitySidebar' + + @containerRequired: false + + constructor: (@props) -> + @state = @_getStateFromStores() + + componentDidMount: => + @_unlisteners = [] + @_unlisteners.push NamespaceStore.listen @_onNamespacesChanged + @_unlisteners.push TaskQueue.listen @_onDataChanged + @_unlisteners.push NotificationStore.listen @_onDataChanged + @_onNamespacesChanged() + + componentWillUnmount: => + unlisten() for unlisten in @_unlisteners + @_workerUnlisten() if @_workerUnlisten + + render: => + items = [].concat(@_renderSyncActivityItem(), @_renderNotificationActivityItems(), @_renderTaskActivityItems()) + + names = classNames + "sidebar-activity": true + "sidebar-activity-empty": items.length is 0 + "sidebar-activity-error": error? + + + {items} + + + _renderSyncActivityItem: => + count = 0 + fetched = 0 + progress = 0 + incomplete = 0 + error = null + + for model, modelState of @state.sync + incomplete += 1 unless modelState.complete + error ?= modelState.error + if modelState.count + count += modelState.count / 1 + fetched += modelState.fetched / 1 + + progress = (fetched / count) * 100 if count > 0 + + if incomplete is 0 + return [] + else if error +
+
Initial sync encountered an error. Waiting to retry... +
Try Again
+
+
+ else +
+
+
+
+
Syncing mail data...
+
+ + _renderTaskActivityItems: => + summary = {} + + @state.tasks.map (task) -> + label = task.label?() + return unless label + summary[label] ?= 0 + summary[label] += 1 + + _.pairs(summary).map ([label, count]) -> +
+
+ {label} ({count}) +
+
+ + _renderNotificationActivityItems: => + @state.notifications.map (notification) -> +
+
+ {notification.message} +
+
+ + _onNamespacesChanged: => + namespace = NamespaceStore.current() + return unless namespace + @_worker = NylasAPI.workerForNamespace(namespace) + @_workerUnlisten() if @_workerUnlisten + @_workerUnlisten = @_worker.listen(@_onDataChanged, @) + @_onDataChanged() + + _onTryAgain: => + @_worker.resumeFetches() + + _onDataChanged: => + @setState(@_getStateFromStores()) + + _getStateFromStores: => + tasks: TaskQueue.queue() + notifications: NotificationStore.notifications() + sync: @_worker?.state() + + +module.exports = ActivitySidebar diff --git a/internal_packages/notifications/lib/main.cjsx b/internal_packages/notifications/lib/main.cjsx index 8c6473d78..feb4b99fb 100644 --- a/internal_packages/notifications/lib/main.cjsx +++ b/internal_packages/notifications/lib/main.cjsx @@ -1,5 +1,5 @@ React = require "react" -Notifications = require "./notifications" +ActivitySidebar = require "./activity-sidebar" NotificationsStickyBar = require "./notifications-sticky-bar" {ComponentRegistry, WorkspaceStore} = require("nylas-exports") @@ -7,14 +7,14 @@ module.exports = item: null # The DOM item the main React component renders into activate: (@state={}) -> - ComponentRegistry.register Notifications, + ComponentRegistry.register ActivitySidebar, location: WorkspaceStore.Location.RootSidebar ComponentRegistry.register NotificationsStickyBar, location: WorkspaceStore.Sheet.Global.Header deactivate: -> - ComponentRegistry.unregister('NotificationsStickyBar') - ComponentRegistry.unregister('Notifications') + ComponentRegistry.unregister(ActivitySidebar) + ComponentRegistry.unregister(NotificationsStickyBar) serialize: -> @state diff --git a/internal_packages/notifications/lib/notifications.cjsx b/internal_packages/notifications/lib/notifications.cjsx deleted file mode 100644 index 7c20b7613..000000000 --- a/internal_packages/notifications/lib/notifications.cjsx +++ /dev/null @@ -1,35 +0,0 @@ -React = require 'react' -NotificationStore = require './notifications-store' - -class Notifications extends React.Component - @displayName: "Notifications" - - @containerRequired: false - - constructor: (@props) -> - @state = notifications: NotificationStore.notifications() - - componentDidMount: => - @unsubscribeStore = NotificationStore.listen @_onStoreChange - - componentWillUnmount: => - @unsubscribeStore() if @unsubscribeStore - - render: => -
- {@_notificationComponents()} -
- - _notificationComponents: => - @state.notifications.map (notification) -> -
- {notification.message} -
- - _onStoreChange: => - @setState - notifications: NotificationStore.notifications() - - -module.exports = Notifications \ No newline at end of file diff --git a/internal_packages/notifications/stylesheets/notifications.less b/internal_packages/notifications/stylesheets/notifications.less index e20572bb8..a6b5bd488 100644 --- a/internal_packages/notifications/stylesheets/notifications.less +++ b/internal_packages/notifications/stylesheets/notifications.less @@ -1,30 +1,77 @@ @import "ui-variables"; @import "ui-mixins"; -// Notifications Above Threads -.notifications-momentary { +.sidebar-activity { width: 100%; bottom: 0; - position: absolute; + order:2; background: @background-off-primary; - border-top: 1px solid @border-secondary-bg; - box-shadow: @standard-shadow-up; + font-size: @font-size-small; + color: @text-color-subtle; + line-height:@line-height-computed * 0.95; + height:140px; + overflow-y:scroll; + box-shadow:inset 0 1px 0 @border-color-divider; - .notification-info { border-color: @background-color-info; } - .notification-error { - border-color: @background-color-error; - color: @error-color; + .item { + border-bottom:1px solid @border-color-divider; + .inner { + padding: @padding-large-vertical @padding-base-horizontal @padding-large-vertical @padding-base-horizontal; + margin-top:3px; + } + .count { + color: @text-color-very-subtle; + float:right; + } + .btn { + display:block; + text-align:center; + margin-top:4px; + margin-bottom:4px; + font-size: @font-size-small; + } + .progress-track { + display:block; + height:3px; + font-size:0; + .progress { + transition: width 0.4s; + height:3px; + background-color: @background-color-info; + } + } } - .notification-success { border-color: @background-color-success; } - .notification-item { - text-align: center; - border-top-width: 3px; - border-top-style: solid; - padding: @spacing-standard; + transition: height 0.4s; + transition-delay: 2s; + &.sidebar-activity-error { + .progress { + background-color: @error-color; + } } } +.activity-item-enter { + opacity:0; + transition: opacity .125s ease-out; +} + +.activity-item-enter.activity-item-enter-active { + opacity:1; +} + +.activity-item-leave { + opacity:1; + transition: opacity .125s ease-in; + transition-delay: 0.5s; +} + +.activity-item-leave.activity-item-leave-active { + transition-delay: 0.5s; + opacity:0; +} + + .notifications-sticky { width:100%; @@ -71,4 +118,4 @@ margin-right:@padding-base-horizontal; } } -} \ No newline at end of file +} diff --git a/internal_packages/thread-list/lib/thread-list-store.coffee b/internal_packages/thread-list/lib/thread-list-store.coffee index d7235082c..e10786492 100644 --- a/internal_packages/thread-list/lib/thread-list-store.coffee +++ b/internal_packages/thread-list/lib/thread-list-store.coffee @@ -139,7 +139,6 @@ ThreadListStore = Reflux.createStore # Archive the current thread task = new AddRemoveTagsTask(focused, ['archive'], ['inbox']) Actions.queueTask(task) - Actions.postNotification({message: "Archived thread", type: 'success'}) # Remove the current thread from selection @_view.selection.remove(focused) diff --git a/spec-nylas/nylas-sync-worker-spec.coffee b/spec-nylas/nylas-sync-worker-spec.coffee index af11b5921..1d791b987 100644 --- a/spec-nylas/nylas-sync-worker-spec.coffee +++ b/spec-nylas/nylas-sync-worker-spec.coffee @@ -7,62 +7,154 @@ describe "NylasSyncWorker", -> beforeEach -> @apiRequests = [] @api = + makeRequest: (requestOptions) => + @apiRequests.push({requestOptions}) getCollection: (namespace, model, params, requestOptions) => @apiRequests.push({namespace, model, params, requestOptions}) getThreads: (namespace, params, requestOptions) => @apiRequests.push({namespace, model:'threads', params, requestOptions}) - @state = - "contacts": {busy: true} - "calendars": {complete: true} - spyOn(atom.config, 'get').andCallFake (key) => expected = "nylas.namespace-id.worker-state" - return throw new Error("Not stubbed!") unless key is expected - return @state + return throw new Error("Not stubbed! #{key}") unless key is expected + return _.extend {}, { + "contacts": + busy: true + complete: false + "calendars": + busy:false + complete: true + } spyOn(atom.config, 'set').andCallFake (key, val) => - expected = "nylas.namespace-id.worker-state" - return throw new Error("Not stubbed!") unless key is expected - @state = val + return @worker = new NylasSyncWorker(@api, 'namespace-id') @connection = @worker.connection() + it "should reset `busy` to false when reading state from disk", -> + state = @worker.state() + expect(state.contacts.busy).toEqual(false) + describe "start", -> it "should open the long polling connection", -> spyOn(@connection, 'start') @worker.start() expect(@connection.start).toHaveBeenCalled() - it "should start querying for model collections that haven't been fully cached", -> + it "should start querying for model collections and counts that haven't been fully cached", -> @worker.start() - expect(@apiRequests.length).toBe(3) - modelsRequested = _.map @apiRequests, (r) -> r.model + expect(@apiRequests.length).toBe(6) + modelsRequested = _.compact _.map @apiRequests, ({model}) -> model expect(modelsRequested).toEqual(['threads', 'contacts', 'files']) + countsRequested = _.compact _.map @apiRequests, ({requestOptions}) -> + if requestOptions.qs?.view is 'count' + return requestOptions.path + + expect(modelsRequested).toEqual(['threads', 'contacts', 'files']) + expect(countsRequested).toEqual(['/n/namespace-id/threads', '/n/namespace-id/contacts', '/n/namespace-id/files']) + it "should mark incomplete collections as `busy`", -> @worker.start() - expect(@state).toEqual({ - "contacts": {busy: true} - "threads": {busy: true} - "files": {busy: true} - "calendars": {complete: true} - }) + nextState = @worker.state() - describe "when an API request completes", -> + for collection in ['contacts','threads','files'] + expect(nextState[collection].busy).toEqual(true) + + it "should initialize count and fetched to 0", -> + @worker.start() + nextState = @worker.state() + + for collection in ['contacts','threads','files'] + expect(nextState[collection].fetched).toEqual(0) + expect(nextState[collection].count).toEqual(0) + + it "should periodically try to restart failed collection syncs", -> + spyOn(@worker, 'resumeFetches').andCallThrough() + @worker.start() + advanceClock(50000) + expect(@worker.resumeFetches.callCount).toBe(2) + + describe "when a count request completes", -> beforeEach -> @worker.start() @request = @apiRequests[0] @apiRequests = [] + it "should update the count on the collection", -> + @request.requestOptions.success({count: 1001}) + nextState = @worker.state() + expect(nextState.threads.count).toEqual(1001) + + describe "resumeFetches", -> + it "should fetch collections", -> + spyOn(@worker, 'fetchCollection') + @worker.resumeFetches() + expect(@worker.fetchCollection.calls.map (call) -> call.args[0]).toEqual(['threads', 'calendars', 'contacts', 'files']) + + describe "fetchCollection", -> + beforeEach -> + @apiRequests = [] + + it "should not start if the collection sync is already in progress", -> + @worker._state.threads = { + 'busy': true + 'complete': false + } + @worker.fetchCollection('threads') + expect(@apiRequests.length).toBe(0) + + it "should not start if the collection sync is already complete", -> + @worker._state.threads = { + 'busy': false + 'complete': true + } + @worker.fetchCollection('threads') + expect(@apiRequests.length).toBe(0) + + it "should start the request for the model count", -> + @worker._state.threads = { + 'busy': false + 'complete': false + } + @worker.fetchCollection('threads') + expect(@apiRequests[0].requestOptions.path).toBe('/n/namespace-id/threads') + expect(@apiRequests[0].requestOptions.qs.view).toBe('count') + + it "should start the first request for models", -> + @worker._state.threads = { + 'busy': false + 'complete': false + } + @worker.fetchCollection('threads') + expect(@apiRequests[1].model).toBe('threads') + expect(@apiRequests[1].params.offset).toBe(0) + + describe "when an API request completes", -> + beforeEach -> + @worker.start() + @request = @apiRequests[1] + @apiRequests = [] + describe "successfully, with models", -> it "should request the next page", -> + pageSize = @request.params.limit models = [] - models.push(new Thread) for i in [0..249] + models.push(new Thread) for i in [0..(pageSize-1)] @request.requestOptions.success(models) expect(@apiRequests.length).toBe(1) - expect(@apiRequests[0].params).toEqual({limit:250; offset: 250}) + expect(@apiRequests[0].params).toEqual + limit: pageSize, + offset: @request.params.offset + pageSize + + it "should update the fetched count on the collection", -> + expect(@worker.state().threads.fetched).toEqual(0) + pageSize = @request.params.limit + models = [] + models.push(new Thread) for i in [0..(pageSize-1)] + @request.requestOptions.success(models) + expect(@worker.state().threads.fetched).toEqual(pageSize) describe "successfully, with fewer models than requested", -> beforeEach -> @@ -71,17 +163,14 @@ describe "NylasSyncWorker", -> @request.requestOptions.success(models) it "should not request another page", -> - @request.requestOptions.success([]) expect(@apiRequests.length).toBe(0) it "should update the state to complete", -> - @request.requestOptions.success([]) - expect(@state).toEqual({ - "contacts": {busy: true} - "files": {busy: true} - "threads": {complete : true} - "calendars": {complete: true} - }) + expect(@worker.state().threads.busy).toEqual(false) + expect(@worker.state().threads.complete).toEqual(true) + + it "should update the fetched count on the collection", -> + expect(@worker.state().threads.fetched).toEqual(101) describe "successfully, with no models", -> it "should not request another page", -> @@ -90,23 +179,16 @@ describe "NylasSyncWorker", -> it "should update the state to complete", -> @request.requestOptions.success([]) - expect(@state).toEqual({ - "contacts": {busy: true} - "files": {busy: true} - "threads": {complete : true} - "calendars": {complete: true} - }) + expect(@worker.state().threads.busy).toEqual(false) + expect(@worker.state().threads.complete).toEqual(true) describe "with an error", -> it "should log the error to the state", -> err = new Error("Oh no a network error") @request.requestOptions.error(err) - expect(@state).toEqual({ - "contacts": {busy: true} - "files": {busy: true} - "threads": {busy: false, error: err.toString()} - "calendars": {complete: true} - }) + expect(@worker.state().threads.busy).toEqual(false) + expect(@worker.state().threads.complete).toEqual(false) + expect(@worker.state().threads.error).toEqual(err.toString()) it "should not request another page", -> @request.requestOptions.error(new Error("Oh no a network error")) @@ -117,3 +199,9 @@ describe "NylasSyncWorker", -> spyOn(@connection, 'end') @worker.cleanup() expect(@connection.end).toHaveBeenCalled() + + it "should stop trying to restart failed collection syncs", -> + spyOn(@worker, 'resumeFetches').andCallThrough() + @worker.cleanup() + advanceClock(50000) + expect(@worker.resumeFetches.callCount).toBe(0) diff --git a/spec-nylas/tasks/send-draft-spec.coffee b/spec-nylas/tasks/send-draft-spec.coffee index a27a03a54..b30824fc2 100644 --- a/spec-nylas/tasks/send-draft-spec.coffee +++ b/spec-nylas/tasks/send-draft-spec.coffee @@ -140,10 +140,6 @@ describe "SendDraftTask", -> waitsForPromise => @task.performRemote().then -> expect(atom.playSound).toHaveBeenCalledWith("mail_sent.ogg") - it "post a notification", -> - waitsForPromise => @task.performRemote().then -> - expect(Actions.postNotification).toHaveBeenCalled() - it "should start an API request to /send", -> waitsForPromise => @task.performRemote().then => diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index 3125ad30a..d6e9ec72a 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -117,6 +117,8 @@ beforeEach -> spyOn(_._, "now").andCallFake -> window.now spyOn(window, "setTimeout").andCallFake window.fakeSetTimeout spyOn(window, "clearTimeout").andCallFake window.fakeClearTimeout + spyOn(window, "setInterval").andCallFake window.fakeSetInterval + spyOn(window, "clearInterval").andCallFake window.fakeClearInterval atom.packages.packageStates = {} diff --git a/src/flux/nylas-api.coffee b/src/flux/nylas-api.coffee index f0db55844..3903f674c 100644 --- a/src/flux/nylas-api.coffee +++ b/src/flux/nylas-api.coffee @@ -52,7 +52,7 @@ class NylasAPI return unless atom.isMainWindow() namespaces = NamespaceStore.items() - workers = _.map(namespaces, @_workerForNamespace) + workers = _.map(namespaces, @workerForNamespace) # Stop the workers that are not in the new workers list. # These namespaces are no longer in our database, so we shouldn't @@ -62,12 +62,10 @@ class NylasAPI @_workers = workers - _cleanupNamespaceWorkers: -> - for worker in @_workers - worker.cleanup() - @_workers = [] + workers: => + @_workers - _workerForNamespace: (namespace) => + workerForNamespace: (namespace) => worker = _.find @_workers, (c) -> c.namespaceId() is namespace.id return worker if worker @@ -91,6 +89,12 @@ class NylasAPI worker.start() worker + _cleanupNamespaceWorkers: -> + for worker in @_workers + worker.cleanup() + @_workers = [] + + # Delegates to node's request object. # On success, it will call the passed in success callback with options. # On error it will create a new APIError object that wraps the error, diff --git a/src/flux/nylas-sync-worker.coffee b/src/flux/nylas-sync-worker.coffee index fb9cdfa02..5a70c2270 100644 --- a/src/flux/nylas-sync-worker.coffee +++ b/src/flux/nylas-sync-worker.coffee @@ -1,11 +1,17 @@ _ = require 'underscore-plus' NylasLongConnection = require './nylas-long-connection' +{Publisher} = require './modules/reflux-coffee' +CoffeeHelpers = require './coffee-helpers' + PAGE_SIZE = 250 module.exports = class NylasSyncWorker + @include: CoffeeHelpers.includeModule + @include Publisher + constructor: (api, namespaceId) -> @_api = api @_namespaceId = namespaceId @@ -13,6 +19,9 @@ class NylasSyncWorker @_terminated = false @_connection = new NylasLongConnection(api, namespaceId) @_state = atom.config.get("nylas.#{namespaceId}.worker-state") ? {} + for model, modelState of @_state + modelState.busy = false + @ namespaceId: -> @@ -21,53 +30,80 @@ class NylasSyncWorker connection: -> @_connection + state: -> + @_state + start: -> + @_resumeTimer = setInterval(@resumeFetches, 20000) @_connection.start() + @resumeFetches() + + cleanup: -> + clearInterval(@_resumeTimer) + @_connection.end() + @_terminated = true + @ + + resumeFetches: => @fetchCollection('threads') @fetchCollection('calendars') @fetchCollection('contacts') @fetchCollection('files') - cleanup: -> - @_connection.end() - @_terminated = true - @ - - fetchCollection: (model, options = {}, callback) -> + fetchCollection: (model, options = {}) -> return if @_state[model]?.complete and not options.force? + return if @_state[model]?.busy - @_state[model] = {busy: true} + @_state[model] = + complete: false + error: null + busy: true + count: 0 + fetched: 0 @writeState() - params = - offset: 0 - limit: PAGE_SIZE - @fetchCollectionPage(model, params, callback) + @fetchCollectionCount(model) + @fetchCollectionPage(model, {offset: 0, limit: PAGE_SIZE}) + + fetchCollectionCount: (model) -> + @_api.makeRequest + path: "/n/#{@_namespaceId}/#{model}" + returnsModel: false + qs: + view: 'count' + success: (response) => + return if @_terminated + @updateTransferState(model, count: response.count) + error: (err) => + return if @_terminated - fetchCollectionPage: (model, params = {}, callback) -> + fetchCollectionPage: (model, params = {}) -> requestOptions = error: (err) => return if @_terminated - @_state[model] = {busy: false, error: err.toString()} - @writeState() - callback(err) if callback + @updateTransferState(model, {busy: false, complete: false, error: err.toString()}) success: (json) => return if @_terminated + lastReceivedIndex = params.offset + json.length if json.length is params.limit - params.offset = params.offset + json.length - @fetchCollectionPage(model, params, callback) + nextParams = _.extend({}, params, {offset: lastReceivedIndex}) + @fetchCollectionPage(model, nextParams) + @updateTransferState(model, {fetched: lastReceivedIndex}) else - @_state[model] = {complete: true} - @writeState() - callback() if callback + @updateTransferState(model, {fetched: lastReceivedIndex, busy: false, complete: true}) if model is 'threads' @_api.getThreads(@_namespaceId, params, requestOptions) else @_api.getCollection(@_namespaceId, model, params, requestOptions) + updateTransferState: (model, {busy, error, complete, fetched, count}) -> + @_state[model] = _.defaults({busy, error, complete, fetched, count}, @_state[model]) + @writeState() + writeState: -> @_writeState ?= _.debounce => atom.config.set("nylas.#{@_namespaceId}.worker-state", @_state) ,100 @_writeState() + @trigger() diff --git a/src/flux/stores/task-queue.coffee b/src/flux/stores/task-queue.coffee index d1cf0c949..56fe66936 100644 --- a/src/flux/stores/task-queue.coffee +++ b/src/flux/stores/task-queue.coffee @@ -100,6 +100,9 @@ class TaskQueue performedRemote: false notifiedOffline: false + queue: => + @_queue + findTask: ({object, matchKey, matchValue}) -> for other in @_queue by -1 if object is object and other[matchKey] is matchValue diff --git a/src/flux/tasks/add-remove-tags.coffee b/src/flux/tasks/add-remove-tags.coffee index 45d8d982f..23e893a88 100644 --- a/src/flux/tasks/add-remove-tags.coffee +++ b/src/flux/tasks/add-remove-tags.coffee @@ -13,7 +13,8 @@ class AddRemoveTagsTask extends Task constructor: (@thread, @tagIdsToAdd = [], @tagIdsToRemove = []) -> super - tagForId: (id) -> + label: -> + "Applying tags..." performLocal: (versionIncrement = 1) -> new Promise (resolve, reject) => diff --git a/src/flux/tasks/file-upload-task.coffee b/src/flux/tasks/file-upload-task.coffee index b9c59a8fb..571136586 100644 --- a/src/flux/tasks/file-upload-task.coffee +++ b/src/flux/tasks/file-upload-task.coffee @@ -1,4 +1,5 @@ fs = require 'fs' +_ = require 'underscore-plus' pathUtils = require 'path' Task = require './task' File = require '../models/file' @@ -122,7 +123,7 @@ class FileUploadTask extends Task fileName: pathUtils.basename(@filePath) @_memoUploadData.bytesUploaded = @_getBytesUploaded() @_memoUploadData.state = state if state? - return @_memoUploadData + return _.extend({}, @_memoUploadData) _getFileSize: (path) -> fs.statSync(path)["size"] diff --git a/src/flux/tasks/send-draft.coffee b/src/flux/tasks/send-draft.coffee index 0c8632b12..b077bb2bc 100644 --- a/src/flux/tasks/send-draft.coffee +++ b/src/flux/tasks/send-draft.coffee @@ -14,6 +14,9 @@ class SendDraftTask extends Task constructor: (@draftLocalId, {@fromPopout}={}) -> super + label: -> + "Sending draft..." + shouldDequeueOtherTask: (other) -> other instanceof SendDraftTask and other.draftLocalId is @draftLocalId @@ -25,7 +28,6 @@ class SendDraftTask extends Task # it actually succeeds. We don't want users to think messages have # already sent when they haven't! return Promise.reject("Attempt to call SendDraftTask.performLocal without @draftLocalId") unless @draftLocalId - Actions.postNotification({message: "Sending messageā€¦", type: 'info'}) Promise.resolve() @@ -59,7 +61,6 @@ class SendDraftTask extends Task _onSendDraftSuccess: (draft, resolve, reject) => (newMessage) => newMessage = (new Message).fromJSON(newMessage) atom.playSound('mail_sent.ogg') - Actions.postNotification({message: "Sent!", type: 'success'}) Actions.sendDraftSuccess draftLocalId: @draftLocalId newMessage: newMessage