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

Added filter to allow for changing the output of die_on_login #593

Merged
merged 1 commit into from
Dec 5, 2018

Conversation

coperator
Copy link

@coperator coperator commented Nov 30, 2018

Motivation

Error messages during login should be shown in a helpful way to the enduser. We want to add further information to the wp-auth0 error message, which are specific for our setup. The filter allows a modification of the shown html, similar to how it is already done for the email verification prompt.

Changes

  • Changed method die_on_login in class WP_Auth0_LoginManager to apply a filter on the html output.

Testing

No tests added & modified.

  • I included manual testing steps above, if applicable
  • This change adds unit test coverage
  • This change has been tested on the latest version of the platform/language or why not

Checklist

@joshcanhelp joshcanhelp added this to the 3.9.0 milestone Nov 30, 2018
@joshcanhelp
Copy link
Contributor

Thank you @coperator, this looks great!

Would you mind writing a few unit tests for this? We already have a test suite for this class and you can see an example of a filter output test here (this is also the suite to add it in):

https://github.com/auth0/wp-auth0/blob/master/tests/testLoginManager.php#L65

... and an example of checking that the hook is actually there:

https://github.com/auth0/wp-auth0/blob/master/tests/testProfileChangePassword.php#L76

You'll need to add the HookHelpers trait to the TestLoginManager class.

Not required to merge it but will save some trouble later.

Thanks again!

@coperator
Copy link
Author

coperator commented Dec 5, 2018 via email

@joshcanhelp
Copy link
Contributor

No problem @coperator.I'll approve and merge this now so we have it for the next release. If you can come back and put through a PR for the tests later, that would be appreciated.

Copy link
Contributor

@joshcanhelp joshcanhelp left a comment

Choose a reason for hiding this comment

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

LGTM!

@joshcanhelp joshcanhelp merged commit fab5038 into auth0:master Dec 5, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants