-
Notifications
You must be signed in to change notification settings - Fork 10
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(DHIS2-16988): download uncompressed json rather than open inline #2037
fix(DHIS2-16988): download uncompressed json rather than open inline #2037
Conversation
🚀 Deployed on https://pr-2037--dhis2-import-export.netlify.app |
2ef47c8
to
400e2b1
Compare
src/utils/helper.js
Outdated
// call stub function if available | ||
const locationAssign = (url) => { | ||
const locationAssign = (relativeUrl) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I thought baseUrl from useConfig
would be the absolute URL? But it may depend if the it's available from system/info
I guess?
Very minor: but not sure if the rename of this variable is better when it can be an absolute url as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some questions really.
src/utils/helper.js
Outdated
const [, file, extension] = urlFilePart.match(/(^[^.]+)(\..+$)/) | ||
|
||
downloadWindow.document.title = downloadWindowTitle | ||
downloadWindow.document.body.innerHTML = downloadWindowHtml // does not work in Chrome | ||
const downloadedFileName = `${file}${extension}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? I thought the browser would handle the file-extension and we could just send the name, if we wanted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in, I can only provide the file
part and the browser will figure out the extension? I don't think so but I will give it a try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right @Birkbjo - there was no need to add the extension, removed it now and addressed your comments
ca7d26b
to
8c727b5
Compare
## [101.1.5](v101.1.4...v101.1.5) (2024-06-03) ### Bug Fixes * **DHIS2-16988:** download uncompressed json rather than open inline ([#2037](#2037)) ([b7732b5](b7732b5))
🎉 This PR is included in version 101.1.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This allows downloading exported
json
files - right now, it just opens in a new window since the API no longer returns thecontent-disposition
header for JSON uncompressed. To fix that, we're using the download property of an anchor to trigger a download in the browser. Since the download is triggered programatically, rather than through a link element, we have to create the button in JS and trigger it, similar to this solution.Testing this solution needs to happen on the same origin, so it won't work on netlify. You need to build the app and install in an instance, then it works.
fixes DHIS2-16988
ps: I had a solution here that creates a longer file name containing other info related to the export, but canned it for now as we didn't agree whether we should do this or not.