From f24af41b98d85d8c69f8ad9f85383b9f8947ceec Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Thu, 11 Aug 2022 11:49:45 +0500 Subject: [PATCH] feat: redirect method for learners to new MFE (#30769) Co-authored-by: adeel.tajamul --- lms/djangoapps/discussion/tests/test_views.py | 103 +++++++++++++++++- lms/djangoapps/discussion/views.py | 37 ++++++- 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 7442ff9bc1..9e73967dde 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -51,7 +51,11 @@ from lms.djangoapps.discussion.django_comment_client.tests.utils import ( topic_name_to_id ) from lms.djangoapps.discussion.django_comment_client.utils import strip_none -from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE +from lms.djangoapps.discussion.toggles import ( + ENABLE_DISCUSSIONS_MFE, + ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE, + ENABLE_DISCUSSIONS_MFE_BANNER +) from lms.djangoapps.discussion.views import _get_discussion_default_topic_id, course_discussions_settings_handler from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory from openedx.core.djangoapps.course_groups.models import CourseUserGroup @@ -2325,3 +2329,100 @@ class ForumMFETestCase(ForumsEnableMixin, SharedModuleStoreTestCase): assert "discussions-mfe-tab-embed" in content else: assert "discussions-mfe-tab-embed" not in content + + @override_settings(DISCUSSIONS_MICROFRONTEND_URL="http://test.url") + @ddt.data(*itertools.product(("learner", "staff"), (True, False))) + @ddt.unpack + def test_redirect_from_legacy_base_url_to_new_experience(self, user_role, toggle_enabled): + """ + Verify that the requested is redirected to MFE homepage when + ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE flag is enabled. Privileged users will + be able to view legacy experience. For learners, if ENABLE_DISCUSSIONS_MFE_BANNER + flag is enabled, they will be able to use legacy otherwise they will be redirected + to MFE. + """ + if user_role == "staff": + user = self.staff_user + elif user_role == "learner": + user = self.user + + with override_waffle_flag(ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE, toggle_enabled): + self.client.login(username=user.username, password='test') + url = reverse("forum_form_discussion", args=[self.course.id]) + response = self.client.get(url) + content = response.content.decode('utf8') + + if toggle_enabled: + assert "discussions-mfe-tab-embed" in content + else: + assert "discussions-mfe-tab-embed" not in content + + @override_settings(DISCUSSIONS_MICROFRONTEND_URL="http://test.url") + @ddt.data(*itertools.product(("learner", "staff"), (True, False))) + @ddt.unpack + def test_redirect_from_legacy_profile_url_to_new_experience(self, user_role, toggle_enabled): + """ + Verify that the requested is redirected to MFE homepage when + ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE flag is enabled. This redirect is only + for learners and not for privileged users. + """ + if user_role == "staff": + user = self.staff_user + elif user_role == "learner": + user = self.user + + with override_waffle_flag(ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE, toggle_enabled): + self.client.login(username=user.username, password='test') + url = reverse("user_profile", args=[self.course.id, user.id]) + response = self.client.get(url) + content = response.content.decode('utf8') + if toggle_enabled and user == "learner": + assert "discussions-mfe-tab-embed" in content + else: + assert "discussions-mfe-tab-embed" not in content + + @override_settings(DISCUSSIONS_MICROFRONTEND_URL="http://test.url") + @ddt.data(*itertools.product(("learner", "staff"), (True, False))) + @ddt.unpack + def test_correct_experience_for_single_thread_url_for_everyone_flag(self, user_role, toggle_enabled): + """ + Verify that the correct experience is shown based on the MFE toggle for everyone + for Legacy single thread url + """ + if user_role == "staff": + user = self.staff_user + elif user_role == "learner": + user = self.user + + with override_waffle_flag(ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE, toggle_enabled): + self.client.login(username=user.username, password='test') + url = reverse("single_thread", args=[self.course.id, "test_discussion", "test_thread"]) + response = self.client.get(url) + content = response.content.decode('utf8') + if toggle_enabled and user == "learner": + assert "discussions-mfe-tab-embed" in content + else: + assert "discussions-mfe-tab-embed" not in content + + @override_settings(DISCUSSIONS_MICROFRONTEND_URL="http://test.url") + @ddt.data(*itertools.product(("legacy", "new"), (True, False))) + @ddt.unpack + def test_correct_experience_for_learner_banner_flag(self, experience, toggle_enabled): + """ + Verify that the correct experience is shown based on the MFE toggle for everyone + for Legacy single thread url + """ + with override_waffle_flag(ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE, True): + with override_waffle_flag(ENABLE_DISCUSSIONS_MFE_BANNER, toggle_enabled): + self.client.login(username=self.user.username, password='test') + url = reverse("forum_form_discussion", args=[self.course.id]) + url += f"?discussions_experience={experience}" + response = self.client.get(url) + content = response.content.decode('utf8') + + if experience == "new": + assert "discussions-mfe-tab-embed" in content + elif not toggle_enabled and experience == "legacy": + assert response.status_code == 302 + else: + assert "discussions-mfe-tab-embed" not in content diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index 0c1ff1d24f..0403a4e1b2 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -11,7 +11,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.contrib.staticfiles.storage import staticfiles_storage from django.http import Http404, HttpResponseForbidden, HttpResponseServerError -from django.shortcuts import render +from django.shortcuts import redirect, render from django.template.context_processors import csrf from django.template.loader import render_to_string from django.urls import reverse @@ -307,11 +307,36 @@ def forum_form_discussion(request, course_key): 'corrected_text': query_params['corrected_text'], }) else: + discussions_mfe_enabled_for_everyone = ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE.is_enabled(course_key) + privileged_user = is_privileged_user(course_key, request.user) + if discussions_mfe_enabled_for_everyone and (not privileged_user): + discussion_experience = request.GET.get('discussions_experience', None) + banner_enabled = ENABLE_DISCUSSIONS_MFE_BANNER.is_enabled(course_key) + redirect_to_mfe = (discussion_experience is None) or (not banner_enabled) + if redirect_to_mfe and discussion_experience == "legacy": + mfe_context = _discussions_mfe_context(request.GET, course_key, True, False, False) + return redirect(mfe_context['mfe_url']) + course_id = str(course.id) tab_view = CourseTabView() return tab_view.get(request, course_id, 'discussion') +def redirect_url_to_new_mfe(request, course_id, thread_id): + """ + Returns MFE url of the thread if the user is not privileged + """ + course_key = CourseKey.from_string(course_id) + discussions_mfe_enabled_for_everyone = ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE.is_enabled(course_key) + if discussions_mfe_enabled_for_everyone and (not is_privileged_user(course_key, request.user)): + discussion_experience = request.GET.get('discussions_experience', None) + if (discussion_experience is None) and (thread_id is not None): + mfe_context = _discussions_mfe_context(request.GET, course_key, True, False, False) + redirect_url = f"{mfe_context['mfe_url']}#posts/{thread_id}" + return redirect_url + return None + + @require_GET @login_required @use_bulk_ops @@ -361,6 +386,10 @@ def single_thread(request, course_key, discussion_id, thread_id): 'annotated_content_info': annotated_content_info, }) else: + redirect_url = redirect_url_to_new_mfe(request, str(course.id), thread_id) + if redirect_url: + return redirect(redirect_url) + course_id = str(course.id) tab_view = CourseTabView() return tab_view.get(request, course_id, 'discussion', discussion_id=discussion_id, thread_id=thread_id) @@ -620,6 +649,12 @@ def user_profile(request, course_key, user_id): 'annotated_content_info': context['annotated_content_info'], }) else: + discussions_mfe_enabled_for_everyone = ENABLE_DISCUSSIONS_MFE_FOR_EVERYONE.is_enabled(course_key) + privileged_user = is_privileged_user(course_key, request.user) + if discussions_mfe_enabled_for_everyone and (not privileged_user): + mfe_context = _discussions_mfe_context(request.GET, course_key, True, False, False) + return redirect(mfe_context['mfe_url']) + tab_view = CourseTabView() # To avoid mathjax loading from 'mathjax_include.html'