-
Notifications
You must be signed in to change notification settings - Fork 43
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
Fix urls for root domains #1507
base: master
Are you sure you want to change the base?
Conversation
test plan 🧪
|
@Pralish I see the API surface for URL's and asset linking as changed, could you write some documentation to include as part of the release for server engineers to use as reference? |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
@Pralish could you please take a look at the review app? There are some major breakages that prevent UAT: Admin redirect broken:Screen.Recording.2023-05-02.at.7.37.24.AM.movpassword reset email link is broken |
Deployed review-app can be viewed at https://review-1507.violet-test.net |
We are familiar with the fact that we need to pass
host
and/orsubdomain
parameters in url helper for it to work properly, especially for asset urls.And we were passing either
Subdomain.current.hostname
orApartment::Tenant.current
as the subdomain, which is not consistent.Subdomain.current.hostname
returns nil for root domain, which is fine, butApartment::Tenant.current
returns 'public' which causes all the asset urls to be prefixed with 'public'rails_blob_url(attachment, subdomain: Apartment::Tenant.current, subdomain: Apartment::Tenant.current)
Issue: #1537
With this PR, we will no longer have to pass host and subdomain value to url helper. They are set by default based on the request host.
rails_blob_url(attachment)
You can pass the host and subdomain parameters if you want to override the default value