Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OKTA 561098 : Reminder element rerender timing fix #3077

Merged
merged 11 commits into from
Feb 11, 2023
Merged

OKTA 561098 : Reminder element rerender timing fix #3077

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c65b8a9
refactored reminder prompt
leemchale-okta Feb 9, 2023
d86a0b4
updated spec test
leemchale-okta Feb 9, 2023
a1524b9
updated unit test
leemchale-okta Feb 9, 2023
028d06d
lint
leemchale-okta Feb 9, 2023
d35ae0a
updated integration test
leemchale-okta Feb 9, 2023
b095811
test
leemchale-okta Feb 9, 2023
0f0a690
test
leemchale-okta Feb 9, 2023
58f8fae
updated snapshot
leemchale-okta Feb 9, 2023
33ff33a
mocked integration tests session storage
leemchale-okta Feb 10, 2023
61187cc
lint
leemchale-okta Feb 10, 2023
ecf4ea2
refactored integration tests
leemchale-okta Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/v3/src/components/ReminderPrompt/ReminderPrompt.test.tsx
View file
Open in desktop
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason setSystemTime does not exist on the jest type

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to update @types/jest version to match the jest version. This should solve all of the type errors that we're ignoring @ts-expect-error

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
View file
Open in desktop
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when you omit the window part.

Based on light research, window.clearInterval and clearInterval are equivalent so you should be able to leave off the window part

}

setShow(false);

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

timerRef.current = window.setTimeout(() => setShow(true), timeout);
timerRef.current = window.setInterval(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, can you try omitting the window part and see what happens? It seems like it isn't necessary and they are equivalent.

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
2 changes: 1 addition & 1 deletion ...test/integration/__snapshots__/authenticator-reset-password-revoke-sessions.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ exports[`authenticator-reset-password-revoke-sessions should render form 1`] = `
<span
class="MuiTypography-root MuiTypography-body1 MuiFormControlLabel-label emotion-94"
>
Sign me out of all other devices.
leemchale-okta marked this conversation as resolved.
Show resolved Hide resolved
Sign me out of all other devices
</span>
</label>
</fieldset>
Expand Down
58 changes: 7 additions & 51 deletions src/v3/test/integration/__snapshots__/authenticator-verification-email.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,59 +288,15 @@ exports[`Email authenticator verification when email magic link = undefined rend
>
<div
class="MuiBox-root emotion-14"
>
<div
class="MuiBox-root emotion-14"
>
<div
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation0 MuiAlert-root MuiAlert-infoboxWarning MuiAlert-infobox emotion-16"
role="alert"
>
<div
class="MuiAlert-icon emotion-17"
>
<svg
aria-hidden="true"
class="MuiSvgIcon-root MuiSvgIcon-fontSizeInherit emotion-18"
fill="none"
focusable="false"
viewBox="0 0 16 16"
xmlns="http://www.w3.org/2000/svg"
>
<path
clip-rule="evenodd"
d="M13.983 14.6471C13.4706 15 12.6329 15 10.9575 15H5.04248C3.36707 15 2.52937 15 2.01699 14.6471C1.56939 14.3387 1.26655 13.8615 1.17816 13.3252C1.07698 12.7114 1.43367 11.9534 2.14706 10.4374L5.10455 4.15277C6.02878 2.18879 6.49089 1.2068 7.12576 0.898255C7.6777 0.630012 8.32225 0.630012 8.87419 0.898255C9.50906 1.2068 9.97117 2.18879 10.8954 4.15277L13.8529 10.4374C14.5663 11.9534 14.923 12.7114 14.8218 13.3252C14.7334 13.8615 14.4306 14.3387 13.983 14.6471ZM7.99998 10C7.72383 10 7.49998 9.77614 7.49998 9.5L7.49998 5H8.49998L8.49998 9.5C8.49998 9.77614 8.27612 10 7.99998 10ZM7.99998 13C8.14831 13 8.29332 12.956 8.41665 12.8736C8.53999 12.7912 8.63612 12.6741 8.69288 12.537C8.74965 12.4 8.7645 12.2492 8.73556 12.1037C8.70662 11.9582 8.6352 11.8246 8.53031 11.7197C8.42542 11.6148 8.29178 11.5434 8.14629 11.5144C8.00081 11.4855 7.85001 11.5003 7.71296 11.5571C7.57592 11.6139 7.45878 11.71 7.37637 11.8333C7.29396 11.9567 7.24998 12.1017 7.24998 12.25C7.24998 12.4489 7.32899 12.6397 7.46964 12.7803C7.6103 12.921 7.80106 13 7.99998 13Z"
fill="currentColor"
fill-rule="evenodd"
/>
</svg>
</div>
<div
class="MuiAlert-message emotion-19"
>
<div
class="MuiBox-root emotion-20"
>
Haven't received an email?
</div>
<a
class="MuiTypography-root MuiTypography-inherit MuiLink-root MuiLink-underlineAlways emotion-21"
href="javascript:void(0);"
>
Send again
</a>
</div>
</div>
</div>
</div>
leemchale-okta marked this conversation as resolved.
Show resolved Hide resolved
/>
<div
class="MuiBox-root emotion-14"
>
<div
class="MuiBox-root emotion-23"
class="MuiBox-root emotion-16"
>
<h2
class="MuiTypography-root MuiTypography-h4 emotion-24"
class="MuiTypography-root MuiTypography-h4 emotion-17"
data-se="o-form-head"
id="challenge-authenticator_Title_Verify_with_your_email_okta_email_eae1egri5eK5zce0V1d7_3_1"
>
Expand All @@ -352,10 +308,10 @@ exports[`Email authenticator verification when email magic link = undefined rend
class="MuiBox-root emotion-14"
>
<div
class="MuiBox-root emotion-23"
class="MuiBox-root emotion-16"
>
<p
class="MuiTypography-root MuiTypography-body1 MuiTypography-paragraph emotion-27"
class="MuiTypography-root MuiTypography-body1 MuiTypography-paragraph emotion-20"
data-se="o-form-explain"
id="challenge-authenticator_Description_We_sent_an_email_to_f****************e@examplecom_Click_the_verification_link_in_your_email_to_continue_or_enter_the_code_below_okta_email_eae1egri5eK5zce0V1d7_4_1"
>
Expand All @@ -368,7 +324,7 @@ exports[`Email authenticator verification when email magic link = undefined rend
>
<button
aria-describedby="challenge-authenticator_Title_Verify_with_your_email_okta_email_eae1egri5eK5zce0V1d7_3_1 challenge-authenticator_Description_We_sent_an_email_to_f****************e@examplecom_Click_the_verification_link_in_your_email_to_continue_or_enter_the_code_below_okta_email_eae1egri5eK5zce0V1d7_4_1"
class="MuiButtonBase-root MuiButton-root MuiButton-secondary MuiButton-secondaryPrimary MuiButton-sizeMedium MuiButton-secondarySizeMedium MuiButton-disableElevation MuiButton-fullWidth Mui-focusVisible MuiButton-root MuiButton-secondary MuiButton-secondaryPrimary MuiButton-sizeMedium MuiButton-secondarySizeMedium MuiButton-disableElevation MuiButton-fullWidth emotion-29"
class="MuiButtonBase-root MuiButton-root MuiButton-secondary MuiButton-secondaryPrimary MuiButton-sizeMedium MuiButton-secondarySizeMedium MuiButton-disableElevation MuiButton-fullWidth Mui-focusVisible MuiButton-root MuiButton-secondary MuiButton-secondaryPrimary MuiButton-sizeMedium MuiButton-secondarySizeMedium MuiButton-disableElevation MuiButton-fullWidth emotion-22"
tabindex="0"
type="button"
>
Expand All @@ -380,7 +336,7 @@ exports[`Email authenticator verification when email magic link = undefined rend
class="MuiBox-root emotion-14"
>
<a
class="MuiTypography-root MuiTypography-inherit MuiLink-root MuiLink-underlineAlways emotion-21"
class="MuiTypography-root MuiTypography-inherit MuiLink-root MuiLink-underlineAlways emotion-24"
data-se="cancel"
href="javascript:void(0)"
>
Expand Down