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

Use APP_URL env var in assets URI #3621

Closed
wants to merge 1 commit into from
Closed

Conversation

nalysius
Copy link
Contributor

Cachet may be installed at the root of its vhost or in a subdirectory,
for example in "/" or under "/status".

The URI we found pointing to some assets were usually not prefixed with
the APP_URL value, so if Cachet was installed under "/status" the asset
URI pointed to the server root like "/my-asset.js".
It is a problem because that means the behaviour is broken in this case.
The problem was present from the setup, it was not possible to fill the
setup since the path to the scripts and CSS weres wrong.

The content of the APP_URL environment variable is now read and used
in the assets URI.

See: #3618

Cachet may be installed at the root of its vhost or in a subdirectory,
for example in "/" or under "/status".

The URI we found pointing to some assets were usually not prefixed with
the APP_URL value, so if Cachet was installed under "/status" the asset
URI pointed to the server root like "/my-asset.js".
It is a problem because that means the behaviour is broken in this case.
The problem was present from the setup, it was not possible to fill the
setup since the path to the scripts and CSS weres wrong.

The content of the APP_URL environment variable is now read and used
in the assets URI.

See: cachethq#3618
@jbrooksuk
Copy link
Member

I believe we should be using the asset helper, instead :)

@nalysius
Copy link
Contributor Author

I'd forgotten this helper in Laravel. You can close this PR so, I'll check that and do it using asset. Sorry :)

@jbrooksuk jbrooksuk closed this May 27, 2019
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.

None yet

2 participants