From 8169aa99da32902b00a7f76a20c627ea1d44576c Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Tue, 28 Jun 2022 16:19:32 +0000 Subject: [PATCH] fix: if pages and resources view is disabled, show all pages in studio (#30550) In a previous PR #28686, the ability to see and enable/disable wiki and progress tabs was removed from studio along with the ability to re-order non-static tabs. The ability to toggle the Wiki tab was moved to the pages and resources section of the course authoring MFE. If that MFE is unavailable this means there is no way to show/hide the Wiki. This reverts some of the old changes if the pages and resources view is disabled. --- .../rest_api/v0/tests/test_tabs.py | 35 ++------ .../contentstore/rest_api/v0/views/tabs.py | 6 +- cms/djangoapps/contentstore/views/tabs.py | 21 ++--- .../contentstore/views/tests/test_tabs.py | 20 ++--- cms/static/images/preview-lms-staticpage.png | Bin 3214 -> 0 bytes cms/templates/edit-tabs.html | 78 ++++++++++++++++-- xmodule/tabs.py | 3 +- 7 files changed, 103 insertions(+), 60 deletions(-) delete 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 5162b50b06..5daf2f55a0 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py @@ -43,15 +43,11 @@ class TabsAPITests(CourseTestCase): ) # add a static tab to the course, for code coverage - # 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.test_tab = ItemFactory.create( + parent_location=self.course.location, + category="static_tab", + display_name="Static_1", + ) self.reload_course() def check_invalid_response(self, resp): @@ -129,27 +125,6 @@ class TabsAPITests(CourseTestCase): new_tab_ids = [tab.tab_id for tab in self.course.tabs] assert new_tab_ids == reordered_tab_ids - def test_reorder_tabs_invalid_list(self): - """ - Test re-ordering of tabs with invalid tab list. - - Not all tabs can be rearranged. Here we are trying to swap the first - two tabs, which is disallowed since the first tab is the "Course" tab - which is immovable. - """ - - orig_tab_ids = [tab.tab_id for tab in self.course.tabs] - tab_ids = list(orig_tab_ids) - - # reorder the first two tabs - tab_ids[0], tab_ids[1] = tab_ids[1], tab_ids[0] - - # post the request - resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids]) - assert resp.status_code == 400 - error = self.check_invalid_response(resp) - assert "error" in error - def test_reorder_tabs_invalid_tab_ids(self): """ Test re-ordering of tabs with invalid tab. diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index ce5e72de43..518381a85a 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_static_tabs, reorder_tabs_handler +from ....views.tabs import edit_tab_handler, get_course_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 static tabs in a course including hidden tabs. + Get a list of all the 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_static_tabs(course_module, request.user) + tabs_to_render = get_course_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 85f6b89850..72c7c90c72 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -19,7 +19,7 @@ from xmodule.tabs import CourseTab, CourseTabList, InvalidTabsException, StaticT from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from ..utils import get_lms_link_for_item +from ..utils import get_lms_link_for_item, get_pages_and_resources_url __all__ = ["tabs_handler", "update_tabs_handler"] @@ -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 and select only static tabs (a.k.a. user-created tabs) + # get all tabs from the tabs list: static tabs (a.k.a. user-created tabs) and built-in tabs # present in the same order they are displayed in LMS - tabs_to_render = list(get_course_static_tabs(course_item, request.user)) + tabs_to_render = list(get_course_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_static_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: +def get_course_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: """ - Yields all the static tabs in a course including hidden tabs. + Yields all the course tabs in a course including hidden tabs. Args: course_item (CourseBlock): The course object from which to get the tabs @@ -90,12 +90,14 @@ def get_course_static_tabs(course_item: CourseBlock, user: User) -> Iterable[Cou Iterable[CourseTab]: An iterable containing course tab objects from the course """ - + pages_and_resources_mfe_enabled = bool(get_pages_and_resources_url(course_item.id)) for tab in CourseTabList.iterate_displayable(course_item, user=user, inline_collections=False, include_hidden=True): if isinstance(tab, StaticTab): # 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 + # If the course apps MFE is set up and pages and resources is enabled, then only show static tabs + if isinstance(tab, StaticTab) or not pages_and_resources_mfe_enabled: yield tab @@ -119,7 +121,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 static tabs request + Helper function for handling reorder of tabs request """ # Static tabs are identified by locators (a UsageKey) instead of a tab id like @@ -152,9 +154,8 @@ def create_new_list(tab_locators, old_tab_list): tab = get_tab_by_tab_id_locator(old_tab_list, tab_locator) if tab is None: raise ValidationError({"error": f"Tab with id_locator '{tab_locator}' does not exist."}) - if not isinstance(tab, StaticTab): - raise ValidationError({"error": f"Cannot reorder tabs of type '{tab.type}'"}) - new_tab_list.append(tab) + if isinstance(tab, StaticTab): + new_tab_list.append(tab) # the old_tab_list may contain additional tabs that were not rendered in the UI because of # global or course settings. so add those to the end of the list. diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 58a6393585..8df8c7b3c9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,5 +1,6 @@ """ Tests for tab functions (just primitive). """ + import json import random @@ -26,15 +27,12 @@ class TabsPageTests(CourseTestCase): # Set the URL for tests self.url = reverse_course_url('tabs_handler', self.course.id) - # 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) + # 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" + ) self.reload_course() def check_invalid_tab_id_response(self, resp): @@ -95,7 +93,7 @@ class TabsPageTests(CourseTestCase): # Remove one tab randomly. This shouldn't delete the tab. tabs_data.pop() - # post the request with the reordered static tabs only + # post the request resp = self.client.ajax_post( self.url, data={ @@ -167,7 +165,7 @@ class TabsPageTests(CourseTestCase): """ Verify that the static tab renders itself with the correct HTML """ - preview_url = f'/xblock/{self.test_tabs[0].location}/{STUDENT_VIEW}' + preview_url = f'/xblock/{self.test_tab.location}/{STUDENT_VIEW}' resp = self.client.get(preview_url, HTTP_ACCEPT='application/json') assert resp.status_code == 200 diff --git a/cms/static/images/preview-lms-staticpage.png b/cms/static/images/preview-lms-staticpage.png deleted file mode 100644 index 7940a106bf47f02991988c1bda329d209ec371cc..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 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 ${_("Content")} ## Translators: Custom Pages refer to the tabs that appear in the top navigation of each course. - > ${_("Custom pages")} + > ${_("Pages")} % endif @@ -101,8 +101,59 @@ css_class = css_class + " is-fixed" %> -
  • + % if isinstance(tab, StaticTab): +
  • + % else: +
  • +
    + + % if tab.is_collection: + +

    ${_(tab.name)}

    +
      + % for item in tab.items(context_course): +
    • + ${_(item.name)} +
      • + % endfor +
    + + % else: +

    ${_(tab.name)}

    + % endif +
    + +
    +
      + + % if tab.is_hideable: +
    • + + % if tab.is_hidden: + + % else: + + % endif +
      +
      • + % endif + +
    +
    + + % if tab.is_movable: +
    + ${_("Drag to reorder")} +
    + % else: +
    + ${_("This page cannot be reordered")} +
    + % endif +
    • + + % endif % endfor
  • @@ -116,6 +167,22 @@ + % if pages_and_resources_mfe_enabled: + + % else: + % endif
    -

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

    +

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

    - ${_('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.")}
    + ${_('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.")}
    diff --git a/xmodule/tabs.py b/xmodule/tabs.py index 56c3faaf08..b18f6ca12d 100644 --- a/xmodule/tabs.py +++ b/xmodule/tabs.py @@ -53,7 +53,7 @@ class CourseTab(metaclass=ABCMeta): priority = None # Class property that specifies whether the tab can be moved within a course's list of tabs - is_movable = True + is_movable = False # Class property that specifies whether the tab is a collection of other tabs is_collection = False @@ -301,6 +301,7 @@ class StaticTab(CourseTab): """ type = 'static_tab' is_default = False # A static tab is never added to a course by default + is_movable = True allow_multiple = True priority = 100