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
Auxiliary file download #7729
Auxiliary file download #7729
Conversation
This is just a stub for now with many FIXMEs.
Not sure why they didn't work before but they work now.
All other aux files go under "other".
Thanks @pdurbin for the PR and for the note about the docs. I took another read through and I think they are fine for now, since we flag this as experimental. |
Over at opendp/dpcreator#135 (comment) @TaniaSchlatter asked if this could be changed to "dp_" (adding an underscore) and this got a thumbs up from @raprasad. Yes, this is certainly possible. Let's see what other changes are requested and do them at once. |
I made many commits to address the feedback above. In addition I just merged the latest from develop to pick up the change to 5.4.1 and renamed the SQL update script. Ready for more review. |
restrictedaccess=Restricted Access | ||
restrictedWithAccessGranted=Restricted with Access Granted |
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.
I checked is only uses as a title on an image lock of a file search card; so let's have consistency and just have the one (restrictedaccess=Restricted with Access Granted)
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.
Done in d277aa1. Now the search card is consistent with other places:
@NamedQuery(name = "AuxiliaryFile.findAuxiliaryFiles", | ||
query = "select object(o) from AuxiliaryFile as o where o.dataFile.id = :dataFileId"), | ||
@NamedQuery(name = "AuxiliaryFile.findAuxiliaryFilesByType", | ||
query = "select object(o) from AuxiliaryFile as o where o.dataFile.id = :dataFileId and o.type like :type"), |
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.
Just noticed this - why "like" and not straight "="
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.
No reason. Fixed in e1d5541.
file.downloadBtn.restricted=Restricted | ||
file.downloadBtn.restricted.with.access.granted=Restricted with Access Granted |
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.
We can now get rid of these and just use:
restricted=Restricted
restrictedaccess=Restricted with Access Granted
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.
With the exception of deciding on the API (which we documented as a todo and also explicitly mentioned in the "experimental" section, I think this is good to go. Next up, QA!
What this PR does / why we need it:
Adds the ability to download auxiliary files from the web interface. Also, other cleanup was done.
Which issue(s) this PR closes:
Closes #7400
Special notes for your reviewer:
I talked to @djbrooke about trimming down the changes he made but ultimately didn't because I wasn't sure what to take out or how to best word things. I think it's fine to leave it all in but I'm also fine with some of it being removed.
Suggestions on how to test this:
Use AuxiliaryFilesIT or your own methods to add some auxiliary files. Here are the changes to watch for (from the release note):
Auxiliary Files can now be downloaded from the web interface.
In addition, related changes were made, including the following:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Yes. Mockups at https://docs.google.com/presentation/d/1XuX4qStdHjkFf4g1d1vKxxb85kuuUSEzdKN41dNM2mM/
Here's a screenshot showing the DP files:
And another showing "other" files:
Is there a release notes update needed for this change?:
Added.
Additional documentation:
Included.