From cfbbcd23d3f01a22430b4f82f305aa1df68c9428 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Mon, 14 Mar 2016 15:34:16 -0700 Subject: [PATCH] fix(snooze): Correctly query and create snooze categories per account Summary: - Was not properly updating the references to snoozed categories when accounts were added or removed - Update whenCategoriesReady to make sure we listen until category syncing has concluded (Move inside CategoryStore) - #1676, #1658 Test Plan: - TODO Reviewers: evan, drew, bengotow Reviewed By: bengotow Differential Revision: https://phab.nylas.com/D2723 --- build/config/eslint.json | 3 +- .../notifications/lib/activity-sidebar.cjsx | 2 +- .../thread-snooze/lib/snooze-mail-label.jsx | 12 +++ .../thread-snooze/lib/snooze-store.js | 13 +++- .../thread-snooze/lib/snooze-utils.js | 17 +---- .../thread-snooze/spec/snooze-utils-spec.es6 | 2 +- .../worker-sync/lib/nylas-sync-worker.coffee | 2 +- spec/stores/category-store-spec.es6 | 61 +++++++++++++++ spec/stores/nylas-sync-status-store-spec.es6 | 76 +++++++++++++++++++ src/flux/models/account.coffee | 3 + src/flux/stores/category-store.coffee | 33 +++++++- .../stores/nylas-sync-status-store.coffee | 15 +++- src/global/nylas-exports.coffee | 4 + 13 files changed, 215 insertions(+), 28 deletions(-) create mode 100644 spec/stores/category-store-spec.es6 create mode 100644 spec/stores/nylas-sync-status-store-spec.es6 diff --git a/build/config/eslint.json b/build/config/eslint.json index 3c5e7cea9..0065fbe02 100644 --- a/build/config/eslint.json +++ b/build/config/eslint.json @@ -3,7 +3,8 @@ "globals": { "NylasEnv": false, "$n": false, - "waitsForPromise": false + "waitsForPromise": false, + "advanceClock": false }, "env": { "browser": true, diff --git a/internal_packages/notifications/lib/activity-sidebar.cjsx b/internal_packages/notifications/lib/activity-sidebar.cjsx index a95d47825..4181e59bd 100644 --- a/internal_packages/notifications/lib/activity-sidebar.cjsx +++ b/internal_packages/notifications/lib/activity-sidebar.cjsx @@ -108,7 +108,7 @@ class ActivitySidebar extends React.Component _getStateFromStores: => notifications: NotificationStore.notifications() tasks: TaskQueueStatusStore.queue() - isInitialSyncComplete: NylasSyncStatusStore.isComplete() + isInitialSyncComplete: NylasSyncStatusStore.isSyncComplete() _onDeltaReceived: (countDeltas) => tooSmallForNotification = countDeltas <= 10 diff --git a/internal_packages/thread-snooze/lib/snooze-mail-label.jsx b/internal_packages/thread-snooze/lib/snooze-mail-label.jsx index 14332b2cb..d67e4b943 100644 --- a/internal_packages/thread-snooze/lib/snooze-mail-label.jsx +++ b/internal_packages/thread-snooze/lib/snooze-mail-label.jsx @@ -1,5 +1,6 @@ import _ from 'underscore'; import React, {Component, PropTypes} from 'react'; +import {FocusedPerspectiveStore} from 'nylas-exports'; import {RetinaImg, MailLabel} from 'nylas-component-kit'; import {SNOOZE_CATEGORY_NAME, PLUGIN_ID} from './snooze-constants'; import {snoozedUntilMessage} from './snooze-utils'; @@ -15,6 +16,17 @@ class SnoozeMailLabel extends Component { static containerRequired = false; render() { + const current = FocusedPerspectiveStore.current() + const isSnoozedPerspective = ( + current.categories && + current.categories.length > 0 && + current.categories[0].displayName === SNOOZE_CATEGORY_NAME + ) + + if (!isSnoozedPerspective) { + return + } + const {thread} = this.props; if (_.findWhere(thread.categories, {displayName: SNOOZE_CATEGORY_NAME})) { const metadata = thread.metadataForPluginId(PLUGIN_ID); diff --git a/internal_packages/thread-snooze/lib/snooze-store.js b/internal_packages/thread-snooze/lib/snooze-store.js index cc747e87f..331cd3915 100644 --- a/internal_packages/thread-snooze/lib/snooze-store.js +++ b/internal_packages/thread-snooze/lib/snooze-store.js @@ -15,11 +15,14 @@ class SnoozeStore { constructor(pluginId = PLUGIN_ID, pluginName = PLUGIN_NAME) { this.pluginId = pluginId this.pluginName = pluginName - this.snoozeCategoriesPromise = getSnoozeCategoriesByAccount() + this.snoozeCategoriesPromise = getSnoozeCategoriesByAccount(AccountStore.accounts()) } activate() { - this.unsubscribe = SnoozeActions.snoozeThreads.listen(this.onSnoozeThreads) + this.unsubscribers = [ + AccountStore.listen(this.onAccountsChanged), + SnoozeActions.snoozeThreads.listen(this.onSnoozeThreads), + ] } recordSnoozeEvent(threads, snoozeDate, label) { @@ -55,6 +58,10 @@ class SnoozeStore { return Promise.resolve(threadsByAccountId); }; + onAccountsChanged = ()=> { + this.snoozeCategoriesPromise = getSnoozeCategoriesByAccount(AccountStore.accounts()) + }; + onSnoozeThreads = (threads, snoozeDate, label) => { this.recordSnoozeEvent(threads, label) @@ -85,7 +92,7 @@ class SnoozeStore { }; deactivate() { - this.unsubscribe() + this.unsubscribers.forEach(unsub => unsub()) } } diff --git a/internal_packages/thread-snooze/lib/snooze-utils.js b/internal_packages/thread-snooze/lib/snooze-utils.js index 1906c0d02..311a2eb3e 100644 --- a/internal_packages/thread-snooze/lib/snooze-utils.js +++ b/internal_packages/thread-snooze/lib/snooze-utils.js @@ -58,23 +58,8 @@ const SnoozeUtils = { }) }, - whenCategoriesReady(accountId) { - const categoriesReady = ()=> CategoryStore.categories(accountId).length > 0; - if (!categoriesReady()) { - return new Promise((resolve)=> { - const unsubscribe = CategoryStore.listen(()=> { - if (categoriesReady()) { - unsubscribe() - resolve() - } - }) - }) - } - return Promise.resolve() - }, - getSnoozeCategory(accountId, categoryName = SNOOZE_CATEGORY_NAME) { - return SnoozeUtils.whenCategoriesReady(accountId) + return CategoryStore.whenCategoriesReady(accountId) .then(()=> { const allCategories = CategoryStore.categories(accountId) const category = _.findWhere(allCategories, {displayName: categoryName}) diff --git a/internal_packages/thread-snooze/spec/snooze-utils-spec.es6 b/internal_packages/thread-snooze/spec/snooze-utils-spec.es6 index bd07ae944..4832b40ba 100644 --- a/internal_packages/thread-snooze/spec/snooze-utils-spec.es6 +++ b/internal_packages/thread-snooze/spec/snooze-utils-spec.es6 @@ -22,7 +22,7 @@ describe('Snooze Utils', ()=> { beforeEach(()=> { this.name = 'Snoozed Folder' this.accId = 123 - spyOn(SnoozeUtils, 'whenCategoriesReady').andReturn(Promise.resolve()) + spyOn(CategoryStore, 'whenCategoriesReady').andReturn(Promise.resolve()) }) describe('snoozedUntilMessage', ()=> { diff --git a/internal_packages/worker-sync/lib/nylas-sync-worker.coffee b/internal_packages/worker-sync/lib/nylas-sync-worker.coffee index d9d01075b..696d82b23 100644 --- a/internal_packages/worker-sync/lib/nylas-sync-worker.coffee +++ b/internal_packages/worker-sync/lib/nylas-sync-worker.coffee @@ -108,7 +108,7 @@ class NylasSyncWorker needed = [ {model: 'threads'}, - {model: "#{@_account.organizationUnit}s", initialPageSize: 1000} + {model: @_account.categoryCollection(), initialPageSize: 1000} {model: 'drafts'}, {model: 'contacts'}, {model: 'calendars'}, diff --git a/spec/stores/category-store-spec.es6 b/spec/stores/category-store-spec.es6 new file mode 100644 index 000000000..a51d5e22a --- /dev/null +++ b/spec/stores/category-store-spec.es6 @@ -0,0 +1,61 @@ +import { + Rx, + AccountStore, + CategoryStore, + NylasSyncStatusStore, +} from 'nylas-exports'; + +describe('CategoryStore', ()=> { + beforeEach(()=> { + spyOn(AccountStore, 'accountForId').andReturn({categoryCollection: ()=> 'labels'}) + }); + + describe('whenCategoriesReady', ()=> { + it('resolves immediately if sync is done and cache is populated', ()=> { + spyOn(NylasSyncStatusStore, 'isSyncCompleteForAccount').andReturn(true) + spyOn(CategoryStore, 'categories').andReturn([{name: 'inbox'}]) + spyOn(Rx.Observable, 'fromStore') + waitsForPromise(()=> { + const promise = CategoryStore.whenCategoriesReady('a1') + expect(promise.isResolved()).toBe(true) + return promise.then(()=> { + expect(Rx.Observable.fromStore).not.toHaveBeenCalled() + }) + }) + }); + + it('resolves only when sync is done even if cache is already populated', ()=> { + spyOn(NylasSyncStatusStore, 'isSyncCompleteForAccount').andReturn(false) + spyOn(CategoryStore, 'categories').andReturn([{name: 'inbox'}]) + waitsForPromise(()=> { + const promise = CategoryStore.whenCategoriesReady('a1') + expect(promise.isResolved()).toBe(false) + + jasmine.unspy(NylasSyncStatusStore, 'isSyncCompleteForAccount') + spyOn(NylasSyncStatusStore, 'isSyncCompleteForAccount').andReturn(true) + NylasSyncStatusStore.trigger() + + return promise.then(()=> { + expect(promise.isResolved()).toBe(true) + }) + }) + }); + + it('resolves only when cache is populated even if sync is done', ()=> { + spyOn(NylasSyncStatusStore, 'isSyncCompleteForAccount').andReturn(true) + spyOn(CategoryStore, 'categories').andReturn([]) + waitsForPromise(()=> { + const promise = CategoryStore.whenCategoriesReady('a1') + expect(promise.isResolved()).toBe(false) + + jasmine.unspy(CategoryStore, 'categories') + spyOn(CategoryStore, 'categories').andReturn([{name: 'inbox'}]) + CategoryStore.trigger() + + return promise.then(()=> { + expect(promise.isResolved()).toBe(true) + }) + }) + }); + }); +}); diff --git a/spec/stores/nylas-sync-status-store-spec.es6 b/spec/stores/nylas-sync-status-store-spec.es6 new file mode 100644 index 000000000..3fd15c729 --- /dev/null +++ b/spec/stores/nylas-sync-status-store-spec.es6 @@ -0,0 +1,76 @@ +import {NylasSyncStatusStore} from 'nylas-exports' + +const store = NylasSyncStatusStore + +fdescribe('NylasSyncStatusStore', ()=> { + beforeEach(()=> { + store._statesByAccount = {} + }); + + describe('isSyncCompleteForAccount', ()=> { + describe('when model (collection) provided', ()=> { + it('returns true if syncing for the given model and account is complete', ()=> { + store._statesByAccount = { + a1: { + labels: {complete: true}, + }, + } + expect(store.isSyncCompleteForAccount('a1', 'labels')).toBe(true) + }); + + it('returns false otherwise', ()=> { + const states = [ + { a1: { labels: {complete: false} } }, + { a1: {} }, + {}, + ] + states.forEach((state)=> { + store._statesByAccount = state + expect(store.isSyncCompleteForAccount('a1', 'labels')).toBe(false) + }) + }); + }); + + describe('when model not provided', ()=> { + it('returns true if sync is complete for all models for the given account', ()=> { + store._statesByAccount = { + a1: { + labels: {complete: true}, + threads: {complete: true}, + }, + } + expect(store.isSyncCompleteForAccount('a1')).toBe(true) + }); + + it('returns false otherwise', ()=> { + store._statesByAccount = { + a1: { + labels: {complete: true}, + threads: {complete: false}, + }, + } + expect(store.isSyncCompleteForAccount('a1')).toBe(false) + }); + }); + }); + + describe('isSyncComplete', ()=> { + it('returns true if sync is complete for all accounts', ()=> { + spyOn(store, 'isSyncCompleteForAccount').andReturn(true) + store._statesByAccount = { + a1: {}, + a2: {}, + } + expect(store.isSyncComplete('a1')).toBe(true) + }); + + it('returns false otherwise', ()=> { + spyOn(store, 'isSyncCompleteForAccount').andCallFake((acctId) => acctId === 'a1' ? true : false) + store._statesByAccount = { + a1: {}, + a2: {}, + } + expect(store.isSyncComplete('a1')).toBe(false) + }); + }); +}); diff --git a/src/flux/models/account.coffee b/src/flux/models/account.coffee index 27664cf77..4d5fb055e 100644 --- a/src/flux/models/account.coffee +++ b/src/flux/models/account.coffee @@ -111,6 +111,9 @@ class Account extends ModelWithMetadata else 'Unknown' + categoryCollection: -> + "#{@organizationUnit}s" + categoryIcon: -> if @usesFolders() 'folder.png' diff --git a/src/flux/stores/category-store.coffee b/src/flux/stores/category-store.coffee index 725d06532..922a63602 100644 --- a/src/flux/stores/category-store.coffee +++ b/src/flux/stores/category-store.coffee @@ -1,10 +1,11 @@ _ = require 'underscore' +Rx = require 'rx-lite' NylasStore = require 'nylas-store' AccountStore = require './account-store' +NylasSyncStatusStore = require './nylas-sync-status-store' Account = require '../models/account' {StandardCategoryNames} = require '../models/category' {Categories} = require 'nylas-observables' -Rx = require 'rx-lite' asAccount = (a) -> throw new Error("You must pass an Account or Account Id") unless a @@ -129,6 +130,36 @@ class CategoryStore extends NylasStore getSpamCategory: (accountOrId) => @getStandardCategory(accountOrId, "spam") + # Public: Returns a promise that resolves when the categories for a given + # account have been loaded into the local cache + # + whenCategoriesReady: (accountOrId) => + if not accountOrId + Promise.reject('whenCategoriesReady: must pass an account or accountId') + + account = asAccount(accountOrId) + categoryCollection = account.categoryCollection() + categoriesReady = => ( + @categories(account).length > 0 and + NylasSyncStatusStore.isSyncCompleteForAccount(account.id, categoryCollection) + ) + + if not categoriesReady() + return new Promise (resolve) => + syncStatusObservable = Rx.Observable.fromStore(NylasSyncStatusStore) + categoryObservable = Rx.Observable.fromStore(@) + + disposable = Rx.Observable.merge( + syncStatusObservable, + categoryObservable + ).subscribe => + if categoriesReady() + disposable.dispose() + resolve() + + return Promise.resolve() + + _onCategoriesChanged: (categories) => @_categoryResult = categories @_categoryCache = {} diff --git a/src/flux/stores/nylas-sync-status-store.coffee b/src/flux/stores/nylas-sync-status-store.coffee index f16df0870..3d2f053b6 100644 --- a/src/flux/stores/nylas-sync-status-store.coffee +++ b/src/flux/stores/nylas-sync-status-store.coffee @@ -25,10 +25,17 @@ class NylasSyncStatusStore extends NylasStore state: => @_statesByAccount - isComplete: -> - for acctId, state of @_statesByAccount - for model, modelState of state - return false if not modelState.complete + isSyncCompleteForAccount: (acctId, model) => + return false unless @_statesByAccount[acctId] + if model + return @_statesByAccount[acctId][model]?.complete ? false + for _model, modelState of @_statesByAccount[acctId] + return false if not modelState.complete + return true + + isSyncComplete: => + for acctId of @_statesByAccount + return false if not @isSyncCompleteForAccount(acctId) return true busy: => diff --git a/src/global/nylas-exports.coffee b/src/global/nylas-exports.coffee index 4adf9fcdc..22ece2510 100644 --- a/src/global/nylas-exports.coffee +++ b/src/global/nylas-exports.coffee @@ -42,6 +42,10 @@ class NylasExports return exported enumerable: true + # Make sure our custom observable helpers are defined immediately + # (fromStore, fromQuery, etc...) + require 'nylas-observables' + # Actions @load "Actions", 'flux/actions'