From 9eb06241f60535c2b3e6a7097697640caa4ec1cc Mon Sep 17 00:00:00 2001 From: "O.Bilenko" Date: Thu, 14 Feb 2019 16:51:37 +0100 Subject: [PATCH 1/2] PWA-1703 Wrong page tracked when user comes back after checkout --- libraries/common/reducers/router/index.js | 2 ++ libraries/common/streams/main.js | 1 + libraries/tracking/action-creators/index.js | 13 +++++++++- libraries/tracking/constants/index.js | 2 ++ libraries/tracking/helpers/index.js | 24 +---------------- libraries/tracking/helpers/index.spec.js | 26 ------------------- libraries/tracking/streams/app.js | 23 +++++++++++++++- libraries/tracking/streams/pages.js | 26 ++++++++++++++----- libraries/tracking/streams/pages.spec.js | 25 ++++++++++++------ libraries/tracking/subscriptions/checkout.js | 4 +-- .../tracking/subscriptions/checkout.spec.js | 4 --- libraries/tracking/subscriptions/setup.js | 9 ++++--- .../tracking/subscriptions/setup.spec.js | 5 ---- 13 files changed, 82 insertions(+), 82 deletions(-) delete mode 100644 libraries/tracking/helpers/index.spec.js diff --git a/libraries/common/reducers/router/index.js b/libraries/common/reducers/router/index.js index 520632621a..d72429fa0c 100644 --- a/libraries/common/reducers/router/index.js +++ b/libraries/common/reducers/router/index.js @@ -5,6 +5,7 @@ import { ROUTE_DID_LEAVE, ROUTE_WILL_LEAVE, ROUTE_DID_UPDATE, + ROUTE_DID_ENTER, } from '../../constants/ActionTypes'; const defaultState = { @@ -27,6 +28,7 @@ export default function router(state = defaultState, action) { }; } case ROUTE_WILL_ENTER: + case ROUTE_DID_ENTER: return { ...state, stack: Array.from(stack.getAll().values()), diff --git a/libraries/common/streams/main.js b/libraries/common/streams/main.js index 1e7081601b..7295d63658 100644 --- a/libraries/common/streams/main.js +++ b/libraries/common/streams/main.js @@ -10,6 +10,7 @@ import 'rxjs/add/operator/do'; import 'rxjs/add/operator/shareReplay'; import 'rxjs/add/operator/distinctUntilChanged'; import 'rxjs/add/operator/switchMap'; +import 'rxjs/add/operator/withLatestFrom'; import { mainSubject } from '../store/middelwares/streams'; /** diff --git a/libraries/tracking/action-creators/index.js b/libraries/tracking/action-creators/index.js index a06e1e1820..07f7d8d854 100644 --- a/libraries/tracking/action-creators/index.js +++ b/libraries/tracking/action-creators/index.js @@ -1,4 +1,7 @@ -import { PWA_DID_APPEAR } from '../constants'; +import { + PWA_DID_APPEAR, + PWA_DID_DISAPPEAR, +} from '../constants'; /** * Creates the dispatched PWA_DID_APPEAR action object. @@ -7,3 +10,11 @@ import { PWA_DID_APPEAR } from '../constants'; export const pwaDidAppear = () => ({ type: PWA_DID_APPEAR, }); + +/** + * Creates the dispatched PWA_DID_DISAPPEAR action + * @return {Object} The dispatched action object. + */ +export const pwaDidDisappear = () => ({ + type: PWA_DID_DISAPPEAR, +}); diff --git a/libraries/tracking/constants/index.js b/libraries/tracking/constants/index.js index 7fd786fcbb..ace33112bf 100644 --- a/libraries/tracking/constants/index.js +++ b/libraries/tracking/constants/index.js @@ -1,3 +1,5 @@ export const APP_EVENT_VIEW_DID_APPEAR = 'viewDidAppear'; export const APP_EVENT_VIEW_DID_DISAPPEAR = 'viewDidDisappear'; + export const PWA_DID_APPEAR = 'PWA_DID_APPEAR'; +export const PWA_DID_DISAPPEAR = 'PWA_DID_DISAPPEAR'; diff --git a/libraries/tracking/helpers/index.js b/libraries/tracking/helpers/index.js index 105a8f98ac..b060833304 100644 --- a/libraries/tracking/helpers/index.js +++ b/libraries/tracking/helpers/index.js @@ -1,7 +1,7 @@ import get from 'lodash/get'; import find from 'lodash/find'; import core from '@shopgate/tracking-core/core/Core'; -import { logger, hasSGJavaScriptBridge } from '@shopgate/pwa-core/helpers'; +import { logger } from '@shopgate/pwa-core/helpers'; /** * Converts a price to a formatted string. @@ -164,25 +164,3 @@ export const track = (eventName, data, state) => { return core; }; - -/** - * Initialize the visible state. When the PWA runs within a real app, it will be set to TRUE when - * the app sends the APP_EVENT_VIEW_DID_APPEAR event. Within a browser it needs to be set manually. - * Otherwise page tracking would be suprpessed. - */ -let pwaWebviewVisible = !hasSGJavaScriptBridge(); - -/** - * Sets the visible state of the PWA webview. - * It's used to determine, if a legacy page is currently active. - * @param {boolean} [value=true] Tells if the PWA currently visible. - */ -export const setPWAVisibleState = (value = true) => { - pwaWebviewVisible = value; -}; - -/** - * Checks if the PWA webview is currently visible. - * @return {boolean} - */ -export const isPWAVisible = () => pwaWebviewVisible; diff --git a/libraries/tracking/helpers/index.spec.js b/libraries/tracking/helpers/index.spec.js deleted file mode 100644 index 90fa716ffe..0000000000 --- a/libraries/tracking/helpers/index.spec.js +++ /dev/null @@ -1,26 +0,0 @@ -import { setPWAVisibleState, isPWAVisible } from './index'; - -describe('Tracking helpers', () => { - describe('isPWAVisible()', () => { - it('should return true when nothing was set before', () => { - expect(isPWAVisible()).toBe(true); - }); - - it('should return false when the state was set to false', () => { - setPWAVisibleState(false); - expect(isPWAVisible()).toBe(false); - }); - }); - - describe('setPWAVisibleState()', () => { - it('should set the state to false', () => { - setPWAVisibleState(false); - expect(isPWAVisible()).toBe(false); - }); - - it('should set the state to true', () => { - setPWAVisibleState(true); - expect(isPWAVisible()).toBe(true); - }); - }); -}); diff --git a/libraries/tracking/streams/app.js b/libraries/tracking/streams/app.js index 91f3932ac2..161dfaaba0 100644 --- a/libraries/tracking/streams/app.js +++ b/libraries/tracking/streams/app.js @@ -1,6 +1,9 @@ import { getCurrentRoute } from '@shopgate/pwa-common/selectors/router'; import { main$ } from '@shopgate/pwa-common/streams/main'; -import { PWA_DID_APPEAR } from '../constants'; +import { + PWA_DID_APPEAR, + PWA_DID_DISAPPEAR, +} from '../constants'; /** * Emits when the PWA appeared again after navigating back from a legacy page. @@ -14,3 +17,21 @@ export const pwaDidAppear$ = main$ route: getCurrentRoute(params.getState()), }, })); + +/** + * Emits when the PWA disappears + */ +export const pwaDidDisappear$ = main$ + .filter(({ action }) => action.type === PWA_DID_DISAPPEAR) + .map(params => ({ + ...params, + action: { + ...params.action, + route: getCurrentRoute(params.getState()), + }, + })); + +/** + * Emits when the PWA visibility changes + */ +export const pwaVisibility$ = pwaDidAppear$.merge(pwaDidDisappear$); diff --git a/libraries/tracking/streams/pages.js b/libraries/tracking/streams/pages.js index 0cb77f67ad..751c8d7eb0 100644 --- a/libraries/tracking/streams/pages.js +++ b/libraries/tracking/streams/pages.js @@ -1,4 +1,6 @@ import { routeDidEnter$ } from '@shopgate/pwa-common/streams/router'; +import { appWillStart$ } from '@shopgate/pwa-common/streams/app'; +import { APP_WILL_START } from '@shopgate/pwa-common/constants/ActionTypes'; import { SEARCH_PATH, SEARCH_FILTER_PATTERN, @@ -15,11 +17,14 @@ import { ITEM_WRITE_REVIEW_PATTERN, } from '@shopgate/pwa-common-commerce/product/constants'; import { FAVORITES_PATH } from '@shopgate/pwa-common-commerce/favorites/constants'; -import { pwaDidAppear$ } from './app'; -import { isPWAVisible } from '../helpers'; +import { + pwaDidAppear$, + pwaVisibility$, +} from './app'; +import { PWA_DID_APPEAR } from '../constants'; /** - * A blacklist of paths that should be tracked whithin their individual subscriptions. + * A blacklist of paths that should be tracked within their individual subscriptions. * @type {Array} */ export const blacklistedPatterns = [ @@ -37,9 +42,16 @@ export const blacklistedPatterns = [ ]; /** - * Emits when one of the tracked paths is entered except some special one. + * Route did enter and PWA is visible and route is not blacklisted + * @type {Rx.Observable<*[]>} + */ +const routeDidEnterForVisiblePwa$ = routeDidEnter$ + .withLatestFrom(appWillStart$.merge(pwaVisibility$)) + .filter(([, { action: { type } }]) => type === APP_WILL_START || type === PWA_DID_APPEAR) + .map(([routeDidEnter]) => routeDidEnter); + +/** + * Emits when one of the tracked paths is entered or pwa appeared */ -export const pagesAreReady$ = routeDidEnter$ - .filter(() => isPWAVisible()) - .merge(pwaDidAppear$) +export const pagesAreReady$ = routeDidEnterForVisiblePwa$.merge(pwaDidAppear$) .filter(({ action }) => !blacklistedPatterns.find(pattern => action.route.pattern === pattern)); diff --git a/libraries/tracking/streams/pages.spec.js b/libraries/tracking/streams/pages.spec.js index ced30fac01..0710a6f860 100644 --- a/libraries/tracking/streams/pages.spec.js +++ b/libraries/tracking/streams/pages.spec.js @@ -1,7 +1,10 @@ import { createMockStore } from '@shopgate/pwa-common/store'; import { routeDidEnter } from '@shopgate/pwa-common/action-creators/router'; -import { setPWAVisibleState } from '../helpers'; -import { pwaDidAppear } from '../action-creators'; +import { appWillStart } from '@shopgate/pwa-common/action-creators/app'; +import { + pwaDidAppear, + pwaDidDisappear, +} from '../action-creators'; import { blacklistedPatterns, pagesAreReady$ } from './pages'; let mockedPattern; @@ -34,16 +37,16 @@ describe('Page streams', () => { mockedPattern = ''; ({ dispatch } = createMockStore()); + // Simulate app is started + dispatch(appWillStart('/')); + pagesAreReadySubscriber = jest.fn(); pagesAreReady$.subscribe(pagesAreReadySubscriber); }); describe('pagesAreReady$', () => { - beforeEach(() => { - setPWAVisibleState(true); - }); - - it('should emit when a route is active which is not blacklisted', () => { + it('should emit when a route not blacklisted', () => { + dispatch(appWillStart('/')); dispatch(routeDidEnterWrapped('/somepath')); expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(1); }); @@ -54,9 +57,15 @@ describe('Page streams', () => { }); it('should not emit when a route is active, but the PWA is not visible', () => { - setPWAVisibleState(false); + // Simulate pwa is disappear + dispatch(pwaDidDisappear()); + dispatch(routeDidEnterWrapped('/somepath')); expect(pagesAreReadySubscriber).not.toHaveBeenCalled(); + + // Simulate pwa is appeared again + dispatch(pwaDidAppear()); + expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(1); }); }); diff --git a/libraries/tracking/subscriptions/checkout.js b/libraries/tracking/subscriptions/checkout.js index 360f61eeef..5376da25f0 100644 --- a/libraries/tracking/subscriptions/checkout.js +++ b/libraries/tracking/subscriptions/checkout.js @@ -2,7 +2,7 @@ import event from '@shopgate/pwa-core/classes/Event'; import analyticsSetCustomValues from '@shopgate/pwa-core/commands/analyticsSetCustomValues'; import { appDidStart$ } from '@shopgate/pwa-common/streams/app'; import getCart from '../selectors/cart'; -import { track, formatPurchaseData, setPWAVisibleState } from '../helpers'; +import { track, formatPurchaseData } from '../helpers'; import { checkoutDidEnter$ } from '../streams/checkout'; /** @@ -11,8 +11,6 @@ import { checkoutDidEnter$ } from '../streams/checkout'; */ export default function checkout(subscribe) { subscribe(checkoutDidEnter$, ({ getState }) => { - setPWAVisibleState(false); - const state = getState(); track('initiatedCheckout', { cart: getCart(state) }, state); diff --git a/libraries/tracking/subscriptions/checkout.spec.js b/libraries/tracking/subscriptions/checkout.spec.js index 42424f14c1..7def28e724 100644 --- a/libraries/tracking/subscriptions/checkout.spec.js +++ b/libraries/tracking/subscriptions/checkout.spec.js @@ -30,7 +30,6 @@ describe('Checkout subscriptions', () => { }); it('should call the expected commands', () => { - const setPWAVisibleStateSpy = jest.spyOn(helpers, 'setPWAVisibleState'); const trackSpy = jest.spyOn(helpers, 'track'); /** * Mocked getState function. @@ -45,9 +44,6 @@ describe('Checkout subscriptions', () => { { cart: getCart(getState()) }, getState() ); - - expect(setPWAVisibleStateSpy).toHaveBeenCalledTimes(1); - expect(setPWAVisibleStateSpy).toHaveBeenCalledWith(false); }); }); }); diff --git a/libraries/tracking/subscriptions/setup.js b/libraries/tracking/subscriptions/setup.js index 44b9cba8b5..ad26fc11dc 100644 --- a/libraries/tracking/subscriptions/setup.js +++ b/libraries/tracking/subscriptions/setup.js @@ -14,8 +14,10 @@ import { } from '@shopgate/pwa-common/streams/app'; import UnifiedPlugin from '@shopgate/tracking-core/plugins/trackers/Unified'; import { APP_EVENT_VIEW_DID_APPEAR, APP_EVENT_VIEW_DID_DISAPPEAR } from '../constants'; -import { pwaDidAppear } from '../action-creators'; -import { setPWAVisibleState } from '../helpers'; +import { + pwaDidAppear, + pwaDidDisappear, +} from '../action-creators'; /** * Setup tracking subscriptions. @@ -29,12 +31,11 @@ export default function setup(subscribe) { ]); event.addCallback(APP_EVENT_VIEW_DID_APPEAR, () => { - setPWAVisibleState(true); dispatch(pwaDidAppear()); }); event.addCallback(APP_EVENT_VIEW_DID_DISAPPEAR, () => { - setPWAVisibleState(false); + dispatch(pwaDidDisappear()); }); }); diff --git a/libraries/tracking/subscriptions/setup.spec.js b/libraries/tracking/subscriptions/setup.spec.js index b9ccc643d2..475b55769d 100644 --- a/libraries/tracking/subscriptions/setup.spec.js +++ b/libraries/tracking/subscriptions/setup.spec.js @@ -3,7 +3,6 @@ import registerEvents from '@shopgate/pwa-core/commands/registerEvents'; import { appWillStart$ } from '@shopgate/pwa-common/streams/app'; import { pwaDidAppear } from '../action-creators'; import { APP_EVENT_VIEW_DID_APPEAR, APP_EVENT_VIEW_DID_DISAPPEAR } from '../constants'; -import * as helpers from '../helpers'; import subscription from './setup'; jest.mock('@shopgate/pwa-core/classes/Event'); @@ -60,12 +59,8 @@ describe('setup subscriptions', () => { }); it('should dispatch pwaDidAppear when the app event is triggered', () => { - const setPWAVisibleStateSpy = jest.spyOn(helpers, 'setPWAVisibleState'); - callback({ dispatch }); event.call(APP_EVENT_VIEW_DID_APPEAR); - expect(setPWAVisibleStateSpy).toHaveBeenCalledTimes(1); - expect(setPWAVisibleStateSpy).toHaveBeenCalledWith(true); expect(dispatch).toHaveBeenCalledTimes(1); expect(dispatch).toHaveBeenCalledWith(pwaDidAppear()); From bf80fcb530b404b5de966b1fb86d9952fcc893c5 Mon Sep 17 00:00:00 2001 From: "O.Bilenko" Date: Fri, 15 Feb 2019 16:05:32 +0100 Subject: [PATCH 2/2] PWA-1703 Adjust tracking duplicates and checkout flow --- libraries/tracking/streams/checkout.js | 8 ++--- libraries/tracking/streams/pages.js | 19 +++++++--- libraries/tracking/streams/pages.spec.js | 44 +++++++++++++++++++----- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/libraries/tracking/streams/checkout.js b/libraries/tracking/streams/checkout.js index c91624d8a2..9d63df0876 100644 --- a/libraries/tracking/streams/checkout.js +++ b/libraries/tracking/streams/checkout.js @@ -3,6 +3,8 @@ import { navigate$ } from '@shopgate/pwa-common/streams/router'; import { isUserLoggedIn } from '@shopgate/pwa-common/selectors/user'; import { CHECKOUT_PATH } from '@shopgate/pwa-common/constants/RoutePaths'; +const actionsWl = [ACTION_PUSH, ACTION_REPLACE]; + export const checkoutDidEnter$ = navigate$ .filter(({ action, getState }) => { if (action.params.pathname !== CHECKOUT_PATH) { @@ -21,9 +23,5 @@ export const checkoutDidEnter$ = navigate$ * A checkout route can be pushed when a logged in user opens the checkout. It can also replace * the current route when a user is redirected from the login form after a successful login. */ - if ([ACTION_PUSH, ACTION_REPLACE].includes(navigateAction)) { - return true; - } - - return false; + return actionsWl.includes(navigateAction); }); diff --git a/libraries/tracking/streams/pages.js b/libraries/tracking/streams/pages.js index 751c8d7eb0..9003f0e374 100644 --- a/libraries/tracking/streams/pages.js +++ b/libraries/tracking/streams/pages.js @@ -17,11 +17,13 @@ import { ITEM_WRITE_REVIEW_PATTERN, } from '@shopgate/pwa-common-commerce/product/constants'; import { FAVORITES_PATH } from '@shopgate/pwa-common-commerce/favorites/constants'; +import { PWA_DID_APPEAR } from '../constants'; import { pwaDidAppear$, + pwaDidDisappear$, pwaVisibility$, } from './app'; -import { PWA_DID_APPEAR } from '../constants'; +import { checkoutDidEnter$ } from './checkout'; /** * A blacklist of paths that should be tracked within their individual subscriptions. @@ -41,17 +43,26 @@ export const blacklistedPatterns = [ ITEM_WRITE_REVIEW_PATTERN, ]; +const latestAppActions$ = appWillStart$.merge(pwaVisibility$, checkoutDidEnter$); + /** * Route did enter and PWA is visible and route is not blacklisted * @type {Rx.Observable<*[]>} */ const routeDidEnterForVisiblePwa$ = routeDidEnter$ - .withLatestFrom(appWillStart$.merge(pwaVisibility$)) + .withLatestFrom(latestAppActions$) .filter(([, { action: { type } }]) => type === APP_WILL_START || type === PWA_DID_APPEAR) .map(([routeDidEnter]) => routeDidEnter); /** - * Emits when one of the tracked paths is entered or pwa appeared + * PWA reappear after disappear + * @type {Rx.Observable} + */ +const pwaDidAppearAfterDisappear = pwaDidDisappear$.switchMap(() => pwaDidAppear$.first()); + +/** + * Emits when one of the tracked paths is entered or pwa reappear */ -export const pagesAreReady$ = routeDidEnterForVisiblePwa$.merge(pwaDidAppear$) +export const pagesAreReady$ = routeDidEnterForVisiblePwa$ + .merge(pwaDidAppearAfterDisappear) .filter(({ action }) => !blacklistedPatterns.find(pattern => action.route.pattern === pattern)); diff --git a/libraries/tracking/streams/pages.spec.js b/libraries/tracking/streams/pages.spec.js index 0710a6f860..8e8058753c 100644 --- a/libraries/tracking/streams/pages.spec.js +++ b/libraries/tracking/streams/pages.spec.js @@ -1,6 +1,10 @@ +import { combineReducers } from 'redux'; +import { ACTION_REPLACE } from '@virtuous/conductor'; import { createMockStore } from '@shopgate/pwa-common/store'; -import { routeDidEnter } from '@shopgate/pwa-common/action-creators/router'; +import user from '@shopgate/pwa-common/reducers/user'; +import { routeDidEnter, navigate } from '@shopgate/pwa-common/action-creators/router'; import { appWillStart } from '@shopgate/pwa-common/action-creators/app'; +import { CHECKOUT_PATH } from '@shopgate/pwa-common/constants/RoutePaths'; import { pwaDidAppear, pwaDidDisappear, @@ -35,18 +39,19 @@ describe('Page streams', () => { jest.clearAllMocks(); mockedPattern = ''; - ({ dispatch } = createMockStore()); - - // Simulate app is started - dispatch(appWillStart('/')); + ({ dispatch } = createMockStore(combineReducers({ user }))); pagesAreReadySubscriber = jest.fn(); pagesAreReady$.subscribe(pagesAreReadySubscriber); }); describe('pagesAreReady$', () => { - it('should emit when a route not blacklisted', () => { + beforeEach(() => { + // Simulate app is started dispatch(appWillStart('/')); + }); + + it('should emit when a route not blacklisted', () => { dispatch(routeDidEnterWrapped('/somepath')); expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(1); }); @@ -70,16 +75,39 @@ describe('Page streams', () => { }); describe('coming back from legacy pages', () => { - it('should emit when pwaDidAppear is dispatched and a not blacklisted route is active', () => { + beforeEach(() => { + // Simulate app is started + dispatch(appWillStart('/')); + }); + it('should emit when PWA is reappear and a not blacklisted route is active', () => { dispatch(routeDidEnterWrapped('/somepath')); + dispatch(pwaDidDisappear()); dispatch(pwaDidAppear()); - expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(1); + expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(2); }); it('should not emit when pwaDidAppear is dispatched and a blacklisted is active', () => { dispatch(routeDidEnterWrapped(blacklistedPatterns[0])); + dispatch(pwaDidDisappear()); dispatch(pwaDidAppear()); expect(pagesAreReadySubscriber).not.toHaveBeenCalled(); }); }); + + describe('go to checkout', () => { + beforeEach(() => { + // Simulate app is started + dispatch(appWillStart('/')); + }); + it('should not emit events when user goes to checkout', () => { + dispatch(routeDidEnterWrapped('/somepath')); + dispatch(navigate({ + action: ACTION_REPLACE, + pathname: CHECKOUT_PATH, + })); + // should not track this path + dispatch(routeDidEnterWrapped('/somepath2')); + expect(pagesAreReadySubscriber).toHaveBeenCalledTimes(1); + }); + }); });