-
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
dm-4270 relay resource links #733
Conversation
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.
Working as expected - great job Phil!
…e download endpoint
update path helper in partial for resource link
…ion to `practice_resource` id in params
edits `set` method, changes name, adds case statement to determine resource type adds strong params
adds `resource_type` param
changes file link to use new practice resource endpoint
adds factories for 'PracticeSolutionResource', 'PracticeResultsResource', and 'PracticeProblemResource' that inherit from `:practice_resource` factory
adds tests for additional file types
…write method allows practice resource link for non-public practice to be protected by authentication
adds logic to allow traffic to private practices for admins only changes download session token to hash object containing `download_url`, `practice_path` and `practice_name`
now has centered text indicating download status and instructions provides links to re-attempt download and practice path
edits styling to be more centered on page adds `rel="noopener noreferrer"` to re-download link
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.
@PhilipDeFraties This is working well for me. Good job handling the various flavors of permissions involved and restricting attachment access appropriately. I made a small accessibility request re: "Click here" for link text. It looks like one of the practice_resource feature specs is currently failing in CI. Please also rebase in the Rails 6.1 changes from master (and Ruby 3.2.2, depending on the timing!).
e0f1368
to
0c1d64d
Compare
JIRA issue link
https://agile6.atlassian.net/browse/DM-4270
Description - what does this code do?
/practice_resources/:id/download
to act as relay for innovation resource file downloads, redirects to s3 download via presigned url. These links will now work when saved, however for non-public innovations the user will be required to first authenticate, if not already logged in, and in turn will redirect to a new resource download page (see screenshot) and will simultaneously open a new tab with the file (or in the case of a docx file will initiate the download).Testing done - how did you test it/steps on how can another person can test it
Public Innovations:
practice.published && practice.enabled && practice.approved && practice.published
Non-public Innovations:
practice.update!(is_public: false
Non-published Innovations:
Screenshots, Gifs, Videos from application (if applicable)
Link to mock-ups/mock ups (image file if you have it) (if applicable)
Acceptance criteria
Definition of done