From 0ee8097a16c937d53c39800bf54705355a5c61fc Mon Sep 17 00:00:00 2001 From: Ahmad Abdolsaheb Date: Fri, 13 Sep 2024 12:43:00 +0300 Subject: [PATCH] feat: move session related challenge data to sessionStorage (#55918) Co-authored-by: sembauke Co-authored-by: Oliver Eyton-Williams --- client/src/redux/action-types.js | 1 - client/src/redux/actions.js | 4 +- client/src/redux/donation-saga.js | 13 +- client/src/redux/donation-saga.test.js | 8 +- client/src/redux/index.js | 17 +-- client/src/redux/selectors.js | 27 ++-- .../src/redux/session-completed-challenges.js | 11 ++ client/src/redux/types.ts | 2 - client/src/utils/session-storage.test.ts | 134 ++++++++++++++++++ client/src/utils/session-storage.ts | 52 +++++++ 10 files changed, 225 insertions(+), 44 deletions(-) create mode 100644 client/src/redux/session-completed-challenges.js create mode 100644 client/src/utils/session-storage.test.ts create mode 100644 client/src/utils/session-storage.ts diff --git a/client/src/redux/action-types.js b/client/src/redux/action-types.js index 319fcfb2589ced..ecf2cd649e467a 100644 --- a/client/src/redux/action-types.js +++ b/client/src/redux/action-types.js @@ -9,7 +9,6 @@ export const actionTypes = createTypes( 'allowBlockDonationRequests', 'setRenderStartTime', 'preventBlockDonationRequests', - 'setCompletionCountWhenShownProgressModal', 'setShowMultipleProgressModals', 'openDonationModal', 'closeDonationModal', diff --git a/client/src/redux/actions.js b/client/src/redux/actions.js index 1d6e0f843e7b1a..78188582dc52b7 100644 --- a/client/src/redux/actions.js +++ b/client/src/redux/actions.js @@ -17,9 +17,7 @@ export const openDonationModal = createAction(actionTypes.openDonationModal); export const preventBlockDonationRequests = createAction( actionTypes.preventBlockDonationRequests ); -export const setCompletionCountWhenShownProgressModal = createAction( - actionTypes.setCompletionCountWhenShownProgressModal -); + export const setShowMultipleProgressModals = createAction( actionTypes.setShowMultipleProgressModals ); diff --git a/client/src/redux/donation-saga.js b/client/src/redux/donation-saga.js index 2361324e2f3977..6b227ab44e256d 100644 --- a/client/src/redux/donation-saga.js +++ b/client/src/redux/donation-saga.js @@ -18,6 +18,10 @@ import { import { stringifyDonationEvents } from '../utils/analytics-strings'; import { stripe } from '../utils/stripe'; import { PaymentProvider } from '../../../shared/config/donation-settings'; +import { + getSessionChallengeData, + saveCurrentCount +} from '../utils/session-storage'; import { actionTypes as appTypes } from './action-types'; import { openDonationModal, @@ -25,7 +29,6 @@ import { postChargeProcessing, postChargeError, preventBlockDonationRequests, - setCompletionCountWhenShownProgressModal, updateCardError, updateCardRedirecting } from './actions'; @@ -34,7 +37,6 @@ import { recentlyClaimedBlockSelector, shouldRequestDonationSelector, isSignedInSelector, - completionCountSelector, completedChallengesSelector } from './selectors'; @@ -56,7 +58,7 @@ function* showDonateModalSaga() { if (recentlyClaimedBlock) { yield put(preventBlockDonationRequests()); } else { - yield put(setCompletionCountWhenShownProgressModal()); + yield call(saveCurrentCount); } } } @@ -124,9 +126,8 @@ export function* postChargeSaga({ }); } else { const completedChallenges = yield select(completedChallengesSelector); - const completedChallengesInSession = yield select( - completionCountSelector - ); + const sessionChallengeData = yield call(getSessionChallengeData); + const completedChallengesInSession = sessionChallengeData.currentCount; yield call(callGA, { event: 'donation', action: stringifyDonationEvents(paymentContext, paymentProvider), diff --git a/client/src/redux/donation-saga.test.js b/client/src/redux/donation-saga.test.js index f2e8053e19f184..3bb437107e10b9 100644 --- a/client/src/redux/donation-saga.test.js +++ b/client/src/redux/donation-saga.test.js @@ -50,7 +50,6 @@ const analyticsDataMock = { const signedInStoreMock = { app: { appUsername: 'devuser', - completionCount: 2, user: { devuser: { completedChallenges: [ @@ -83,7 +82,6 @@ const signedInStoreMock = { const signedOutStoreMock = { app: { appUsername: '', - completionCount: 0, user: { '': { completedChallenges: [] @@ -94,6 +92,9 @@ const signedOutStoreMock = { describe('donation-saga', () => { it('calls postChargeStrip for Stripe', () => { + // The number of completed challenges per session is stored in the session storage + sessionStorage.setItem('session-completed-challenges', '2'); + return expectSaga(postChargeSaga, postChargeDataMock) .withState(signedInStoreMock) .put(postChargeProcessing()) @@ -149,6 +150,8 @@ describe('donation-saga', () => { payload: { ...postChargeDataMock.payload, paymentProvider: 'paypal' } }; + sessionStorage.setItem('session-completed-challenges', '0'); + const paypalAnalyticsDataMock = { ...analyticsDataMock, action: 'Donate Page Paypal Payment Submission', @@ -160,7 +163,6 @@ describe('donation-saga', () => { const signedOutStoreMock = { app: { appUsername: '', - completionCount: 0, user: { '': { completedChallenges: [] diff --git a/client/src/redux/index.js b/client/src/redux/index.js index 2720b42186ccf4..a5290c55eb7077 100644 --- a/client/src/redux/index.js +++ b/client/src/redux/index.js @@ -15,13 +15,14 @@ import { createFetchUserSaga } from './fetch-user-saga'; import hardGoToEpic from './hard-go-to-epic'; import { createReportUserSaga } from './report-user-saga'; import { createSaveChallengeSaga } from './save-challenge-saga'; -import { completionCountSelector, savedChallengesSelector } from './selectors'; +import { savedChallengesSelector } from './selectors'; import { actionTypes as settingsTypes } from './settings/action-types'; import { createShowCertSaga } from './show-cert-saga'; import updateCompleteEpic from './update-complete-epic'; import { createUserTokenSaga } from './user-token-saga'; import { createMsUsernameSaga } from './ms-username-saga'; import { createSurveySaga } from './survey-saga'; +import { createSessionCompletedChallengesSaga } from './session-completed-challenges'; const defaultFetchState = { pending: true, @@ -51,9 +52,6 @@ const initialState = { appUsername: '', showMultipleProgressModals: false, recentlyClaimedBlock: null, - completionCountWhenShownProgressModal: 0, - progressDonationModalShown: false, - completionCount: 0, currentChallengeId: store.get(CURRENT_CHALLENGE_KEY), examInProgress: false, isProcessing: false, @@ -97,7 +95,8 @@ export const sagas = [ ...createUserTokenSaga(actionTypes), ...createSaveChallengeSaga(actionTypes), ...createMsUsernameSaga(actionTypes), - ...createSurveySaga(actionTypes) + ...createSurveySaga(actionTypes), + ...createSessionCompletedChallengesSaga(actionTypes) ]; function spreadThePayloadOnUser(state, payload) { @@ -274,13 +273,6 @@ export const reducer = handleActions( ...state, recentlyClaimedBlock: null }), - [actionTypes.setCompletionCountWhenShownProgressModal]: state => ({ - ...state, - progressDonationModalShown: true, - completionCountWhenShownProgressModal: completionCountSelector({ - [MainApp]: state - }) - }), [actionTypes.setShowMultipleProgressModals]: (state, { payload }) => ({ ...state, showMultipleProgressModals: payload @@ -346,7 +338,6 @@ export const reducer = handleActions( } : { ...state, - completionCount: state.completionCount + 1, user: { ...state.user, [appUsername]: { diff --git a/client/src/redux/selectors.js b/client/src/redux/selectors.js index 4f56facbbf3ea4..c8a56981a69363 100644 --- a/client/src/redux/selectors.js +++ b/client/src/redux/selectors.js @@ -1,4 +1,5 @@ import { Certification } from '../../../shared/config/certification-settings'; +import { getSessionChallengeData } from '../utils/session-storage'; import { ns as MainApp } from './action-types'; export const savedChallengesSelector = state => @@ -13,10 +14,6 @@ export const currentChallengeIdSelector = state => export const completionCountSelector = state => state[MainApp].completionCount; export const showMultipleProgressModalsSelector = state => state[MainApp].showMultipleProgressModals; -export const completionCountWhenShownProgressModalSelector = state => - state[MainApp].completionCountWhenShownProgressModal; -export const progressDonationModalShownSelector = state => - state[MainApp].progressDonationModalShown; export const isDonatingSelector = state => userSelector(state).isDonating; export const isOnlineSelector = state => state[MainApp].isOnline; export const isServerOnlineSelector = state => state[MainApp].isServerOnline; @@ -36,11 +33,7 @@ export const showCertSelector = state => state[MainApp].showCert; export const showCertFetchStateSelector = state => state[MainApp].showCertFetchState; export const shouldRequestDonationSelector = state => { - const completedChallengesLength = completedChallengesSelector(state).length; - const completionCount = completionCountSelector(state); - const lastCompletionCount = - completionCountWhenShownProgressModalSelector(state); - const progressDonationModalShown = progressDonationModalShownSelector(state); + const completedChallengeCount = completedChallengesSelector(state).length; const isDonating = isDonatingSelector(state); const recentlyClaimedBlock = recentlyClaimedBlockSelector(state); @@ -50,22 +43,24 @@ export const shouldRequestDonationSelector = state => { // a block has been completed if (recentlyClaimedBlock) return true; + const sessionChallengeData = getSessionChallengeData(); /* Different intervals need to be tested for optimization. */ - if (progressDonationModalShown && completionCount - lastCompletionCount >= 20) - return true; - - // a donation has already been requested - if (progressDonationModalShown) return false; + // the assumption is that we save the count when we request donations + if (sessionChallengeData.isSaved) { + // only request if sufficient challenges have been completed since last + // request + return sessionChallengeData.countSinceSave >= 20; + } // donations only appear after the user has completed ten challenges (i.e. // not before the 11th challenge has mounted) - if (completedChallengesLength < 10) return false; + if (completedChallengeCount < 10) return false; // this will mean we have completed 3 or more challenges this browser session // and enough challenges overall to not be new - return completionCount >= 3; + return sessionChallengeData.currentCount >= 3; }; export const userTokenSelector = state => { diff --git a/client/src/redux/session-completed-challenges.js b/client/src/redux/session-completed-challenges.js new file mode 100644 index 00000000000000..8cbe111d39eaba --- /dev/null +++ b/client/src/redux/session-completed-challenges.js @@ -0,0 +1,11 @@ +import { takeEvery, call } from 'redux-saga/effects'; + +import { incrementCurrentCount } from '../utils/session-storage'; + +function* SessionCompletedChallengesSaga() { + yield call(incrementCurrentCount); +} + +export function createSessionCompletedChallengesSaga(types) { + return [takeEvery(types.submitComplete, SessionCompletedChallengesSaga)]; +} diff --git a/client/src/redux/types.ts b/client/src/redux/types.ts index 244fde893149aa..67475a3c4ecf9e 100644 --- a/client/src/redux/types.ts +++ b/client/src/redux/types.ts @@ -14,9 +14,7 @@ export interface State { [MainApp]: { appUsername: string; recentlyClaimedBlock: null | string; - completionCountWhenShownProgressModal: number | null; showMultipleProgressModals: boolean; - completionCount: number; currentChallengId: string; showCert: Record; showCertFetchState: DefaultFetchState; diff --git a/client/src/utils/session-storage.test.ts b/client/src/utils/session-storage.test.ts new file mode 100644 index 00000000000000..ad3c4b2283c7b0 --- /dev/null +++ b/client/src/utils/session-storage.test.ts @@ -0,0 +1,134 @@ +/* eslint-disable @typescript-eslint/unbound-method */ +import { + CURRENT_COUNT_KEY, + SAVED_COUNT_KEY, + getSessionChallengeData, + incrementCurrentCount, + saveCurrentCount +} from './session-storage'; + +describe('Session Storage', () => { + let sessionStorageMock: Storage; + + beforeEach(() => { + sessionStorageMock = (() => { + let store: { [key: string]: string } = {}; + + const mockStorage: Storage = { + length: 0, + + clear: jest.fn(() => { + store = {}; + }), + + getItem: jest.fn((key: string) => { + return store[key] || null; + }), + + key: jest.fn((index: number) => { + const keys = Object.keys(store); + return keys[index] || null; + }), + + removeItem: jest.fn((key: string) => { + delete store[key]; + }), + + setItem: jest.fn((key: string, value: string) => { + store[key] = value; + }) + }; + + Object.defineProperty(mockStorage, 'length', { + get: () => Object.keys(store).length + }); + + return mockStorage; + })(); + + Object.defineProperty(window, 'sessionStorage', { + value: sessionStorageMock, + configurable: true + }); + }); + + afterEach(() => { + sessionStorage.clear(); + jest.clearAllMocks(); + }); + + describe('getSessionChallengeData', () => { + describe('countSinceSave', () => { + it('is not included if nothing has been saved', () => { + expect(getSessionChallengeData()).not.toHaveProperty('countSinceSave'); + }); + + it('is included if the count has been saved', () => { + sessionStorage.setItem(SAVED_COUNT_KEY, '7'); + sessionStorage.setItem(CURRENT_COUNT_KEY, '10'); + expect(getSessionChallengeData()).toMatchObject({ + countSinceSave: 3 + }); + }); + }); + + describe('currentCount', () => { + it('defaults to 0 if no challenges have been completed', () => { + expect(getSessionChallengeData()).toMatchObject({ + currentCount: 0 + }); + }); + + it('returns the stored number if it exists', () => { + sessionStorage.setItem(CURRENT_COUNT_KEY, '5'); + expect(getSessionChallengeData()).toMatchObject({ + currentCount: 5 + }); + }); + }); + + describe('isSaved', () => { + it('is false if we haved saved the count', () => { + expect(getSessionChallengeData()).toMatchObject({ + isSaved: false + }); + }); + + it('is true if we have saved something', () => { + sessionStorage.setItem(SAVED_COUNT_KEY, '7'); + expect(getSessionChallengeData()).toMatchObject({ + isSaved: true + }); + }); + }); + }); + + describe('incrementCurrentCount', () => { + test('increments the completed challenge count', () => { + sessionStorage.setItem(CURRENT_COUNT_KEY, '5'); + incrementCurrentCount(); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + CURRENT_COUNT_KEY, + '6' + ); + expect(getSessionChallengeData()).toMatchObject({ currentCount: 6 }); + }); + + test('sets the count to 1 if no previous value exists', () => { + incrementCurrentCount(); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + CURRENT_COUNT_KEY, + '1' + ); + expect(getSessionChallengeData()).toMatchObject({ currentCount: 1 }); + }); + }); + + describe('saveCurrentCount', () => { + test('sets correct value in sessionStorage', () => { + sessionStorage.setItem(CURRENT_COUNT_KEY, '5'); + saveCurrentCount(); + expect(sessionStorage.setItem).toHaveBeenCalledWith(SAVED_COUNT_KEY, '5'); + }); + }); +}); diff --git a/client/src/utils/session-storage.ts b/client/src/utils/session-storage.ts new file mode 100644 index 00000000000000..c441d8a5276031 --- /dev/null +++ b/client/src/utils/session-storage.ts @@ -0,0 +1,52 @@ +const parseCount = (rawCount: string | null) => + rawCount == null ? 0 : parseInt(rawCount, 10); + +export const CURRENT_COUNT_KEY = 'session-completed-challenges'; +export const SAVED_COUNT_KEY = 'saved-session-completed-challenges'; + +const getCurrentCount = () => + parseCount(sessionStorage.getItem(CURRENT_COUNT_KEY)); + +const getSavedCount = () => parseCount(sessionStorage.getItem(SAVED_COUNT_KEY)); + +export const incrementCurrentCount = () => { + const newCount = getCurrentCount() + 1; + sessionStorage.setItem(CURRENT_COUNT_KEY, newCount.toString()); +}; + +/** + * Saves the current count so we can compare it later. + */ +export const saveCurrentCount = () => { + sessionStorage.setItem(SAVED_COUNT_KEY, getCurrentCount().toString()); +}; + +const isSaved = () => sessionStorage.getItem(SAVED_COUNT_KEY) !== null; + +type AfterSave = { + isSaved: true; + currentCount: number; + countSinceSave: number; +}; + +type BeforeSave = { + isSaved: false; + currentCount: number; +}; + +type SessionChallengeData = AfterSave | BeforeSave; + +export const getSessionChallengeData = (): SessionChallengeData => { + if (isSaved()) { + return { + isSaved: true, + currentCount: getCurrentCount(), + countSinceSave: getCurrentCount() - getSavedCount() + }; + } else { + return { + isSaved: false, + currentCount: getCurrentCount() + }; + } +};