From 6aabe915e891bbbac5cc52658136d87ef21eddfd Mon Sep 17 00:00:00 2001 From: Matias Szylkowski Date: Tue, 21 Dec 2021 14:45:09 -0500 Subject: [PATCH 1/4] Added tasts --- .vscode/tasks.json | 70 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .vscode/tasks.json diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 000000000000..ea550d21e7a9 --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,70 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "amp: Run unit test in this file", + "detail": "amp unit --files=${file} --headless --verbose", + "type": "shell", + "command": "amp", + "args": ["unit", "--files=${file}", "--headless", "--verbose"], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": true + } + }, + { + "label": "amp: Watch unit test in this file", + "detail": "amp unit --files=${file} --headless --verbose --watch", + "type": "shell", + "command": "amp", + "args": ["unit", "--files=${file}", "--headless", "--verbose", "--watch"], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": true + }, + "isBackground": true + }, + { + "label": "amp: Run unit tests in all changed files", + "detail": "amp unit --local_changes --headless --verbose", + "type": "shell", + "command": "amp", + "args": ["unit", "--local_changes", "--headless", "--verbose"], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": true + } + }, + { + "label": "amp: Check PR", + "detail": "amp pr-check --nobuild", + "type": "shell", + "command": "amp", + "args": ["pr-check", "--nobuild"], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": true + } + }, + ] +} \ No newline at end of file From dc1fa876f8e029a54a7cfd4bf2f70795da071102 Mon Sep 17 00:00:00 2001 From: Matias Szylkowski Date: Tue, 21 Dec 2021 14:47:32 -0500 Subject: [PATCH 2/4] Undo --- .vscode/tasks.json | 70 ---------------------------------------------- 1 file changed, 70 deletions(-) delete mode 100644 .vscode/tasks.json diff --git a/.vscode/tasks.json b/.vscode/tasks.json deleted file mode 100644 index ea550d21e7a9..000000000000 --- a/.vscode/tasks.json +++ /dev/null @@ -1,70 +0,0 @@ -{ - "version": "2.0.0", - "tasks": [ - { - "label": "amp: Run unit test in this file", - "detail": "amp unit --files=${file} --headless --verbose", - "type": "shell", - "command": "amp", - "args": ["unit", "--files=${file}", "--headless", "--verbose"], - "group": "test", - "presentation": { - "echo": true, - "reveal": "always", - "focus": false, - "panel": "shared", - "showReuseMessage": true, - "clear": true - } - }, - { - "label": "amp: Watch unit test in this file", - "detail": "amp unit --files=${file} --headless --verbose --watch", - "type": "shell", - "command": "amp", - "args": ["unit", "--files=${file}", "--headless", "--verbose", "--watch"], - "group": "test", - "presentation": { - "echo": true, - "reveal": "always", - "focus": false, - "panel": "shared", - "showReuseMessage": true, - "clear": true - }, - "isBackground": true - }, - { - "label": "amp: Run unit tests in all changed files", - "detail": "amp unit --local_changes --headless --verbose", - "type": "shell", - "command": "amp", - "args": ["unit", "--local_changes", "--headless", "--verbose"], - "group": "test", - "presentation": { - "echo": true, - "reveal": "always", - "focus": false, - "panel": "shared", - "showReuseMessage": true, - "clear": true - } - }, - { - "label": "amp: Check PR", - "detail": "amp pr-check --nobuild", - "type": "shell", - "command": "amp", - "args": ["pr-check", "--nobuild"], - "group": "test", - "presentation": { - "echo": true, - "reveal": "always", - "focus": false, - "panel": "shared", - "showReuseMessage": true, - "clear": true - } - }, - ] -} \ No newline at end of file From d5c17dbd593d0de7b2b6014ccd6fccf1642ada2d Mon Sep 17 00:00:00 2001 From: Matias Szylkowski Date: Mon, 16 May 2022 17:50:51 -0400 Subject: [PATCH 3/4] Make subscriptions localize the template async --- .../0.1/amp-story-subscriptions.js | 129 +++++++++--------- .../0.1/test/test-amp-story-subscriptions.js | 10 +- 2 files changed, 73 insertions(+), 66 deletions(-) diff --git a/extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js b/extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js index 62c0a575fbcd..a5a1d05d395f 100644 --- a/extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js +++ b/extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js @@ -8,6 +8,8 @@ import {LocalizedStringId_Enum} from '#service/localization/strings'; import {dev, devAssert} from '#utils/log'; +import {localizeTemplate} from 'extensions/amp-story/1.0/amp-story-localization-service'; + import {CSS} from '../../../build/amp-story-subscriptions-0.1.css'; import { Action, @@ -43,9 +45,6 @@ export class AmpStorySubscriptions extends AMP.BaseElement { /** @private {?../../../extensions/amp-subscriptions/0.1/amp-subscriptions.SubscriptionService} */ this.subscriptionService_ = null; - /** @private {?../../../src/service/localization.LocalizationService} */ - this.localizationService_ = null; - /** @private {?../../../src/service/viewer-interface.ViewerInterface} */ this.viewer_ = null; @@ -64,50 +63,53 @@ export class AmpStorySubscriptions extends AMP.BaseElement { Services.storyStoreServiceForOrNull(this.win), Services.subscriptionsServiceForDoc(this.element), Services.localizationServiceForOrNull(this.element), - ]).then(([storeService, subscriptionService, localizationService]) => { - this.storeService_ = storeService; - this.subscriptionService_ = subscriptionService; - this.localizationService_ = localizationService; - - const pages = this.win.document.querySelectorAll( - 'amp-story-page:not([ad])' - ); - devAssert( - pages.length >= 4, - 'The number of pages should be at least 4 to enable subscriptions feature, got %s', - pages.length - ); + ]).then( + ([storeService, subscriptionService, unusedLocalizationService]) => { + this.storeService_ = storeService; + this.subscriptionService_ = subscriptionService; - let subscriptionsPageIndex = parseInt( - this.element.getAttribute('subscriptions-page-index'), - 10 - ); - subscriptionsPageIndex = isNaN(subscriptionsPageIndex) - ? DEFAULT_SUBSCRIPTIONS_PAGE_INDEX - : subscriptionsPageIndex; - this.storeService_.dispatch( - Action.SET_SUBSCRIPTIONS_PAGE_INDEX, - clamp( - subscriptionsPageIndex, - DEFAULT_SUBSCRIPTIONS_PAGE_INDEX, - pages.length - 1 - ) - ); + const pages = this.win.document.querySelectorAll( + 'amp-story-page:not([ad])' + ); + devAssert( + pages.length >= 4, + 'The number of pages should be at least 4 to enable subscriptions feature, got %s', + pages.length + ); - // Get grant status immediately to set up the initial subscriptions state. - this.getGrantStatusAndUpdateState_(); - // When the user finishes any of the actions, e.g. log in or subscribe, new entitlements would be - // re-fetched and this callback would be executed. Update states based on new entitlements. - this.subscriptionService_.addOnEntitlementResolvedCallback(() => - this.getGrantStatusAndUpdateState_() - ); + let subscriptionsPageIndex = parseInt( + this.element.getAttribute('subscriptions-page-index'), + 10 + ); + subscriptionsPageIndex = isNaN(subscriptionsPageIndex) + ? DEFAULT_SUBSCRIPTIONS_PAGE_INDEX + : subscriptionsPageIndex; + this.storeService_.dispatch( + Action.SET_SUBSCRIPTIONS_PAGE_INDEX, + clamp( + subscriptionsPageIndex, + DEFAULT_SUBSCRIPTIONS_PAGE_INDEX, + pages.length - 1 + ) + ); - // Create a paywall dialog element that have required attributes to be able to be - // rendered by amp-subscriptions. - this.element.appendChild(this.renderSubscriptionsDialogTemplate_()); + // Get grant status immediately to set up the initial subscriptions state. + this.getGrantStatusAndUpdateState_(); + // When the user finishes any of the actions, e.g. log in or subscribe, new entitlements would be + // re-fetched and this callback would be executed. Update states based on new entitlements. + this.subscriptionService_.addOnEntitlementResolvedCallback(() => + this.getGrantStatusAndUpdateState_() + ); - this.initializeListeners_(); - }); + // Create a paywall dialog element that have required attributes to be able to be + // rendered by amp-subscriptions. + const template = this.renderSubscriptionsDialogTemplate_(); + return localizeTemplate(template, this.element).then(() => { + this.element.appendChild(template); + this.initializeListeners_(); + }); + } + ); } /** @override */ @@ -244,11 +246,12 @@ export class AmpStorySubscriptions extends AMP.BaseElement { return (
-
@@ -274,9 +277,11 @@ export class AmpStorySubscriptions extends AMP.BaseElement { > - {this.localizationService_.getLocalizedString( - LocalizedStringId_Enum.AMP_STORY_SUBSCRIPTIONS_CTA - )} +   {getStoryAttributeSrc(this.element, 'publisher', /* warn */ true)} @@ -289,25 +294,27 @@ export class AmpStorySubscriptions extends AMP.BaseElement { subscriptions-decorate="false" > - - {this.localizationService_.getLocalizedString( + + } + >
diff --git a/extensions/amp-story-subscriptions/0.1/test/test-amp-story-subscriptions.js b/extensions/amp-story-subscriptions/0.1/test/test-amp-story-subscriptions.js index 81e23d4ce73a..f1945537d203 100644 --- a/extensions/amp-story-subscriptions/0.1/test/test-amp-story-subscriptions.js +++ b/extensions/amp-story-subscriptions/0.1/test/test-amp-story-subscriptions.js @@ -1,12 +1,13 @@ import * as Preact from '#core/dom/jsx'; import {Services} from '#service'; -import {LocalizationService} from '#service/localization'; import {afterRenderPromise} from '#testing/helpers'; +import {getLocalizationService} from 'extensions/amp-story/1.0/amp-story-localization-service'; import {AdvancementMode} from 'extensions/amp-story/1.0/story-analytics'; +import LocalizedStringsEn from '../../../amp-story/1.0/_locales/en.json' assert {type: 'json'}; // lgtm[js/syntax-error] import { Action, AmpStoryStoreService, @@ -45,10 +46,9 @@ describes.realWin( .returns(Promise.resolve(storeService)); env.sandbox.stub(Services, 'storyStoreService').returns(storeService); - const localizationService = new LocalizationService(doc.body); - env.sandbox - .stub(Services, 'localizationServiceForOrNull') - .returns(Promise.resolve(localizationService)); + getLocalizationService(doc.body).registerLocalizedStringBundles({ + 'en': LocalizedStringsEn, + }); const platformConfig = { 'services': [ From 2ad5bcb62d8bfb5e28929fce9ab56cad8f8257ac Mon Sep 17 00:00:00 2001 From: Matias Szylkowski Date: Mon, 16 May 2022 17:50:59 -0400 Subject: [PATCH 4/4] Add dep check --- build-system/test-configs/dep-check-config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/build-system/test-configs/dep-check-config.js b/build-system/test-configs/dep-check-config.js index 1539630eb5a3..d497c30665f0 100644 --- a/build-system/test-configs/dep-check-config.js +++ b/build-system/test-configs/dep-check-config.js @@ -283,6 +283,7 @@ exports.rules = [ 'extensions/amp-story-page-attachment/0.1/amp-story-form.js->extensions/amp-story/1.0/amp-story-localization-service.js', 'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/amp-story-localization-service.js', 'extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js->extensions/amp-story/1.0/amp-story-localization-service.js', + 'extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js->extensions/amp-story/1.0/amp-story-localization-service.js', // Subscriptions. 'extensions/amp-subscriptions/0.1/expr.js->extensions/amp-access/0.1/access-expr.js',