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

[GH-43] Use SiteUrl for calls from the webapp #58

Closed

Conversation

sudiptog81
Copy link

@sudiptog81 sudiptog81 commented Oct 23, 2020

Summary

Added getServerRoute() and updated the Client to get SiteURL from the client configuration in the state to support subpath deployments.

Ticket Link

Fixes #43.

@mattermod
Copy link
Contributor

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.

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Oct 23, 2020
Copy link
Contributor

@mickmister mickmister left a 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

webapp/src/client/client.js Outdated Show resolved Hide resolved
Copy link
Contributor

@mickmister mickmister left a 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!

Copy link
Contributor

@marianunez marianunez left a 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!

@marianunez marianunez removed the 2: Dev Review Requires review by a core committer label Oct 30, 2020
@DHaussermann
Copy link

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?

Screen Shot 2020-11-06 at 1 49 02 PM

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

@jasonblais
Copy link
Contributor

@sudiptog81 let us know if you have any comments or feedback to @DHaussermann's question above :)

@larkox
Copy link
Contributor

larkox commented Jan 13, 2021

Hi @sudiptog81, do you think you could keep working on this issue?

@sudiptog81
Copy link
Author

I'm pre-occupied with some coursework, closing this, sorry for not making it :)

@jasonblais
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use SiteURL for all calls from the webapp
8 participants