Fix paywall race condition and display issues

extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js

@@ -218,12 +218,13 @@ export class GoogleSubscriptionsPlatform {
   }
 
   /** @override */
-  activate(entitlement) {
+  activate(entitlement, grantEntitlement) {
+    const best = grantEntitlement || entitlement;
     // Offers or abbreviated offers may need to be shown depending on
     // whether the access has been granted and whether user is a subscriber.
-    if (!entitlement.granted) {
+    if (!best.granted) {
       this.runtime_.showOffers({list: 'amp'});
-    } else if (!entitlement.isSubscriber()) {
+    } else if (!best.isSubscriber()) {
       this.runtime_.showAbbrvOffer({list: 'amp'});
     }
   }

extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js

@@ -196,12 +196,32 @@ describes.realWin('amp-subscriptions-google', {amp: true}, env => {
 
   it('should show abbrv offer on activate when granted non-subscriber', () => {
     platform.activate(new Entitlement({service: 'subscribe.google.com',
-      granted: true, subscribed: true}));
+      granted: true, grantReason: GrantReason.METERING}));
     expect(methods.showAbbrvOffer).to.be.calledOnce
         .calledWithExactly({list: 'amp'});
     expect(methods.showOffers).to.not.be.called;
   });
 
+  it('should override show offers with the grant for subscriber', () => {
+    const entitlement = new Entitlement({service: 'subscribe.google.com',
+      granted: false});
+    const grantEntitlement = new Entitlement({service: 'local',
+      granted: true, grantReason: GrantReason.SUBSCRIBER});
+    platform.activate(entitlement, grantEntitlement);
+    expect(methods.showOffers).to.not.be.called;
+    expect(methods.showAbbrvOffer).to.not.be.called;
+  });
+
+  it('should override show offers with the grant non-subscriber', () => {
+    const entitlement = new Entitlement({service: 'subscribe.google.com',
+      granted: false});
+    const grantEntitlement = new Entitlement({service: 'local',
+      granted: true, grantReason: GrantReason.METERING});
+    platform.activate(entitlement, grantEntitlement);
+    expect(methods.showOffers).to.not.be.called;
+    expect(methods.showAbbrvOffer).to.be.calledOnce;
+  });
+
   it('should start linking flow when requested', () => {
     serviceAdapterMock.expects('delegateActionToLocal').never();
     callback(callbacks.loginRequest)({linkRequested: true});

extensions/amp-subscriptions/0.1/amp-subscriptions.js

@@ -412,14 +412,17 @@ export class SubscriptionService {
     const requireValuesPromise = Promise.all([
       this.platformStore_.getGrantStatus(),
       this.platformStore_.selectPlatform(),
+      this.platformStore_.getGrantEntitlement(),
     ]);
 
     return requireValuesPromise.then(resolvedValues => {
       const selectedPlatform = resolvedValues[1];
+      const grantEntitlement = resolvedValues[2];
       const selectedEntitlement = this.platformStore_.getResolvedEntitlementFor(
           selectedPlatform.getServiceId());
+      const bestEntitlement = grantEntitlement || selectedEntitlement;
 
-      selectedPlatform.activate(selectedEntitlement);
+      selectedPlatform.activate(selectedEntitlement, grantEntitlement);
 
       if (sendAnalyticsEvents === false) {
         return;
@@ -432,10 +435,10 @@ export class SubscriptionService {
       this.subscriptionAnalytics_.serviceEvent(
           SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED_DEPRECATED,
           selectedPlatform.getServiceId());
-      if (selectedEntitlement.granted) {
+      if (bestEntitlement.granted) {
         this.subscriptionAnalytics_.serviceEvent(
             SubscriptionAnalyticsEvents.ACCESS_GRANTED,
-            selectedPlatform.getServiceId());
+            bestEntitlement.service);
       } else {
         this.subscriptionAnalytics_.serviceEvent(
             SubscriptionAnalyticsEvents.PAYWALL_ACTIVATED,

extensions/amp-subscriptions/0.1/platform-store.js

@@ -75,9 +75,6 @@ export class PlatformStore {
     /** @private {?Deferred} */
     this.grantStatusPromise_ = null;
 
-    /** @private @const {!Observable} */
-    this.onGrantStateResolvedCallbacks_ = new Observable();
-
     /** @private {?Entitlement} */
     this.grantStatusEntitlement_ = null;
 
@@ -211,6 +208,9 @@ export class PlatformStore {
       this.failedPlatforms_.splice(this.failedPlatforms_.indexOf(serviceId));
     }
     // Call all onChange callbacks.
+    if (entitlement.granted) {
+      this.saveGrantEntitlement_(entitlement);
+    }
     this.onEntitlementResolvedCallbacks_.fire({serviceId, entitlement});
   }
 
@@ -262,7 +262,6 @@ export class PlatformStore {
       // Listen if any upcoming entitlements unblock the reader
       this.onChange(({entitlement}) => {
         if (entitlement.granted) {
-          this.saveGrantEntitlement_(entitlement);
           this.grantStatusPromise_.resolve(true);
         } else if (this.areAllPlatformsResolved_()) {
           this.grantStatusPromise_.resolve(false);
@@ -286,7 +285,6 @@ export class PlatformStore {
           && !this.grantStatusEntitlement_.isSubscriber()
           && entitlement.isSubscriber())) {
       this.grantStatusEntitlement_ = entitlement;
-      this.onGrantStateResolvedCallbacks_.fire();
     }
   }
 
@@ -304,9 +302,10 @@ export class PlatformStore {
           || this.areAllPlatformsResolved_()) {
       this.grantStatusEntitlementPromise_.resolve(this.grantStatusEntitlement_);
     } else {
-      this.onGrantStateResolvedCallbacks_.add(() => {
+      this.onEntitlementResolvedCallbacks_.add(() => {
         // Grant entitlement only if subscriber
-        if (this.grantStatusEntitlement_.isSubscriber()
+        if ((this.grantStatusEntitlement_
+             && this.grantStatusEntitlement_.isSubscriber())
             || this.areAllPlatformsResolved_()) {
           this.grantStatusEntitlementPromise_.resolve(
               this.grantStatusEntitlement_);

extensions/amp-subscriptions/0.1/subscription-platform.js

@@ -40,8 +40,9 @@ export class SubscriptionPlatform {
    * Activates the subscription platform and hands over the control for
    * rendering.
    * @param {!./entitlement.Entitlement} unusedEntitlement
+   * @param {?./entitlement.Entitlement} unusedGrantEntitlement
    */
-  activate(unusedEntitlement) {}
+  activate(unusedEntitlement, unusedGrantEntitlement) {}
 
   /**
    * Reset the platform and renderer.

extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js

@@ -293,12 +293,43 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
   });
 
   describe('selectAndActivatePlatform_', () => {
-    it('should wait for grantStatus and selectPlatform promise', () => {
+    function resolveRequiredPromises(entitlementSpec, grantEntitlementSpec) {
+      entitlementSpec = Object.assign({
+        service: 'local',
+        source: 'local',
+        raw: 'raw',
+      }, entitlementSpec);
+      if (!grantEntitlementSpec && entitlementSpec.granted) {
+        grantEntitlementSpec = entitlementSpec;
+      }
+      const entitlement = new Entitlement(entitlementSpec);
+      const grantEntitlement = grantEntitlementSpec ?
+        new Entitlement(grantEntitlementSpec) : null;
+      const granted = !!grantEntitlementSpec;
+      const localPlatform =
+          subscriptionService.platformStore_.getLocalPlatform();
+      sandbox.stub(subscriptionService.platformStore_, 'getGrantStatus')
+          .callsFake(() => Promise.resolve(granted));
+      sandbox.stub(subscriptionService.platformStore_, 'getGrantEntitlement')
+          .callsFake(() => Promise.resolve(grantEntitlement));
+      subscriptionService.platformStore_.resolveEntitlement(
+          entitlementSpec.source,
+          entitlement);
+      sandbox.stub(
+          subscriptionService.platformStore_,
+          'selectPlatform'
+      ).callsFake(() => Promise.resolve(localPlatform));
+    }
+
+    it('should wait for grantStatus/ent and selectPlatform promise', () => {
       sandbox.stub(subscriptionService, 'fetchEntitlements_');
       subscriptionService.start();
       subscriptionService.viewTrackerPromise_ = Promise.resolve();
       return subscriptionService.initialize_().then(() => {
-        resolveRequiredPromises(subscriptionService, /* subscriber */ true);
+        resolveRequiredPromises({
+          granted: true,
+          grantReason: GrantReason.SUBSCRIBER,
+        });
         const localPlatform =
             subscriptionService.platformStore_.getLocalPlatform();
         const selectPlatformStub =
@@ -307,6 +338,8 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
         expect(localPlatform).to.be.not.null;
         return subscriptionService.selectAndActivatePlatform_().then(() => {
           expect(activateStub).to.be.calledOnce;
+          expect(activateStub.firstCall.args[0].source).to.equal('local');
+          expect(activateStub.firstCall.args[1].source).to.equal('local');
           expect(selectPlatformStub).to.be.called;
           expect(analyticsEventStub).to.be.calledWith(
               SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED,
@@ -326,12 +359,57 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
       });
     });
 
+    it('should activate with a different grant entitlement', () => {
+      sandbox.stub(subscriptionService, 'fetchEntitlements_');
+      subscriptionService.start();
+      subscriptionService.viewTrackerPromise_ = Promise.resolve();
+      return subscriptionService.initialize_().then(() => {
+        resolveRequiredPromises({
+          granted: false,
+        }, {
+          service: 'other',
+          source: 'other',
+          granted: true,
+          grantReason: GrantReason.SUBSCRIBER,
+        });
+        const localPlatform =
+            subscriptionService.platformStore_.getLocalPlatform();
+        const selectPlatformStub =
+            subscriptionService.platformStore_.selectPlatform;
+        const activateStub = sandbox.stub(localPlatform, 'activate');
+        expect(localPlatform).to.be.not.null;
+        return subscriptionService.selectAndActivatePlatform_().then(() => {
+          expect(activateStub).to.be.calledOnce;
+          expect(activateStub.firstCall.args[0].source).to.equal('local');
+          expect(activateStub.firstCall.args[1].source).to.equal('other');
+          expect(selectPlatformStub).to.be.called;
+          expect(analyticsEventStub).to.be.calledWith(
+              SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED,
+              {
+                'serviceId': 'local',
+              }
+          );
+          expect(analyticsEventStub).to.be.calledWith(
+              SubscriptionAnalyticsEvents.ACCESS_GRANTED,
+              {
+                'serviceId': 'other',
+              }
+          );
+          expect(analyticsEventStub).to.not.be.calledWith(
+              SubscriptionAnalyticsEvents.PAYWALL_ACTIVATED);
+        });
+      });
+    });
+
     it('should call selectPlatform with preferViewerSupport config', () => {
       sandbox.stub(subscriptionService, 'fetchEntitlements_');
       subscriptionService.start();
       subscriptionService.viewTrackerPromise_ = Promise.resolve();
       return subscriptionService.initialize_().then(() => {
-        resolveRequiredPromises(subscriptionService, /* subscriber */ true);
+        resolveRequiredPromises({
+          granted: true,
+          grantReason: GrantReason.SUBSCRIBER,
+        });
         const selectPlatformStub =
           subscriptionService.platformStore_.selectPlatform;
         subscriptionService.platformConfig_['preferViewerSupport'] = false;
@@ -346,7 +424,7 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
       subscriptionService.start();
       subscriptionService.viewTrackerPromise_ = Promise.resolve();
       return subscriptionService.initialize_().then(() => {
-        resolveRequiredPromises(subscriptionService, /* subscriber */ false);
+        resolveRequiredPromises({granted: false});
         const localPlatform =
             subscriptionService.platformStore_.getLocalPlatform();
         sandbox.stub(localPlatform, 'activate');
@@ -370,25 +448,6 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
         });
       });
     });
-
-    function resolveRequiredPromises(subscriptionService, subscriber) {
-      const entitlement = new Entitlement({
-        source: 'local',
-        raw: 'raw',
-        granted: subscriber,
-        grantReason: subscriber ? GrantReason.SUBSCRIBER : null,
-      });
-      const localPlatform =
-        subscriptionService.platformStore_.getLocalPlatform();
-      sandbox.stub(subscriptionService.platformStore_, 'getGrantStatus')
-          .callsFake(() => Promise.resolve());
-      subscriptionService.platformStore_.resolveEntitlement('local',
-          entitlement);
-      sandbox.stub(
-          subscriptionService.platformStore_,
-          'selectPlatform'
-      ).callsFake(() => Promise.resolve(localPlatform));
-    }
   });
 
   describe('startAuthorizationFlow_', () => {

extensions/amp-subscriptions/0.1/test/test-platform-store.js

@@ -508,55 +508,60 @@ describes.realWin('Platform store', {}, () => {
   });
 
   describe('getGrantEntitlement', () => {
-    const subscribedMeteredEntitlement = new Entitlement({
+    const subscribedEntitlement = new Entitlement({
       source: 'local',
       service: 'local',
       granted: true,
       grantReason: GrantReason.SUBSCRIBER,
     });
+    const meteringEntitlement = new Entitlement({
+      source: 'local',
+      service: 'local',
+      granted: true,
+      grantReason: GrantReason.METERING,
+    });
+    const noEntitlement = new Entitlement({
+      source: 'local',
+      service: 'local',
+      granted: false,
+    });
+
     it('should resolve with existing entitlement with subscriptions', () => {
-      platformStore.grantStatusEntitlement_ = subscribedMeteredEntitlement;
+      platformStore.grantStatusEntitlement_ = subscribedEntitlement;
       return platformStore.getGrantEntitlement().then(entitlement => {
-        expect(entitlement.json()).to.deep.equal(
-            subscribedMeteredEntitlement.json());
+        expect(entitlement).to.equal(subscribedEntitlement);
       });
     });
 
     it('should resolve with first entitlement with subscriptions', () => {
-      const meteringEntitlement = new Entitlement({
-        source: 'local',
-        service: 'local',
-        granted: true,
-        data: {
-          metering: {
-            'left': 5,
-            'total': 10,
-            'token': 'token',
-          },
-        },
-      });
-      platformStore.grantStatusEntitlement_ = meteringEntitlement;
-      platformStore.saveGrantEntitlement_(subscribedMeteredEntitlement);
+      platformStore.resolveEntitlement('service1', subscribedEntitlement);
       return platformStore.getGrantEntitlement().then(entitlement => {
-        expect(entitlement.json()).to.deep.equal(
-            subscribedMeteredEntitlement.json());
+        expect(entitlement).to.equal(subscribedEntitlement);
       });
     });
 
     it('should resolve with metered entitlement when no '
         + 'platform is subscribed', () => {
-      const meteringEntitlement = new Entitlement({
-        source: 'local',
-        service: 'local',
-        granted: true,
-        grantReason: GrantReason.METERING,
+      platformStore.resolveEntitlement('service1', noEntitlement);
+      platformStore.resolveEntitlement('service2', meteringEntitlement);
+      return platformStore.getGrantEntitlement().then(entitlement => {
+        expect(entitlement).to.equal(meteringEntitlement);
       });
-      sandbox.stub(platformStore, 'areAllPlatformsResolved_')
-          .callsFake(() => true);
-      platformStore.saveGrantEntitlement_(meteringEntitlement);
+    });
+
+    it('should override metering with subscription', () => {
+      platformStore.resolveEntitlement('service1', meteringEntitlement);
+      platformStore.resolveEntitlement('service2', subscribedEntitlement);
+      return platformStore.getGrantEntitlement().then(entitlement => {
+        expect(entitlement).to.equal(subscribedEntitlement);
+      });
+    });
+
+    it('should resolve to null if nothing matched', () => {
+      platformStore.resolveEntitlement('service1', noEntitlement);
+      platformStore.resolveEntitlement('service2', noEntitlement);
       return platformStore.getGrantEntitlement().then(entitlement => {
-        expect(entitlement.json()).to.deep.equal(
-            meteringEntitlement.json());
+        expect(entitlement).to.be.null;
       });
     });
   });