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

Split out redirect filters into separate methods #234

Merged
merged 3 commits into from
May 4, 2023
Merged

Conversation

earnjam
Copy link
Member

@earnjam earnjam commented May 4, 2023

The login_redirect filter and the newfold_sso_success_url filters accept differing numbers of parameters. They also require different ways of determining the current user. The former does not have $current_user set and the latter does not have a $user global variable.

To make them both work correctly, and avoid a lot of logic to check values and types of multiple variables, I've created separate methods for each hook that each establish the logged in user, and then pass the url and user on to a single filter_redirect method for evaluation.

Copy link
Member

@arunshenoy99 arunshenoy99 left a comment

Choose a reason for hiding this comment

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

Code looks fine, tested, and it works for the WordPress Login.

Copy link
Member

@circlecube circlecube left a comment

Choose a reason for hiding this comment

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

I like using unique callbacks for these scenarios. 👍

@earnjam earnjam merged commit c9a53da into trunk May 4, 2023
@earnjam earnjam deleted the login-sso-redirect branch May 4, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants