[SE-4175] Add CCX ID to generated filename prefixes - #27028
Conversation
|
Thanks for the pull request, @gabor-boros! I've created OSPR-5675 to keep track of it in JIRA, where we prioritize reviews. 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:
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. |
8bd2100 to
9bfb630
Compare
|
@gabor-boros Thank you for your contribution. Please let me know once it is ready for our review. |
|
@natabene I'll let you know as soon this is ready for a review. |
There was a problem hiding this comment.
filename = separator.join([filename, 'ccx', course_id.ccx]) - the additional + is causing tests to fail.
mavidser
left a comment
There was a problem hiding this comment.
LGTM. This should be ready for edx's review once the tests pass :)
|
@natabene This is ready for edX review 🚀 |
|
@pdpinch Can you review and merge this PR related to CCX? |
|
jenkins run all |
1 similar comment
|
jenkins run all |
|
FTR: Oh, there's an invalid syntax error: |
|
@pdpinch FYI, I just fixed the failing tests 😇 |
|
@pdpinch Could you have another look? |
|
@natabene Could you please help me scheduling this PR for review? 😇 |
|
Sorry I missed both your updates -- I'll take a look this week. |
|
@gabor-boros I don't understand what you mean by this, in the PR description:
|
pdpinch
left a comment
There was a problem hiding this comment.
Looking good to me now. 👍
|
@pdpinch Is this fit to merge? If so, let's merge. |
|
Sure thing! I wasn't sure if it needed another approval. |
|
@gabor-boros 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
(cherry picked from commit f6ab4e0)
…27028) This is the backport of https://github.com/edx/edx-platform/pull/27028. --- This pull requests backport a feature to include CCX course IDs in the generated course filenames used for downloading reports, etc. The suffix appended to the generated filename prefix is following the `<separator>_ccx_<ccx id>` pattern, to ensure unique filename generation per ccx - previously all CCX courses were downloaded with the same name. **Dependencies**: None **Screenshots**:  **Sandbox URL**: TBD **Merge deadline**: None **Testing instructions**: 1. Navigate to https://pr29101.sandbox.opencraft.hosting 2. Login as staff 3. Navigate to https://pr29101.sandbox.opencraft.hosting/courses/ccx-v1:edX+DemoX+Demo_Course+ccx@1/instructor#view-data_download 4. Click "Download profile information as a CSV" button 5. Check that the file name contains CCX
(cherry picked from commit f6ab4e0)
…27028) This is the backport of https://github.com/edx/edx-platform/pull/27028. --- This pull requests backport a feature to include CCX course IDs in the generated course filenames used for downloading reports, etc. The suffix appended to the generated filename prefix is following the `<separator>_ccx_<ccx id>` pattern, to ensure unique filename generation per ccx - previously all CCX courses were downloaded with the same name. **Dependencies**: None **Screenshots**:  **Sandbox URL**: TBD **Merge deadline**: None **Testing instructions**: 1. Navigate to https://pr29101.sandbox.opencraft.hosting 2. Login as staff 3. Navigate to https://pr29101.sandbox.opencraft.hosting/courses/ccx-v1:edX+DemoX+Demo_Course+ccx@1/instructor#view-data_download 4. Click "Download profile information as a CSV" button 5. Check that the file name contains CCX
This pull requests adds a new feature to include CCX course IDs in the generated course filenames used for downloading reports, etc.
The suffix appended to the generated filename prefix is following the
<separator>_ccx_<ccx id>pattern, to ensure unique filename generation per ccx - previously all CCX courses were downloaded with the same name.Dependencies: None
Screenshots:

Sandbox URL: Due to an
DeprecatedEdxPlatformImportErrorexception the sandbox is not provisioned.Merge deadline: None
Testing instructions:
ALLOW_COURSE_STAFF_GRADE_DOWNLOADS,ENABLE_GRADE_DOWNLOADS, andENABLE_COURSE_FILENAME_CCX_SUFFIXfeatures.Author notes and concerns:
This change should be cherry-picked to Koa and Lilac as well.
Reviewers