Skip to content

Commit

Permalink
✨ Support multi entitlement pingback in amp-subscriptions (ampproject…
Browse files Browse the repository at this point in the history
…#22469)

* Support multi entitlement pingback

* add pingbackReturnsAllEntitlements support
  • Loading branch information
jpettitt authored May 28, 2019
1 parent 93fc3cb commit 352aa45
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,11 @@ export class GoogleSubscriptionsPlatform {
return false;
}

/** @override */
pingbackReturnsAllEntitlements() {
return false;
}

/**
* Performs the pingback to the subscription platform
*/
Expand Down
18 changes: 12 additions & 6 deletions extensions/amp-subscriptions/0.1/amp-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -509,16 +509,22 @@ export class SubscriptionService {
*/
performPingback_() {
if (this.viewTrackerPromise_) {
const localPlatform = this.platformStore_.getLocalPlatform();
return this.viewTrackerPromise_
.then(() => {
return this.platformStore_.getGrantEntitlement();
if (localPlatform.pingbackReturnsAllEntitlements()) {
return this.platformStore_.getAllPlatformsEntitlements();
}
return this.platformStore_
.getGrantEntitlement()
.then(
grantStateEntitlement =>
grantStateEntitlement || Entitlement.empty('local')
);
})
.then(grantStateEntitlement => {
const localPlatform = this.platformStore_.getLocalPlatform();
.then(resolveEntitlements => {
if (localPlatform.isPingbackEnabled()) {
localPlatform.pingback(
grantStateEntitlement || Entitlement.empty('local')
);
localPlatform.pingback(resolveEntitlements);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ export class LocalSubscriptionBasePlatform {
/** @protected {!JsonObject} */
this.serviceConfig_ = platformConfig;

/** @private @const {boolean} */
this.pingbackAllEntitlements_ = !!this.serviceConfig_[
'pingbackAllEntitlements'
];

/** @protected @const {!./service-adapter.ServiceAdapter} */
this.serviceAdapter_ = serviceAdapter;

Expand Down Expand Up @@ -195,6 +200,14 @@ export class LocalSubscriptionBasePlatform {
*/
pingback(unusedEntitlement) {}

/**
* @override
* @return {boolean}
*/
pingbackReturnsAllEntitlements() {
return this.pingbackAllEntitlements_;
}

/** @override */
isPingbackEnabled() {
return false;
Expand Down
5 changes: 2 additions & 3 deletions extensions/amp-subscriptions/0.1/platform-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,9 @@ export class PlatformStore {

/**
* Returns entitlements when all services are done fetching them.
* @private
* @return {!Promise<!Array<!./entitlement.Entitlement>>}
*/
getAllPlatformsEntitlements_() {
getAllPlatformsEntitlements() {
if (this.allResolvedPromise_) {
return this.allResolvedPromise_.promise;
}
Expand Down Expand Up @@ -383,7 +382,7 @@ export class PlatformStore {
* @return {!Promise<!./subscription-platform.SubscriptionPlatform>}
*/
selectPlatform() {
return this.getAllPlatformsEntitlements_().then(() => {
return this.getAllPlatformsEntitlements().then(() => {
// TODO(@prateekbh): explain why sometimes a quick resolve is possible vs
// waiting for all entitlement.
return this.selectApplicablePlatform_();
Expand Down
6 changes: 6 additions & 0 deletions extensions/amp-subscriptions/0.1/subscription-platform.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ export class SubscriptionPlatform {
*/
isPingbackEnabled() {}

/**
* True if pingback returns all entitlments
* @return {boolean}
*/
pingbackReturnsAllEntitlements() {}

/**
* Performs the pingback to the subscription platform.
* @param {!./entitlement.Entitlement} unusedSelectedPlatform
Expand Down
23 changes: 23 additions & 0 deletions extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,29 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => {
});
});

it('should send pingback with all entitlements if "pingbackAllEntitlements" is set', () => {
const entitlementData = {
source: 'local',
granted: true,
grantReason: GrantReason.SUBSCRIBER,
};
const entitlement = Entitlement.parseFromJson(entitlementData);
subscriptionService.viewTrackerPromise_ = Promise.resolve();
subscriptionService.platformStore_ = new PlatformStore(['local']);
const platform = new SubscriptionPlatform();
platform.isPingbackEnabled = () => true;
platform.pingbackReturnsAllEntitlements = () => true;
subscriptionService.platformStore_.resolvePlatform('local', platform);
const entitlementStub = sandbox
.stub(subscriptionService.platformStore_, 'getAllPlatformsEntitlements')
.callsFake(() => Promise.resolve([entitlement]));
const pingbackStub = sandbox.stub(platform, 'pingback');
return subscriptionService.performPingback_().then(() => {
expect(entitlementStub).to.be.called;
expect(pingbackStub).to.be.calledWith([entitlement]);
});
});

it('should send empty pingback if resolved entitlement is null', () => {
subscriptionService.viewTrackerPromise_ = Promise.resolve();
subscriptionService.platformStore_ = new PlatformStore(['local']);
Expand Down
17 changes: 17 additions & 0 deletions extensions/amp-subscriptions/0.1/test/test-local-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,23 @@ describes.fakeWin('LocalSubscriptionsPlatform', {amp: true}, env => {
expect(localSubscriptionPlatform.getBaseScore()).to.be.equal(99);
});

it('should default to single entitlement pingback', () => {
expect(localSubscriptionPlatform.pingbackReturnsAllEntitlements()).to.be
.false;
});

it('pingbackReturnsAllEntitlements should "pingbackAllEntitlements" config value', () => {
const testConfig = Object.assign({}, serviceConfig.services[0], {
'pingbackAllEntitlements': true,
});
const testLocalPlatform = localSubscriptionPlatformFactory(
ampdoc,
testConfig,
serviceAdapter
);
expect(testLocalPlatform.pingbackReturnsAllEntitlements()).to.be.true;
});

it('Should not allow prerender', () => {
expect(localSubscriptionPlatform.isPrerenderSafe()).to.be.false;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ describes.realWin('Platform store', {}, () => {
() => {
const fakeResult = [entitlementsForService1, entitlementsForService2];
const getAllPlatformsStub = sandbox
.stub(platformStore, 'getAllPlatformsEntitlements_')
.stub(platformStore, 'getAllPlatformsEntitlements')
.callsFake(() => Promise.resolve(fakeResult));
const selectApplicablePlatformStub = sandbox
.stub(platformStore, 'selectApplicablePlatform_')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ export class ViewerSubscriptionPlatform {
return this.platform_.isPingbackEnabled();
}

/** @override */
pingbackReturnsAllEntitlements() {
return this.platform_.pingbackReturnsAllEntitlements();
}

/** @override */
pingback(selectedPlatform) {
this.platform_.pingback(selectedPlatform);
Expand Down

0 comments on commit 352aa45

Please sign in to comment.