From fe318b3f47902b404f14aa308bc9d471a9a88e79 Mon Sep 17 00:00:00 2001 From: Thomas Lo Date: Fri, 7 Oct 2016 14:18:28 -0700 Subject: [PATCH 1/4] If user is not signed-in, still delete data and return resolved promise --- lib/destroy.js | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/lib/destroy.js b/lib/destroy.js index e1f4bd5..b528061 100644 --- a/lib/destroy.js +++ b/lib/destroy.js @@ -6,19 +6,33 @@ var internals = module.exports.internals = {} internals.request = require('../utils/request') internals.clearSession = require('../utils/clear-session') internals.get = require('./get') +internals.isSignedIn = require('./is-signed-in') function destroy (state) { var accountProperties = internals.get(state) - return internals.request({ - method: 'DELETE', - url: state.url + '/session/account', - headers: { - authorization: 'Session ' + state.account.session.id - } - }) + if (internals.isSignedIn(state)) { + return internals.request({ + method: 'DELETE', + url: state.url + '/session/account', + headers: { + authorization: 'Session ' + state.account.session.id + } + }) + + .then(function () { + internals.clearSession({ + cacheKey: state.cacheKey + }) + + state.emitter.emit('signout', clone(state.account)) + state.emitter.emit('destroy', clone(state.account)) - .then(function () { + delete state.account + + return clone(accountProperties) + }) + } else { internals.clearSession({ cacheKey: state.cacheKey }) @@ -28,6 +42,6 @@ function destroy (state) { delete state.account - return clone(accountProperties) - }) + return Promise.resolve(clone(accountProperties)) + } } From facd35db36476d00dfea7decbaecd016f1466088 Mon Sep 17 00:00:00 2001 From: Thomas Lo Date: Fri, 7 Oct 2016 14:19:45 -0700 Subject: [PATCH 2/4] test: account.destroy works with invalid session --- test/integration/destroy-test.js | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/integration/destroy-test.js b/test/integration/destroy-test.js index 51f2846..ef4d418 100644 --- a/test/integration/destroy-test.js +++ b/test/integration/destroy-test.js @@ -5,6 +5,7 @@ var nock = require('nock') var clone = require('lodash/clone') var signInResponse = clone(require('../fixtures/signin.json')) var simple = require('simple-mock') +var lolex = require('lolex') var baseURL = 'http://localhost:3000' var options = { @@ -60,3 +61,46 @@ test('destroy account', function (t) { .catch(t.error) }) + +test('destroy account even when session is invalid', function (t) { + store.clear() + t.plan(5) + + // mock the Date object to always return 1970-01-01T00:00:00.000Z + var clock = lolex.install(0) + var account = new Account({ + url: baseURL, + id: 'abc4567' + }) + + nock(baseURL) + .delete('/session/account') + .reply(204) + + var signOutHandler = simple.stub() + var destroyHandler = simple.stub() + account.on('signout', signOutHandler) + account.on('destroy', destroyHandler) + + account.destroy(options) + + .then(function () { + clock.uninstall() + t.pass('destroys account') + + t.deepEqual(signOutHandler.lastCall.arg, { + createdAt: '1970-01-01T00:00:00.000Z', + id: 'abc4567' + }, '"signout" event emitted with account object') + + t.deepEqual(destroyHandler.lastCall.arg, { + createdAt: '1970-01-01T00:00:00.000Z', + id: 'abc4567' + }, '"destroy" event emitted with account object') + + t.is(signOutHandler.callCount, 1, '"signout" event emitted once') + t.is(destroyHandler.callCount, 1, '"destroy" event emitted once') + }) + + .catch(t.error) +}) From 5d4c901a00aa1a579a9f38947752b7ea1116afc7 Mon Sep 17 00:00:00 2001 From: Thomas Lo Date: Fri, 7 Oct 2016 19:31:52 -0700 Subject: [PATCH 3/4] Refactor: create initial promise object, send request only if signed-in --- lib/destroy.js | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/lib/destroy.js b/lib/destroy.js index b528061..680dd9a 100644 --- a/lib/destroy.js +++ b/lib/destroy.js @@ -11,28 +11,21 @@ internals.isSignedIn = require('./is-signed-in') function destroy (state) { var accountProperties = internals.get(state) - if (internals.isSignedIn(state)) { - return internals.request({ - method: 'DELETE', - url: state.url + '/session/account', - headers: { - authorization: 'Session ' + state.account.session.id - } - }) + var promise = Promise.resolve() - .then(function () { - internals.clearSession({ - cacheKey: state.cacheKey + if (internals.isSignedIn(state)) { + promise = promise.then(function () { + internals.request({ + method: 'DELETE', + url: state.url + '/session/account', + headers: { + authorization: 'Session ' + state.account.session.id + } }) - - state.emitter.emit('signout', clone(state.account)) - state.emitter.emit('destroy', clone(state.account)) - - delete state.account - - return clone(accountProperties) }) - } else { + } + + return promise.then(function () { internals.clearSession({ cacheKey: state.cacheKey }) @@ -42,6 +35,6 @@ function destroy (state) { delete state.account - return Promise.resolve(clone(accountProperties)) - } + return clone(accountProperties) + }) } From 08143fc5084dd39b68402c12cd79684bbf091861 Mon Sep 17 00:00:00 2001 From: Thomas Lo Date: Fri, 7 Oct 2016 19:33:09 -0700 Subject: [PATCH 4/4] Remove uncessay mocking of delete request, it never get's called --- test/integration/destroy-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/destroy-test.js b/test/integration/destroy-test.js index ef4d418..b3a831b 100644 --- a/test/integration/destroy-test.js +++ b/test/integration/destroy-test.js @@ -73,10 +73,6 @@ test('destroy account even when session is invalid', function (t) { id: 'abc4567' }) - nock(baseURL) - .delete('/session/account') - .reply(204) - var signOutHandler = simple.stub() var destroyHandler = simple.stub() account.on('signout', signOutHandler)