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

feat: [AXIMST-10] Refactor Unit page view as DRF #2484

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

ruzniaievdm
Copy link

@ruzniaievdm ruzniaievdm force-pushed the ruzniaievdm/container-handler-drf branch 2 times, most recently from f581f73 to 1d4d2a9 Compare December 28, 2023 15:50
@ruzniaievdm ruzniaievdm marked this pull request as ready for review December 28, 2023 15:51
@ruzniaievdm ruzniaievdm self-assigned this Dec 28, 2023
@ruzniaievdm ruzniaievdm force-pushed the ruzniaievdm/container-handler-drf branch from 1d4d2a9 to 276bafe Compare December 28, 2023 16:06
@ruzniaievdm ruzniaievdm force-pushed the ruzniaievdm/container-handler-drf branch from 276bafe to f5eef05 Compare December 28, 2023 16:23
Comment on lines 1755 to 1760
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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?

Copy link

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?

Copy link
Author

@ruzniaievdm ruzniaievdm Jan 3, 2024

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

Copy link
Author

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

@ruzniaievdm ruzniaievdm force-pushed the ruzniaievdm/container-handler-drf branch from 7199ce2 to 507311b Compare January 5, 2024 10:40
@ruzniaievdm ruzniaievdm changed the base branch from master to ts-develop January 5, 2024 10:41
@ruzniaievdm ruzniaievdm requested a review from monteri January 5, 2024 10:45
@ruzniaievdm ruzniaievdm force-pushed the ruzniaievdm/container-handler-drf branch from 9ea7255 to fe2b4aa Compare January 5, 2024 12:10
@@ -113,6 +113,7 @@ function($, _, XBlockView, ModuleUtils, gettext, StringUtils, NotificationView)
return $(child).data('locator');
}
);
console.log('childLocators:', childLocators)
Copy link

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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debug, please

@@ -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()
Copy link

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
Copy link

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?

@monteri monteri merged commit 4a09682 into ts-develop Jan 5, 2024
43 of 45 checks passed
'user_clipboard': user_clipboard,
'is_fullwidth_content': is_library_xblock,
})
container_handler_context = get_container_handler_context(request, usage_key)
Copy link
Collaborator

@GlugovGrGlib GlugovGrGlib Jan 6, 2024

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed here #2487

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

Successfully merging this pull request may close these issues.

3 participants