fix: cast course_id to CourseKey when loading CourseOverview #29686

mariajgrimaldi · 2021-12-27T14:27:45Z

Description

When creating a course mode using the API for a course which course overview object version is less than the CourseOverview class version, an assertion error is raised.

  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/mongodb_proxy.py", line 55, in wrapper
    return func(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/mongo/base.py", line 1128, in has_course
    assert isinstance(course_key, CourseKey)
AssertionError

This happens because the API serializer calls CourseOverview.get_from_id (here) with a bare string, and then loads the course overview from the modulestore if the version condition is met (here). When the method tries to load the object with the string as an identifier, raises the assertion error(here).

Supporting information

The course modes API was created based on this Jira issue

Testing instructions

With a course that meets the requirement instance.version < CourseOverview.version make the following call

curl -X POST "https://<domain>/api/course_modes/v1/courses/course-v1%3Atesting-site%2BTS%2B00/" -H  "accept: application/json" -H  "Content-Type: application/json" -H  "X-CSRFToken: 5lillyFTelMhHy9ne4i8HJ7IbU7SQe4mpAsRr0OZBoWZrqYqNWhxEmP2BHHClYYd" -d "{  \"course_id\": \"course-v1:testing-site+TS+00\",  \"mode_slug\": \"honor\",  \"mode_display_name\": \"Honor\",  \"currency\": \"usd\"}"

That results in:

2021-12-27 19:52:55,381 INFO 2870331 [openedx.core.djangoapps.content.course_overviews.models] [user 261689] [ip 201.234.236.10] models.py:260 - Attempting to load CourseOverview for course course-v1:testing-site+TS+00 from modulestore.
2021-12-27 19:52:55,661 ERROR 2870331 [root] [user None] [ip None] signals.py:22 - Uncaught exception from None
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python3.8/contextlib.py", line 75, in inner
    return func(*args, **kwds)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/generics.py", line 242, in post
    return self.create(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/mixins.py", line 19, in create
    self.perform_create(serializer)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/mixins.py", line 24, in perform_create
    serializer.save()
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/rest_framework/serializers.py", line 205, in save
    self.instance = self.create(validated_data)
  File "/edx/app/edxapp/edx-platform/common/djangoapps/course_modes/rest_api/serializers.py", line 44, in create
    course_overview = CourseOverview.get_from_id(validated_data['course_id'])
  File "/edx/app/edxapp/edx-platform/openedx/core/lib/cache_utils.py", line 74, in decorator
    result = wrapped(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/content/course_overviews/models.py", line 373, in get_from_id
    course_overview = cls.load_from_module_store(course_id)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/content/course_overviews/models.py", line 265, in load_from_module_store
    with store.bulk_operations(course_id):
  File "/usr/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/ccx/modulestore.py", line 305, in bulk_operations
    with self._modulestore.bulk_operations(course_id, emit_signals=emit_signals, ignore_case=ignore_case):
  File "/usr/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/mixed.py", line 1046, in bulk_operations
    store = self._get_modulestore_for_courselike(course_id)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/mixed.py", line 217, in _get_modulestore_for_courselike
    if has_locator(store):
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/mixed.py", line 215, in <lambda>
    has_locator = lambda store: store.has_course(locator)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/mongodb_proxy.py", line 55, in wrapper
    return func(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/mongo/base.py", line 1128, in has_course
    assert isinstance(course_key, CourseKey)
AssertionError
2021-12-27 19:52:56,423 ERROR 2870331 [django.request] [user 261689] [ip 201.234.236.10] log.py:222 - Internal Server Error: /api/course_modes/v1/courses/course-v1:testing-site+TS+00/

Then make the change, and you'll be able to create the course mode.

Other information

We found this error in our Lilac staging version while creating course modes through this new feature with course overviews taken from juniper DB backups.

openedx-webhooks · 2021-12-27T14:27:50Z

Thanks for the pull request, @mariajgrimaldi! I've created OSPR-6351 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 as you can:

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 · 2022-01-06T19:59:58Z

@mariajgrimaldi Thank you for your contribution. This is ready for our review, correct?

mariajgrimaldi · 2022-01-07T14:12:32Z

Hi @natabene! Happy new year. Yes, this is ready for review.

Agrendalath · 2022-01-07T16:05:27Z

@natabene, can I review this one?

natabene · 2022-01-10T15:49:04Z

@Agrendalath Thanks for offering, please feel free to review and provide feedback. Our team will also want to review.

mariajgrimaldi · 2022-03-09T19:50:19Z

Hi there @Agrendalath! Thanks for the review. It took me a while to come back to this issue 😅

Let me know what do you need to move this forward:smile:

Agrendalath

👍

I tested this: verified that this fixes the falsy assertion
I read through the code
I checked for accessibility issues: n/a
Includes documentation: n/a
I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

Agrendalath · 2022-03-10T19:57:20Z

Hi @mariajgrimaldi. This looks good to me. Just please amend the CurseKey object typo in the commit message :)

Thank you for providing extensive info in the PR's description!

cc: @natabene

mariajgrimaldi · 2022-04-21T17:28:09Z

Hi @natabene, you mentioned your team would want to review; who should we mention? 🤔

natabene · 2022-04-29T09:25:11Z

@mariajgrimaldi It is actually my team :) We will look into this.

pshiu

Thank you @mariajgrimaldi & @Agrendalath!

openedx-webhooks · 2022-04-29T14:35:55Z

@mariajgrimaldi 🎉 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 · 2022-04-29T15:56:14Z

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

edx-pipeline-bot · 2022-04-29T16:51:37Z

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

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

mariajgrimaldi marked this pull request as draft December 27, 2021 14:27

mariajgrimaldi changed the title ~~fix: fix course_id usage in course modes API~~ fix: cast course_id to CourseKey when loading CourseOverview Dec 27, 2021

mariajgrimaldi marked this pull request as ready for review December 27, 2021 20:16

openedx-webhooks added awaiting prioritization and removed needs triage labels Jan 6, 2022

mariajgrimaldi force-pushed the MJG/fix_course_modes_api branch from bc9fab9 to af3479d Compare March 9, 2022 19:47

Agrendalath approved these changes Mar 10, 2022

View reviewed changes

fix: cast course_id string to CourseKey object

d4ba5cc

mariajgrimaldi force-pushed the MJG/fix_course_modes_api branch from af3479d to d4ba5cc Compare March 15, 2022 17:57

pshiu approved these changes Apr 29, 2022

View reviewed changes

natabene closed this Apr 29, 2022

openedx-webhooks added rejected and removed awaiting prioritization labels Apr 29, 2022

natabene reopened this Apr 29, 2022

openedx-webhooks added awaiting prioritization and removed rejected labels Apr 29, 2022

pshiu self-assigned this Apr 29, 2022

pshiu merged commit 2e41384 into openedx:master Apr 29, 2022

openedx-webhooks removed the awaiting prioritization label Apr 29, 2022

openedx-webhooks added the merged label Apr 29, 2022

jawad-khan pushed a commit that referenced this pull request Jun 14, 2022

fix: cast course_id string to CourseKey object (#29686)

0b00282

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: cast course_id to CourseKey when loading CourseOverview #29686

fix: cast course_id to CourseKey when loading CourseOverview #29686

mariajgrimaldi commented Dec 27, 2021 •

edited

Loading

openedx-webhooks commented Dec 27, 2021 •

edited

Loading

natabene commented Jan 6, 2022

mariajgrimaldi commented Jan 7, 2022

Agrendalath commented Jan 7, 2022

natabene commented Jan 10, 2022

mariajgrimaldi commented Mar 9, 2022

Agrendalath left a comment

Agrendalath commented Mar 10, 2022

mariajgrimaldi commented Apr 21, 2022

natabene commented Apr 29, 2022

pshiu left a comment

openedx-webhooks commented Apr 29, 2022

edx-pipeline-bot commented Apr 29, 2022

edx-pipeline-bot commented Apr 29, 2022

fix: cast course_id to CourseKey when loading CourseOverview #29686

fix: cast course_id to CourseKey when loading CourseOverview #29686

Conversation

mariajgrimaldi commented Dec 27, 2021 • edited Loading

Description

Supporting information

Testing instructions

Other information

openedx-webhooks commented Dec 27, 2021 • edited Loading

natabene commented Jan 6, 2022

mariajgrimaldi commented Jan 7, 2022

Agrendalath commented Jan 7, 2022

natabene commented Jan 10, 2022

mariajgrimaldi commented Mar 9, 2022

Agrendalath left a comment

Choose a reason for hiding this comment

Agrendalath commented Mar 10, 2022

mariajgrimaldi commented Apr 21, 2022

natabene commented Apr 29, 2022

pshiu left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Apr 29, 2022

edx-pipeline-bot commented Apr 29, 2022

edx-pipeline-bot commented Apr 29, 2022

mariajgrimaldi commented Dec 27, 2021 •

edited

Loading

openedx-webhooks commented Dec 27, 2021 •

edited

Loading