diff --git a/extensions/amp-user-notification/0.1/amp-user-notification.js b/extensions/amp-user-notification/0.1/amp-user-notification.js index 3c042bf999d4..572f7cb5b60d 100644 --- a/extensions/amp-user-notification/0.1/amp-user-notification.js +++ b/extensions/amp-user-notification/0.1/amp-user-notification.js @@ -30,15 +30,6 @@ import {xhrFor} from '../../../src/xhr'; const TAG = 'amp-user-notification'; -/** - * @export - * @typedef {{ - * elementId: string, - * ampUserId: string - * }} - */ -let PostRequestMetadataDef; - /** * @export * @typedef {{ @@ -74,6 +65,14 @@ class NotificationInterface { * @return {!Promise} */ show() {} + + /** + * Returns whether this notification has been dismissed and the dismissal + * has been persisted in storage. Returns false if storage throws error or + * 'data-persist-dismissal' is disabled. + * @return {!Promise} + */ + isDismissed() {} } /** @@ -239,21 +238,7 @@ export class AmpUserNotification extends AMP.BaseElement { /** @override */ shouldShow() { - let maybeCheckStoragePromise; - - if (this.persistDismissal_) { - maybeCheckStoragePromise = this.storagePromise_.then(storage => { - return storage.get(this.storageKey_); - }); - } else { - // Skip reading storage when not data-persist-dismissal. - maybeCheckStoragePromise = Promise.resolve(null); - } - - return maybeCheckStoragePromise.catch(reason => { - dev.error(TAG, 'Failed to read storage', reason); - return false; - }).then(dismissed => { + return this.isDismissed().then(dismissed => { if (dismissed) { // Consent has been accepted. Nothing more to do. return false; @@ -286,6 +271,19 @@ export class AmpUserNotification extends AMP.BaseElement { return this.dialogPromise_; } + /** @override */ + isDismissed() { + if (!this.persistDismissal_) { + return Promise.resolve(false); + } + return this.storagePromise_ + .then(storage => storage.get(this.storageKey_)) + .then(persistedValue => !!persistedValue, reason => { + dev.error(TAG, 'Failed to read storage', reason); + return false; + }); + } + /** * Hides the current user notification and invokes the `dialogResolve_` * method. Removes the `.amp-active` class from the element. @@ -339,7 +337,7 @@ export class UserNotificationManager { * Retrieve a promise associated to an `amp-user-notification` component * that is resolved when user agrees to the terms. * @param {string} id - * @return {!Promise} + * @return {!Promise} */ get(id) { this.managerReadyPromise_.then(() => { @@ -347,19 +345,18 @@ export class UserNotificationManager { user.warn(TAG, `Did not find amp-user-notification element ${id}.`); } }); - return this.getElementDeferById_(id).promise; + return this.getOrCreateDeferById_(id).promise; } /** * Register an instance of `amp-user-notification`. * @param {string} id - * @param {!UserNotification} userNotification + * @param {!NotificationInterface} userNotification * @return {!Promise} * @package */ registerUserNotification(id, userNotification) { - const deferred = this.getElementDeferById_(id); - + const deferred = this.getOrCreateDeferById_(id); // Compose the registered notifications into a promise queue // that blocks until one notification is dismissed. return this.nextInQueue_ = this.nextInQueue_ @@ -370,27 +367,19 @@ export class UserNotificationManager { } }); }) - .then(deferred.resolve) + .then(deferred.resolve.bind(this, userNotification)) .catch(rethrowAsync.bind(null, 'Notification service failed amp-user-notification', id)); } /** - * Retrieve UserNotificationDeferDef object. + * Retrieves UserNotificationDeferDef object. Creates an defer if it doesn't + * exist. * @param {string} id * @return {!UserNotificationDeferDef} * @private */ - getElementDeferById_(id) { - return this.createOrReturnDefer_(id); - } - - /** - * Create an defer if it doesnt exist, else just return the one in the - * registry. - * @return {!UserNotificationDeferDef} - */ - createOrReturnDefer_(id) { + getOrCreateDeferById_(id) { if (this.deferRegistry_[id]) { return this.deferRegistry_[id]; } diff --git a/extensions/amp-user-notification/0.1/test/test-amp-user-notification.js b/extensions/amp-user-notification/0.1/test/test-amp-user-notification.js index e783fa3c2cb6..9cfe9e973f0e 100644 --- a/extensions/amp-user-notification/0.1/test/test-amp-user-notification.js +++ b/extensions/amp-user-notification/0.1/test/test-amp-user-notification.js @@ -115,6 +115,57 @@ describe('amp-user-notification', () => { }); }); + it('isDismissed should return true if dismissal has been recorded', () => { + return getUserNotification(dftAttrs).then(el => { + const impl = el.implementation_; + impl.buildCallback(); + + storageMock.expects('get') + .withExactArgs('amp-user-notification:n1') + .returns(Promise.resolve(true)) + .once(); + return expect(impl.isDismissed()).to.eventually.equal(true); + }); + }); + + it('isDismissed should return false if dismissal has not been recorded', + () => { + return getUserNotification(dftAttrs).then(el => { + const impl = el.implementation_; + impl.buildCallback(); + + storageMock.expects('get') + .withExactArgs('amp-user-notification:n1') + .returns(Promise.resolve(null)) + .once(); + return expect(impl.isDismissed()).to.eventually.equal(false); + }); + }); + + it('isDismissed should return false if data-persist-dismissal=false', () => { + dftAttrs['data-persist-dismissal'] = false; + return getUserNotification(dftAttrs).then(el => { + const impl = el.implementation_; + impl.buildCallback(); + + storageMock.expects('get').never(); + return expect(impl.isDismissed()).to.eventually.equal(false); + }); + }); + + it('isDismissed should return false if storage throws error', () => { + return getUserNotification(dftAttrs).then(el => { + const impl = el.implementation_; + impl.buildCallback(); + + storageMock.expects('get') + .withExactArgs('amp-user-notification:n1') + .returns(Promise.reject('intentional')) + .once(); + return expect(impl.isDismissed()).to.eventually.equal(false); + }); + }); + it('shouldShow should return false if storage has been recorded', () => { return getUserNotification(dftAttrs).then(el => { const impl = el.implementation_; @@ -471,10 +522,17 @@ describe('amp-user-notification', () => { }; }); - it('should be able to get a resolved service', () => { - service.registerUserNotification('n1', tag); + it('should be able to get AmpUserNotification object by ID', () => { + let userNotification; - return service.get('n1'); + return getUserNotification().then(element => { + return new AmpUserNotification(element); + }).then(un => { + userNotification = un; + service.registerUserNotification('n1', userNotification); + }).then(() => { + return expect(service.get('n1')).to.eventually.equal(userNotification); + }); }); it('should queue up multiple amp-user-notification elements', () => {