Add root to all static asset paths that mis it #5229
Conversation
Static assets are already prefixed with the root denomination in most of Redash. There are just a few places where these paths are missing. Normally this doesn't really matter if Redash is used like most people do with the Open Source edition. We however had to toggle the organizations feature on (using REDASH_MULTI_ORG), at which point a prefixed organisation "name-slug" is added before all page paths (http://localhost:5000/org_a/). Now the static assets without root path were suddenly expected to be in our organizations respective slug-namespace. (http://localhost:5000/org_a/static/images/db-logos/postgres.png for example). This Commit (and pull request) updates the 6 cases of "wrongly routed" asset paths back to "/".
HendrikPetertje
changed the title
|
Hey there, @kravets-levko & @arikfr, what is the status of reviewing this? I can continue to stay checked out on my own fork over on our EC3 instance for a while, but I'd much rather skip the overhead of having to merge in in future updates to my fork and instead just track your Github version without the asset bug ;) |
|
@HendrikPetertje your change breaks other things. I don't mind making the base path configurable, so the default remains |
|
I've been trying to figure out what would break, the only thing I've come up with so far where it breaks is if the entire Redash application sits in a sub-path (example.org/redash/...), But I needed to do some pretty awful unconventional stuff in NGinx to actually get there. I figure that anyone doing such in NGinx would need to add extra location paths to fix asset locations anyway. Is this really a problem and would you be able to point me to what would actually break (if I'm overlooking things) if we merged this as is? If it is as broken as you say, then we should probably update most of the static JS & images paths that are already prefixed with a / (some of which I already mentioned above)
|
What type of PR is this? (check all applicable)
Description
Static assets are already prefixed with the root denomination in most of
Redash. There are just a few places where these paths are missing.
Normally this doesn't really matter if Redash is used like most people
do with the Open Source edition.
We however had to toggle the organizations feature on (using REDASH_MULTI_ORG),
at which point a prefixed organisation "name-slug" is added before all
page paths (http://localhost:5000/org_a/).
Now the static assets without root path were suddenly expected to be in
our organizations respective slug-namespace.
(http://localhost:5000/org_a/static/images/db-logos/postgres.png for
example).
This pull update 6 cases of "wrongly routed"
asset paths to be in line with a "/" like the already existing cases listed below:
redash/redash/templates/layouts/signed_out.html
Lines 10 to 12 in a682265
redash/client/app/multi_org.html
Lines 10 to 12 in 9097feb
redash/redash/templates/invite.html
Line 26 in 5afd055
redash/redash/templates/login.html
Line 18 in 5afd055
redash/client/cypress/integration/visualizations/map_spec.js
Line 21 in de052ff
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Before the fix:
After the fix:
