-
Notifications
You must be signed in to change notification settings - Fork 195
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: forming Response Files link in the report #1982
fix: forming Response Files link in the report #1982
Conversation
Thanks for the pull request, @DmytroAlipov! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
@DmytroAlipov Thanks for the contribution! Is it ready for review? |
Hi @itsjeyd. Yes of course ;) |
@DmytroAlipov OK great, we'll get tests enabled for you next. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1982 +/- ##
==========================================
+ Coverage 94.98% 95.01% +0.02%
==========================================
Files 154 154
Lines 17056 17060 +4
Branches 1611 1610 -1
==========================================
+ Hits 16201 16209 +8
+ Misses 641 638 -3
+ Partials 214 213 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
@DmytroAlipov it looks like there's a unit test error that needs attention. |
1af28f0
to
eccc020
Compare
@e0d Hi! There was a small problem with linting due to an unused variable, but I have fixed it. Could you please run the tests again? |
@DmytroAlipov We are 🟢 |
Hi @mattcarter, this PR is ready for review by the Aurora team. |
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 👍🏻
TL;DR - This is a fix forming "Response Files" link in the reports (in the context of the ORA functionality).
What changed?
The link in the formed report doesn't allow checking the uploaded file in the installation server for Instructor:
Correspond link allows to download file for instructor:
I rechecked S3 storage forming link too. It works correctly.
Developer Checklist
Testing Instructions
[ How should a reviewer test this PR? ]
Reviewer Checklist
Collectively, these should be completed by reviewers of this PR:
FYI: @openedx/content-aurora