You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
The text was updated successfully, but these errors were encountered: