Skip to content

Commit

Permalink
OKTA 561098 : Reminder element rerender timing fix (#3077)
Browse files Browse the repository at this point in the history
OKTA-561098 refactored reminder prompt
updated spec test
updated unit test
lint
updated integration test
test
test
updated snapshot
mocked integration tests session storage
lint
refactored integration tests
  • Loading branch information
leemchale-okta committed Feb 11, 2023
1 parent b1d67fd commit d54f3c4
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 41 deletions.
16 changes: 11 additions & 5 deletions src/v3/src/components/ReminderPrompt/ReminderPrompt.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import {
} from '../../types';
import ReminderPrompt, { DEFAULT_TIMEOUT_MS } from './ReminderPrompt';

jest.useFakeTimers();
// @ts-expect-error Expected 0 arguments, but got 1
jest.useFakeTimers('modern');

const mockSubmitHook = jest.fn().mockImplementation(() => ({}));
jest.mock('../../hooks', () => ({
Expand Down Expand Up @@ -58,9 +59,10 @@ describe('ReminderPrompt', () => {
const { container, getByRole } = render(<ReminderPrompt {...props} />);

expect(container.firstChild).toBeNull();

act(() => {
jest.advanceTimersByTime(DEFAULT_TIMEOUT_MS);
// @ts-expect-error setSystemTime does not exist on type 'typeof jest'
jest.setSystemTime(Date.now() + DEFAULT_TIMEOUT_MS);
jest.runOnlyPendingTimers();
});

expect(container.firstChild).not.toBeNull();
Expand Down Expand Up @@ -101,7 +103,9 @@ describe('ReminderPrompt', () => {
expect(container.firstChild).toBeNull();

act(() => {
jest.advanceTimersByTime(DEFAULT_TIMEOUT_MS);
// @ts-expect-error setSystemTime does not exist on type 'typeof jest'
jest.setSystemTime(Date.now() + DEFAULT_TIMEOUT_MS);
jest.runOnlyPendingTimers();
});

expect(container.firstChild).not.toBeNull();
Expand Down Expand Up @@ -143,7 +147,9 @@ describe('ReminderPrompt', () => {
expect(container).toMatchSnapshot();

act(() => {
jest.advanceTimersByTime(CUSTOM_TIMEOUT);
// @ts-expect-error setSystemTime does not exist on type 'typeof jest'
jest.setSystemTime(Date.now() + CUSTOM_TIMEOUT);
jest.runOnlyPendingTimers();
});

expect(container.firstChild).not.toBeNull();
Expand Down
38 changes: 31 additions & 7 deletions src/v3/src/components/ReminderPrompt/ReminderPrompt.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,47 @@ const ReminderPrompt: UISchemaElementComponent<{
const [show, setShow] = useState<boolean>(false);
const timerRef = useRef<number | undefined>();

const RESEND_TIMESTAMP_SESSION_STORAGE_KEY = 'osw-oie-resend-timestamp';
const removeResendTimestamp = () => {
sessionStorage.removeItem(RESEND_TIMESTAMP_SESSION_STORAGE_KEY);
};
const setResendTimestamp = (token: string) => {
sessionStorage.setItem(RESEND_TIMESTAMP_SESSION_STORAGE_KEY, token);
};

// eslint-disable-next-line arrow-body-style
const getResendTimestamp = (): string | null => {
return sessionStorage.getItem(RESEND_TIMESTAMP_SESSION_STORAGE_KEY);
};

const startTimer = () => {
setShow(false);
const timeStamp = getResendTimestamp();
if (!timeStamp) {
setResendTimestamp(Date.now().toString());
}
if (timerRef) {
window.clearTimeout(timerRef.current);
window.clearInterval(timerRef.current);
}

setShow(false);

const timeout = typeof customTimeout === 'number' ? customTimeout : DEFAULT_TIMEOUT_MS;

timerRef.current = window.setTimeout(() => setShow(true), timeout);
timerRef.current = window.setInterval(() => {
const start = parseInt(getResendTimestamp() as string);
const now = Date.now();
const timeout = typeof customTimeout === 'number' ? customTimeout : DEFAULT_TIMEOUT_MS;
if (now - start >= timeout) {
setShow(true);
window.clearInterval(timerRef.current);
removeResendTimestamp();
}
}, 250);
};

useEffect(() => {
startTimer();

return () => {
window.clearTimeout(timerRef.current);
window.clearInterval(timerRef.current);
removeResendTimestamp();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down
41 changes: 28 additions & 13 deletions src/v3/test/integration/authenticator-verification-email.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* See the License for the specific language governing permissions and limitations under the License.
*/

import { act } from 'preact/test-utils';
import { waitFor, within } from '@testing-library/preact';
import { createAuthJsPayloadArgs, setup } from './util';

Expand All @@ -20,8 +19,24 @@ import sessionExpiredResponse from '../../src/mocks/response/idp/idx/identify/er

describe('Email authenticator verification when email magic link = undefined', () => {
describe('renders correct form', () => {
let mockSystemTime: number;

beforeEach(() => {
// Mock system time for triggering resend email reminder element
mockSystemTime = 1676068045456;
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => mockSystemTime);
jest.useFakeTimers();
// sessionStorage 'get' method is mocked for the ReminderPrompts start timestamp variable
jest.spyOn(global, 'sessionStorage', 'get').mockReturnValue({
length: 0,
clear: () => jest.fn(),
getItem: () => '1676068045456',
setItem: () => jest.fn(),
key: () => null,
removeItem: () => jest.fn(),
});
});

afterEach(() => {
Expand Down Expand Up @@ -65,9 +80,9 @@ describe('Email authenticator verification when email magic link = undefined', (
'button', { name: 'Enter a code from the email instead' },
) as HTMLButtonElement;
await waitFor(() => expect(codeEntryBtn).toHaveFocus());
act(() => {
jest.advanceTimersByTime(31_000);
});
// Advance system time to show resend email reminder element
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);
expect(container).toMatchSnapshot();
});

Expand Down Expand Up @@ -99,9 +114,9 @@ describe('Email authenticator verification when email magic link = undefined', (
await user.click(await findByText(/Enter a code from the email instead/));
await findByText(/Enter Code/);

act(() => {
jest.advanceTimersByTime(31_000);
});
// Advance system time to show resend email reminder element
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);
await findByText(/Haven't received an email?/);

const codeEle = await findByTestId('credentials.passcode') as HTMLInputElement;
Expand All @@ -114,9 +129,9 @@ describe('Email authenticator verification when email magic link = undefined', (
expect(queryByText(/Haven't received an email?/)).toBeNull();
await findByText(/We found some errors./);

act(() => {
jest.advanceTimersByTime(31_000);
});
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);

// after delay, reminder should be displayed as well as global error
await findByText(/We found some errors./);
await findByText(/Haven't received an email?/);
Expand Down Expand Up @@ -163,9 +178,9 @@ describe('Email authenticator verification when email magic link = undefined', (
...createAuthJsPayloadArgs('POST', 'idp/idx/challenge/poll'),
);

act(() => {
jest.advanceTimersByTime(31_000);
});
// Advance system time to show resend email reminder element
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);
await findByText(/Haven't received an email?/);
// render invalid otp message
const codeEle = await findByTestId('credentials.passcode') as HTMLInputElement;
Expand Down
29 changes: 22 additions & 7 deletions src/v3/test/integration/flow-okta-verify-enrollment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
*/

import { HttpRequestClient } from '@okta/okta-auth-js';
import { act } from 'preact/test-utils';
import { waitFor } from '@testing-library/preact';
import { createAuthJsPayloadArgs, setup, updateStateHandleInMock } from './util';
import qrPollingResponse from '../../src/mocks/response/idp/idx/credential/enroll/enroll-okta-verify-mfa.json';
Expand Down Expand Up @@ -75,8 +74,24 @@ const createTestContext = async () => {
};

describe('flow-okta-verify-enrollment', () => {
let mockSystemTime: number;

beforeEach(() => {
// Mock system time for triggering resend email reminder element
mockSystemTime = 1676068045456;
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => mockSystemTime);
jest.useFakeTimers();
// sessionStorage 'get' method is mocked for the ReminderPrompts start timestamp variable
jest.spyOn(global, 'sessionStorage', 'get').mockReturnValue({
length: 0,
clear: () => jest.fn(),
getItem: () => '1676068045456',
setItem: () => jest.fn(),
key: () => null,
removeItem: () => jest.fn(),
});
});

afterEach(() => {
Expand Down Expand Up @@ -135,9 +150,9 @@ describe('flow-okta-verify-enrollment', () => {
// email polling
await findByText(/Check your email/);

act(() => {
jest.advanceTimersByTime(31_000);
});
// Advance system time to show resend email reminder element
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);
expect(container).toMatchSnapshot();
await user.click(await findByText(/try a different way/));
expect(authClient.options.httpRequestClient).toHaveBeenCalledWith(
Expand Down Expand Up @@ -223,9 +238,9 @@ describe('flow-okta-verify-enrollment', () => {

// sms polling
await findByText(/Check your text messages/);
act(() => {
jest.advanceTimersByTime(31_000);
});
// Advance system time to show resend email reminder element
mockSystemTime += 31_000;
jest.advanceTimersByTime(500);
expect(container).toMatchSnapshot();
await user.click(await findByText(/try a different way/));
expect(authClient.options.httpRequestClient).toHaveBeenCalledWith(
Expand Down
12 changes: 3 additions & 9 deletions test/testcafe/spec/EnrollAuthenticatorEmail_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,7 @@ test
await t.expect(lastRequestUrl).eql('http://localhost:3000/idp/idx/challenge/resend');
});

// Test fails in v3. After re-render we still have to wait for 30 seconds
// Enable after fixing - OKTA-561098
test.meta('v3', false)
test
.requestHooks(logger, validOTPmock)('resend after 30 seconds at most even after re-render', async t => {
const enrollEmailPageObject = await setup(t);
await t.expect(await enrollEmailPageObject.resendEmailExists()).eql(false);
Expand All @@ -445,9 +443,7 @@ test.meta('v3', false)
await t.expect(enrollEmailPageObject.resendEmailText()).contains('Haven\'t received an email?');
});

// Test fails in v3. After re-render we still have to wait for 30 seconds
// Enable after fixing - OKTA-561098
test.meta('v3', false)
test
.requestHooks(logger, validOTPmockWithEmailMagicLink)('resend after 30 seconds at most even after re-render', async t => {
const enrollEmailPageObject = await setup(t);
await t.expect(await enrollEmailPageObject.resendEmailExists()).eql(false);
Expand All @@ -458,9 +454,7 @@ test.meta('v3', false)
await t.expect(enrollEmailPageObject.resendEmailText()).contains('Haven\'t received an email?');
});

// Test fails in v3. After re-render we still have to wait for 30 seconds
// Enable after fixing - OKTA-561098
test.meta('v3', false)
test
.requestHooks(logger, validOTPmockWithoutEmailMagicLink)('resend after 30 seconds at most even after re-render', async t => {
const enrollEmailPageObject = await setup(t);
await t.expect(await enrollEmailPageObject.resendEmailExists()).eql(false);
Expand Down

0 comments on commit d54f3c4

Please sign in to comment.