Skip to content
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

Notes not being saved with the correct course id in CCXs #34427

Open
alexjmpb opened this issue Mar 26, 2024 · 0 comments
Open

Notes not being saved with the correct course id in CCXs #34427

alexjmpb opened this issue Mar 26, 2024 · 0 comments

Comments

@alexjmpb
Copy link
Contributor

Description

Currently, when a note is created in a CCX (this process is handled by a decorator https://github.com/open-craft/edx-platform/blob/master/lms/djangoapps/edxnotes/decorators.py#L14-L69), it is saving the note using a versioned course_id (e.g. ccx-v1:edX+DemoX.1+2014+branch@published-branch+version@651d7121f4285e6db96715e0+ccx@1) and the right behavior, based on how notes for regular courses are saved and how it worked before, should be to save it using the CCX course_id without any versions (e.g. ccx-v1:edX+DemoX.1+2014+ccx@1).

Cause of the issue

I have a vague understanding of the context for applying the change that causes the issue. However, here is the PR that contains these changes (https://github.com/openedx/edx-platform/pull/30715/files#diff-aee892f54ee20b3aae56aceefea75775fdb3b1f77421976e7eb0eb626201936eR36-R37). From what I could understand, the way the code is now obtaining the course_key causes, when calling modulestore, to obtain a course block with the parameter id being a versioned CCXCourseLocator, then notes obtain the course_id string from this parameter causing it to save it wrongly.

Potential solution

The solution that we applied in our fork, consists on putting a conditional in the notes decorator to verify if the course is a CCX and if so, remove any versions from the course_key. The solution can be viewed in the following PR https://github.com/Pearson-Advance/edx-platform/pull/117/files.

Discarded solutions

We reviewed other places to put the fix and enforce the behavior lower. Those were xmodule.modulestore.DraftVersioningModuleStore.get_course and xmodule.course_module.CourseBlock.id, however these were discarded because it may affect other functionalities of the platform. It's worth to open the discussion of reviewing if is feasible to put the fix in these places or others than the discussed before.

Migration of the data

Finally, it's worth to note that instances of the platform which have notes with the issue should perform a migration process.

@pdpinch This issue aims to continue the conversation from slack about the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant