NASA-IMPACT / veda-config

Configuration template for the Dashboard Evolution Project
https://visex.netlify.app/
Other
21 stars 37 forks source link

Fix issue 451 #452

Closed amarouane-ABDELHAK closed 1 month ago

amarouane-ABDELHAK commented 1 month ago

Why are you creating this Pull Request?

netlify[bot] commented 1 month ago

Deploy Preview for visex ready!

Name Link
Latest commit 5f9c95f2c73af954b8b1bba538bf7fa84a430154
Latest deploy log https://app.netlify.com/sites/visex/deploys/670d2bb888e01f00079ddf61
Deploy Preview https://deploy-preview-452--visex.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

amarouane-ABDELHAK commented 1 month ago

@hanbyul-here @anayeaye and I were wondering why we didn't use Domain_PROD as a relative path instead of a hardcoded domain name, if both work. Do you know the reason behind having the Domain_PROD variable as a full URL?

hanbyul-here commented 1 month ago

Thanks for working on the fix. The variable is used to specify where the bundled assets are served. where it is passed to be used for application base url: (https://github.com/NASA-IMPACT/veda-config/blob/8654ea1a196b37a44841e9a28492c689b8e3ae0c/.github/workflows/deploy-prod.yml#L65) The absolute path should work in theory, but I am honestly unsure if I am missing anything and if this change will bring unexpected outcomes.

As far as I know, we don't have a staging environment for earthdata.nasa.gov to test the URL change. Considering earthdata.nasa.gov has low traffic, should we just test on the production? or do you have any idea how we can test this change to not introduce any unexpected outcome without deploying it to production?

amarouane-ABDELHAK commented 1 month ago

I have build it locally with PUBLIC_URL as a path instead of URL and did not notice any side effect