From 706df06638352ea161bbd4b3643f39fb3d0f3b92 Mon Sep 17 00:00:00 2001 From: Tinuade Adeleke Date: Thu, 21 Oct 2021 16:06:22 +0100 Subject: [PATCH] feat: updated pages to the new custom pages design (#28686) made changes to pages template refactored method to handle reordering of static tabs refactored test for the refactored method added link to the pages and resources MFE on the updated page --- .../rest_api/v0/tests/test_tabs.py | 22 ++-- .../contentstore/rest_api/v0/views/tabs.py | 6 +- cms/djangoapps/contentstore/views/tabs.py | 59 ++++++---- .../contentstore/views/tests/test_tabs.py | 27 ++--- cms/static/images/preview-lms-staticpage.png | Bin 0 -> 3214 bytes cms/templates/edit-tabs.html | 104 +++++++----------- 6 files changed, 109 insertions(+), 109 deletions(-) create mode 100644 cms/static/images/preview-lms-staticpage.png diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py index 6722ed2d84..3a159cf042 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py @@ -42,11 +42,15 @@ class TabsAPITests(CourseTestCase): ) # add a static tab to the course, for code coverage - self.test_tab = ItemFactory.create( - parent_location=self.course.location, - category="static_tab", - display_name="Static_1", - ) + # add 4 static tabs to the course, for code coverage + self.test_tabs = [] + for i in range(1, 5): + tab = ItemFactory.create( + parent_location=self.course.location, + category="static_tab", + display_name=f"Static_{i}" + ) + self.test_tabs.append(tab) self.reload_course() def check_invalid_response(self, resp): @@ -107,13 +111,13 @@ class TabsAPITests(CourseTestCase): # reorder the last two tabs tab_ids[num_orig_tabs - 1], tab_ids[num_orig_tabs - 2] = tab_ids[num_orig_tabs - 2], tab_ids[num_orig_tabs - 1] - # remove the middle tab + # remove the third to the last tab, the removed tab has to be a static tab # (the code needs to handle the case where tabs requested for re-ordering is a subset of the tabs in the course) - removed_tab = tab_ids.pop(num_orig_tabs // 2) - assert len(tab_ids) == num_orig_tabs - 1 + removed_tab = tab_ids.pop(num_orig_tabs - 3) + self.assertEqual(len(tab_ids), num_orig_tabs - 1) # post the request - resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids]) + resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids if 'static' in tab_id]) assert resp.status_code == 204 # reload the course and verify the new tab order diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index 0f6ddd778e..58e0e147f1 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py @@ -13,7 +13,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from ..serializers import CourseTabSerializer, CourseTabUpdateSerializer, TabIDLocatorSerializer -from ....views.tabs import edit_tab_handler, get_course_tabs, reorder_tabs_handler +from ....views.tabs import edit_tab_handler, get_course_static_tabs, reorder_tabs_handler @view_auth_classes(is_authenticated=True) @@ -34,7 +34,7 @@ class CourseTabListView(DeveloperErrorViewMixin, APIView): @verify_course_exists() def get(self, request: Request, course_id: str) -> Response: """ - Get a list of all the tabs in a course including hidden tabs. + Get a list of all the static tabs in a course including hidden tabs. **Example Request** @@ -82,7 +82,7 @@ class CourseTabListView(DeveloperErrorViewMixin, APIView): self.permission_denied(request) course_module = modulestore().get_course(course_key) - tabs_to_render = get_course_tabs(course_module, request.user) + tabs_to_render = get_course_static_tabs(course_module, request.user) return Response(CourseTabSerializer(tabs_to_render, many=True).data) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 356087f51f..4788ddf4e7 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -61,10 +61,10 @@ def tabs_handler(request, course_key_string): return JsonResponse() elif request.method == "GET": # assume html - # get all tabs from the tabs list: static tabs (a.k.a. user-created tabs) and built-in tabs + # get all tabs from the tabs list and select only static tabs (a.k.a. user-created tabs) # present in the same order they are displayed in LMS - tabs_to_render = list(get_course_tabs(course_item, request.user)) + tabs_to_render = list(get_course_static_tabs(course_item, request.user)) return render_to_response( "edit-tabs.html", @@ -78,9 +78,9 @@ def tabs_handler(request, course_key_string): return HttpResponseNotFound() -def get_course_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: +def get_course_static_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: """ - Yields all the course tabs in a course including hidden tabs. + Yields all the static tabs in a course including hidden tabs. Args: course_item (CourseBlock): The course object from which to get the tabs @@ -96,7 +96,7 @@ def get_course_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab] # static tab needs its locator information to render itself as an xmodule static_tab_loc = course_item.id.make_usage_key("static_tab", tab.url_slug) tab.locator = static_tab_loc - yield tab + yield tab def update_tabs_handler(course_item: CourseBlock, tabs_data: Dict, user: User) -> None: @@ -119,7 +119,7 @@ def update_tabs_handler(course_item: CourseBlock, tabs_data: Dict, user: User) - def reorder_tabs_handler(course_item, tabs_data, user): """ - Helper function for handling reorder of tabs request + Helper function for handling reorder of static tabs request """ # Tabs are identified by tab_id or locators. @@ -128,10 +128,40 @@ def reorder_tabs_handler(course_item, tabs_data, user): # their tab_ids since the xmodule editor uses only locators to identify new objects. requested_tab_id_locators = tabs_data["tabs"] - # original tab list in original order - old_tab_list = course_item.tabs + #get original tab list of only static tabs with their original index(position) in the full course tabs list + old_tab_dict = {} + for idx, tab in enumerate(course_item.tabs): + if isinstance(tab, StaticTab): + old_tab_dict[tab] = idx + old_tab_list = list(old_tab_dict.keys()) - # create a new list in the new order + new_tab_list = create_new_list(requested_tab_id_locators, old_tab_list) + + # Creates a full new course tab list of both default and static course tabs + # by looping through the new tab list of static only tabs and + # putting them in their new position in the list of course item tabs + # original_idx gives the list of positions of all static tabs in course tabs originally + full_new_tab_list = course_item.tabs + original_idx = list(old_tab_dict.values()) + for i in range(len(new_tab_list)): + full_new_tab_list[original_idx[i]] = new_tab_list[i] + + # validate the tabs to make sure everything is Ok (e.g., did the client try to reorder unmovable tabs?) + try: + CourseTabList.validate_tabs(full_new_tab_list) + except InvalidTabsException as exception: + raise ValidationError({"error": f"New list of tabs is not valid: {str(exception)}."}) from exception + + # persist the new order of the tabs + course_item.tabs = full_new_tab_list + modulestore().update_item(course_item, user.id) + + +def create_new_list(requested_tab_id_locators, old_tab_list): + """ + Helper function for creating a new course tab list in the new order + of reordered tabs + """ new_tab_list = [] for tab_id_locator in requested_tab_id_locators: tab = get_tab_by_tab_id_locator(old_tab_list, tab_id_locator) @@ -143,16 +173,7 @@ def reorder_tabs_handler(course_item, tabs_data, user): # global or course settings. so add those to the end of the list. non_displayed_tabs = set(old_tab_list) - set(new_tab_list) new_tab_list.extend(non_displayed_tabs) - - # validate the tabs to make sure everything is Ok (e.g., did the client try to reorder unmovable tabs?) - try: - CourseTabList.validate_tabs(new_tab_list) - except InvalidTabsException as exception: - raise ValidationError({"error": f"New list of tabs is not valid: {str(exception)}."}) from exception - - # persist the new order of the tabs - course_item.tabs = new_tab_list - modulestore().update_item(course_item, user.id) + return new_tab_list def edit_tab_handler(course_item: CourseBlock, tabs_data: Dict, user: User): diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 9d3ebee96d..0ccc8878ca 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,6 +1,5 @@ """ Tests for tab functions (just primitive). """ - import json from cms.djangoapps.contentstore.tests.utils import CourseTestCase @@ -25,12 +24,15 @@ class TabsPageTests(CourseTestCase): # Set the URL for tests self.url = reverse_course_url('tabs_handler', self.course.id) - # add a static tab to the course, for code coverage - self.test_tab = ItemFactory.create( - parent_location=self.course.location, - category="static_tab", - display_name="Static_1" - ) + # add 4 static tabs to the course, for code coverage + self.test_tabs = [] + for i in range(1, 5): + tab = ItemFactory.create( + parent_location=self.course.location, + category="static_tab", + display_name=f"Static_{i}" + ) + self.test_tabs.append(tab) self.reload_course() def check_invalid_tab_id_response(self, resp): @@ -39,7 +41,6 @@ class TabsPageTests(CourseTestCase): self.assertEqual(resp.status_code, 400) resp_content = json.loads(resp.content.decode('utf-8')) self.assertIn("error", resp_content) - self.assertIn("invalid_tab_id", resp_content['error']) def test_not_implemented(self): """Verify not implemented errors""" @@ -85,15 +86,15 @@ class TabsPageTests(CourseTestCase): # reorder the last two tabs tab_ids[num_orig_tabs - 1], tab_ids[num_orig_tabs - 2] = tab_ids[num_orig_tabs - 2], tab_ids[num_orig_tabs - 1] - # remove the middle tab + # remove the third to the last tab, the removed tab has to be a static tab # (the code needs to handle the case where tabs requested for re-ordering is a subset of the tabs in the course) - removed_tab = tab_ids.pop(num_orig_tabs // 2) + removed_tab = tab_ids.pop(num_orig_tabs - 3) self.assertEqual(len(tab_ids), num_orig_tabs - 1) - # post the request + # post the request with the reordered static tabs only resp = self.client.ajax_post( self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]}, + data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids if 'static' in tab_id]}, ) self.assertEqual(resp.status_code, 204) @@ -178,7 +179,7 @@ class TabsPageTests(CourseTestCase): """ Verify that the static tab renders itself with the correct HTML """ - preview_url = f'/xblock/{self.test_tab.location}/{STUDENT_VIEW}' + preview_url = f'/xblock/{self.test_tabs[0].location}/{STUDENT_VIEW}' resp = self.client.get(preview_url, HTTP_ACCEPT='application/json') self.assertEqual(resp.status_code, 200) diff --git a/cms/static/images/preview-lms-staticpage.png b/cms/static/images/preview-lms-staticpage.png new file mode 100644 index 0000000000000000000000000000000000000000..7940a106bf47f02991988c1bda329d209ec371cc GIT binary patch literal 3214 zcmV;93~}>`P)-+or`uh6c-``+hU_d}XARr)6P*A_Wzi)4EkdTm{pPxZMK!AXNprD|Ce}6we zKL7v#nVFe_f`ZS_&+qT=5D*YBFfdF^Os}u6JUl!vFE3wTUyqNEA0Hohcz9V^Sq~2n zPft$(00D4ta8XcDaB*;`sHjj;P=A1bO-xLHe}6A8FdrZvFfT9B(a{eO55T~{U|(Nf zUtm8!K!1OL0096`Pf$NVKOY|;0RR9XAR)cIy?k|S+1cj%{{BEeKTuCkKtMsBo}NBF zK59=cYN6GBetzuN$grY_US4d{@cZiO>f77f?AODvu)5Ob_MDuYPEJm3ZEgSk{r~*@ zjEs!@#>W1-y8pw&|JT?5=H~zU`u+F!{^{xd%F6!0!2jCX|L*SXP298q01DHFeYCjr*d|S{Y_jb(>3ye1+NSsY|NjA<8A-MQ1KxKm zFZ=Z6mj||`(Kn+{BZ&nu=HYQc1dL54j7{O~M?U(okADJQCzs~r;o;%o;o;%&a1r4D zfJqRzOAn7r0>lV#!i0aUoo*Z2674BU+ z=}kNN*~|XZ8Qn&ktQDk9Vaa6y?uvM?e%Lk{nLFrruvvFc;mj&Ixa^s;BbUP-3 zOQ+tz!VdGxlKIde=n|(9rtRg+sdEsw(4FLFPr)uN&y0F+-oTA8af51lX(smz7>&JI z*#We=-)%uJOT91!&unW}Q&wMh_enQ}#3^_-xoGNjgNjne16Hn^cuuPBY=nZzor@N; z-NlogQSj6*l^KQa5nM0OahqkTG`l6wJYH_J%hTK4+&BfFQ12aX zr#xP6*d%wSlk$LqeNOeb8D8izGVuQb<>BGs;o;%o@e+d1!^KiuuWl5E;iDTfkgtT{ z>eb$jn2)Iy+FC-5D4mS)@ROfv-wHYKJU(a%nTll4SiF0ghIm7V(E+pGd-MsxhE1M> zUWHls>iKd6K3^}wHY#y^P@orA`RV7gwvYTXV;GI^%Z7rRSC@u7eLZAIyXMJcK9Nt8xdYJ8=>O`^u3bH;h(XY+HC2&hzPqKR5 zh;aiLQIuTgxx2clWOuFYW1M6^KQeKu68}Op)9n%mC{mZ;xs;*Q+i)pq@UDh?TtR8M zU@~2WZK*0;<&yDLx{S~!CSkdJWy_hEhx0*FMhSJy^rH|t$Yj0}Xm1Pl&e!5VQ7pqP zr@|{}8k^+IvacW+n-QGP(Gr*Waz3nblx&>NzaSV77b|(-SEQ&hG!Cu-Jz`4jRZD7L zR4!C;vRZML&X+dpiKK+8MWsxLL6+bxF=NOK-J2?6afEX_z3JlQH+HYarh-9 zkPgiO-G~Cuu@+4M;ENOx2NMQXeR*qq*NN>?@DIWg$B(U>ODZ6j+g!D3kY7O#EoAyTH^ z*!63pjH1A}0M}$nOtC%Hz*7!UtAxn`z(NQufL9UQ>$G5LjrEZqWVN_~j-{)ZcpF&B zHo!Gyyq2O2X*P=?Nh>h_wXcIX%EO#!djp}N7}mnBg?7Y330^4*0~nIE(duTIn!=ID z1u;jaRDk0Yme;CP17_Kx1Q(CaRrT^4Eqi6mLC+gM1WDFfMjz^BB${u68;!`js9`X* z<#`IJ8*C-Wl_#BKQKJP7=IUGcb}-YaaJ9`OQ&3l8g*vC<7NKO}wW)A4v)7i?Qhm%{ zBfdPNQje60b=6R-BWgjI#DYp~NZTaYHwAl>*vE4!4JPragt8mfCWSqaWZ{X*jrD!M z%1*LMI*ThLIiL{=yqe4g&Q7lyNW=4on+HG z!-YyhnWBZ2EVc&yivi38$qH&v8il^6;e3d~_7Sa1RnG|i9~D9y&`K-;OEOH4%O zM(b@WS=6Wzp}?(aVIg_EL83iuW#C92{fQt>j z4raQZWH}H_!BYkMpV~?`2Ag06g;Z2%AsS2%*j74+by5V5Hi^mjw$%}}AWT#$V$Ga3 zk{e_qEPHC4CP|zdVWu7@S?MIpzhnKXn0}^cwu5D=aZQ7KDB2oU4tkOAN*2d%*lvpO zc4}tMn)V08FH5mOwI$xBTwk(sK>^gbtj)eVQSZF_J>2is!(GV}-0{Q|Ist)!b(vmR z$zm94p>e{uU5T=f3L9r9SttS&re!;1y^lOB!n+ zS!enCU6$3gq+2d(5JcBqr;*oLxPeOLVqN2oZW9)Tl`Kr`XccbJqeJv0i={A?@*gA$ zw_Mb55!8Hx>MNaY!XFM>iIXHzC3a4|UX%%%fOKz56$Pl_#Ey9|Z zC`>ZIzQSeAq9l##V*O0%k&=bQjN0)FnkX9FLxXX+ubV6cy&+Q);|3QoJCX%{LN|+b zVc6T-*m@2oUkm1(b73 z*-F+nCLky_i4|G+%y#&?a*~Aw#2Ve@udfSOxjQxB_A9T$@Ob`%mKSI(-mFVn%hpsl zk;MXHj@vJ5xwkN&Td);+VxtDZ6qe^XxDPh@&dbh|Q6KWQg?g*>l*Xm`R5U$rWhW%2 z)e*JYcANoI#j@|zc|1v+1mEGN9Un1N=vd`lr27_*f3|EL&rfR#IAO9@G z7aigl44wTF+-xO#T*Y*zm^-b+OV-08p#RL <%block name="title">${_("Pages")} <%block name="bodyclass">is-signedin course view-static-pages @@ -27,12 +28,40 @@ <%block name="content">
+ % if context_course: + <% + pages_and_resources_mfe_url = get_pages_and_resources_url(context_course.id) + pages_and_resources_mfe_enabled = bool(pages_and_resources_mfe_url) + %> + % endif + + % if pages_and_resources_mfe_enabled: +
+
+ +
+

+ > ${_("Custom Pages")} +

+ % else:

${_("Content")} - ## Translators: Pages refer to the tabs that appear in the top navigation of each course. - > ${_("Pages")} + ## Translators: Custom Pages refer to the tabs that appear in the top navigation of each course. + > ${_("Custom Pages")}

+ % endif
-

${_("Pages in Your Course")}

+

${_("Custom pages in your course")}

- ${_('Preview of Pages in your course')} -
${_("Pages appear in your course's top navigation bar. The default pages (Home, Course, Discussion, Wiki, and Progress) are followed by textbooks and custom pages.")}
+ ${_('Preview of Pages in your course')} +
${_("Custom pages are listed horizontally at the top of your course after default pages and textbooks. In the above example, custom pages for \"Course Schedule\" and \"Supplements\" have been added to a course.")}