-
Notifications
You must be signed in to change notification settings - Fork 23
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
[GH-43] Use SiteUrl for calls from the webapp #58
[GH-43] Use SiteUrl for calls from the webapp #58
Conversation
Hello @sudiptog81, Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sudiptog81 Thanks for your contribution 👍 I have one request for changing where we set the siteURL, otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks for the contribution @sudiptog81!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks @sudiptog81!
Hi @sudiptog81 thank you for adding the subpath support on this 🎉 I ran into snag testing this on a subpath server. I'm not able to configure the groups via the admin side UI. Searching for users or teams will return a 404 that seems like it's missing the subdomain. This seems like the very issue you've resolved. Is it possible we need the solution to also be applied to the admin UI or could something else be happening? |
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
@sudiptog81 let us know if you have any comments or feedback to @DHaussermann's question above :) |
Hi @sudiptog81, do you think you could keep working on this issue? |
I'm pre-occupied with some coursework, closing this, sorry for not making it :) |
Thanks for letting us know, hope to see you around the community in the future :) Big thanks for your help here and getting the work started! |
Summary
Added
getServerRoute()
and updated the Client to getSiteURL
from the client configuration in the state to support subpath deployments.Ticket Link
Fixes #43.