feat(auth): add boolean flag that skips Stripe comparison in favour of Contentful

Because:

* We want a seamless cutover from Stripe metadata to Contentful.

This commit:

* Adds a boolean feature flag to use Contentful data directly.

Closes FXA-9062
This commit is contained in:
Meghan Sardesai 2024-02-21 15:39:48 -05:00
Родитель 0f1da4adbc
Коммит 380caf6ab9
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 9A46BEBC2E8A3934
11 изменённых файлов: 266 добавлений и 30 удалений

Просмотреть файл

@ -24,7 +24,8 @@ describe('PlanMapperUtil', () => {
defaultMapper = new PlanMapperUtil(
defaultCommonContent,
defaultPurchaseDetails,
stripeMetadata
stripeMetadata,
false
);
});
@ -85,7 +86,8 @@ describe('PlanMapperUtil', () => {
const defaultMapper = new PlanMapperUtil(
commonContent,
defaultPurchaseDetails,
stripeMetadata
stripeMetadata,
false
);
// Instead of having a test for each Stripe Metadata Key, only test
// for keys with mapping logic, e.g. newsletterSlug
@ -105,7 +107,8 @@ describe('PlanMapperUtil', () => {
const mapper = new PlanMapperUtil(
commonContent,
defaultPurchaseDetails,
stripeMetadata
stripeMetadata,
false
);
const expected = null;
const actual = mapper.getContentfulForMetadataKey(
@ -131,7 +134,8 @@ describe('PlanMapperUtil', () => {
const mapper = new PlanMapperUtil(
defaultCommonContent,
defaultPurchaseDetails,
metadata
metadata,
false
);
const expected = 'hubs,mdnplus,snp';
const actual = mapper.getStripeForMetadataKey(
@ -145,7 +149,8 @@ describe('PlanMapperUtil', () => {
const mapper = new PlanMapperUtil(
defaultCommonContent,
defaultPurchaseDetails,
metadata
metadata,
false
);
const expected = undefined;
const actual = mapper.getStripeForMetadataKey(
@ -160,7 +165,8 @@ describe('PlanMapperUtil', () => {
const defaultMapper = new PlanMapperUtil(
defaultCommonContent,
defaultPurchaseDetails,
stripeMetadata
stripeMetadata,
false
);
const result = defaultMapper.mergeStripeAndContentful();
expect(result.errorFields.length).toBe(1);

Просмотреть файл

@ -13,7 +13,8 @@ export class PlanMapperUtil {
constructor(
private commonContent: OfferingCommonContentResult,
private purchaseDetails: PurchaseDetailsTransformed,
private stripeMetadata: Stripe.Metadata | null
private stripeMetadata: Stripe.Metadata | null,
private contentfulEnabled: boolean
) {}
/**
@ -25,6 +26,12 @@ export class PlanMapperUtil {
stripeValue?: string,
contentfulValue?: string | null
) {
// If contentful config enabled, skip comparison and
// return undefined if null, otherwise contentfulValue
if (this.contentfulEnabled) {
return contentfulValue === null ? undefined : contentfulValue;
}
// Return undefined if stripe and contentful aren't provided
if (stripeValue === undefined && contentfulValue === null) {
return undefined;

Просмотреть файл

@ -42,7 +42,11 @@ describe('StripeMapperService', () => {
metadata: validMetadata,
});
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans([stripePlan], 'en');
await stripeMapper.mapContentfulToStripePlans(
[stripePlan],
'en',
false
);
expect(mappedPlans[0].metadata?.['webIconURL']).toBe(
validMetadata.webIconURL
);
@ -63,7 +67,11 @@ describe('StripeMapperService', () => {
}),
});
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans([stripePlan], 'en');
await stripeMapper.mapContentfulToStripePlans(
[stripePlan],
'en',
false
);
const actualProduct = mappedPlans[0].product as Stripe.Product;
expect(actualProduct.metadata?.['webIconURL']).toBe(
validMetadata.webIconURL
@ -87,7 +95,11 @@ describe('StripeMapperService', () => {
}),
});
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans([stripePlan], 'en');
await stripeMapper.mapContentfulToStripePlans(
[stripePlan],
'en',
false
);
const actualProduct = mappedPlans[0].product as Stripe.Product;
expect(actualProduct.metadata?.['webIconURL']).toBe(
validMetadata.webIconURL
@ -115,7 +127,11 @@ describe('StripeMapperService', () => {
metadata: planOverrideMetadata,
});
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans([stripePlan], 'en');
await stripeMapper.mapContentfulToStripePlans(
[stripePlan],
'en',
false
);
const actualProduct = mappedPlans[0].product as Stripe.Product;
expect(actualProduct.metadata?.['webIconURL']).toBe(
planOverrideMetadata.webIconURL
@ -137,7 +153,11 @@ describe('StripeMapperService', () => {
}),
});
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans([stripePlan], 'en');
await stripeMapper.mapContentfulToStripePlans(
[stripePlan],
'en',
false
);
const actualProduct = mappedPlans[0].product as Stripe.Product;
expect(mappedPlans[0].metadata?.['webIconURL']).toBe(
expected.purchaseDetails.webIcon
@ -186,7 +206,8 @@ describe('StripeMapperService', () => {
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans(
[stripePlan1, stripePlan2],
'en'
'en',
false
);
expect(nonMatchingPlans.length).toBe(0);
@ -238,7 +259,8 @@ describe('StripeMapperService', () => {
const { mappedPlans, nonMatchingPlans } =
await stripeMapper.mapContentfulToStripePlans(
[stripePlan1, stripePlan2],
'en'
'en',
false
);
expect(nonMatchingPlans[0].split(' - ')[1]).toBe(

Просмотреть файл

@ -58,7 +58,8 @@ export class StripeMapperService {
*/
async mapContentfulToStripePlans(
plans: Stripe.Plan[],
acceptLanguage: string
acceptLanguage: string,
contentfulEnabled: boolean
) {
const mappedPlans: Stripe.Plan[] = [];
const validPlanIds = plans.map((plan) => plan.id);
@ -100,7 +101,8 @@ export class StripeMapperService {
const planMapper = new PlanMapperUtil(
commonContent,
purchaseDetails,
mergedStripeMetadata
mergedStripeMetadata,
contentfulEnabled
);
const { metadata, errorFields } = planMapper.mergeStripeAndContentful();

Просмотреть файл

@ -576,6 +576,14 @@ const conf = convict({
env: 'NEWSLETTER_HOST',
},
},
contentful: {
enabled: {
doc: 'Whether to use Contentful',
format: Boolean,
default: false,
env: 'CONTENTFUL_ENABLED',
},
},
});
const configDir = __dirname;

Просмотреть файл

@ -130,6 +130,7 @@ export class StripeService extends StripeHelper {
authFirestore: configService.get('authFirestore'),
subhub: configService.get('subhub'),
redis: configService.get('redis'),
contentful: configService.get('contentful'),
};
super(config, metrics, logger);

Просмотреть файл

@ -2017,6 +2017,12 @@ const convictConf = convict({
env: 'CONTENTFUL_FIRESTORE_CACHE_COLLECTION_NAME',
format: String,
},
enabled: {
default: false,
doc: 'Whether to use Contentful',
env: 'CONTENTFUL_ENABLED',
format: Boolean,
},
},
cloudTasks: {

Просмотреть файл

@ -34,7 +34,8 @@ import { SeverityLevel } from '@sentry/types';
import error from '../error';
import { authEvents } from '../events';
import { AuthLogger, AuthRequest, ProfileClient } from '../types';
import { AppConfig, AuthLogger, AuthRequest, ProfileClient } from '../types';
import { ConfigType } from '../../config';
import { PaymentConfigManager } from './configuration/manager';
import { AppleIAP } from './iap/apple-app-store/apple-iap';
import { AppStoreSubscriptionPurchase } from './iap/apple-app-store/subscription-purchase';
@ -74,6 +75,7 @@ export class CapabilityService {
private capabilityManager?: CapabilityManager;
private sentryLogCounter = new Map<string, number>();
private eligibilityManager?: EligibilityManager;
private config: ConfigType;
constructor() {
// TODO: the mock stripeHelper here fixes this specific instance when
@ -103,6 +105,7 @@ export class CapabilityService {
this.eligibilityManager = Container.get(EligibilityManager);
}
this.config = Container.get(AppConfig);
this.log = Container.get(AuthLogger);
// Register the event handlers for capability changes.
@ -168,7 +171,7 @@ export class CapabilityService {
{
subscriptionId: sub.id,
},
'Subscription customer was not a string.'
new Error('Subscription customer was not a string.')
);
}
if (!uid) {
@ -321,6 +324,8 @@ export class CapabilityService {
targetPlanId: string,
useFirestoreProductConfigs = false
): Promise<SubscriptionChangeEligibility> {
const contentfulEnabled = this.config.contentful.enabled;
const allPlansByPlanId = await this.allAbbrevPlansByPlanId();
const targetPlan = allPlansByPlanId[targetPlanId];
@ -329,6 +334,34 @@ export class CapabilityService {
const [stripeSubscribedPlans, iapSubscribedPlans] =
await this.getAllSubscribedAbbrevPlans(uid, allPlansByPlanId);
if (contentfulEnabled) {
if (!this.eligibilityManager) {
throw error.internalValidationError(
'eligibilityResult',
{},
new Error('CapabilityManager not found.')
);
} else {
try {
const eligibilityManagerResult =
await this.eligibilityFromEligibilityManager(
stripeSubscribedPlans,
iapSubscribedPlans,
targetPlan
);
return eligibilityManagerResult;
} catch (err) {
throw error.internalValidationError(
'subscriptions.getPlanEligibility',
{},
err
);
}
}
}
// TODO: will be removed in FXA-8918
const stripeEligibilityResult = await this.eligibilityFromStripeMetadata(
stripeSubscribedPlans,
iapSubscribedPlans,
@ -361,11 +394,12 @@ export class CapabilityService {
);
});
}
} catch (error) {
this.log.error('subscriptions.getPlanEligibility', { error: error });
Sentry.captureException(error);
} catch (err) {
this.log.error('subscriptions.getPlanEligibility', { error: err });
Sentry.captureException(err);
}
return stripeEligibilityResult;
// END TODO: will be removed in FXA-8918
}
/**
@ -764,6 +798,7 @@ export class CapabilityService {
/**
* Fetch the list of capabilities for the given plan ids from Stripe.
*/
// TODO: will be removed in FXA-8918
private async planIdsToClientCapabilitiesFromStripe(
subscribedPrices: string[]
): Promise<ClientIdCapabilityMap> {
@ -800,6 +835,7 @@ export class CapabilityService {
/**
* Retrieve the client capabilities from Stripe
*/
// TODO: will be removed in FXA-8918
async getClientsFromStripe() {
let result: ClientIdCapabilityMap = {};
@ -856,6 +892,32 @@ export class CapabilityService {
* Retrieve the client capabilities
*/
async getClients() {
const contentfulEnabled = this.config.contentful.enabled;
if (contentfulEnabled) {
if (!this.capabilityManager) {
throw error.internalValidationError(
'getClients',
{},
new Error('CapabilityManager not found.')
);
} else {
try {
const clientsFromContentful =
await this.capabilityManager.getClients();
return clientsFromContentful;
} catch (err) {
throw error.internalValidationError(
'subscriptions.getClients',
{},
err
);
}
}
}
// TODO: will be removed in FXA-8918
const clientsFromStripe = await this.getClientsFromStripe();
if (!this.capabilityManager) {
@ -892,11 +954,12 @@ export class CapabilityService {
);
});
}
} catch (error) {
this.log.error('subscriptions.getClients', { error: error });
Sentry.captureException(error);
} catch (err) {
this.log.error('subscriptions.getClients', { error: err });
Sentry.captureException(err);
}
return clientsFromStripe;
// END TODO: will be removed in FXA-8918
}
/**
@ -905,6 +968,34 @@ export class CapabilityService {
async planIdsToClientCapabilities(
subscribedPrices: string[]
): Promise<ClientIdCapabilityMap> {
const contentfulEnabled = this.config.contentful.enabled;
if (contentfulEnabled) {
if (!this.capabilityManager) {
throw error.internalValidationError(
'planIdsToClientCapabilities',
{},
new Error('CapabilityManager not found.')
);
} else {
try {
const contentfulCapabilities =
await this.capabilityManager.planIdsToClientCapabilities(
subscribedPrices
);
return contentfulCapabilities;
} catch (err) {
throw error.internalValidationError(
'subscriptions.planIdsToClientCapabilities',
{},
err
);
}
}
}
// TODO: will be removed in FXA-8918
const stripeCapabilities = await this.planIdsToClientCapabilitiesFromStripe(
subscribedPrices
);
@ -950,13 +1041,14 @@ export class CapabilityService {
'error' as SeverityLevel
);
});
} catch (error) {
} catch (err) {
this.log.error('subscriptions.planIdsToClientCapabilities', {
error: error,
error: err,
});
Sentry.captureException(error);
Sentry.captureException(err);
}
return stripeCapabilities;
// END TODO: will be removed in FXA-8918
}
}

Просмотреть файл

@ -4,6 +4,7 @@
'use strict';
const config = require('../../../config').default.getProperties();
const sinon = require('sinon');
const assert = { ...sinon.assert, ...require('chai').assert };
const { Container } = require('typedi');
@ -14,7 +15,7 @@ const {
mockPlans,
mockContentfulPlanIdsToClientCapabilities,
} = require('../../mocks');
const { AuthLogger } = require('../../../lib/types');
const { AppConfig, AuthLogger } = require('../../../lib/types');
const { StripeHelper } = require('../../../lib/payments/stripe');
const { PlayBilling } = require('../../../lib/payments/iap/google-play');
const { AppleIAP } = require('../../../lib/payments/iap/apple-app-store');
@ -94,6 +95,7 @@ describe('CapabilityService', () => {
let mockPaymentConfigManager;
let mockConfigPlans;
let mockCapabilityManager;
let mockConfig;
beforeEach(async () => {
mockAuthEvents.on = sinon.fake.returns({});
@ -170,7 +172,10 @@ describe('CapabilityService', () => {
mockContentfulPlanIdsToClientCapabilities
),
};
mockConfig = {...config, contentful: { enabled: false }};
log = mockLog();
Container.set(AppConfig, mockConfig);
Container.set(AuthLogger, log);
Container.set(StripeHelper, mockStripeHelper);
Container.set(PlayBilling, mockPlayBilling);
@ -1343,4 +1348,54 @@ describe('CapabilityService', () => {
});
});
});
describe('Contentful flag is enabled', () => {
it('returns planIdsToClientCapabilities from Contentful', async () => {
mockConfig.contentful.enabled = true;
capabilityService.subscribedPriceIds = sinon.fake.resolves([
UID,
]);
const mockContentfulCapabilities =
await mockCapabilityManager.planIdsToClientCapabilities(
capabilityService.subscribedPrices
);
const expected = {
'*': [ 'capAll' ],
c1: [ 'capZZ', 'cap4', 'cap5', 'capAlpha' ],
c2: [ 'cap5', 'cap6', 'capC', 'capD' ],
c3: [ 'capD', 'capE' ]
};
assert.deepEqual(mockContentfulCapabilities, expected);
});
it('returns getClients from Contentful', async () => {
mockConfig.contentful.enabled = true;
const mockClientsFromContentful =
await mockCapabilityManager.getClients();
const expected = [
{
capabilities: ['exampleCap0', 'exampleCap1', 'exampleCap3'],
clientId: 'client1',
},
{
capabilities: [
'exampleCap0',
'exampleCap2',
'exampleCap4',
'exampleCap5',
'exampleCap6',
'exampleCap7',
],
clientId: 'client2',
},
];
assert.deepEqual(mockClientsFromContentful, expected);
});
});
});

Просмотреть файл

@ -146,6 +146,9 @@ const mockConfig = {
stripeTaxRatesCacheTtlSeconds: 60,
},
currenciesToCountries: { ZAR: ['AS', 'CA'] },
contentful: {
enabled: false
},
};
const mockRedisConfig = {
@ -3453,6 +3456,36 @@ describe('#integration - StripeHelper', () => {
sinon.assert.calledOnce(Sentry.withScope);
sinon.assert.calledOnce(sentryScope.setContext);
});
it('returns Contentful values when flag is enabled', async () => {
// enable flag
mockConfig.contentful.enabled = true;
// set container
const mockContentfulManager = {
getPurchaseWithDetailsOfferingContentByPlanIds: sinon.fake.resolves(),
};
Container.set(ContentfulManager, mockContentfulManager);
// set stripeHelper
const stripeHelper = new StripeHelper(log, mockConfig, mockStatsd);
// set sandbox and spies
sandbox
.stub(stripeHelper.stripe.plans, 'list')
.returns(asyncIterable([plan1, plan2, plan3]));
sandbox.spy(stripeHelper, 'allPlans');
sandbox.spy(stripeHelper, 'allConfiguredPlans');
// test method
await stripeHelper.allAbbrevPlans();
// test that flag is enabled and all spies called
assert.isTrue(mockConfig.contentful.enabled);
assert(stripeHelper.allConfiguredPlans.calledOnce);
assert(stripeHelper.allPlans.calledOnce);
assert(stripeHelper.stripe.plans.list.calledOnce);
});
});
describe('fetchProductById', () => {

Просмотреть файл

@ -94,6 +94,9 @@ export type StripeHelperConfig = {
plansCacheTtlSeconds: number;
};
redis: any; // TODO
contentful: {
enabled: boolean;
};
};
/**
@ -475,7 +478,8 @@ export abstract class StripeHelper {
const validPlansMapped =
await contentfulToStripeMapper.mapContentfulToStripePlans(
validPlans,
acceptLanguage
acceptLanguage,
this.config.contentful.enabled
);
validPlansFinal.push(...validPlansMapped.mappedPlans);
@ -536,7 +540,7 @@ export abstract class StripeHelper {
*/
async expandResource<T>(
resource: string | T,
resourceType: (typeof VALID_RESOURCE_TYPES)[number],
resourceType: typeof VALID_RESOURCE_TYPES[number],
statusFilter?: Stripe.Subscription.Status[]
): Promise<T> {
if (typeof resource !== 'string') {