-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: [AXIMST-10] Refactor Unit page view as DRF #2484
Conversation
f581f73
to
1d4d2a9
Compare
1d4d2a9
to
276bafe
Compare
276bafe
to
f5eef05
Compare
cms/djangoapps/contentstore/utils.py
Outdated
assert unit is not None, "Could not determine unit page" | ||
subsection = get_parent_xblock(unit) | ||
assert subsection is not None, "Could not determine parent subsection from unit " + str( | ||
unit.location) | ||
section = get_parent_xblock(subsection) | ||
assert section is not None, "Could not determine ancestor section from unit " + str(unit.location) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert unit is not None, "Could not determine unit page" | |
subsection = get_parent_xblock(unit) | |
assert subsection is not None, "Could not determine parent subsection from unit " + str( | |
unit.location) | |
section = get_parent_xblock(subsection) | |
assert section is not None, "Could not determine ancestor section from unit " + str(unit.location) | |
if unit is None: | |
raise ValueError("Could not determine unit page") | |
subsection = get_parent_xblock(unit) | |
if subsection is None: | |
raise ValueError(f"Could not determine parent subsection from unit {unit.location}") | |
section = get_parent_xblock(subsection) | |
if section is None: | |
raise ValueError(f"Could not determine ancestor section from unit {unit.location}") |
Is it better to replace with raise
instead of assert since it is not a test function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we don't want to change original code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest leaving it as it was and don't change original code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added new commit with this replacement
7199ce2
to
507311b
Compare
9ea7255
to
fe2b4aa
Compare
cms/static/js/views/container.js
Outdated
@@ -113,6 +113,7 @@ function($, _, XBlockView, ModuleUtils, gettext, StringUtils, NotificationView) | |||
return $(child).data('locator'); | |||
} | |||
); | |||
console.log('childLocators:', childLocators) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug, please
@@ -131,6 +131,8 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H | |||
}, | |||
|
|||
render: function() { | |||
debugger; | |||
console.log('MODEL', this.model); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug, please
xmodule/studio_editable.py
Outdated
@@ -32,6 +32,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): | |||
'id': str(child.location), | |||
'content': rendered_child.content | |||
}) | |||
# import pdb; pdb.set_trace() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove debug, please
@@ -72,7 +72,7 @@ | |||
|
|||
|
|||
@require_http_methods(("DELETE", "GET", "PUT", "POST", "PATCH")) | |||
@login_required | |||
# @login_required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be returned back?
'user_clipboard': user_clipboard, | ||
'is_fullwidth_content': is_library_xblock, | ||
}) | ||
container_handler_context = get_container_handler_context(request, usage_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a bug
You can't put statement
if is_unit_page and use_new_unit_page(course.id):
return redirect(get_unit_url(course.id, unit.location))
Inside get_container_handler_context
function.
Because redirect()
returns Response object, and should be returned by the View itself.
Currently on dev server the following error is thrown when enabling feature flag contentstore.new_studio_mfe.use_new_unit_page
│[2024-01-06 13:33:38 +0000] [47665] [INFO] Autorestarting worker after current request. │
│2024-01-06 13:33:38,471 ERROR 47665 [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 47, in inner │
│ response = get_response(request) │
│ File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, 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/http.py", line 40, in inner │
│ return func(request, *args, **kwargs) │
│ File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view │
│ return view_func(request, *args, **kwargs) │
│ File "/edx/app/edxapp/edx-platform/cms/djangoapps/contentstore/views/component.py", line 121, in container_handler │
│ return render_to_response('container.html', container_handler_context) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/shortcuts.py", line 188, in render_to_response │
│ return HttpResponse(render_to_string(template_name, dictionary, namespace, request), **kwargs) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/shortcuts.py", line 178, in render_to_string │
│ return template.render(dictionary, request) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/template.py", line 77, in render │
│ context_dictionary.update(context) │
│ValueError: dictionary update sequence element #0 has length 0; 2 is required │
│2024-01-06 13:33:38,491 ERROR 47665 [django.request] [user 19] [ip 88.135.201.166] log.py:224 - Internal Server Error: /container/block-v1:eDx+123+12+type@vertical+block@bf197d5f929d421980c6acbd8583964d │
│Traceback (most recent call last): │
│ File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner │
│ response = get_response(request) │
│ File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, 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/http.py", line 40, in inner │
│ return func(request, *args, **kwargs) │
│ File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view │
│ return view_func(request, *args, **kwargs) │
│ File "/edx/app/edxapp/edx-platform/cms/djangoapps/contentstore/views/component.py", line 121, in container_handler │
│ return render_to_response('container.html', container_handler_context) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/shortcuts.py", line 188, in render_to_response │
│ return HttpResponse(render_to_string(template_name, dictionary, namespace, request), **kwargs) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/shortcuts.py", line 178, in render_to_string │
│ return template.render(dictionary, request) │
│ File "/edx/app/edxapp/edx-platform/common/djangoapps/edxmako/template.py", line 77, in render │
│ context_dictionary.update(context) │
│ValueError: dictionary update sequence element #0 has length 0; 2 is required │
│[2024-01-06 13:33:38 +0000] [47665] [INFO] Worker exiting (pid: 47665) │
│[2024-01-06 13:33:41 +0000] [62159] [INFO] Booting worker with pid: 62159
Ticket returned to rework
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed here #2487
AXIMST-10