From 53f7eca83f4b31b574b34d649ff723fb072e312c Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Wed, 17 Dec 2014 17:44:14 +0300 Subject: [PATCH 01/18] List of CAPA input types + setting to choose one --- .../xmodule/xmodule/library_content_module.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 5004a5d51ae2..45452a088d19 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -5,6 +5,7 @@ from bson.objectid import ObjectId, InvalidId from collections import namedtuple from copy import copy + from .mako_module import MakoModuleDescriptor from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocator @@ -19,6 +20,7 @@ from .xml_module import XmlDescriptor from pkg_resources import resource_string + # Make '_' a no-op so we can scrape strings _ = lambda text: text @@ -28,6 +30,40 @@ def enum(**enums): return type('Enum', (), enums) +def _get_capa_types(): + capa_types = { + 'annotationinput': _('Annotation'), + 'checkboxgroup': _('Checkbox Group'), + 'checkboxtextgroup': _('Checkbox Text Group'), + 'chemicalequationinput': _('Chemical Equation'), + 'choicegroup': _('Choice Group'), + 'codeinput': _('Code Input'), + 'crystallography': _('Crystallography'), + 'designprotein2dinput': _('Design Protein 2D'), + 'drag_and_drop_input': _('Drag and Drop'), + 'editageneinput': _('Edit A Gene'), + 'editamoleculeinput': _('Edit A Molecule'), + 'filesubmission': _('File Submission'), + 'formulaequationinput': _('Formula Equation'), + 'imageinput': _('Image'), + 'javascriptinput': _('Javascript Input'), + 'jsinput': _('JS Input'), + 'matlabinput': _('Matlab'), + 'optioninput': _('Select option'), + 'radiogroup': _('Radio Group'), + 'radiotextgroup': _('Radio Text Group'), + 'schematic': _('Schematic'), + 'textbox': _('Code Text Input'), + 'textline': _('Text Line'), + 'vsepr_input': _('VSEPR'), + } + + return sorted([ + {'value': capa_type, 'display_name': caption} + for capa_type, caption in capa_types.items() + ], key=lambda item: item.get('display_name')) + + class LibraryVersionReference(namedtuple("LibraryVersionReference", "library_id version")): """ A reference to a specific library, with an optional version. @@ -146,6 +182,13 @@ class LibraryContentFields(object): default=1, scope=Scope.settings, ) + capa_type = String( + display_name=_("Problem Type"), + help=_("The type of components to include in this block"), + default="any", + values=[{"display_name": _("Any Type"), "value": "any"}] + _get_capa_types(), + scope=Scope.settings, + ) filters = String(default="") # TBD has_score = Boolean( display_name=_("Scored"), From 93452281ca86659c77a9f3256bb39c708b840d2b Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Wed, 17 Dec 2014 19:07:45 +0300 Subject: [PATCH 02/18] Filtering children by CAPA input type. --- .../xmodule/xmodule/library_content_module.py | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 45452a088d19..8a59c4a76449 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -25,6 +25,10 @@ _ = lambda text: text +ANY_CAPA_TYPE_VALUE = 'any' +CAPA_BLOCK_TYPE = 'problem' + + def enum(**enums): """ enum helper in lieu of enum34 """ return type('Enum', (), enums) @@ -32,6 +36,7 @@ def enum(**enums): def _get_capa_types(): capa_types = { + ANY_CAPA_TYPE_VALUE: _('Any Type'), 'annotationinput': _('Annotation'), 'checkboxgroup': _('Checkbox Group'), 'checkboxtextgroup': _('Checkbox Text Group'), @@ -185,8 +190,8 @@ class LibraryContentFields(object): capa_type = String( display_name=_("Problem Type"), help=_("The type of components to include in this block"), - default="any", - values=[{"display_name": _("Any Type"), "value": "any"}] + _get_capa_types(), + default=ANY_CAPA_TYPE_VALUE, + values=_get_capa_types(), scope=Scope.settings, ) filters = String(default="") # TBD @@ -215,6 +220,21 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): as children of this block, but only a subset of those children are shown to any particular student. """ + def _filter_children(self, child_locator): + if self.capa_type == ANY_CAPA_TYPE_VALUE: + return True + + if child_locator.block_type != CAPA_BLOCK_TYPE: + return False + + block = self.runtime.get_block(child_locator) + + if not hasattr(block, 'lcp'): + return True + + return any(self.capa_type in capa_input.tags for capa_input in block.lcp.inputs.values()) + + def selected_children(self): """ Returns a set() of block_ids indicating which of the possible children @@ -231,7 +251,7 @@ def selected_children(self): return self._selected_set # pylint: disable=access-member-before-definition # Determine which of our children we will show: selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples - valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member + valid_block_keys = set([(c.block_type, c.block_id) for c in self.children if self._filter_children(c)]) # pylint: disable=no-member # Remove any selected blocks that are no longer valid: selected -= (selected - valid_block_keys) # If max_count has been decreased, we may have to drop some previously selected blocks: @@ -407,7 +427,8 @@ def editor_saved(self, user, old_metadata, old_content): If source_libraries has been edited, refresh_children automatically. """ old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) - if set(old_source_libraries) != set(self.source_libraries): + if (set(old_source_libraries) != set(self.source_libraries) or + old_metadata.get('capa_type', ANY_CAPA_TYPE_VALUE) != self.capa_type): try: self.refresh_children(None, None, update_db=False) # update_db=False since update_item() is about to be called anyways except ValueError: From 13c19ac1664e14cabb82ac277c031da8123de215 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 22 Dec 2014 18:37:05 +0300 Subject: [PATCH 03/18] Bok choy acceptance tests --- .../xmodule/xmodule/library_content_module.py | 16 +- common/test/acceptance/pages/lms/library.py | 11 ++ .../test/acceptance/pages/studio/library.py | 19 ++ .../test/acceptance/tests/lms/test_library.py | 162 ++++++++++++++++-- 4 files changed, 188 insertions(+), 20 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 8a59c4a76449..e2e2481324e1 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -35,8 +35,10 @@ def enum(**enums): def _get_capa_types(): + """ + Gets capa types tags and labels + """ capa_types = { - ANY_CAPA_TYPE_VALUE: _('Any Type'), 'annotationinput': _('Annotation'), 'checkboxgroup': _('Checkbox Group'), 'checkboxtextgroup': _('Checkbox Text Group'), @@ -54,7 +56,7 @@ def _get_capa_types(): 'javascriptinput': _('Javascript Input'), 'jsinput': _('JS Input'), 'matlabinput': _('Matlab'), - 'optioninput': _('Select option'), + 'optioninput': _('Select Option'), 'radiogroup': _('Radio Group'), 'radiotextgroup': _('Radio Text Group'), 'schematic': _('Schematic'), @@ -63,7 +65,7 @@ def _get_capa_types(): 'vsepr_input': _('VSEPR'), } - return sorted([ + return [{'value': ANY_CAPA_TYPE_VALUE, 'display_name': _('Any Type')}] + sorted([ {'value': capa_type, 'display_name': caption} for capa_type, caption in capa_types.items() ], key=lambda item: item.get('display_name')) @@ -221,6 +223,9 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): any particular student. """ def _filter_children(self, child_locator): + """ + Filters children by CAPA problem type, if configured + """ if self.capa_type == ANY_CAPA_TYPE_VALUE: return True @@ -234,7 +239,6 @@ def _filter_children(self, child_locator): return any(self.capa_type in capa_input.tags for capa_input in block.lcp.inputs.values()) - def selected_children(self): """ Returns a set() of block_ids indicating which of the possible children @@ -427,8 +431,8 @@ def editor_saved(self, user, old_metadata, old_content): If source_libraries has been edited, refresh_children automatically. """ old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) - if (set(old_source_libraries) != set(self.source_libraries) or - old_metadata.get('capa_type', ANY_CAPA_TYPE_VALUE) != self.capa_type): + if set(old_source_libraries) != set(self.source_libraries) or \ + old_metadata.get('capa_type', ANY_CAPA_TYPE_VALUE) != self.capa_type: try: self.refresh_children(None, None, update_db=False) # update_db=False since update_item() is about to be called anyways except ValueError: diff --git a/common/test/acceptance/pages/lms/library.py b/common/test/acceptance/pages/lms/library.py index 8655fae79f55..33970823445f 100644 --- a/common/test/acceptance/pages/lms/library.py +++ b/common/test/acceptance/pages/lms/library.py @@ -16,6 +16,9 @@ def __init__(self, browser, locator): self.locator = locator def is_browser_on_page(self): + """ + Checks if page is opened + """ return self.q(css='{}[data-id="{}"]'.format(self.BODY_SELECTOR, self.locator)).present def _bounded_selector(self, selector): @@ -35,3 +38,11 @@ def children_contents(self): """ child_blocks = self.q(css=self._bounded_selector("div[data-id]")) return frozenset(child.text for child in child_blocks) + + @property + def children_headers(self): + """ + Gets headers if all child XBlocks as list of strings + """ + child_blocks_headers = self.q(css=self._bounded_selector("div[data-id] h2.problem-header")) + return frozenset(child.text for child in child_blocks_headers) diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index ea7f2299f961..71df4c4ad18d 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -122,6 +122,7 @@ class StudioLibraryContentXBlockEditModal(CourseOutlineModal, PageObject): LIBRARY_LABEL = "Libraries" COUNT_LABEL = "Count" SCORED_LABEL = "Scored" + PROBLEM_TYPE_LABEL = "Problem Type" def is_browser_on_page(self): """ @@ -196,6 +197,24 @@ def scored(self, scored): scored_select.select_by_value(str(scored)) EmptyPromise(lambda: self.scored == scored, "scored is updated in modal.").fulfill() + @property + def capa_type(self): + """ + Gets value of CAPA type select + """ + return self.get_metadata_input(self.PROBLEM_TYPE_LABEL).get_attribute('value') + + @capa_type.setter + def capa_type(self, value): + """ + Sets value of CAPA type select + """ + select_element = self.get_metadata_input(self.PROBLEM_TYPE_LABEL) + select_element.click() + problem_type_select = Select(select_element) + problem_type_select.select_by_value(value) + EmptyPromise(lambda: self.capa_type == value, "problem type is updated in modal.").fulfill() + def _add_library_key(self): """ Adds library key input diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index f83e6b94e9d5..4d6f34e8220c 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -19,22 +19,22 @@ UNIT_NAME = 'Test Unit' -@ddt.ddt -class LibraryContentTest(UniqueCourseTest): - """ - Test courseware. - """ +class LibraryContentTestBase(UniqueCourseTest): + """ Base class for library content block tests """ USERNAME = "STUDENT_TESTER" EMAIL = "student101@example.com" STAFF_USERNAME = "STAFF_TESTER" STAFF_EMAIL = "staff101@example.com" + def populate_library_fixture(self, library_fixture): + pass + def setUp(self): """ Set up library, course and library content XBlock """ - super(LibraryContentTest, self).setUp() + super(LibraryContentTestBase, self).setUp() self.courseware_page = CoursewarePage(self.browser, self.course_id) @@ -46,11 +46,7 @@ def setUp(self): ) self.library_fixture = LibraryFixture('test_org', self.unique_id, 'Test Library {}'.format(self.unique_id)) - self.library_fixture.add_children( - XBlockFixtureDesc("html", "Html1", data='html1'), - XBlockFixtureDesc("html", "Html2", data='html2'), - XBlockFixtureDesc("html", "Html3", data='html3'), - ) + self.populate_library_fixture(self.library_fixture) self.library_fixture.install() self.library_info = self.library_fixture.library_info @@ -83,7 +79,7 @@ def setUp(self): self.course_fixture.install() - def _refresh_library_content_children(self, count=1): + def _change_library_content_settings(self, count=1, capa_type=None): """ Performs library block refresh in Studio, configuring it to show {count} children """ @@ -91,6 +87,8 @@ def _refresh_library_content_children(self, count=1): library_container_block = StudioLibraryContainerXBlockWrapper.from_xblock_wrapper(unit_page.xblocks[0]) modal = StudioLibraryContentXBlockEditModal(library_container_block.edit()) modal.count = count + if capa_type is not None: + modal.capa_type = capa_type library_container_block.save_settings() self._go_to_unit_page(change_login=False) unit_page.wait_for_page() @@ -124,6 +122,7 @@ def _goto_library_block_page(self, block_id=None): block_id = block_id if block_id is not None else self.lib_block.locator #pylint: disable=attribute-defined-outside-init self.library_content_page = LibraryContentXBlockWrapper(self.browser, block_id) + self.library_content_page.wait_for_page() def _auto_auth(self, username, email, staff): """ @@ -132,6 +131,22 @@ def _auto_auth(self, username, email, staff): AutoAuthPage(self.browser, username=username, email=email, course_id=self.course_id, staff=staff).visit() + +@ddt.ddt +class LibraryContentTest(LibraryContentTestBase): + """ + Test courseware. + """ + def populate_library_fixture(self, library_fixture): + """ + Populates library fixture with XBlock Fixtures + """ + library_fixture.add_children( + XBlockFixtureDesc("html", "Html1", data='html1'), + XBlockFixtureDesc("html", "Html2", data='html2'), + XBlockFixtureDesc("html", "Html3", data='html3'), + ) + @ddt.data(1, 2, 3) def test_shows_random_xblocks_from_configured(self, count): """ @@ -143,7 +158,7 @@ def test_shows_random_xblocks_from_configured(self, count): When I go to LMS courseware page for library content xblock as student Then I can see {count} random xblocks from the library """ - self._refresh_library_content_children(count=count) + self._change_library_content_settings(count=count) self._auto_auth(self.USERNAME, self.EMAIL, False) self._goto_library_block_page() children_contents = self.library_content_page.children_contents @@ -160,9 +175,128 @@ def test_shows_all_if_max_set_to_greater_value(self): When I go to LMS courseware page for library content xblock as student Then I can see all xblocks from the library """ - self._refresh_library_content_children(count=10) + self._change_library_content_settings(count=10) self._auto_auth(self.USERNAME, self.EMAIL, False) self._goto_library_block_page() children_contents = self.library_content_page.children_contents self.assertEqual(len(children_contents), 3) self.assertEqual(children_contents, self.library_xblocks_texts) + + +@ddt.ddt +class StudioLibraryContainerCapaFilterTest(LibraryContentTestBase): + """ + Test Library Content block in LMS + """ + def _get_problem_choice_group_text(self, name, items): + """ Generates Choice Group CAPA problem XML """ + items_text = "\n".join([ + "{item}".format(correct=correct, item=item) + for item, correct in items + ]) + + return """ +

{name}

+ + {items} + +""".format(name=name, items=items_text) + + def _get_problem_select_text(self, name, items, correct): + """ Generates Select Option CAPA problem XML """ + items_text = ",".join(map(lambda item: "'{0}'".format(item), items)) + + return """ +

{name}

+ + + +""".format(name=name, options=items_text, correct=correct) + + def populate_library_fixture(self, library_fixture): + """ + Populates library fixture with XBlock Fixtures + """ + library_fixture.add_children( + XBlockFixtureDesc( + "problem", "Problem Choice Group 1", + data=self._get_problem_choice_group_text("Problem Choice Group 1 Text", [("1", False), ('2', True)]) + ), + XBlockFixtureDesc( + "problem", "Problem Choice Group 2", + data=self._get_problem_choice_group_text("Problem Choice Group 2 Text", [("Q", True), ('W', False)]) + ), + XBlockFixtureDesc( + "problem", "Problem Select 1", + data=self._get_problem_select_text("Problem Select 1 Text", ["Option 1", "Option 2"], "Option 1") + ), + XBlockFixtureDesc( + "problem", "Problem Select 2", + data=self._get_problem_select_text("Problem Select 2 Text", ["Option 3", "Option 4"], "Option 4") + ), + ) + + @property + def _problem_headers(self): + """ Expected XBLock headers according to populate_library_fixture """ + return frozenset(child.display_name.upper() for child in self.library_fixture.children) + + @ddt.data(1, 3) + def test_any_capa_type_shows_all(self, count): + """ + Scenario: Ensure setting "Any Type" for Problem Type does not filter out Problems + Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing + LibraryContent XBlock configured to draw XBlocks from that library + When I go to studio unit page for library content xblock as staff + And I set library content xblock Problem Type to "Any Type" and Count to {count} + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see {count} xblocks from the library of any type + """ + self._change_library_content_settings(count=count, capa_type="Any Type") + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_headers = self.library_content_page.children_headers + self.assertEqual(len(children_headers), count) + self.assertLessEqual(children_headers, self._problem_headers) + + @ddt.data( + ('Choice Group', 1, ["Problem Choice Group 1", "Problem Choice Group 2"]), + ('Select Option', 2, ["Problem Select 1", "Problem Select 2"]), + ) + @ddt.unpack + def test_capa_type_shows_only_chosen_type(self, capa_type, count, expected_headers): + """ + Scenario: Ensure setting "{capa_type}" for Problem Type draws aonly problem of {capa_type} from library + Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing + LibraryContent XBlock configured to draw XBlocks from that library + When I go to studio unit page for library content xblock as staff + And I set library content xblock Problem Type to "{capa_type}" and Count to {count} + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see {count} xblocks from the library of {capa_type} + """ + self._change_library_content_settings(count=count, capa_type=capa_type) + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_headers = self.library_content_page.children_headers + self.assertEqual(len(children_headers), count) + self.assertLessEqual(children_headers, self._problem_headers) + self.assertLessEqual(children_headers, set(map(lambda header: header.upper(), expected_headers))) + + def test_missing_capa_type_shows_none(self): + """ + Scenario: Ensure setting "{capa_type}" for Problem Type that is not present in library results in empty XBlock + Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing + LibraryContent XBlock configured to draw XBlocks from that library + When I go to studio unit page for library content xblock as staff + And I set library content xblock Problem Type to type not present in library + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see no xblocks + """ + self._change_library_content_settings(count=1, capa_type="Matlab") + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_headers = self.library_content_page.children_headers + self.assertEqual(len(children_headers), 0) From 31440f4204e60f70cb1021d8e875919afcabb1ae Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 23 Dec 2014 11:38:33 +0300 Subject: [PATCH 04/18] Fixed typo + combined problem type tests into single test --- common/test/acceptance/pages/lms/library.py | 2 +- .../test/acceptance/tests/lms/test_library.py | 89 +++++++++---------- 2 files changed, 43 insertions(+), 48 deletions(-) diff --git a/common/test/acceptance/pages/lms/library.py b/common/test/acceptance/pages/lms/library.py index 33970823445f..6978b5fa0b1e 100644 --- a/common/test/acceptance/pages/lms/library.py +++ b/common/test/acceptance/pages/lms/library.py @@ -42,7 +42,7 @@ def children_contents(self): @property def children_headers(self): """ - Gets headers if all child XBlocks as list of strings + Gets headers of all child XBlocks as list of strings """ child_blocks_headers = self.q(css=self._bounded_selector("div[data-id] h2.problem-header")) return frozenset(child.text for child in child_blocks_headers) diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index 4d6f34e8220c..53b26238c537 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -119,6 +119,9 @@ def _goto_library_block_page(self, block_id=None): Open library page in LMS """ self.courseware_page.visit() + paragraphs = self.courseware_page.q(css='.course-content p') + if paragraphs and "You were most recently in" in paragraphs.text[0]: + paragraphs[0].find_element_by_tag_name('a').click() block_id = block_id if block_id is not None else self.lib_block.locator #pylint: disable=attribute-defined-outside-init self.library_content_page = LibraryContentXBlockWrapper(self.browser, block_id) @@ -241,62 +244,54 @@ def _problem_headers(self): """ Expected XBLock headers according to populate_library_fixture """ return frozenset(child.display_name.upper() for child in self.library_fixture.children) - @ddt.data(1, 3) - def test_any_capa_type_shows_all(self, count): + def _set_library_content_settings(self, count=1, capa_type="Any Type"): """ - Scenario: Ensure setting "Any Type" for Problem Type does not filter out Problems - Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing - LibraryContent XBlock configured to draw XBlocks from that library - When I go to studio unit page for library content xblock as staff - And I set library content xblock Problem Type to "Any Type" and Count to {count} - And I refresh library content xblock and pulbish unit - When I go to LMS courseware page for library content xblock as student - Then I can see {count} xblocks from the library of any type + Sets library content XBlock parameters, saves, publishes unit, goes to LMS unit page and + gets children XBlock headers to assert against them """ - self._change_library_content_settings(count=count, capa_type="Any Type") + self._change_library_content_settings(count=count, capa_type=capa_type) self._auto_auth(self.USERNAME, self.EMAIL, False) self._goto_library_block_page() - children_headers = self.library_content_page.children_headers - self.assertEqual(len(children_headers), count) - self.assertLessEqual(children_headers, self._problem_headers) + return self.library_content_page.children_headers - @ddt.data( - ('Choice Group', 1, ["Problem Choice Group 1", "Problem Choice Group 2"]), - ('Select Option', 2, ["Problem Select 1", "Problem Select 2"]), - ) - @ddt.unpack - def test_capa_type_shows_only_chosen_type(self, capa_type, count, expected_headers): + def test_problem_type_selector(self): """ - Scenario: Ensure setting "{capa_type}" for Problem Type draws aonly problem of {capa_type} from library + Scenario: Ensure setting "Any Type" for Problem Type does not filter out Problems Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing LibraryContent XBlock configured to draw XBlocks from that library - When I go to studio unit page for library content xblock as staff - And I set library content xblock Problem Type to "{capa_type}" and Count to {count} - And I refresh library content xblock and pulbish unit + When I set library content xblock Problem Type to "Any Type" and Count to 3 and publish unit + When I go to LMS courseware page for library content xblock as student + Then I can see 3 xblocks from the library of any type + When I set library content xblock Problem Type to "Choice Group" and Count to 1 and publish unit When I go to LMS courseware page for library content xblock as student - Then I can see {count} xblocks from the library of {capa_type} + Then I can see 1 xblock from the library of "Choice Group" type + When I set library content xblock Problem Type to "Select Option" and Count to 2 and publish unit + When I go to LMS courseware page for library content xblock as student + Then I can see 2 xblock from the library of "Select Option" type + When I set library content xblock Problem Type to "Matlab" and Count to 2 and publish unit + When I go to LMS courseware page for library content xblock as student + Then I can see 0 xblocks from the library """ - self._change_library_content_settings(count=count, capa_type=capa_type) - self._auto_auth(self.USERNAME, self.EMAIL, False) - self._goto_library_block_page() - children_headers = self.library_content_page.children_headers - self.assertEqual(len(children_headers), count) + children_headers = self._set_library_content_settings(count=3, capa_type="Any Type") + self.assertEqual(len(children_headers), 3) self.assertLessEqual(children_headers, self._problem_headers) - self.assertLessEqual(children_headers, set(map(lambda header: header.upper(), expected_headers))) - def test_missing_capa_type_shows_none(self): - """ - Scenario: Ensure setting "{capa_type}" for Problem Type that is not present in library results in empty XBlock - Given I have a library with two "Select Option" and two "Choice Group" problems, and a course containing - LibraryContent XBlock configured to draw XBlocks from that library - When I go to studio unit page for library content xblock as staff - And I set library content xblock Problem Type to type not present in library - And I refresh library content xblock and pulbish unit - When I go to LMS courseware page for library content xblock as student - Then I can see no xblocks - """ - self._change_library_content_settings(count=1, capa_type="Matlab") - self._auto_auth(self.USERNAME, self.EMAIL, False) - self._goto_library_block_page() - children_headers = self.library_content_page.children_headers - self.assertEqual(len(children_headers), 0) + # Choice group test + children_headers = self._set_library_content_settings(count=1, capa_type="Choice Group") + self.assertEqual(len(children_headers), 1) + self.assertLessEqual( + children_headers, + set(map(lambda header: header.upper(), ["Problem Choice Group 1", "Problem Choice Group 2"])) + ) + + # Choice group test + children_headers = self._set_library_content_settings(count=2, capa_type="Select Option") + self.assertEqual(len(children_headers), 2) + self.assertLessEqual( + children_headers, + set(map(lambda header: header.upper(), ["Problem Select 1", "Problem Select 2"])) + ) + + # Missing problem type test + children_headers = self._set_library_content_settings(count=2, capa_type="Matlab") + self.assertEqual(children_headers, set()) From f632d695ed7655a851823b692a6b7de10418e966 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Wed, 24 Dec 2014 11:33:53 +0300 Subject: [PATCH 05/18] Problem type filtering on `update_children` event --- common/lib/xmodule/xmodule/capa_module.py | 9 ++++++++ .../xmodule/xmodule/library_content_module.py | 19 +--------------- common/lib/xmodule/xmodule/library_tools.py | 22 ++++++++++++++++--- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 776dc36e3fb9..4c7f785c6d28 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -2,10 +2,12 @@ import json import logging import sys +from lxml import etree from pkg_resources import resource_string from .capa_base import CapaMixin, CapaFields, ComplexEncoder +from capa import inputtypes from .progress import Progress from xmodule.x_module import XModule, module_attr from xmodule.raw_module import RawDescriptor @@ -172,6 +174,13 @@ def non_editable_metadata_fields(self): ]) return non_editable_fields + @property + def problem_types(self): + """ Low-level problem type introspection for content libraries filtering by problem type """ + tree = etree.XML(self.data) + registered_tas = inputtypes.registry.registered_tags() + return set([node.tag for node in tree.iter() if node.tag in registered_tas]) + # Proxy to CapaModule for access to any of its attributes answer_available = module_attr('answer_available') check_button_name = module_attr('check_button_name') diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index e2e2481324e1..11d23106f4c0 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -222,23 +222,6 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): as children of this block, but only a subset of those children are shown to any particular student. """ - def _filter_children(self, child_locator): - """ - Filters children by CAPA problem type, if configured - """ - if self.capa_type == ANY_CAPA_TYPE_VALUE: - return True - - if child_locator.block_type != CAPA_BLOCK_TYPE: - return False - - block = self.runtime.get_block(child_locator) - - if not hasattr(block, 'lcp'): - return True - - return any(self.capa_type in capa_input.tags for capa_input in block.lcp.inputs.values()) - def selected_children(self): """ Returns a set() of block_ids indicating which of the possible children @@ -255,7 +238,7 @@ def selected_children(self): return self._selected_set # pylint: disable=access-member-before-definition # Determine which of our children we will show: selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples - valid_block_keys = set([(c.block_type, c.block_id) for c in self.children if self._filter_children(c)]) # pylint: disable=no-member + valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member # Remove any selected blocks that are no longer valid: selected -= (selected - valid_block_keys) # If max_count has been decreased, we may have to drop some previously selected blocks: diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index f8dcadf80e16..d0e6768ed868 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -5,8 +5,9 @@ from django.core.exceptions import PermissionDenied from opaque_keys.edx.locator import LibraryLocator from xblock.fields import Scope -from xmodule.library_content_module import LibraryVersionReference +from xmodule.library_content_module import LibraryVersionReference, ANY_CAPA_TYPE_VALUE from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.capa_module import CapaDescriptor class LibraryToolsService(object): @@ -45,6 +46,18 @@ def get_library_version(self, lib_key): return library.location.library_key.version_guid return None + def _filter_child(self, dest_block, child_descriptor): + """ + Filters children by CAPA problem type, if configured + """ + if dest_block.capa_type == ANY_CAPA_TYPE_VALUE: + return True + + if not isinstance(child_descriptor, CapaDescriptor): + return False + + return dest_block.capa_type in child_descriptor.problem_types + def update_children(self, dest_block, user_id, user_perms=None, update_db=True): """ This method is to be used when any of the libraries that a LibraryContentModule @@ -91,13 +104,16 @@ def update_children(self, dest_block, user_id, user_perms=None, update_db=True): new_libraries = [] for library_key, library in libraries: - def copy_children_recursively(from_block): + def copy_children_recursively(from_block, filter_problem_type=True): """ Internal method to copy blocks from the library recursively """ new_children = [] for child_key in from_block.children: child = self.store.get_item(child_key, depth=9) + + if filter_problem_type and not self._filter_child(dest_block, child): + continue # We compute a block_id for each matching child block found in the library. # block_ids are unique within any branch, but are not unique per-course or globally. # We need our block_ids to be consistent when content in the library is updated, so @@ -125,7 +141,7 @@ def copy_children_recursively(from_block): ) new_children.append(new_child_info.location) return new_children - root_children.extend(copy_children_recursively(from_block=library)) + root_children.extend(copy_children_recursively(from_block=library, filter_problem_type=True)) new_libraries.append(LibraryVersionReference(library_key, library.location.library_key.version_guid)) dest_block.source_libraries = new_libraries dest_block.children = root_children From cf4fb861b18de033294966344d643295232c8e0e Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 29 Dec 2014 15:29:30 +0300 Subject: [PATCH 06/18] Switched to filtering by response type rather than input type --- common/lib/xmodule/xmodule/capa_module.py | 6 +-- .../xmodule/xmodule/library_content_module.py | 46 +++++++++---------- common/lib/xmodule/xmodule/library_tools.py | 2 +- .../test/acceptance/tests/lms/test_library.py | 6 +-- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 4c7f785c6d28..47583d97065a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -7,7 +7,7 @@ from pkg_resources import resource_string from .capa_base import CapaMixin, CapaFields, ComplexEncoder -from capa import inputtypes +from capa import responsetypes from .progress import Progress from xmodule.x_module import XModule, module_attr from xmodule.raw_module import RawDescriptor @@ -178,8 +178,8 @@ def non_editable_metadata_fields(self): def problem_types(self): """ Low-level problem type introspection for content libraries filtering by problem type """ tree = etree.XML(self.data) - registered_tas = inputtypes.registry.registered_tags() - return set([node.tag for node in tree.iter() if node.tag in registered_tas]) + registered_tags = responsetypes.registry.registered_tags() + return set([node.tag for node in tree.iter() if node.tag in registered_tags]) # Proxy to CapaModule for access to any of its attributes answer_available = module_attr('answer_available') diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 11d23106f4c0..d8512bcdbe5e 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -39,30 +39,28 @@ def _get_capa_types(): Gets capa types tags and labels """ capa_types = { - 'annotationinput': _('Annotation'), - 'checkboxgroup': _('Checkbox Group'), - 'checkboxtextgroup': _('Checkbox Text Group'), - 'chemicalequationinput': _('Chemical Equation'), - 'choicegroup': _('Choice Group'), - 'codeinput': _('Code Input'), - 'crystallography': _('Crystallography'), - 'designprotein2dinput': _('Design Protein 2D'), - 'drag_and_drop_input': _('Drag and Drop'), - 'editageneinput': _('Edit A Gene'), - 'editamoleculeinput': _('Edit A Molecule'), - 'filesubmission': _('File Submission'), - 'formulaequationinput': _('Formula Equation'), - 'imageinput': _('Image'), - 'javascriptinput': _('Javascript Input'), - 'jsinput': _('JS Input'), - 'matlabinput': _('Matlab'), - 'optioninput': _('Select Option'), - 'radiogroup': _('Radio Group'), - 'radiotextgroup': _('Radio Text Group'), - 'schematic': _('Schematic'), - 'textbox': _('Code Text Input'), - 'textline': _('Text Line'), - 'vsepr_input': _('VSEPR'), + # basic tab + 'choiceresponse': _('Checkboxes'), + 'optionresponse': _('Dropdown'), + 'multiplechoiceresponse': _('Multiple Choice'), + 'truefalseresponse': _('True/False Choice'), + 'numericalresponse': _('Numerical Input'), + 'stringresponse': _('Text Input'), + + # advanced tab + 'schematicresponse': _('Circuit Schematic Builder'), + 'customresponse': _('Custom Evaluated Script'), + 'imageresponse': _('Image Mapped Input'), + 'formularesponse': _('Math Expression Input'), + 'jsmeresponse': _('Molecular Structure'), + + # not in "Add Component" menu + 'javascriptresponse': _('Javascript Input'), + 'symbolicresponse': _('Symbolic Math Input'), + 'coderesponse': _('Code Input'), + 'externalresponse': _('External Grader'), + 'annotationresponse': _('Annotation Input'), + 'choicetextresponse': _('Checkboxes With Text Input'), } return [{'value': ANY_CAPA_TYPE_VALUE, 'display_name': _('Any Type')}] + sorted([ diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index d0e6768ed868..3b902622c51c 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -104,7 +104,7 @@ def update_children(self, dest_block, user_id, user_perms=None, update_db=True): new_libraries = [] for library_key, library in libraries: - def copy_children_recursively(from_block, filter_problem_type=True): + def copy_children_recursively(from_block, filter_problem_type=False): """ Internal method to copy blocks from the library recursively """ diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index 53b26238c537..cd28f81da2cd 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -277,7 +277,7 @@ def test_problem_type_selector(self): self.assertLessEqual(children_headers, self._problem_headers) # Choice group test - children_headers = self._set_library_content_settings(count=1, capa_type="Choice Group") + children_headers = self._set_library_content_settings(count=1, capa_type="Multiple Choice") self.assertEqual(len(children_headers), 1) self.assertLessEqual( children_headers, @@ -285,7 +285,7 @@ def test_problem_type_selector(self): ) # Choice group test - children_headers = self._set_library_content_settings(count=2, capa_type="Select Option") + children_headers = self._set_library_content_settings(count=2, capa_type="Dropdown") self.assertEqual(len(children_headers), 2) self.assertLessEqual( children_headers, @@ -293,5 +293,5 @@ def test_problem_type_selector(self): ) # Missing problem type test - children_headers = self._set_library_content_settings(count=2, capa_type="Matlab") + children_headers = self._set_library_content_settings(count=2, capa_type="Custom Evaluated Script") self.assertEqual(children_headers, set()) From 6ad8b070bfb49df17ad32638efac719bb6ca96f2 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 29 Dec 2014 16:07:32 +0300 Subject: [PATCH 07/18] Validation warning when no content matches configured filters --- .../xmodule/xmodule/library_content_module.py | 64 +++++++++++++------ common/lib/xmodule/xmodule/library_tools.py | 35 ++++++---- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index d8512bcdbe5e..49237c7881ac 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -361,6 +361,31 @@ def refresh_children(self, request, suffix, update_db=True): # pylint: disable= lib_tools.update_children(self, user_id, user_perms, update_db) return Response() + def _validate_library_version(self, validation, lib_tools, version, library_key): + latest_version = lib_tools.get_library_version(library_key) + if latest_version is not None: + if version is None or version != latest_version: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _(u'This component is out of date. The library has new content.'), + action_class='library-update-btn', # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + action_label=_(u"↻ Update now") + ) + ) + return False + else: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.ERROR, + _(u'Library is invalid, corrupt, or has been deleted.'), + action_class='edit-button', + action_label=_(u"Edit Library List") + ) + ) + return False + return True + def validate(self): """ Validates the state of this Library Content Module Instance. This @@ -381,30 +406,27 @@ def validate(self): ) return validation lib_tools = self.runtime.service(self, 'library_tools') + has_children_matching_filter = False for library_key, version in self.source_libraries: - latest_version = lib_tools.get_library_version(library_key) - if latest_version is not None: - if version is None or version != latest_version: - validation.set_summary( - StudioValidationMessage( - StudioValidationMessage.WARNING, - _(u'This component is out of date. The library has new content.'), - action_class='library-update-btn', # TODO: change this to action_runtime_event='...' once the unit page supports that feature. - action_label=_(u"↻ Update now") - ) - ) - break - else: - validation.set_summary( - StudioValidationMessage( - StudioValidationMessage.ERROR, - _(u'Library is invalid, corrupt, or has been deleted.'), - action_class='edit-button', - action_label=_(u"Edit Library List") - ) - ) + if not self._validate_library_version(validation, lib_tools, version, library_key): break + library = lib_tools.get_library(library_key) + children_matching_filter = lib_tools.get_filtered_children(library, self.capa_type) + # get_filtered_children returns generator, so we're basically checking if there are at least one child + # that satisfy filtering. Children are never equal to None, so None is returned only if generator was empty + has_children_matching_filter |= next(children_matching_filter, None) is not None + + if not has_children_matching_filter and validation.empty: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _(u'There are no content matching configured filters in the selected libraries.'), + action_class='edit-button', + action_label=_(u"Edit Library List") + ) + ) + return validation def editor_saved(self, user, old_metadata, old_content): diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index 3b902622c51c..ad0d78a35b54 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -18,7 +18,7 @@ class LibraryToolsService(object): def __init__(self, modulestore): self.store = modulestore - def _get_library(self, library_key): + def get_library(self, library_key): """ Given a library key like "library-v1:ProblemX+PR0B", return the 'library' XBlock with meta-information about the library. @@ -39,24 +39,39 @@ def get_library_version(self, lib_key): Get the version (an ObjectID) of the given library. Returns None if the library does not exist. """ - library = self._get_library(lib_key) + library = self.get_library(lib_key) if library: # We need to know the library's version so ensure it's set in library.location.library_key.version_guid assert library.location.library_key.version_guid is not None return library.location.library_key.version_guid return None - def _filter_child(self, dest_block, child_descriptor): + def _filter_child(self, capa_type, child_descriptor): """ Filters children by CAPA problem type, if configured """ - if dest_block.capa_type == ANY_CAPA_TYPE_VALUE: + if capa_type == ANY_CAPA_TYPE_VALUE: return True if not isinstance(child_descriptor, CapaDescriptor): return False - return dest_block.capa_type in child_descriptor.problem_types + return capa_type in child_descriptor.problem_types + + def get_filtered_children(self, from_block, capa_type=ANY_CAPA_TYPE_VALUE): + """ + Filters children of `from_block` that satisfy filter criteria + Returns generator containing (child_key, child) for all children matching filter criteria + """ + children = ( + (child_key, self.store.get_item(child_key, depth=9)) + for child_key in from_block.children + ) + return ( + (child_key, child) + for child_key, child in children + if self._filter_child(capa_type, child) + ) def update_children(self, dest_block, user_id, user_perms=None, update_db=True): """ @@ -89,7 +104,7 @@ def update_children(self, dest_block, user_id, user_perms=None, update_db=True): # First, load and validate the source_libraries: libraries = [] for library_key, old_version in dest_block.source_libraries: # pylint: disable=unused-variable - library = self._get_library(library_key) + library = self.get_library(library_key) if library is None: raise ValueError("Required library not found.") if user_perms and not user_perms.can_read(library_key): @@ -109,11 +124,9 @@ def copy_children_recursively(from_block, filter_problem_type=False): Internal method to copy blocks from the library recursively """ new_children = [] - for child_key in from_block.children: - child = self.store.get_item(child_key, depth=9) - - if filter_problem_type and not self._filter_child(dest_block, child): - continue + target_capa_type = dest_block.capa_type if filter_problem_type else ANY_CAPA_TYPE_VALUE + filtered_children = self.get_filtered_children(from_block, target_capa_type) + for child_key, child in filtered_children: # We compute a block_id for each matching child block found in the library. # block_ids are unique within any branch, but are not unique per-course or globally. # We need our block_ids to be consistent when content in the library is updated, so From 5d190dc4f166002b0d720d55d9edea95ec3f4a0f Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 29 Dec 2014 16:32:48 +0300 Subject: [PATCH 08/18] Test for warning message when no content is configured. --- .../xmodule/xmodule/library_content_module.py | 6 ++- .../studio/test_studio_library_container.py | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 49237c7881ac..fcbef1fc00f3 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -26,7 +26,6 @@ ANY_CAPA_TYPE_VALUE = 'any' -CAPA_BLOCK_TYPE = 'problem' def enum(**enums): @@ -362,6 +361,9 @@ def refresh_children(self, request, suffix, update_db=True): # pylint: disable= return Response() def _validate_library_version(self, validation, lib_tools, version, library_key): + """ + Validates library version + """ latest_version = lib_tools.get_library_version(library_key) if latest_version is not None: if version is None or version != latest_version: @@ -423,7 +425,7 @@ def validate(self): StudioValidationMessage.WARNING, _(u'There are no content matching configured filters in the selected libraries.'), action_class='edit-button', - action_label=_(u"Edit Library List") + action_label=_(u"Edit Problem Type Filter") ) ) diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index 42fdfc4dc542..747db98752c4 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -42,6 +42,14 @@ def populate_library_fixture(self, library_fixture): XBlockFixtureDesc("html", "Html1"), XBlockFixtureDesc("html", "Html2"), XBlockFixtureDesc("html", "Html3"), + + XBlockFixtureDesc( + "problem", "Dropdown", + data=""" + +

Dropdown

+ +""") ) def populate_course_fixture(self, course_fixture): @@ -171,3 +179,37 @@ def test_out_of_date_message(self): self.assertFalse(library_block.has_validation_message) #self.assertIn("4 matching components", library_block.author_content) # Removed this assert until a summary message is added back to the author view (SOL-192) + + def test_no_content_message(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I set Problem Type selector so that no libraries have matching content + Then I can see that "No matching content" warning is shown + """ + expected_text = 'There are no content matching configured filters in the selected libraries. ' \ + 'Edit Problem Type Filter' + + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert library has children matching filter criteria + self.assertFalse(library_container.has_validation_error) + self.assertFalse(library_container.has_validation_warning) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + self.assertEqual(edit_modal.capa_type, "Any Type") # precondition check + edit_modal.capa_type = "Custom Evaluated Script" + + library_container.save_settings() + + self.assertTrue(library_container.has_validation_warning) + self.assertIn(expected_text, library_container.validation_warning_text) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + self.assertEqual(edit_modal.capa_type, "Custom Evaluated Script") # precondition check + edit_modal.capa_type = "Dropdown" + library_container.save_settings() + + # Library should contain single Dropdown problem, so now there should be no errors again + self.assertFalse(library_container.has_validation_error) + self.assertFalse(library_container.has_validation_warning) From 3acdf72671d252a1d892407ef85f7db13ec05902 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 30 Dec 2014 11:12:10 +0300 Subject: [PATCH 09/18] Validation warning if library content XBlock configured to fetch more problems than libraries and filtering allow --- .../xmodule/xmodule/library_content_module.py | 30 +++++++++++++++---- .../studio/test_studio_library_container.py | 27 +++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index fcbef1fc00f3..b8058e8e7fe2 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -388,6 +388,11 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) return False return True + def _set_validation_error_if_empty(self, validation, summary): + """ Helper method to only set validation summary if it's empty """ + if validation.empty: + validation.set_summary(summary) + def validate(self): """ Validates the state of this Library Content Module Instance. This @@ -408,19 +413,20 @@ def validate(self): ) return validation lib_tools = self.runtime.service(self, 'library_tools') - has_children_matching_filter = False + matching_children_count = 0 for library_key, version in self.source_libraries: if not self._validate_library_version(validation, lib_tools, version, library_key): break library = lib_tools.get_library(library_key) children_matching_filter = lib_tools.get_filtered_children(library, self.capa_type) - # get_filtered_children returns generator, so we're basically checking if there are at least one child - # that satisfy filtering. Children are never equal to None, so None is returned only if generator was empty - has_children_matching_filter |= next(children_matching_filter, None) is not None + # get_filtered_children returns generator, so can't use len. + # And we don't actually need those children, so no point of constructing a list + matching_children_count += sum(1 for child in children_matching_filter) - if not has_children_matching_filter and validation.empty: - validation.set_summary( + if matching_children_count == 0: + self._set_validation_error_if_empty( + validation, StudioValidationMessage( StudioValidationMessage.WARNING, _(u'There are no content matching configured filters in the selected libraries.'), @@ -429,6 +435,18 @@ def validate(self): ) ) + if matching_children_count < self.max_count: + self._set_validation_error_if_empty( + validation, + StudioValidationMessage( + StudioValidationMessage.WARNING, + _(u'Configured to fetch {count} blocks, library and filter settings yield only {actual} blocks.') + .format(actual=matching_children_count, count=self.max_count), + action_class='edit-button', + action_label=_(u"Edit block configuration") + ) + ) + return validation def editor_saved(self, user, old_metadata, old_content): diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index 747db98752c4..4634fb09ac34 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -186,6 +186,8 @@ def test_no_content_message(self): When I go to studio unit page for library content block And I set Problem Type selector so that no libraries have matching content Then I can see that "No matching content" warning is shown + When I set Problem Type selector so that there are matching content + Then I can see that warning messages are not shown """ expected_text = 'There are no content matching configured filters in the selected libraries. ' \ 'Edit Problem Type Filter' @@ -213,3 +215,28 @@ def test_no_content_message(self): # Library should contain single Dropdown problem, so now there should be no errors again self.assertFalse(library_container.has_validation_error) self.assertFalse(library_container.has_validation_warning) + + def test_not_enough_children_blocks(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I set Problem Type selector so "Any" + Then I can see that "No matching content" warning is shown + """ + expected_tpl = "Configured to fetch {count} blocks, library and filter settings yield only {actual} blocks." + + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert block is configured fine + self.assertFalse(library_container.has_validation_error) + self.assertFalse(library_container.has_validation_warning) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.count = 50 + library_container.save_settings() + + self.assertTrue(library_container.has_validation_warning) + self.assertIn( + expected_tpl.format(count=50, actual=len(self.library_fixture.children)), + library_container.validation_warning_text + ) From e6e7f9122bce7fd20195f91265f1bec58bfb5145 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 30 Dec 2014 11:18:17 +0300 Subject: [PATCH 10/18] Improved help message. --- common/lib/xmodule/xmodule/library_content_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index b8058e8e7fe2..0d2886ac9d89 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -188,7 +188,7 @@ class LibraryContentFields(object): ) capa_type = String( display_name=_("Problem Type"), - help=_("The type of components to include in this block"), + help=_('Choose a problem type to fetch from the library. If "Any Type" is selected no filtering is applied.'), default=ANY_CAPA_TYPE_VALUE, values=_get_capa_types(), scope=Scope.settings, From f15a94f55c393a6a754031eb221339dcd0882d70 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 31 Dec 2014 19:31:42 +0000 Subject: [PATCH 11/18] Addressed notes from reviewers about Library content filters. --- common/lib/capa/capa/responsetypes.py | 18 ++++++++ .../xmodule/xmodule/library_content_module.py | 42 +++++++------------ .../test/acceptance/tests/lms/test_library.py | 11 +++-- .../studio/test_studio_library_container.py | 6 +-- 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index a0b74372e255..0ff086ea6214 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -58,6 +58,8 @@ CorrectMap = correctmap.CorrectMap # pylint: disable=invalid-name CORRECTMAP_PY = None +# Make '_' a no-op so we can scrape strings +_ = lambda text: text #----------------------------------------------------------------------------- # Exceptions @@ -439,6 +441,7 @@ class JavascriptResponse(LoncapaResponse): Javascript using Node.js. """ + human_name = _('Javascript Input') tags = ['javascriptresponse'] max_inputfields = 1 allowed_inputfields = ['javascriptinput'] @@ -684,6 +687,7 @@ class ChoiceResponse(LoncapaResponse): """ + human_name = _('Checkboxes') tags = ['choiceresponse'] max_inputfields = 1 allowed_inputfields = ['checkboxgroup', 'radiogroup'] @@ -754,6 +758,7 @@ class MultipleChoiceResponse(LoncapaResponse): """ # TODO: handle direction and randomize + human_name = _('Multiple Choice') tags = ['multiplechoiceresponse'] max_inputfields = 1 allowed_inputfields = ['choicegroup'] @@ -1042,6 +1047,7 @@ def sample_from_answer_pool(self, choices, rng, num_pool): @registry.register class TrueFalseResponse(MultipleChoiceResponse): + human_name = _('True/False Choice') tags = ['truefalseresponse'] def mc_setup_response(self): @@ -1073,6 +1079,7 @@ class OptionResponse(LoncapaResponse): TODO: handle direction and randomize """ + human_name = _('Dropdown') tags = ['optionresponse'] hint_tag = 'optionhint' allowed_inputfields = ['optioninput'] @@ -1108,6 +1115,7 @@ class NumericalResponse(LoncapaResponse): to a number (e.g. `4+5/2^2`), and accepts with a tolerance. """ + human_name = _('Numerical Input') tags = ['numericalresponse'] hint_tag = 'numericalhint' allowed_inputfields = ['textline', 'formulaequationinput'] @@ -1308,6 +1316,7 @@ class StringResponse(LoncapaResponse): """ + human_name = _('Text Input') tags = ['stringresponse'] hint_tag = 'stringhint' allowed_inputfields = ['textline'] @@ -1426,6 +1435,7 @@ class CustomResponse(LoncapaResponse): or in a """ + human_name = _('Custom Evaluated Script') tags = ['customresponse'] allowed_inputfields = ['textline', 'textbox', 'crystallography', @@ -1797,6 +1807,7 @@ class SymbolicResponse(CustomResponse): Symbolic math response checking, using symmath library. """ + human_name = _('Symbolic Math Input') tags = ['symbolicresponse'] max_inputfields = 1 @@ -1865,6 +1876,7 @@ class CodeResponse(LoncapaResponse): """ + human_name = _('Code Input') tags = ['coderesponse'] allowed_inputfields = ['textbox', 'filesubmission', 'matlabinput'] max_inputfields = 1 @@ -2142,6 +2154,7 @@ class ExternalResponse(LoncapaResponse): """ + human_name = _('External Grader') tags = ['externalresponse'] allowed_inputfields = ['textline', 'textbox'] awdmap = { @@ -2299,6 +2312,7 @@ class FormulaResponse(LoncapaResponse): Checking of symbolic math response using numerical sampling. """ + human_name = _('Math Expression Input') tags = ['formularesponse'] hint_tag = 'formulahint' allowed_inputfields = ['textline', 'formulaequationinput'] @@ -2511,6 +2525,7 @@ class SchematicResponse(LoncapaResponse): """ Circuit schematic response type. """ + human_name = _('Circuit Schematic Builder') tags = ['schematicresponse'] allowed_inputfields = ['schematic'] @@ -2589,6 +2604,7 @@ class ImageResponse(LoncapaResponse): True, if click is inside any region or rectangle. Otherwise False. """ + human_name = _('Image Mapped Input') tags = ['imageresponse'] allowed_inputfields = ['imageinput'] @@ -2707,6 +2723,7 @@ class AnnotationResponse(LoncapaResponse): The response contains both a comment (student commentary) and an option (student tag). Only the tag is currently graded. Answers may be incorrect, partially correct, or correct. """ + human_name = _('Annotation Input') tags = ['annotationresponse'] allowed_inputfields = ['annotationinput'] max_inputfields = 1 @@ -2831,6 +2848,7 @@ class ChoiceTextResponse(LoncapaResponse): ChoiceResponse. """ + human_name = _('Checkboxes With Text Input') tags = ['choicetextresponse'] max_inputfields = 1 allowed_inputfields = ['choicetextgroup', diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 0d2886ac9d89..c6e728bb8aff 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -5,6 +5,7 @@ from bson.objectid import ObjectId, InvalidId from collections import namedtuple from copy import copy +from capa.responsetypes import registry from .mako_module import MakoModuleDescriptor from opaque_keys import InvalidKeyError @@ -33,34 +34,18 @@ def enum(**enums): return type('Enum', (), enums) +def _get_human_name(problem_class): + """ + Get the human-friendly name for a problem type. + """ + return getattr(problem_class, 'human_name', problem_class.__name__) + + def _get_capa_types(): """ Gets capa types tags and labels """ - capa_types = { - # basic tab - 'choiceresponse': _('Checkboxes'), - 'optionresponse': _('Dropdown'), - 'multiplechoiceresponse': _('Multiple Choice'), - 'truefalseresponse': _('True/False Choice'), - 'numericalresponse': _('Numerical Input'), - 'stringresponse': _('Text Input'), - - # advanced tab - 'schematicresponse': _('Circuit Schematic Builder'), - 'customresponse': _('Custom Evaluated Script'), - 'imageresponse': _('Image Mapped Input'), - 'formularesponse': _('Math Expression Input'), - 'jsmeresponse': _('Molecular Structure'), - - # not in "Add Component" menu - 'javascriptresponse': _('Javascript Input'), - 'symbolicresponse': _('Symbolic Math Input'), - 'coderesponse': _('Code Input'), - 'externalresponse': _('External Grader'), - 'annotationresponse': _('Annotation Input'), - 'choicetextresponse': _('Checkboxes With Text Input'), - } + capa_types = {tag: _get_human_name(registry.get_class_for_tag(tag)) for tag in registry.registered_tags()} return [{'value': ANY_CAPA_TYPE_VALUE, 'display_name': _('Any Type')}] + sorted([ {'value': capa_type, 'display_name': caption} @@ -429,9 +414,9 @@ def validate(self): validation, StudioValidationMessage( StudioValidationMessage.WARNING, - _(u'There are no content matching configured filters in the selected libraries.'), + _(u'There are no matching problem types in the specified libraries.'), action_class='edit-button', - action_label=_(u"Edit Problem Type Filter") + action_label=_(u"Select another problem type") ) ) @@ -440,10 +425,11 @@ def validate(self): validation, StudioValidationMessage( StudioValidationMessage.WARNING, - _(u'Configured to fetch {count} blocks, library and filter settings yield only {actual} blocks.') + _(u'The specified libraries are configured to fetch {count} problems, ' + u'but there are only {actual} matching problems.') .format(actual=matching_children_count, count=self.max_count), action_class='edit-button', - action_label=_(u"Edit block configuration") + action_label=_(u"Edit configuration") ) ) diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index cd28f81da2cd..d152f4338687 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -28,7 +28,10 @@ class LibraryContentTestBase(UniqueCourseTest): STAFF_EMAIL = "staff101@example.com" def populate_library_fixture(self, library_fixture): - pass + """ + To be overwritten by subclassed tests. Used to install a library to + run tests on. + """ def setUp(self): """ @@ -207,7 +210,7 @@ def _get_problem_choice_group_text(self, name, items): def _get_problem_select_text(self, name, items, correct): """ Generates Select Option CAPA problem XML """ - items_text = ",".join(map(lambda item: "'{0}'".format(item), items)) + items_text = ",".join(["'{0}'".format(item) for item in items]) return """

{name}

@@ -281,7 +284,7 @@ def test_problem_type_selector(self): self.assertEqual(len(children_headers), 1) self.assertLessEqual( children_headers, - set(map(lambda header: header.upper(), ["Problem Choice Group 1", "Problem Choice Group 2"])) + set([header.upper() for header in ["Problem Choice Group 1", "Problem Choice Group 2"]]) ) # Choice group test @@ -289,7 +292,7 @@ def test_problem_type_selector(self): self.assertEqual(len(children_headers), 2) self.assertLessEqual( children_headers, - set(map(lambda header: header.upper(), ["Problem Select 1", "Problem Select 2"])) + set([header.upper() for header in ["Problem Select 1", "Problem Select 2"]]) ) # Missing problem type test diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index 4634fb09ac34..c482ce090fcf 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -189,8 +189,7 @@ def test_no_content_message(self): When I set Problem Type selector so that there are matching content Then I can see that warning messages are not shown """ - expected_text = 'There are no content matching configured filters in the selected libraries. ' \ - 'Edit Problem Type Filter' + expected_text = 'There are no matching problem types in the specified libraries. Select another problem type' library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) @@ -223,7 +222,8 @@ def test_not_enough_children_blocks(self): And I set Problem Type selector so "Any" Then I can see that "No matching content" warning is shown """ - expected_tpl = "Configured to fetch {count} blocks, library and filter settings yield only {actual} blocks." + expected_tpl = "The specified libraries are configured to fetch {count} problems, " \ + "but there are only {actual} matching problems." library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) From 95bec64f502aa09c753e577fbe6fab2ab8109157 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 5 Jan 2015 13:57:52 +0300 Subject: [PATCH 12/18] Added reference to TNL ticket mentioned in TODO + improved formatting of XML templates in tests --- .../xmodule/xmodule/library_content_module.py | 4 ++- .../test/acceptance/tests/lms/test_library.py | 27 ++++++++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index c6e728bb8aff..fa8341ef1118 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -356,7 +356,9 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) StudioValidationMessage( StudioValidationMessage.WARNING, _(u'This component is out of date. The library has new content.'), - action_class='library-update-btn', # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + # See https://openedx.atlassian.net/browse/TNL-993 + action_class='library-update-btn', action_label=_(u"↻ Update now") ) ) diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index d152f4338687..91bec1661769 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -3,6 +3,7 @@ End-to-end tests for LibraryContent block in LMS """ import ddt +import textwrap from ..helpers import UniqueCourseTest from ...pages.studio.auto_auth import AutoAuthPage @@ -201,23 +202,25 @@ def _get_problem_choice_group_text(self, name, items): for item, correct in items ]) - return """ -

{name}

- - {items} - -""".format(name=name, items=items_text) + return textwrap.dedent(""" + +

{name}

+ + {items} + + """).format(name=name, items=items_text) def _get_problem_select_text(self, name, items, correct): """ Generates Select Option CAPA problem XML """ items_text = ",".join(["'{0}'".format(item) for item in items]) - return """ -

{name}

- - - -""".format(name=name, options=items_text, correct=correct) + return textwrap.dedent(""" + +

{name}

+ + + + """).format(name=name, options=items_text, correct=correct) def populate_library_fixture(self, library_fixture): """ From 3b1c5de1f30872726f9feb50d1981c349b52ce06 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 6 Jan 2015 11:13:10 +0300 Subject: [PATCH 13/18] Addressed review notes: * Added tests for `problem_types` CapaDescriptor property * Improved indentation in tests * Improved validation messages --- common/lib/capa/capa/responsetypes.py | 2 +- .../xmodule/xmodule/library_content_module.py | 27 +++++-- .../xmodule/xmodule/tests/test_capa_module.py | 75 +++++++++++++++---- .../test/acceptance/tests/lms/test_library.py | 2 +- .../studio/test_studio_library_container.py | 13 ++-- 5 files changed, 91 insertions(+), 28 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 0ff086ea6214..6b64ccbd3304 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -441,7 +441,7 @@ class JavascriptResponse(LoncapaResponse): Javascript using Node.js. """ - human_name = _('Javascript Input') + human_name = _('JavaScript Input') tags = ['javascriptresponse'] max_inputfields = 1 allowed_inputfields = ['javascriptinput'] diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index fa8341ef1118..3b624adf904b 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -6,6 +6,7 @@ from collections import namedtuple from copy import copy from capa.responsetypes import registry +from gettext import ngettext from .mako_module import MakoModuleDescriptor from opaque_keys import InvalidKeyError @@ -359,7 +360,8 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) # TODO: change this to action_runtime_event='...' once the unit page supports that feature. # See https://openedx.atlassian.net/browse/TNL-993 action_class='library-update-btn', - action_label=_(u"↻ Update now") + # Translators: ↻ is an UTF icon symbol, no need translating it. + action_label=_(u"↻ Update now.") ) ) return False @@ -369,7 +371,7 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) StudioValidationMessage.ERROR, _(u'Library is invalid, corrupt, or has been deleted.'), action_class='edit-button', - action_label=_(u"Edit Library List") + action_label=_(u"Edit Library List.") ) ) return False @@ -395,7 +397,7 @@ def validate(self): StudioValidationMessage.NOT_CONFIGURED, _(u"A library has not yet been selected."), action_class='edit-button', - action_label=_(u"Select a Library") + action_label=_(u"Select a Library.") ) ) return validation @@ -418,7 +420,7 @@ def validate(self): StudioValidationMessage.WARNING, _(u'There are no matching problem types in the specified libraries.'), action_class='edit-button', - action_label=_(u"Select another problem type") + action_label=_(u"Select another problem type.") ) ) @@ -427,11 +429,20 @@ def validate(self): validation, StudioValidationMessage( StudioValidationMessage.WARNING, - _(u'The specified libraries are configured to fetch {count} problems, ' - u'but there are only {actual} matching problems.') - .format(actual=matching_children_count, count=self.max_count), + ( + ngettext( + u'The specified libraries are configured to fetch {count} problem, ', + u'The specified libraries are configured to fetch {count} problems, ', + self.max_count + ) + + ngettext( + u'but there are only {actual} matching problem.', + u'but there are only {actual} matching problems.', + matching_children_count + ) + ).format(count=self.max_count, actual=matching_children_count), action_class='edit-button', - action_label=_(u"Edit configuration") + action_label=_(u"Edit the library configuration.") ) ) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index c829e29654a4..667aefc993aa 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -19,10 +19,11 @@ import xmodule from xmodule.tests import DATA_DIR +from capa import responsetypes from capa.responsetypes import (StudentInputError, LoncapaProblemError, ResponseError) from capa.xqueue_interface import XQueueInterface -from xmodule.capa_module import CapaModule, ComplexEncoder +from xmodule.capa_module import CapaModule, CapaDescriptor, ComplexEncoder from opaque_keys.edx.locations import Location from xblock.field_data import DictFieldData from xblock.fields import ScopeIds @@ -1660,6 +1661,62 @@ def test_check_unmask_answerpool(self): ('answerpool', ['choice_1', 'choice_3', 'choice_2', 'choice_0'])) self.assertEquals(event_info['success'], 'incorrect') +@ddt.ddt +class CapaDescriptorTest(unittest.TestCase): + def _create_descriptor(self, xml): + """ Creates a CapaDescriptor to run test against """ + descriptor = CapaDescriptor(get_test_system(), scope_ids=1) + descriptor.data = xml + return descriptor + + @ddt.data(*responsetypes.registry.registered_tags()) + def test_all_response_types(self, response_tag): + """ Tests that every registered response tag is correctly returned """ + xml = "<{response_tag}>".format(response_tag=response_tag) + descriptor = self._create_descriptor(xml) + self.assertEquals(descriptor.problem_types, {response_tag}) + + def test_response_types_ignores_non_response_tags(self): + xml = textwrap.dedent(""" + +

Label

+
Some comment
+ + + Apple + Banana + Chocolate + Donut + + + + """) + descriptor = self._create_descriptor(xml) + self.assertEquals(descriptor.problem_types, {"multiplechoiceresponse"}) + + def test_response_types_multiple_tags(self): + xml = textwrap.dedent(""" + +

Label

+
Some comment
+ + + Donut + + + + + Buggy + + + + + + + """) + descriptor = self._create_descriptor(xml) + self.assertEquals(descriptor.problem_types, {"multiplechoiceresponse", "optionresponse"}) + class ComplexEncoderTest(unittest.TestCase): def test_default(self): @@ -1690,18 +1747,10 @@ def test_choice_answer_text(self):

Which piece of furniture is built for sitting?

- - a table - - - a desk - - - a chair - - - a bookshelf - + a table + a desk + a chair + a bookshelf

Which of the following are musical instruments?

diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index 91bec1661769..cb4fd238de1f 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -293,7 +293,7 @@ def test_problem_type_selector(self): # Choice group test children_headers = self._set_library_content_settings(count=2, capa_type="Dropdown") self.assertEqual(len(children_headers), 2) - self.assertLessEqual( + self.assertEqual( children_headers, set([header.upper() for header in ["Problem Select 1", "Problem Select 2"]]) ) diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index c482ce090fcf..ac385eb7cee7 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -1,6 +1,7 @@ """ Acceptance tests for Library Content in LMS """ +import textwrap import ddt from .base_studio_test import StudioLibraryTest from ...fixtures.course import CourseFixture @@ -45,11 +46,13 @@ def populate_library_fixture(self, library_fixture): XBlockFixtureDesc( "problem", "Dropdown", - data=""" - -

Dropdown

- -""") + data=textwrap.dedent(""" + +

Dropdown

+ + + """) + ) ) def populate_course_fixture(self, course_fixture): From 7c7be2298ac5a26be56dbfc96a80cf7100744d38 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 6 Jan 2015 13:55:16 +0300 Subject: [PATCH 14/18] Added problem type filtering related tests. --- .../xmodule/xmodule/tests/test_capa_module.py | 1 + .../xmodule/tests/test_library_content.py | 84 ++++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 667aefc993aa..2e0661dbbe93 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1661,6 +1661,7 @@ def test_check_unmask_answerpool(self): ('answerpool', ['choice_1', 'choice_3', 'choice_2', 'choice_0'])) self.assertEquals(event_info['success'], 'incorrect') + @ddt.ddt class CapaDescriptorTest(unittest.TestCase): def _create_descriptor(self, xml): diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 2b52386e3740..0ae8a60c2894 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -5,7 +5,7 @@ Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. """ import ddt -from xmodule.library_content_module import LibraryVersionReference +from xmodule.library_content_module import LibraryVersionReference, ANY_CAPA_TYPE_VALUE from xmodule.modulestore.tests.factories import LibraryFactory, CourseFactory, ItemFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.tests import get_test_system @@ -80,6 +80,29 @@ def get_module(descriptor): module_system.get_module = get_module module.xmodule_runtime = module_system + def _get_capa_problem_type_xml(self, problem_type): + """ Helper function to create empty CAPA problem definition """ + return "<{problem_type}>".format(problem_type=problem_type) + + def _create_capa_problems(self): + """ Helper function to create two capa problems: multiplechoiceresponse and optionresponse """ + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user_id, + publish_item=False, + data=self._get_capa_problem_type_xml("multiplechoiceresponse"), + modulestore=self.store, + ) + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user_id, + publish_item=False, + data=self._get_capa_problem_type_xml("optionresponse"), + modulestore=self.store, + ) + def test_lib_content_block(self): """ Test that blocks from a library are copied and added as children @@ -140,3 +163,62 @@ def test_validation(self): # Now if we update the block, all validation should pass: self.lc_block.refresh_children(None, None) self.assertTrue(self.lc_block.validate()) + + # Set max_count to higher value than exists in library + self.lc_block.max_count = 50 + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.WARNING, result.summary.type) + self.assertIn("only 4 matching problems", result.summary.text) + + # Add some capa problems so we can check problem type validation messages + self.lc_block.max_count = 1 + self._create_capa_problems() + self.lc_block.refresh_children(None, None) + self.assertTrue(self.lc_block.validate()) + + # Existing problem type should pass validation + self.lc_block.max_count = 1 + self.lc_block.capa_type = 'multiplechoiceresponse' + self.assertTrue(self.lc_block.validate()) + + # ... unless requested more blocks than exists in library + self.lc_block.max_count = 3 + self.lc_block.capa_type = 'multiplechoiceresponse' + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.WARNING, result.summary.type) + self.assertIn("only 1 matching problem", result.summary.text) + + # Missing problem type should always fail validation + self.lc_block.max_count = 1 + self.lc_block.capa_type = 'customresponse' + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.WARNING, result.summary.type) + self.assertIn("no matching problem types", result.summary.text) + + def test_capa_type_filtering(self): + """ + Test that the capa type filter is actually filtering children + """ + self._create_capa_problems() + self.assertEqual(len(self.lc_block.children), 0) # precondition check + self.lc_block.capa_type = "multiplechoiceresponse" + self.lc_block.refresh_children(None, None) + self.assertEqual(len(self.lc_block.children), 1) + + self.lc_block.capa_type = "optionresponse" + self.lc_block.refresh_children(None, None) + self.assertEqual(len(self.lc_block.children), 1) + + self.lc_block.capa_type = "customresponse" + self.lc_block.refresh_children(None, None) + self.assertEqual(len(self.lc_block.children), 0) + + self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE + self.lc_block.refresh_children(None, None) + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 2) From c2942860d45497b4e11f6f55df54824cb34d9e19 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 6 Jan 2015 21:16:09 +0000 Subject: [PATCH 15/18] Addressed nits. --- common/lib/xmodule/xmodule/library_content_module.py | 6 +++--- common/lib/xmodule/xmodule/library_tools.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 3b624adf904b..7918c54eccff 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -361,7 +361,7 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) # See https://openedx.atlassian.net/browse/TNL-993 action_class='library-update-btn', # Translators: ↻ is an UTF icon symbol, no need translating it. - action_label=_(u"↻ Update now.") + action_label=_(u"{0} Update now.").format(u"↻") ) ) return False @@ -453,8 +453,8 @@ def editor_saved(self, user, old_metadata, old_content): If source_libraries has been edited, refresh_children automatically. """ old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) - if set(old_source_libraries) != set(self.source_libraries) or \ - old_metadata.get('capa_type', ANY_CAPA_TYPE_VALUE) != self.capa_type: + if (set(old_source_libraries) != set(self.source_libraries) or + old_metadata.get('capa_type', ANY_CAPA_TYPE_VALUE) != self.capa_type): try: self.refresh_children(None, None, update_db=False) # update_db=False since update_item() is about to be called anyways except ValueError: diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index ad0d78a35b54..94d4e0fdf46c 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -64,7 +64,7 @@ def get_filtered_children(self, from_block, capa_type=ANY_CAPA_TYPE_VALUE): Returns generator containing (child_key, child) for all children matching filter criteria """ children = ( - (child_key, self.store.get_item(child_key, depth=9)) + (child_key, self.store.get_item(child_key, depth=None)) for child_key in from_block.children ) return ( From 4c89f2c4f3618dc8050155f3bdb6b906aaf4467a Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 6 Jan 2015 21:35:39 +0000 Subject: [PATCH 16/18] Test filter for problems with multiple capa response types. --- .../xmodule/tests/test_library_content.py | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 0ae8a60c2894..5ab2eaa57565 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -80,28 +80,33 @@ def get_module(descriptor): module_system.get_module = get_module module.xmodule_runtime = module_system - def _get_capa_problem_type_xml(self, problem_type): + def _get_capa_problem_type_xml(self, *args): """ Helper function to create empty CAPA problem definition """ - return "<{problem_type}>".format(problem_type=problem_type) + problem = "" + for problem_type in args: + problem += "<{problem_type}>".format(problem_type=problem_type) + problem += "" + return problem def _create_capa_problems(self): - """ Helper function to create two capa problems: multiplechoiceresponse and optionresponse """ - ItemFactory.create( - category="problem", - parent_location=self.library.location, - user_id=self.user_id, - publish_item=False, - data=self._get_capa_problem_type_xml("multiplechoiceresponse"), - modulestore=self.store, - ) - ItemFactory.create( - category="problem", - parent_location=self.library.location, - user_id=self.user_id, - publish_item=False, - data=self._get_capa_problem_type_xml("optionresponse"), - modulestore=self.store, - ) + """ + Helper function to create a set of capa problems to test against. + + Creates four blocks total. + """ + problem_types = [ + ["multiplechoiceresponse"], ["optionresponse"], ["optionresponse", "coderesponse"], + ["coderesponse", "optionresponse"] + ] + for problem_type in problem_types: + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user_id, + publish_item=False, + data=self._get_capa_problem_type_xml(*problem_type), + modulestore=self.store, + ) def test_lib_content_block(self): """ @@ -184,7 +189,7 @@ def test_validation(self): self.assertTrue(self.lc_block.validate()) # ... unless requested more blocks than exists in library - self.lc_block.max_count = 3 + self.lc_block.max_count = 10 self.lc_block.capa_type = 'multiplechoiceresponse' result = self.lc_block.validate() self.assertFalse(result) # Validation fails due to at least one warning/message @@ -213,7 +218,11 @@ def test_capa_type_filtering(self): self.lc_block.capa_type = "optionresponse" self.lc_block.refresh_children(None, None) - self.assertEqual(len(self.lc_block.children), 1) + self.assertEqual(len(self.lc_block.children), 3) + + self.lc_block.capa_type = "coderesponse" + self.lc_block.refresh_children(None, None) + self.assertEqual(len(self.lc_block.children), 2) self.lc_block.capa_type = "customresponse" self.lc_block.refresh_children(None, None) @@ -221,4 +230,4 @@ def test_capa_type_filtering(self): self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE self.lc_block.refresh_children(None, None) - self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 2) + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 4) From 2f8a5e67abb029172e46a83f1164ee076e02e9b9 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 6 Jan 2015 23:01:40 -0800 Subject: [PATCH 17/18] Simplifications and changes to reduce conflicts with PR 6399 --- .../xmodule/xmodule/library_content_module.py | 11 ++---- common/lib/xmodule/xmodule/library_tools.py | 38 +++++++------------ .../xmodule/tests/test_library_content.py | 28 +++++++++----- .../studio/test_studio_library_container.py | 23 +++++------ 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 7918c54eccff..218272de8ab3 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -323,7 +323,7 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe js_module_name = "VerticalDescriptor" @XBlock.handler - def refresh_children(self, request, suffix, update_db=True): # pylint: disable=unused-argument + def refresh_children(self, request=None, suffix=None, update_db=True): # pylint: disable=unused-argument """ Refresh children: This method is to be used when any of the libraries that this block @@ -402,17 +402,12 @@ def validate(self): ) return validation lib_tools = self.runtime.service(self, 'library_tools') - matching_children_count = 0 for library_key, version in self.source_libraries: if not self._validate_library_version(validation, lib_tools, version, library_key): break - library = lib_tools.get_library(library_key) - children_matching_filter = lib_tools.get_filtered_children(library, self.capa_type) - # get_filtered_children returns generator, so can't use len. - # And we don't actually need those children, so no point of constructing a list - matching_children_count += sum(1 for child in children_matching_filter) - + # Note: we assume refresh_children() has been called since the last time fields like source_libraries or capa_types were changed. + matching_children_count = len(self.children) # pylint: disable=no-member if matching_children_count == 0: self._set_validation_error_if_empty( validation, diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index 94d4e0fdf46c..f9b9f3edab83 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -18,7 +18,7 @@ class LibraryToolsService(object): def __init__(self, modulestore): self.store = modulestore - def get_library(self, library_key): + def _get_library(self, library_key): """ Given a library key like "library-v1:ProblemX+PR0B", return the 'library' XBlock with meta-information about the library. @@ -39,39 +39,26 @@ def get_library_version(self, lib_key): Get the version (an ObjectID) of the given library. Returns None if the library does not exist. """ - library = self.get_library(lib_key) + library = self._get_library(lib_key) if library: # We need to know the library's version so ensure it's set in library.location.library_key.version_guid assert library.location.library_key.version_guid is not None return library.location.library_key.version_guid return None - def _filter_child(self, capa_type, child_descriptor): + def _filter_child(self, usage_key, capa_type): """ Filters children by CAPA problem type, if configured """ if capa_type == ANY_CAPA_TYPE_VALUE: return True - if not isinstance(child_descriptor, CapaDescriptor): + if usage_key.block_type != "problem": return False - return capa_type in child_descriptor.problem_types - - def get_filtered_children(self, from_block, capa_type=ANY_CAPA_TYPE_VALUE): - """ - Filters children of `from_block` that satisfy filter criteria - Returns generator containing (child_key, child) for all children matching filter criteria - """ - children = ( - (child_key, self.store.get_item(child_key, depth=None)) - for child_key in from_block.children - ) - return ( - (child_key, child) - for child_key, child in children - if self._filter_child(capa_type, child) - ) + descriptor = self.store.get_item(usage_key, depth=0) + assert isinstance(descriptor, CapaDescriptor) + return capa_type in descriptor.problem_types def update_children(self, dest_block, user_id, user_perms=None, update_db=True): """ @@ -104,7 +91,7 @@ def update_children(self, dest_block, user_id, user_perms=None, update_db=True): # First, load and validate the source_libraries: libraries = [] for library_key, old_version in dest_block.source_libraries: # pylint: disable=unused-variable - library = self.get_library(library_key) + library = self._get_library(library_key) if library is None: raise ValueError("Required library not found.") if user_perms and not user_perms.can_read(library_key): @@ -124,9 +111,12 @@ def copy_children_recursively(from_block, filter_problem_type=False): Internal method to copy blocks from the library recursively """ new_children = [] - target_capa_type = dest_block.capa_type if filter_problem_type else ANY_CAPA_TYPE_VALUE - filtered_children = self.get_filtered_children(from_block, target_capa_type) - for child_key, child in filtered_children: + if filter_problem_type: + filtered_children = [key for key in from_block.children if self._filter_child(key, dest_block.capa_type)] + else: + filtered_children = from_block.children + for child_key in filtered_children: + child = self.store.get_item(child_key, depth=None) # We compute a block_id for each matching child block found in the library. # block_ids are unique within any branch, but are not unique per-course or globally. # We need our block_ids to be consistent when content in the library is updated, so diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 5ab2eaa57565..fec957d0cda5 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -138,9 +138,10 @@ def test_children_seen_by_a_user(self): # Check that get_content_titles() doesn't return titles for hidden/unused children self.assertEqual(len(self.lc_block.get_content_titles()), 1) - def test_validation(self): + def test_validation_of_course_libraries(self): """ - Test that the validation method of LibraryContent blocks is working. + Test that the validation method of LibraryContent blocks can validate + the source_libraries setting. """ # When source_libraries is blank, the validation summary should say this block needs to be configured: self.lc_block.source_libraries = [] @@ -166,11 +167,17 @@ def test_validation(self): self.assertIn("out of date", result.summary.text) # Now if we update the block, all validation should pass: - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertTrue(self.lc_block.validate()) + def test_validation_of_matching_blocks(self): + """ + Test that the validation method of LibraryContent blocks can warn + the user about problems with other settings (max_count and capa_type). + """ # Set max_count to higher value than exists in library self.lc_block.max_count = 50 + self.lc_block.refresh_children() # In the normal studio editing process, editor_saved() calls refresh_children at this point result = self.lc_block.validate() self.assertFalse(result) # Validation fails due to at least one warning/message self.assertTrue(result.summary) @@ -180,17 +187,19 @@ def test_validation(self): # Add some capa problems so we can check problem type validation messages self.lc_block.max_count = 1 self._create_capa_problems() - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertTrue(self.lc_block.validate()) # Existing problem type should pass validation self.lc_block.max_count = 1 self.lc_block.capa_type = 'multiplechoiceresponse' + self.lc_block.refresh_children() self.assertTrue(self.lc_block.validate()) # ... unless requested more blocks than exists in library self.lc_block.max_count = 10 self.lc_block.capa_type = 'multiplechoiceresponse' + self.lc_block.refresh_children() result = self.lc_block.validate() self.assertFalse(result) # Validation fails due to at least one warning/message self.assertTrue(result.summary) @@ -200,6 +209,7 @@ def test_validation(self): # Missing problem type should always fail validation self.lc_block.max_count = 1 self.lc_block.capa_type = 'customresponse' + self.lc_block.refresh_children() result = self.lc_block.validate() self.assertFalse(result) # Validation fails due to at least one warning/message self.assertTrue(result.summary) @@ -213,21 +223,21 @@ def test_capa_type_filtering(self): self._create_capa_problems() self.assertEqual(len(self.lc_block.children), 0) # precondition check self.lc_block.capa_type = "multiplechoiceresponse" - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), 1) self.lc_block.capa_type = "optionresponse" - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), 3) self.lc_block.capa_type = "coderesponse" - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), 2) self.lc_block.capa_type = "customresponse" - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), 0) self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE - self.lc_block.refresh_children(None, None) + self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 4) diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index ac385eb7cee7..ce0464374515 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -43,16 +43,6 @@ def populate_library_fixture(self, library_fixture): XBlockFixtureDesc("html", "Html1"), XBlockFixtureDesc("html", "Html2"), XBlockFixtureDesc("html", "Html3"), - - XBlockFixtureDesc( - "problem", "Dropdown", - data=textwrap.dedent(""" - -

Dropdown

- - - """) - ) ) def populate_course_fixture(self, course_fixture): @@ -189,9 +179,20 @@ def test_no_content_message(self): When I go to studio unit page for library content block And I set Problem Type selector so that no libraries have matching content Then I can see that "No matching content" warning is shown - When I set Problem Type selector so that there are matching content + When I set Problem Type selector so that there is matching content Then I can see that warning messages are not shown """ + # Add a single "Dropdown" type problem to the library (which otherwise has only HTML blocks): + self.library_fixture.create_xblock(self.library_fixture.library_location, XBlockFixtureDesc( + "problem", "Dropdown", + data=textwrap.dedent(""" + +

Dropdown

+ + + """) + )) + expected_text = 'There are no matching problem types in the specified libraries. Select another problem type' library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) From af1b085accc03961808e08a838ee13f8527ed987 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Wed, 7 Jan 2015 14:01:00 +0300 Subject: [PATCH 18/18] Added tests for `editor_saved` library content xblock method Retriggering Jenkins --- .../contentstore/tests/test_libraries.py | 93 +++++++++++++++++++ .../xmodule/xmodule/library_content_module.py | 6 +- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index c6fdc00301cf..bc8975eb5d6d 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -326,6 +326,99 @@ def test_change_after_first_sync(self): html_block = modulestore().get_item(lc_block.children[0]) self.assertEqual(html_block.data, data_value) + def test_refreshes_children_if_libraries_change(self): + library2key = self._create_library("org2", "lib2", "Library2") + library2 = modulestore().get_library(library2key) + data1, data2 = "Hello world!", "Hello other world!" + ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name="Lib1: HTML BLock", + data=data1, + ) + + ItemFactory.create( + category="html", + parent_location=library2.location, + user_id=self.user.id, + publish_item=False, + display_name="Lib 2: HTML BLock", + data=data2, + ) + + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + + # Now, change the block settings to have an invalid library key: + resp = self._update_item( + lc_block.location, + {"source_libraries": [[str(library2key)]]}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.data, data2) + + def test_refreshes_children_if_capa_type_change(self): + name1, name2 = "Option Problem", "Multiple Choice Problem" + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name1, + data="", + ) + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name2, + data="", + ) + + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 2) + + resp = self._update_item( + lc_block.location, + {"capa_type": 'optionresponse'}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.display_name, name1) + + resp = self._update_item( + lc_block.location, + {"capa_type": 'multiplechoiceresponse'}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.display_name, name2) + @ddt.ddt class TestLibraryAccess(LibraryTestCase): diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 218272de8ab3..1b68e27a59c2 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -360,8 +360,8 @@ def _validate_library_version(self, validation, lib_tools, version, library_key) # TODO: change this to action_runtime_event='...' once the unit page supports that feature. # See https://openedx.atlassian.net/browse/TNL-993 action_class='library-update-btn', - # Translators: ↻ is an UTF icon symbol, no need translating it. - action_label=_(u"{0} Update now.").format(u"↻") + # Translators: {refresh_icon} placeholder is substituted to "↻" (without double quotes) + action_label=_(u"{refresh_icon} Update now.").format(refresh_icon=u"↻") ) ) return False @@ -445,7 +445,7 @@ def validate(self): def editor_saved(self, user, old_metadata, old_content): """ - If source_libraries has been edited, refresh_children automatically. + If source_libraries or capa_type has been edited, refresh_children automatically. """ old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) if (set(old_source_libraries) != set(self.source_libraries) or