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.

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

Handle revalidation error in a single place #199

Merged
merged 5 commits into from
Jun 9, 2023

Conversation

renintw
Copy link
Contributor

@renintw renintw commented May 30, 2023

Fixes #187 (comment)

Testing Step

  1. Go to account-status.js and comment out disabled={ ! backupCodesEnabled } on L78.
  2. Go to wp-content/plugins/two-factor/class-two-factor-core.php and remove the ! for the if condition on L1209.
    => if ( self::current_user_can_update_two_factor_options( 'save' ) ) {
  3. Comment out the following snippet in /wp-content/plugins/wporg-two-factor/settings/src/components/backup-codes.js
if ( ! totpEnabled ) {
	const currentUrl = new URL( document.location.href );
	currentUrl.searchParams.set( 'screen', 'account-status' );
	window.history.pushState( {}, '', currentUrl );
	setScreen( 'account-status' );
	return;
}
  1. You will see the Modal popup when visiting /?screen=backup-codes. If you want to observe the same behavior as in the clip below, please use sandbox and make sure you have had 2fa set up, and then do 2. above.
Screen.Capture.on.2023-05-30.at.23-43-23.mp4

TODO

If this method works, pulling up all the setError and error in child components to script.js

@renintw renintw requested review from iandunn and StevenDufresne May 30, 2023 15:53
@renintw renintw self-assigned this May 30, 2023
@renintw renintw added this to the Iteration 1 milestone May 30, 2023
@renintw renintw added enhancement New feature or request ui Related to user interface labels May 30, 2023
@StevenDufresne StevenDufresne requested a review from adamwoodnz May 31, 2023 01:27
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

This works for me, worth trying to use setError everywhere I think. We might need to take care of clearing the error centrally when navigating like we do with the globalNotice

@renintw renintw force-pushed the fix/unauthorized-url-access branch from 6a70eca to a940f64 Compare May 31, 2023 14:54
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch from fb30ddd to cb63d73 Compare May 31, 2023 15:04
Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

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

Looks good :)

@renintw
Copy link
Contributor Author

renintw commented Jun 1, 2023

We might need to take care of clearing the error centrally when navigating like we do with the globalNotice

Good catch! a setError should be added there. Added in 494358f.

Note: Should be merged after #189.

@renintw renintw force-pushed the fix/unauthorized-url-access branch from a940f64 to dba3e95 Compare June 6, 2023 19:37
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch from a3582a0 to 516f972 Compare June 6, 2023 20:44
@renintw renintw force-pushed the fix/unauthorized-url-access branch 2 times, most recently from 0b7b675 to fac049f Compare June 6, 2023 22:50
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch 2 times, most recently from 32139c3 to 494358f Compare June 6, 2023 23:22
@renintw renintw force-pushed the fix/unauthorized-url-access branch from fac049f to 1e76b9c Compare June 8, 2023 11:53
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch from 494358f to 8cca504 Compare June 8, 2023 11:54
@renintw renintw force-pushed the fix/unauthorized-url-access branch from 1e76b9c to 8011dd2 Compare June 8, 2023 19:47
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch from 8cca504 to 5add378 Compare June 9, 2023 14:38
Base automatically changed from fix/unauthorized-url-access to trunk June 9, 2023 14:39
@renintw renintw force-pushed the add/handle-revalidation-error-in-a-single-place branch from 5add378 to 7107688 Compare June 9, 2023 14:41
@renintw renintw merged commit 0f6c716 into trunk Jun 9, 2023
@renintw renintw deleted the add/handle-revalidation-error-in-a-single-place branch June 9, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ui Related to user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants