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

adding returnurl to logoff #13010

Merged
merged 4 commits into from
Dec 29, 2022
Merged

Conversation

DavidStania
Copy link
Contributor

Adding returnUrl to logoff.

@sebastienros
Copy link
Member

@hishamco did some changes

@hishamco
Copy link
Member

The build is failed, why there's two arguments instead of one?

@jtkech
Copy link
Member

jtkech commented Dec 29, 2022

@sebastienros added in the private RedirectToLocal() defined here our custom Redirect extension that uses .ToUriComponents() internally, but this extension is defined in OC.Mvc.Core that is not referenced by this project.

Hmm I would have used our LocalRedirect() extension instead because in RedirectToLocal() we test IsLocalUrl() but for the same reason for now we can't.

So need to use one of the mvc methods by using explicitly ToUriComponents().

Redirect(returnUrl.ToUriComponents());

Or I think as we know it is a local url.

LocalRedirect(returnUrl.ToUriComponents());

@jtkech
Copy link
Member

jtkech commented Dec 29, 2022

Also, not important but I don't think we need to use returnUrl = null normally if no returnUrl is passed the mvc binder let it to null.

@sebastienros
Copy link
Member

Did the change in GH directly, please fix the build and merge.

@jtkech
Copy link
Member

jtkech commented Dec 29, 2022

Okay will do

@jtkech jtkech merged commit 0514eed into OrchardCMS:main Dec 29, 2022
@DavidStania
Copy link
Contributor Author

Nice! Thank you all.

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.

4 participants