[SE-4175] Add CCX ID to generated filename prefixes #27028

gabor-boros · 2021-03-16T17:09:55Z

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 DeprecatedEdxPlatformImportError exception the sandbox is not provisioned.

Merge deadline: None

Testing instructions:

Enable ALLOW_COURSE_STAFF_GRADE_DOWNLOADS, ENABLE_GRADE_DOWNLOADS, and ENABLE_COURSE_FILENAME_CCX_SUFFIX features.
Create a course
Enable CCX for the course in the advanced settings
Create a new CCX for the course
Generate and download grades

Author notes and concerns:

This change should be cherry-picked to Koa and Lilac as well.

Reviewers

@mavidser
TBD

openedx-webhooks · 2021-03-16T17:09:59Z

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:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

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.

natabene · 2021-03-17T15:19:02Z

@gabor-boros Thank you for your contribution. Please let me know once it is ready for our review.

gabor-boros · 2021-03-18T07:40:48Z

@natabene I'll let you know as soon this is ready for a review.

mavidser · 2021-03-22T08:21:28Z

common/djangoapps/util/file.py

+    )
+
+    if enable_course_filename_ccx_suffix and getattr(course_id, 'ccx', None):
+        filename += separator.join([filename, 'ccx', course_id.ccx])


filename = separator.join([filename, 'ccx', course_id.ccx]) - the additional + is causing tests to fail.

mavidser

LGTM. This should be ready for edx's review once the tests pass :)

gabor-boros · 2021-03-22T09:48:52Z

@natabene This is ready for edX review 🚀

nasthagiri · 2021-03-22T22:32:30Z

@pdpinch Can you review and merge this PR related to CCX?

pdpinch · 2021-04-06T15:57:19Z

jenkins run all

gabor-boros · 2021-04-09T13:52:07Z

jenkins run all

gabor-boros · 2021-04-09T13:52:57Z

FTR: Oh, there's an invalid syntax error:

18:04:13    File "/home/jenkins/workspace/edx-platform-quality-pipeline-pr/cms/envs/test.py", line 26, in <module>
18:04:13      from .common import *
18:04:13    File "/home/jenkins/workspace/edx-platform-quality-pipeline-pr/cms/envs/common.py", line 470
18:04:13      'ENABLE_HELP_LINK': True,
18:04:13      ^
18:04:13  SyntaxError: invalid syntax

gabor-boros · 2021-04-18T14:30:25Z

@pdpinch FYI, I just fixed the failing tests 😇

natabene · 2021-04-28T20:44:44Z

@pdpinch Could you have another look?

gabor-boros · 2021-05-07T16:51:58Z

@natabene Could you please help me scheduling this PR for review? 😇

pdpinch · 2021-05-10T12:42:02Z

Sorry I missed both your updates -- I'll take a look this week.

pdpinch · 2021-05-11T17:57:58Z

@gabor-boros I don't understand what you mean by this, in the PR description:

This PR differs from the upstream PR, since the upstream already removed Python2 related syntax from its code.

pdpinch

Looking good to me now. 👍

natabene · 2021-05-24T14:51:36Z

@pdpinch Is this fit to merge? If so, let's merge.

pdpinch · 2021-05-25T14:24:08Z

Sure thing! I wasn't sure if it needed another approval.

openedx-webhooks · 2021-05-25T14:25:09Z

@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-pipeline-bot · 2021-05-25T15:39:55Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2021-05-25T16:32:05Z

EdX Release Notice: This PR has been deployed to the production environment.

edx-pipeline-bot · 2021-05-25T16:41:25Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2021-05-25T17:16:34Z

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**: ![Screenshot 2021-03-16 at 16 22 12](https://user-images.githubusercontent.com/19173947/111349994-f1c42980-8681-11eb-8fff-d28024899957.png) **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**: ![Screenshot 2021-03-16 at 16 22 12](https://user-images.githubusercontent.com/19173947/111349994-f1c42980-8681-11eb-8fff-d28024899957.png) **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

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 16, 2021

gabor-boros marked this pull request as draft March 16, 2021 17:26

gabor-boros force-pushed the gabor/add-ccx-id-to-filename-prefixes branch from 8bd2100 to 9bfb630 Compare March 17, 2021 08:52

openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Mar 18, 2021

mavidser reviewed Mar 22, 2021

View reviewed changes

mavidser approved these changes Mar 22, 2021

View reviewed changes

gabor-boros marked this pull request as ready for review March 22, 2021 09:48

openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 22, 2021

openedx-webhooks added awaiting prioritization and removed needs triage labels Mar 22, 2021

nasthagiri requested a review from pdpinch March 22, 2021 22:32

openedx-webhooks added engineering review and removed awaiting prioritization labels Mar 24, 2021

pdpinch self-assigned this May 10, 2021

pdpinch approved these changes May 21, 2021

View reviewed changes

pdpinch merged commit baca477 into openedx:master May 25, 2021

openedx-webhooks added merged and removed engineering review labels May 25, 2021

gabor-boros deleted the gabor/add-ccx-id-to-filename-prefixes branch May 29, 2021 13:31

blarghmatey pushed a commit to mitodl/edx-platform that referenced this pull request Aug 2, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

b8440dc

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Oct 11, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

8ca84fd

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Oct 11, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

f6ab4e0

gabor-boros added a commit to edx-olive/edx-platform-old that referenced this pull request Oct 11, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

bb506c0

This was referenced Oct 25, 2021

[SE-4631] add CCX ID to generated filename prefixes (#27028) #29100

Merged

[SE-4631] add CCX ID to generated filename prefixes (#27028) #29101

Merged

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Oct 25, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

c996623

This was referenced Oct 25, 2021

[SE-4631] add CCX ID to generated filename prefixes open-craft/edx-platform#434

Closed

[SE-4631] add CCX ID to generated filename prefixes (#27028) open-craft/edx-platform#435

Merged

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Nov 1, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028)

988179f

(cherry picked from commit f6ab4e0)

gabor-boros mentioned this pull request Nov 1, 2021

[SE-4631] feat: add CCX ID to generated filename prefixes (#27028) open-craft/edx-platform#438

Merged

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Nov 8, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028) (#438)

d9822e3

(cherry picked from commit f6ab4e0)

gabor-boros added a commit to open-craft/edx-platform that referenced this pull request Nov 9, 2021

feat: add CCX ID to generated filename prefixes (openedx#27028) (#435)

e82c8d1

fsologureng pushed a commit to eol-uchile/edx-platform that referenced this pull request Dec 13, 2023

feat: add CCX ID to generated filename prefixes (openedx#27028)

c6c8fcb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SE-4175] Add CCX ID to generated filename prefixes #27028

[SE-4175] Add CCX ID to generated filename prefixes #27028

gabor-boros commented Mar 16, 2021 •

edited

Loading

openedx-webhooks commented Mar 16, 2021 •

edited

Loading

natabene commented Mar 17, 2021

gabor-boros commented Mar 18, 2021

mavidser Mar 22, 2021 •

edited

Loading

mavidser left a comment

gabor-boros commented Mar 22, 2021

nasthagiri commented Mar 22, 2021

pdpinch commented Apr 6, 2021

gabor-boros commented Apr 9, 2021

gabor-boros commented Apr 9, 2021

gabor-boros commented Apr 18, 2021

natabene commented Apr 28, 2021

gabor-boros commented May 7, 2021

pdpinch commented May 10, 2021

pdpinch commented May 11, 2021

pdpinch left a comment

natabene commented May 24, 2021

pdpinch commented May 25, 2021

openedx-webhooks commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

[SE-4175] Add CCX ID to generated filename prefixes #27028

[SE-4175] Add CCX ID to generated filename prefixes #27028

Conversation

gabor-boros commented Mar 16, 2021 • edited Loading

openedx-webhooks commented Mar 16, 2021 • edited Loading

natabene commented Mar 17, 2021

gabor-boros commented Mar 18, 2021

mavidser Mar 22, 2021 • edited Loading

Choose a reason for hiding this comment

mavidser left a comment

Choose a reason for hiding this comment

gabor-boros commented Mar 22, 2021

nasthagiri commented Mar 22, 2021

pdpinch commented Apr 6, 2021

gabor-boros commented Apr 9, 2021

gabor-boros commented Apr 9, 2021

gabor-boros commented Apr 18, 2021

natabene commented Apr 28, 2021

gabor-boros commented May 7, 2021

pdpinch commented May 10, 2021

pdpinch commented May 11, 2021

pdpinch left a comment

Choose a reason for hiding this comment

natabene commented May 24, 2021

pdpinch commented May 25, 2021

openedx-webhooks commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

edx-pipeline-bot commented May 25, 2021

gabor-boros commented Mar 16, 2021 •

edited

Loading

openedx-webhooks commented Mar 16, 2021 •

edited

Loading

mavidser Mar 22, 2021 •

edited

Loading