diff --git a/TODO b/TODO index bb033a48061b55769a056e6fff9716f6e8d1a285..7a1e5ed73f780a0562b01b83a9677e99c881eaf8 100644 --- a/TODO +++ b/TODO @@ -30,8 +30,6 @@ Ideas - record end flow with a null page visit -- Flow group ordering, max page count, question/page tags - - Name X out of Y question - Multi text question - Multi MC question @@ -54,3 +52,4 @@ Rejected ideas -> localize an elaborate question and all its 'stuff' (JS, media) -> use a media search path - web hooks for auto-fetch and auto-preview +- question/page tags diff --git a/course/content.py b/course/content.py index 094658eb31ea3d0213ad9f906b18e3803d36d210..fc278cc329e66ab01c8ce8a4749331ad4eac6d8e 100644 --- a/course/content.py +++ b/course/content.py @@ -793,47 +793,124 @@ def _adjust_flow_session_page_data_inner(repo, flow_session, course_identifier, flow_desc, commit_sha): from course.models import FlowPageData - id_to_existing_page = dict( - ((data.group_id, data.page_id), data) - for data in FlowPageData.objects.filter(flow_session=flow_session) - ) + ordinal = [0] + for grp in flow_desc.groups: + shuffle = getattr(grp, "shuffle", False) + max_page_count = getattr(grp, "max_page_count", None) - data = None + available_page_ids = [page_desc.id for page_desc in grp.pages] - ordinal = 0 - for grp in flow_desc.groups: - for page_desc in grp.pages: - key = (grp.id, page_desc.id) - - if key in id_to_existing_page: - data = id_to_existing_page.pop(key) - if data.ordinal != ordinal: - data.ordinal = ordinal - data.save() - else: - data = FlowPageData() - data.flow_session = flow_session - data.ordinal = ordinal - data.group_id = grp.id - data.page_id = page_desc.id - - page = instantiate_flow_page( - "course '%s', flow '%s', page '%s/%s'" - % (course_identifier, flow_session.flow_id, - grp.id, page_desc.id), - repo, page_desc, commit_sha) - data.data = page.make_page_data() - data.save() - - ordinal += 1 - - for data in id_to_existing_page.values(): - if data.ordinal is not None: - data.ordinal = None - data.save() - - if flow_session.page_count != ordinal: - flow_session.page_count = ordinal + if max_page_count is None: + max_page_count = len(available_page_ids) + + group_pages = [] + + # {{{ helper functions + + def find_page_desc(page_id): + new_page_desc = None + + for page_desc in grp.pages: + if page_desc.id == page_id: + new_page_desc = page_desc + break + + assert new_page_desc is not None + + return new_page_desc + + def create_fpd(new_page_desc): + page = instantiate_flow_page( + "course '%s', flow '%s', page '%s/%s'" + % (course_identifier, flow_session.flow_id, + grp.id, page_desc.id), + repo, page_desc, commit_sha) + + return FlowPageData( + flow_session=flow_session, + group_id=grp.id, + page_id=page_desc.id, + ordinal=None, + data=page.make_page_data()) + + def add_page(fpd): + if fpd.ordinal != ordinal[0]: + fpd.ordinal = ordinal[0] + fpd.save() + + ordinal[0] += 1 + available_page_ids.remove(fpd.page_id) + group_pages.append(fpd) + + def remove_page(fpd): + if fpd.ordinal is not None: + fpd.ordinal = None + fpd.save() + + # }}} + + if shuffle: + # maintain order of existing pages as much as possible + for fpd in (FlowPageData.objects + .filter( + flow_session=flow_session, + group_id=grp.id, + ordinal__isnull=False) + .order_by("ordinal")): + + if (fpd.page_id in available_page_ids + and len(group_pages) < max_page_count): + add_page(fpd) + else: + remove_page(fpd) + + assert len(group_pages) <= max_page_count + + from random import choice + + # then add randomly chosen new pages + while len(group_pages) < max_page_count and available_page_ids: + new_page_id = choice(available_page_ids) + + new_page_fpds = (FlowPageData.objects + .filter( + flow_session=flow_session, + group_id=grp.id, + page_id=new_page_id)) + + if new_page_fpds.count(): + # We already have FlowPageData for this page, revive it + new_page_fpd, = new_page_fpds + else: + # Make a new FlowPageData instance + new_page_fpd = create_fpd(find_page_desc(new_page_id)) + + add_page(new_page_fpd) + + else: + # reorder pages to order in flow + id_to_fpd = dict( + ((fpd.group_id, fpd.page_id), fpd) + for fpd in FlowPageData.objects.filter( + flow_session=flow_session, + group_id=grp.id)) + + for page_desc in grp.pages: + key = (grp.id, page_desc.id) + + if key in id_to_fpd: + fpd = id_to_fpd.pop(key) + else: + fpd = create_fpd(page_desc) + + if len(group_pages) < max_page_count: + add_page(fpd) + + for fpd in id_to_fpd.values(): + remove_page(fpd) + + if flow_session.page_count != ordinal[0]: + flow_session.page_count = ordinal[0] flow_session.save() diff --git a/course/validation.py b/course/validation.py index c8782c95194f500429a5e57ec0a6bf0c48345c75..425573d4ae7b18f46b8ba279c8cf604fcc1df68f 100644 --- a/course/validation.py +++ b/course/validation.py @@ -323,7 +323,10 @@ def validate_flow_group(ctx, location, grp): ("id", str), ("pages", list), ], - allowed_attrs=[] + allowed_attrs=[ + ("shuffle", bool), + ("max_page_count", int), + ] ) for i, page_desc in enumerate(grp.pages): @@ -333,6 +336,13 @@ def validate_flow_group(ctx, location, grp): % (location, i+1, getattr(page_desc, "id", None)), page_desc) + if len(grp.pages) == 0: + raise ValidationError("%s, group '%s': group is empty" % (location, grp.id)) + + if hasattr(grp, "max_page_count") and grp.max_page_count <= 0: + raise ValidationError("%s, group '%s': max_page_count is not positive" + % (location, grp.id)) + # {{{ check page id uniqueness page_ids = set() diff --git a/doc/flow.rst b/doc/flow.rst index 6502f67c3b9d667f561fa24061e570cbc8d3905d..b61fa0cf1345c67ccced281a902a28579e0a83fd 100644 --- a/doc/flow.rst +++ b/doc/flow.rst @@ -383,6 +383,52 @@ Here's a commented example: .. autoclass:: grade_aggregation_strategy +.. _flow-groups: + +Page groups +----------- + +Each flow consists of a number of page groups, each of which is made up of +individual :ref:`flow-page`. + +The purpose of page groups is to allow shuffling and random selection of +some subset of pages. For example, this functionality would allow you to +have a flow consisting of: + +* a fixed introduction page (that is always at the beginning) + +* a group of exam questions randomly selected from a bigger pool + +* a fixed final page (that may ask the student to, say, affirm academic + honesty) + +Each of these would be a separate 'group'. + +Each group allows the following attributes: + + +.. class:: FlowGroup + + .. attribute:: id + + (Required) A symbolic name for the page group. + + .. attribute:: pages + + (Required) A list of :ref:`flow-page` + + .. attribute:: shuffle + + (Optional) A boolean (True/False) indicating whether the order + of pages should be as in the list :attr:`FlowGroup.pages` or + determined by random shuffling + + .. attribute:: max_page_count + + (Optional) An integer limiting the page count of this group + to a certain value. Allows selection of a random subset by combining + with :attr:`FlowGroup.shuffle`. + .. _flow-permissions: Permissions