Skip to content

Commit

Permalink
C24_WMDE_Desktop_EN_04 - Fix timer (#601)
Browse files Browse the repository at this point in the history
- Replace window timer with injected timer
         Components that use timers are hard to test. This removed the window timers and replaces them with an interface
         that we can mock. Updates all tests and banners to use the new timer.
- Remove import comments in entry points We previously tried to separate the imports in the banner entry points into
         logical sections. This didn't work well with IDE auto adding them at the bottom each time a new object was added.
         It was also a little confusing as to what should go where. This removes the comments and embraces js import
         chaos.
Ticket: https://phabricator.wikimedia.org/T378589
  • Loading branch information
Sperling-0 authored and moiikana committed Nov 21, 2024
1 parent 2d2987d commit 5a90ddf
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 52 deletions.
6 changes: 2 additions & 4 deletions banners/english/C24_WMDE_Desktop_EN_04/banner_ctrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ import DynamicTextPlugin from '@src/DynamicTextPlugin';
import { LocalImpressionCount } from '@src/utils/LocalImpressionCount';
import { LegacyTrackerWPORG } from '@src/tracking/LegacyTrackerWPORG';
import { Locales } from '@src/domain/Locales';

// Locale-specific imports
import messages from './messages';
import { LocaleFactoryEn } from '@src/utils/LocaleFactory/LocaleFactoryEn';

// Channel specific form setup
import { createFormItems } from './form_items';
import { createFormActions } from '@src/createFormActions';
import eventMappings from './event_map';
import { createFallbackDonationURL } from '@src/createFallbackDonationURL';
import { WindowTimer } from '@src/utils/Timer';

const localeFactory = new LocaleFactoryEn();
const translator = new Translator( messages );
Expand Down Expand Up @@ -69,5 +66,6 @@ app.provide( 'currencyFormatter', currencyFormatter );
app.provide( 'formItems', createFormItems( translator, currencyFormatter.euroAmount.bind( currencyFormatter ) ) );
app.provide( 'formActions', createFormActions( page.getTracking(), impressionCount, { locale: Locales.EN, afo: '1' } ) );
app.provide( 'tracker', tracker );
app.provide( 'timer', new WindowTimer() );

app.mount( page.getBannerContainer() );
6 changes: 2 additions & 4 deletions banners/english/C24_WMDE_Desktop_EN_04/banner_var.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ import DynamicTextPlugin from '@src/DynamicTextPlugin';
import { LocalImpressionCount } from '@src/utils/LocalImpressionCount';
import { LegacyTrackerWPORG } from '@src/tracking/LegacyTrackerWPORG';
import { Locales } from '@src/domain/Locales';

// Locale-specific imports
import messages from './messages';
import { LocaleFactoryEn } from '@src/utils/LocaleFactory/LocaleFactoryEn';

// Channel specific form setup
import { createFormItems } from './form_items';
import { createFormActions } from '@src/createFormActions';
import eventMappings from './event_map';
import { createFallbackDonationURL } from '@src/createFallbackDonationURL';
import { WindowTimer } from '@src/utils/Timer';

const localeFactory = new LocaleFactoryEn();
const translator = new Translator( messages );
Expand Down Expand Up @@ -69,5 +66,6 @@ app.provide( 'currencyFormatter', currencyFormatter );
app.provide( 'formItems', createFormItems( translator, currencyFormatter.euroAmount.bind( currencyFormatter ) ) );
app.provide( 'formActions', createFormActions( page.getTracking(), impressionCount, { locale: Locales.EN, afo: '1' } ) );
app.provide( 'tracker', tracker );
app.provide( 'timer', new WindowTimer() );

app.mount( page.getBannerContainer() );
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, test, vi } from 'vitest';
import { beforeEach, describe, test } from 'vitest';
import { mount, VueWrapper } from '@vue/test-utils';
import Banner from '@banners/english/C24_WMDE_Desktop_EN_04/components/BannerCtrl.vue';
import { BannerStates } from '@src/components/BannerConductor/StateMachine/BannerStates';
Expand All @@ -7,12 +7,7 @@ import { useOfFundsContent } from '@test/banners/useOfFundsContent';
import { formItems } from '@test/banners/formItems';
import { CurrencyEn } from '@src/utils/DynamicContent/formatters/CurrencyEn';
import { useOfFundsFeatures } from '@test/features/UseOfFunds';
import {
bannerContentAnimatedTextFeatures,
bannerContentDateAndTimeFeatures,
bannerContentDisplaySwitchFeatures,
bannerContentFeatures
} from '@test/features/BannerContent';
import { bannerContentAnimatedTextFeatures, bannerContentDateAndTimeFeatures, bannerContentDisplaySwitchFeatures, bannerContentFeatures } from '@test/features/BannerContent';
import { TrackerStub } from '@test/fixtures/TrackerStub';
import { donationFormFeatures } from '@test/features/forms/MainDonation_UpgradeToYearlyButton';
import { useFormModel } from '@src/components/composables/useFormModel';
Expand All @@ -21,6 +16,8 @@ import { bannerAutoHideFeatures, bannerMainFeatures } from '@test/features/MainB
import { DynamicContent } from '@src/utils/DynamicContent/DynamicContent';
import { alreadyDonatedModalFeatures } from '@test/features/AlreadyDonatedModal';
import { softCloseFeatures } from '@test/features/SoftCloseDesktop';
import { Timer } from '@src/utils/Timer';
import { TimerStub } from '@test/fixtures/TimerStub';

const formModel = useFormModel();
const translator = ( key: string ): string => key;
Expand All @@ -29,15 +26,9 @@ describe( 'BannerCtrl.vue', () => {

beforeEach( () => {
resetFormModel( formModel );
vi.useFakeTimers();
} );

afterEach( () => {
vi.restoreAllMocks();
vi.useRealTimers();
} );

const getWrapper = ( dynamicContent: DynamicContent = null ): VueWrapper<any> => {
const getWrapper = ( dynamicContent: DynamicContent = null, timer: Timer = null ): VueWrapper<any> => {
return mount( Banner, {
attachTo: document.body,
props: {
Expand All @@ -55,7 +46,8 @@ describe( 'BannerCtrl.vue', () => {
formActions: { donateWithAddressAction: 'https://example.com', donateWithoutAddressAction: 'https://example.com' },
currencyFormatter: new CurrencyEn(),
formItems,
tracker: new TrackerStub()
tracker: new TrackerStub(),
timer: timer ?? new TimerStub()
}
}
} );
Expand Down Expand Up @@ -126,7 +118,7 @@ describe( 'BannerCtrl.vue', () => {
[ 'expectEmitsBannerContentChangedOnSoftClose' ],
[ 'expectDoesNotShowSoftCloseOnFinalBannerImpression' ]
] )( '%s', async ( testName: string ) => {
await softCloseFeatures[ testName ]( getWrapper() );
await softCloseFeatures[ testName ]( getWrapper );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, test, vi } from 'vitest';
import { beforeEach, describe, test } from 'vitest';
import { mount, VueWrapper } from '@vue/test-utils';
import Banner from '@banners/english/C24_WMDE_Desktop_EN_04/components/BannerVar.vue';
import { BannerStates } from '@src/components/BannerConductor/StateMachine/BannerStates';
Expand All @@ -7,12 +7,7 @@ import { useOfFundsContent } from '@test/banners/useOfFundsContent';
import { formItems } from '@test/banners/formItems';
import { CurrencyEn } from '@src/utils/DynamicContent/formatters/CurrencyEn';
import { useOfFundsFeatures } from '@test/features/UseOfFunds';
import {
bannerContentAnimatedTextFeatures,
bannerContentDateAndTimeFeatures,
bannerContentDisplaySwitchFeatures,
bannerContentFeatures
} from '@test/features/BannerContent';
import { bannerContentAnimatedTextFeatures, bannerContentDateAndTimeFeatures, bannerContentDisplaySwitchFeatures, bannerContentFeatures } from '@test/features/BannerContent';
import { TrackerStub } from '@test/fixtures/TrackerStub';
import { donationFormFeatures } from '@test/features/forms/MainDonation_UpgradeToYearlyButton';
import { useFormModel } from '@src/components/composables/useFormModel';
Expand All @@ -21,6 +16,8 @@ import { bannerAutoHideFeatures, bannerMainFeatures } from '@test/features/MainB
import { DynamicContent } from '@src/utils/DynamicContent/DynamicContent';
import { alreadyDonatedModalFeatures } from '@test/features/AlreadyDonatedModal';
import { softCloseFeatures } from '@test/features/SoftCloseDesktop';
import { Timer } from '@src/utils/Timer';
import { TimerStub } from '@test/fixtures/TimerStub';

const formModel = useFormModel();
const translator = ( key: string ): string => key;
Expand All @@ -29,15 +26,9 @@ describe( 'BannerVar.vue', () => {

beforeEach( () => {
resetFormModel( formModel );
vi.useFakeTimers();
} );

afterEach( () => {
vi.restoreAllMocks();
vi.useRealTimers();
} );

const getWrapper = ( dynamicContent: DynamicContent = null ): VueWrapper<any> => {
const getWrapper = ( dynamicContent: DynamicContent = null, timer: Timer = null ): VueWrapper<any> => {
return mount( Banner, {
attachTo: document.body,
props: {
Expand All @@ -55,7 +46,8 @@ describe( 'BannerVar.vue', () => {
formActions: { donateWithAddressAction: 'https://example.com', donateWithoutAddressAction: 'https://example.com' },
currencyFormatter: new CurrencyEn(),
formItems,
tracker: new TrackerStub()
tracker: new TrackerStub(),
timer: timer ?? new TimerStub()
}
}
} );
Expand Down Expand Up @@ -126,7 +118,7 @@ describe( 'BannerVar.vue', () => {
[ 'expectEmitsBannerContentChangedOnSoftClose' ],
[ 'expectDoesNotShowSoftCloseOnFinalBannerImpression' ]
] )( '%s', async ( testName: string ) => {
await softCloseFeatures[ testName ]( getWrapper() );
await softCloseFeatures[ testName ]( getWrapper );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, test, vi } from 'vitest';
import { describe, test } from 'vitest';
import { mount, VueWrapper } from '@vue/test-utils';
import FallbackBanner from '@banners/english/C24_WMDE_Desktop_EN_04/components/FallbackBanner.vue';
import { BannerStates } from '@src/components/BannerConductor/StateMachine/BannerStates';
Expand All @@ -8,20 +8,13 @@ import { DynamicContent } from '@src/utils/DynamicContent/DynamicContent';
import { Tracker } from '@src/tracking/Tracker';
import { TrackerStub } from '@test/fixtures/TrackerStub';
import { dynamicContentFeatures, fallbackBannerFeatures, submitFeatures } from '@test/features/FallbackBanner';
import { TimerStub } from '@test/fixtures/TimerStub';
import { Timer } from '@src/utils/Timer';

const translator = ( key: string ): string => key;

describe( 'FallbackBanner.vue', () => {

beforeEach( () => {
vi.useFakeTimers();
} );

afterEach( () => {
vi.useRealTimers();
} );

const getWrapperAtWidth = ( width: number, dynamicContent: DynamicContent = null, tracker: Tracker = null ): VueWrapper<any> => {
const getWrapperAtWidth = ( width: number, dynamicContent: DynamicContent = null, tracker: Tracker = null, timer: Timer = null ): VueWrapper<any> => {
Object.defineProperty( window, 'innerWidth', { writable: true, configurable: true, value: width } );
return mount( FallbackBanner, {
props: {
Expand All @@ -36,7 +29,8 @@ describe( 'FallbackBanner.vue', () => {
provide: {
translator: { translate: translator },
dynamicCampaignText: dynamicContent ?? newDynamicContent(),
tracker: tracker ?? new TrackerStub()
tracker: tracker ?? new TrackerStub(),
timer: timer ?? new TimerStub()
}
}
} );
Expand Down

0 comments on commit 5a90ddf

Please sign in to comment.