From af3c1b5b826a4dd19a17cf2e44d9eb368d94a7a2 Mon Sep 17 00:00:00 2001 From: Alexey Grigorev Date: Sat, 26 Sep 2026 21:31:24 +0200 Subject: [PATCH 1/7] Add mixed course trees and YAML homework units --- community_base/content_sync/check.py | 7 +- community_base/content_sync/documents.py | 29 +- community_base/content_sync/kinds/base.py | 5 + community_base/content_sync/kinds/course.py | 67 ++++ community_base/content_sync/kinds/layouts.py | 89 +++++- community_base/content_sync/resolution.py | 21 +- community_base/curriculum/parsers.py | 298 +++++++++++++++++- community_base/curriculum/source.py | 200 ++++++++++-- docs/plan/STATUS.md | 2 +- .../01-module/01-unit.md | 0 .../01-module/02-submodule/module.yaml | 0 .../01-module/module.yaml | 0 .../content.yaml | 0 .../course.yaml | 0 tests/content_sync/test_documents.py | 10 +- tests/curriculum/test_parsers.py | 75 ++++- 16 files changed, 747 insertions(+), 56 deletions(-) rename tests/content_sync/fixtures/{invalid/course_mixed_module => valid_mixed_course}/01-module/01-unit.md (100%) rename tests/content_sync/fixtures/{invalid/course_mixed_module => valid_mixed_course}/01-module/02-submodule/module.yaml (100%) rename tests/content_sync/fixtures/{invalid/course_mixed_module => valid_mixed_course}/01-module/module.yaml (100%) rename tests/content_sync/fixtures/{invalid/course_mixed_module => valid_mixed_course}/content.yaml (100%) rename tests/content_sync/fixtures/{invalid/course_mixed_module => valid_mixed_course}/course.yaml (100%) diff --git a/community_base/content_sync/check.py b/community_base/content_sync/check.py index 26fea3dc..f4d04b1c 100644 --- a/community_base/content_sync/check.py +++ b/community_base/content_sync/check.py @@ -132,6 +132,7 @@ def main(argv: list[str] | None = None) -> int: def _check_dialect(item: ParsedDocument, headings: list[tuple[int, str, str]], diagnostics) -> None: title = item.data.get("title") + body_source = item.raw.body_path or item.raw.path for number, line in _code_free_lines(item.body): located = item.body_line + number for pattern, rule, message, severity in _DIALECT_RULES: @@ -140,7 +141,7 @@ def _check_dialect(item: ParsedDocument, headings: list[tuple[int, str, str]], d continue diagnostics.append( Diagnostic( - item.raw.path, + body_source, "/body", rule, f"{message}: {match.group(0).strip()[:60]}", @@ -154,7 +155,7 @@ def _check_dialect(item: ParsedDocument, headings: list[tuple[int, str, str]], d for message in _check_embed(line): diagnostics.append( Diagnostic( - item.raw.path, + body_source, "/body", "4.1", message, @@ -166,7 +167,7 @@ def _check_dialect(item: ParsedDocument, headings: list[tuple[int, str, str]], d if level == 1 and text.strip().lower() == title.strip().lower(): diagnostics.append( Diagnostic( - item.raw.path, + body_source, "/body", "4.1", "the body repeats the title as a leading H1; the renderer strips it", diff --git a/community_base/content_sync/documents.py b/community_base/content_sync/documents.py index b2f30d4a..2f52cbba 100644 --- a/community_base/content_sync/documents.py +++ b/community_base/content_sync/documents.py @@ -180,6 +180,7 @@ def record(self) -> dict[str, Any]: "kind": self.kind, "part": self.part.name, "source_path": self.raw.path, + "body_source_path": self.raw.body_path or self.raw.path, "parent": self.raw.parent, "path": self.path, "slug": self.slug, @@ -531,9 +532,13 @@ def _node_at(tree: DirNode, path: str) -> DirNode | None: def _read_collection( - repository: Repository, collection: Collection, diagnostics: list[Diagnostic] + repository: Repository, + collection: Collection, + diagnostics: list[Diagnostic], + *, + collection_root: DirNode | None = None, ) -> list[ParsedDocument]: - node = _node_at(repository.tree, collection.path) + node = collection_root or _node_at(repository.tree, collection.path) if node is None and (repository.root / collection.path).is_dir(): # A collection root the repository holds but whose every file `ignore` # hides, or which is empty, is an empty collection and not a missing @@ -588,6 +593,12 @@ def _read_item( diagnostics.append(locate(raw.path, problem)) if problems: return None + if raw.body_path: + body, body_line, body_problems = _load_companion_markdown(repository, raw.body_path) + for problem in body_problems: + diagnostics.append(locate(raw.body_path, problem)) + if body_problems: + return None if isinstance(content, Mapping): data = dict(content) for problem in check_item_keys(content, part): @@ -616,7 +627,7 @@ def _read_item( sort_order=_item_sort_order(raw, data), required_level=0, path=slug, - is_document=_is_document(raw.path), + is_document=_is_document(raw.path) or raw.body_path is not None, ) @@ -846,6 +857,18 @@ def _load_document(repository: Repository, rel: str) -> tuple[Any, str, int, lis return data, body, closing + 1, [] +def _load_companion_markdown(repository: Repository, rel: str) -> tuple[str, int, list[Problem]]: + """Read prose-only Markdown paired with a structured manifest item.""" + + try: + text = repository.read_text(rel) + except UnicodeDecodeError: + return "", 0, [Problem(WHOLE_FILE, "3.2", "must be UTF-8")] + if "\r\n" in text: + return "", 0, [Problem(WHOLE_FILE, "3.2", "must use LF line endings")] + return text, 0, [] + + def one_line(error: Exception) -> str: """An exception message on one line, for a diagnostic.""" diff --git a/community_base/content_sync/kinds/base.py b/community_base/content_sync/kinds/base.py index 2440dacd..21b39ba2 100644 --- a/community_base/content_sync/kinds/base.py +++ b/community_base/content_sync/kinds/base.py @@ -97,6 +97,11 @@ class RawItem: name: str parent: str | None = None contributes_slug: bool = True + # Composite items may keep their machine-readable fields in one file and + # their Markdown prose in a sibling companion. The layout still emits one + # item, keyed by ``path``; this path is the body source for rendering and + # relative references. + body_path: str | None = None @dataclass(frozen=True, slots=True) diff --git a/community_base/content_sync/kinds/course.py b/community_base/content_sync/kinds/course.py index 5937e84f..dddf2ead 100644 --- a/community_base/content_sync/kinds/course.py +++ b/community_base/content_sync/kinds/course.py @@ -33,6 +33,21 @@ "docs_url": KeySpec("url"), "faq_url": KeySpec("url"), "hashtag": KeySpec("hashtag"), + # Kept source-relative so the curriculum parser can resolve the + # authored course hierarchy rather than guessing from a title or + # site-owned route. + "projects": KeySpec( + "object_list", + item_keys={ + "slug": KeySpec("slug", required=True), + "title": KeySpec("string", required=True, max_length=200), + "module_path": KeySpec("string", required=True), + "cohort_key": KeySpec("slug"), + "submission_due_at": KeySpec("datetime", required=True), + "review_due_at": KeySpec("datetime", required=True), + "peer_review_count": KeySpec("integer"), + }, + ), "testimonials": KeySpec( "object_list", item_keys={ @@ -71,6 +86,7 @@ ), "session_position": KeySpec("integer"), "is_bonus": KeySpec("boolean", default=False), + "available_after_days": KeySpec("integer"), "code": KeySpec( "object_list", item_keys={ @@ -81,6 +97,56 @@ }, ) +HOMEWORK_UNIT = PartSpec( + name="homework_unit", + shape=SHAPE_MANIFEST, + keys={ + "due_at": KeySpec("datetime", required=True), + "is_bonus": KeySpec("boolean", default=False), + "available_after_days": KeySpec("integer"), + "form": KeySpec( + "mapping", + item_keys={ + **{name: KeySpec("boolean") for name in FORM_FLAGS}, + "learning_in_public_cap": KeySpec("integer"), + }, + ), + "final_fields": KeySpec( + "object_list", + item_keys={ + "key": KeySpec("slug", required=True), + "label": KeySpec("string", required=True), + "type": KeySpec("choice", choices=("text", "url", "textarea")), + "required": KeySpec("boolean", default=False), + }, + ), + "questions": KeySpec( + "object_list", + required=True, + item_keys={ + "content_id": KeySpec("uuid", required=True), + "id": KeySpec("slug", required=True), + "type": KeySpec("choice", choices=QUESTION_TYPES, required=True), + "prompt": KeySpec("markdown", required=True), + "points": KeySpec("integer", default=1), + "step_label": KeySpec("string"), + "options": KeySpec( + "object_list", + item_keys={ + "id": KeySpec("slug", required=True), + "label": KeySpec("string", required=True), + }, + ), + "answer_type": KeySpec("choice", choices=ANSWER_TYPES), + # The source course repository still owns historical scalar + # answers. This key exists only for course-tree homework units; + # cohort homework manifests continue to require envelopes. + "correct": KeySpec("string"), + }, + ), + }, +) + COHORT = PartSpec( name="cohort", shape=SHAPE_MANIFEST, @@ -159,6 +225,7 @@ "course": COURSE, "module": MODULE, "unit": UNIT, + "homework_unit": HOMEWORK_UNIT, "cohort": COHORT, "homework": HOMEWORK, }, diff --git a/community_base/content_sync/kinds/layouts.py b/community_base/content_sync/kinds/layouts.py index 2c9c6850..c97fa72f 100644 --- a/community_base/content_sync/kinds/layouts.py +++ b/community_base/content_sync/kinds/layouts.py @@ -259,9 +259,20 @@ def walk(self, root: DirNode) -> Found: return items, problems def _walk_module(self, node, container, parent, level, items, problems) -> None: + if node.has(MODULE_MANIFEST) and node.has(HOMEWORK_MANIFEST): + problems.append( + ( + node.path, + Problem("", "3.5", "module.yaml and homework.yaml cannot share a directory"), + ) + ) + return if not node.has(MODULE_MANIFEST): problems.append( - (node.path, Problem("", "3.5", f"a module directory needs {MODULE_MANIFEST}")) + ( + node.path, + Problem("", "3.5", f"a curriculum directory needs {MODULE_MANIFEST}"), + ) ) return if level > self.max_module_levels: @@ -284,23 +295,23 @@ def _walk_module(self, node, container, parent, level, items, problems) -> None: parent=parent, ) ) - units = [name for name in node.files if name.endswith(".md") and name != README] - submodules = [ - child - for child in node.dirs - if _base_name(child.path) != CODE_DIR and not is_asset_dir(child) - ] - if units and submodules: + if node.has(HOMEWORK_MANIFEST): problems.append( ( - node.path, + node.joined(HOMEWORK_MANIFEST), Problem( "", "3.5", - "a module directory holds either submodule directories or unit files", + "a homework unit is a sibling directory with homework.yaml and homework.md", ), ) ) + units = [name for name in node.files if name.endswith(".md") and name != README] + submodules = [ + child + for child in node.dirs + if _base_name(child.path) != CODE_DIR and not is_asset_dir(child) + ] for name in units: items.append( RawItem( @@ -312,8 +323,66 @@ def _walk_module(self, node, container, parent, level, items, problems) -> None: ) ) for child in submodules: + if child.has(MODULE_MANIFEST) and child.has(HOMEWORK_MANIFEST): + problems.append( + ( + child.path, + Problem( + "", "3.5", "module.yaml and homework.yaml cannot share a directory" + ), + ) + ) + continue + if child.has(HOMEWORK_MANIFEST): + self._walk_homework_unit(child, node.path, module_path, items, problems) + continue self._walk_module(child, node.path, module_path, level + 1, items, problems) + def _walk_homework_unit(self, node, container, parent, items, problems) -> None: + """Recognize one YAML-backed homework unit and its prose companion.""" + + companion = node.has("homework.md") + other_markdown = [ + name for name in node.files if name.endswith(".md") and name != "homework.md" + ] + extra_content_dirs = [ + child + for child in node.dirs + if _base_name(child.path) != CODE_DIR and not is_asset_dir(child) + ] + if not companion: + problems.append( + ( + node.path, + Problem("", "3.5", "a homework unit needs exactly one homework.md companion"), + ) + ) + if other_markdown or extra_content_dirs: + offenders = [node.joined(name) for name in other_markdown] + [ + child.path for child in extra_content_dirs + ] + problems.append( + ( + offenders[0], + Problem( + "", + "3.5", + "a homework unit has one homework.md companion and no nested content items", + ), + ) + ) + if companion and not other_markdown and not extra_content_dirs: + items.append( + RawItem( + part="homework_unit", + path=node.joined(HOMEWORK_MANIFEST), + container=container, + name=_base_name(node.path), + parent=parent, + body_path=node.joined("homework.md"), + ) + ) + def _walk_cohorts(self, node, items, problems) -> None: for name in node.files: if name != README and not is_asset_name(name): diff --git a/community_base/content_sync/resolution.py b/community_base/content_sync/resolution.py index 18bf0c09..f0f1b80d 100644 --- a/community_base/content_sync/resolution.py +++ b/community_base/content_sync/resolution.py @@ -393,7 +393,7 @@ def replace(match: re.Match[str]) -> str: line = self._line(item, destination) if _is_external(destination): for problem in check_asset_reference(destination, "/body", line): - self.diagnostics.append(locate(item.raw.path, problem)) + self.diagnostics.append(locate(item.raw.body_path or item.raw.path, problem)) return tag asset = self._asset(item, destination, "/body", line) if asset is None: @@ -517,7 +517,7 @@ def _relative( target, _, fragment = destination.partition("#") if not target: return None - resolved = _resolve_relative(item.raw.path, target) + resolved = _resolve_relative(item.raw.body_path or item.raw.path, target) if resolved is None: self._report( item, "/body", "3.6", f"reference leaves the repository: {destination}", line @@ -596,7 +596,7 @@ def _repository_path(self, item: ParsedDocument, destination: str) -> str | None target = target.split("?")[0] if not target: return None - resolved = _resolve_relative(item.raw.path, target) + resolved = _resolve_relative(item.raw.body_path or item.raw.path, target) if not resolved: return None return resolved if (self.repository.root / resolved).exists() else None @@ -626,13 +626,15 @@ def _asset( ) -> ResolvedAsset | None: problems = check_asset_reference(reference, pointer, line) if problems: + source_path = item.raw.body_path if pointer == "/body" else item.raw.path for problem in problems: - self.diagnostics.append(locate(item.raw.path, problem)) + self.diagnostics.append(locate(source_path or item.raw.path, problem)) return None if reference.startswith("https://"): return None target = reference.split("#")[0].split("?")[0] - resolved = _resolve_relative(item.raw.path, target) + source_path = item.raw.body_path if pointer == "/body" else item.raw.path + resolved = _resolve_relative(source_path or item.raw.path, target) if resolved is None: self._report(item, pointer, "3.6", f"asset leaves the repository: {target}", line) return None @@ -716,7 +718,14 @@ def _report( severity: str = SEVERITY_ERROR, ) -> None: self.diagnostics.append( - Diagnostic(item.raw.path, pointer, rule, message, severity=severity, line=line) + Diagnostic( + item.raw.body_path if pointer == "/body" and item.raw.body_path else item.raw.path, + pointer, + rule, + message, + severity=severity, + line=line, + ) ) def _line(self, item: ParsedDocument, destination: str) -> int | None: diff --git a/community_base/curriculum/parsers.py b/community_base/curriculum/parsers.py index d9e2eeb9..bc3c06e1 100644 --- a/community_base/curriculum/parsers.py +++ b/community_base/curriculum/parsers.py @@ -37,6 +37,7 @@ import datetime as dt from collections.abc import Iterable, Mapping +from pathlib import Path, PurePosixPath from typing import Any from community_base.content_sync.documents import ( @@ -44,9 +45,14 @@ Diagnostic, ParsedDocument, ReadResult, + _check_identity, + _node_at, + _read_collection, + _read_repository, read_repository, ) -from community_base.content_sync.kinds.base import resolve_level +from community_base.content_sync.kinds import DirNode, get_kind +from community_base.content_sync.kinds.base import resolve_level, split_order_prefix from community_base.curriculum.code_annotations import ( CodeAnnotationError, validate_annotated_body, @@ -57,16 +63,23 @@ MODE_SELF_PACED, CohortGraph, CourseGraph, + CourseTreeGraph, CurriculumParseError, + HomeworkFinalFieldGraph, + HomeworkFormGraph, + HomeworkOptionGraph, + HomeworkQuestionGraph, + HomeworkUnitGraph, InstructorGraph, ModuleGraph, ParsedCurriculum, + ProjectGraph, UnitGraph, validate_module_tree, ) #: One parser, one version. It names the format, not a site. -PARSER_VERSION = "course-format-1" +PARSER_VERSION = "course-format-2" #: The `content.yaml` version this parser reads (section 3.1). SCHEMA_VERSION = 1 @@ -110,6 +123,120 @@ def parse_course_repository(source: Any, *, path: str | None = None) -> ParsedCu return parse_course(result, collections[0], commit_sha=getattr(source, "commit_sha", None)) +def parse_course_tree( + source: Any, + *, + path: str = ".", + project_specs: Iterable[Mapping[str, Any]] = (), +) -> CourseTreeGraph: + """Parse one physical module/unit tree without taking ownership of its header. + + This is the adapter seam for existing course repositories whose root + ``course.yaml`` also contains site-owned policy. The file must exist as the + course root marker, but its contents and any ``cohorts/`` directory are + deliberately outside this call. All module and unit sources remain under + the normal course schemas and validators. ``source`` may be an immutable + package checkout, a read-only checkout view exposing ``checkout`` and + ``relative``, or a directory; ``path`` is relative to that repository. + + ``project_specs`` lets an adapter pass project fields it owns through the + generic source-relative module resolver without passing unrelated header + metadata into the package parser. + """ + + result, collection, course_manifest_path = _read_course_tree(source, path) + by_parent: dict[str | None, list[ParsedDocument]] = {} + for document in result.documents: + by_parent.setdefault(document.raw.parent, []).append(document) + modules = _module_graphs( + result, + by_parent, + parent=course_manifest_path, + inherited=None, + ) + validate_module_tree(modules, where=collection.path or ".") + projects = _project_graphs(project_specs, modules, course_manifest_path) + return CourseTreeGraph( + parser_version=PARSER_VERSION, + schema_version=SCHEMA_VERSION, + source_path=course_manifest_path, + modules=modules, + projects=projects, + ) + + +def _read_course_tree(source: Any, path: str) -> tuple[ReadResult, Collection, str]: + """Read the course layout under ``path`` without a repository manifest.""" + + if isinstance(source, str | Path): + checkout = None + root = Path(source) + else: + checkout = getattr(source, "checkout", source) + root_value = getattr(checkout, "root", None) + if root_value is None: + raise CurriculumParseError("course tree source must expose a checkout root") + root = Path(root_value) + if not root.is_dir(): + raise CurriculumParseError(f"{root}: course tree source is not a directory") + collection_path = _course_tree_relative_path(source, path) + content_checkout = checkout if checkout is not None and hasattr(checkout, "read_text") else None + repository = _read_repository(root, (), content_checkout) + node = _node_at(repository.tree, collection_path) + if node is None: + raise CurriculumParseError( + f"{collection_path or '.'}: no course tree directory at that source-relative path" + ) + course_manifest_path = f"{collection_path}/course.yaml" if collection_path else "course.yaml" + if "course.yaml" not in node.files: + raise CurriculumParseError(f"{collection_path or '.'}: a course tree needs course.yaml") + + # The tree-only seam intentionally excludes cohort-owned metadata and + # bindings. It reuses the registered layout and all module/unit parsers. + tree_root = DirNode( + path=node.path, + files=node.files, + dirs=tuple(child for child in node.dirs if child.path.rsplit("/", 1)[-1] != "cohorts"), + ) + collection = Collection(kind=get_kind(COURSE_KIND), path=collection_path, index=0) + diagnostics: list[Diagnostic] = [] + documents = _read_collection( + repository, + collection, + diagnostics, + collection_root=tree_root, + ) + # `course.yaml` is just the physical root marker at this boundary. Its + # fields may be a legacy site's schema, so neither its parse diagnostics + # nor its derived record participate in the generic tree graph. + documents = [item for item in documents if item.raw.path != course_manifest_path] + diagnostics = [item for item in diagnostics if item.path != course_manifest_path] + _check_identity(documents, diagnostics) + errors = [item for item in diagnostics if item.severity == "error"] + if errors: + raise CurriculumParseError("\n".join(item.render() for item in errors)) + result = ReadResult( + root=root, + documents=tuple(documents), + diagnostics=tuple(sorted(diagnostics, key=lambda item: item.sort_key)), + repository=repository, + ) + return result, collection, course_manifest_path + + +def _course_tree_relative_path(source: Any, path: str) -> str: + """Normalize a checkout-relative path and honor read-only checkout views.""" + + if path in ("", "."): + return "" + if hasattr(source, "relative"): + return source.relative(path) + candidate = PurePosixPath(str(path).replace("\\", "/")) + if candidate.is_absolute() or any(part in {"", ".", ".."} for part in candidate.parts): + raise CurriculumParseError(f"invalid course tree path {path!r}") + return candidate.as_posix() + + def parse_course( result: ReadResult, collection: Collection, *, commit_sha: str | None = None ) -> ParsedCurriculum: @@ -178,6 +305,7 @@ def _course_graph( status = values.get("status") or PUBLISHED title = document.title cohorts = _cohort_graphs(documents, modules, course_title=title) + projects = _project_graphs(values.get("projects") or (), modules, document.raw.path) return CourseGraph( content_id=document.content_id, slug=document.slug, @@ -197,6 +325,7 @@ def _course_graph( hashtag=values.get("hashtag") or "", visible=status == PUBLISHED, instructors=_instructors(result, values.get("instructors") or ()), + projects=projects, modules=modules, cohorts=cohorts, ) @@ -244,8 +373,20 @@ def _module_graph( level = resolve_level(document.data.get("required_level")) if level is None: level = inherited - units = [item for item in by_parent.get(document.raw.path, ()) if item.part.name == "unit"] - units.sort(key=lambda item: item.sort_key) + siblings = [ + item + for item in by_parent.get(document.raw.path, ()) + if item.part.name in {"module", "unit", "homework_unit"} + ] + siblings.sort(key=lambda item: item.sort_key) + items = [] + for sibling in siblings: + if sibling.part.name == "module": + items.append(_module_graph(result, by_parent, sibling, level)) + elif sibling.part.name == "homework_unit": + items.append(_homework_unit_graph(sibling, level)) + else: + items.append(_unit_graph(sibling, level)) return ModuleGraph( content_id=document.content_id, slug=document.slug, @@ -256,8 +397,8 @@ def _module_graph( sort_order=document.sort_order, is_bonus=bool(values.get("is_bonus")), available_after_days=values.get("available_after_days"), - units=tuple(_unit_graph(item, level) for item in units), - children=_module_graphs(result, by_parent, parent=document.raw.path, inherited=level), + has_order=_has_order(document), + items=tuple(items), ) @@ -289,9 +430,154 @@ def _unit_graph(document: ParsedDocument, inherited: int | None) -> UnitGraph: kind=values.get("kind") or "lesson", session_position=values.get("session_position"), is_bonus=bool(values.get("is_bonus")), + available_after_days=values.get("available_after_days"), + body_source_path=document.raw.body_path, + has_order=_has_order(document), ) +def _homework_unit_graph(document: ParsedDocument, inherited: int | None) -> UnitGraph: + values = document.values + homework = HomeworkUnitGraph( + due_at=_datetime(values.get("due_at"), document.raw.path, "/due_at"), + form=_homework_form(values.get("form") or {}), + final_fields=tuple( + HomeworkFinalFieldGraph( + key=entry["key"], + label=entry["label"], + type=entry.get("type") or "text", + required=bool(entry.get("required")), + ) + for entry in values.get("final_fields") or () + ), + questions=tuple( + HomeworkQuestionGraph( + content_id=entry["content_id"], + stable_id=entry["id"], + type=entry["type"], + prompt=entry["prompt"], + points=entry.get("points", 1), + options=tuple( + HomeworkOptionGraph(id=option["id"], label=option["label"]) + for option in entry.get("options") or () + ), + answer_type=entry.get("answer_type"), + step_label=entry.get("step_label") or "", + correct=entry.get("correct"), + ) + for entry in values.get("questions") or () + ), + ) + return UnitGraph( + content_id=document.content_id, + slug=document.slug, + title=document.title, + source_path=document.raw.path, + body=document.body, + required_level=_declared_level(document, inherited), + sort_order=document.sort_order, + kind="homework", + is_bonus=bool(values.get("is_bonus")), + available_after_days=values.get("available_after_days"), + body_source_path=document.raw.body_path, + has_order=_has_order(document), + homework_unit=homework, + ) + + +def _homework_form(values: Mapping[str, Any]) -> HomeworkFormGraph: + return HomeworkFormGraph( + homework_url=values.get("homework_url"), + time_spent_lectures=values.get("time_spent_lectures"), + time_spent_homework=values.get("time_spent_homework"), + faq_contribution=values.get("faq_contribution"), + learning_in_public_cap=values.get("learning_in_public_cap"), + ) + + +def _has_order(document: ParsedDocument) -> bool: + declared = document.data.get("sort_order") + if isinstance(declared, int) and not isinstance(declared, bool): + return True + prefix, _ = split_order_prefix(document.raw.name) + return prefix is not None + + +def _datetime(value: Any, path: str, pointer: str) -> dt.datetime: + if isinstance(value, dt.datetime): + parsed = value + elif isinstance(value, str): + try: + parsed = dt.datetime.fromisoformat(value) + except ValueError: + raise CurriculumParseError(f"{path}:{pointer}: expected an ISO datetime") from None + else: + raise CurriculumParseError(f"{path}:{pointer}: expected an ISO datetime") + if parsed.tzinfo is None or parsed.utcoffset() is None: + raise CurriculumParseError(f"{path}:{pointer}: datetime needs a UTC offset") + return parsed + + +def _project_graphs( + entries: Iterable[Mapping[str, Any]], modules, path: str +) -> tuple[ProjectGraph, ...]: + by_path: dict[str, list[ModuleGraph]] = {} + + def walk(siblings, prefix=()): + for module in siblings: + module_path = (*prefix, module.slug) + by_path.setdefault("/".join(module_path), []).append(module) + walk(module.children, module_path) + + walk(modules) + found: list[ProjectGraph] = [] + seen: set[str] = set() + for index, entry in enumerate(entries): + project_slug = entry["slug"] + pointer = f"{path}:/projects/{index}" + if project_slug in seen: + raise CurriculumParseError(f"{pointer}/slug: duplicate project slug {project_slug!r}") + seen.add(project_slug) + module_path = entry["module_path"] + components = module_path.split("/") + if any(not component for component in components): + raise CurriculumParseError( + f"{pointer}/module_path: invalid module path {module_path!r}" + ) + matches = by_path.get(module_path, []) + if not matches: + raise CurriculumParseError( + f"{pointer}/module_path: no module at source path {module_path!r}" + ) + if len(matches) != 1: + raise CurriculumParseError( + f"{pointer}/module_path: ambiguous module path {module_path!r}" + ) + submission_due_at = _datetime( + entry["submission_due_at"], path, f"/projects/{index}/submission_due_at" + ) + review_due_at = _datetime(entry["review_due_at"], path, f"/projects/{index}/review_due_at") + if review_due_at <= submission_due_at: + raise CurriculumParseError(f"{pointer}/review_due_at: must follow submission_due_at") + peer_review_count = entry.get("peer_review_count") + if peer_review_count is not None and not 1 <= peer_review_count <= 10: + raise CurriculumParseError(f"{pointer}/peer_review_count: must be between 1 and 10") + module = matches[0] + found.append( + ProjectGraph( + slug=project_slug, + title=entry["title"], + module_path=module_path, + module_content_id=module.content_id, + submission_due_at=submission_due_at, + review_due_at=review_due_at, + cohort_key=entry.get("cohort_key"), + peer_review_count=peer_review_count, + ) + ) + return tuple(found) + + def _declared_level(document: ParsedDocument, inherited: int | None) -> int | None: """The unit's own level, else the one its module ancestors declared. diff --git a/community_base/curriculum/source.py b/community_base/curriculum/source.py index 00959129..e7e3f3a7 100644 --- a/community_base/curriculum/source.py +++ b/community_base/curriculum/source.py @@ -9,7 +9,7 @@ from collections.abc import Mapping from dataclasses import dataclass, field -from datetime import date +from datetime import date, datetime from typing import Any MODE_COHORT = "cohort" @@ -49,11 +49,59 @@ class UnitGraph: kind: str = "lesson" session_position: int | None = None is_bonus: bool = False + available_after_days: int | None = None + body_source_path: str | None = None + has_order: bool = False + homework_unit: HomeworkUnitGraph | None = None + + +@dataclass(frozen=True, slots=True) +class HomeworkOptionGraph: + id: str + label: str + + +@dataclass(frozen=True, slots=True) +class HomeworkQuestionGraph: + content_id: str + stable_id: str + type: str + prompt: str + points: int + options: tuple[HomeworkOptionGraph, ...] = field(default=()) + answer_type: str | None = None + step_label: str = "" + correct: str | None = None + + +@dataclass(frozen=True, slots=True) +class HomeworkFormGraph: + homework_url: bool | None = None + time_spent_lectures: bool | None = None + time_spent_homework: bool | None = None + faq_contribution: bool | None = None + learning_in_public_cap: int | None = None + + +@dataclass(frozen=True, slots=True) +class HomeworkFinalFieldGraph: + key: str + label: str + type: str = "text" + required: bool = False + + +@dataclass(frozen=True, slots=True) +class HomeworkUnitGraph: + due_at: datetime + form: HomeworkFormGraph = field(default_factory=HomeworkFormGraph) + questions: tuple[HomeworkQuestionGraph, ...] = field(default=()) + final_fields: tuple[HomeworkFinalFieldGraph, ...] = field(default=()) @dataclass(frozen=True, slots=True) class ModuleGraph: - """One module, course-owned. Either ``children`` or ``units`` is non-empty, never both.""" + """One physical module and its ordered, mixed child sequence.""" content_id: str | None slug: str @@ -64,8 +112,48 @@ class ModuleGraph: sort_order: int = 0 is_bonus: bool = False available_after_days: int | None = None - units: tuple[UnitGraph, ...] = field(default=()) - children: tuple[ModuleGraph, ...] = field(default=()) + has_order: bool = False + items: tuple[ModuleGraph | UnitGraph, ...] = field(default=()) + + @property + def units(self) -> tuple[UnitGraph, ...]: + """Direct units in sibling order, retained for older read-only consumers.""" + + return tuple(item for item in self.items if isinstance(item, UnitGraph)) + + @property + def children(self) -> tuple[ModuleGraph, ...]: + """Child modules in sibling order, retained for older read-only consumers.""" + + return tuple(item for item in self.items if isinstance(item, ModuleGraph)) + + +@dataclass(frozen=True, slots=True) +class ProjectGraph: + slug: str + title: str + module_path: str + module_content_id: str | None + submission_due_at: datetime + review_due_at: datetime + cohort_key: str | None = None + peer_review_count: int | None = None + + +@dataclass(frozen=True, slots=True) +class CourseTreeGraph: + """Physical course content without course-, cohort- or site-owned metadata. + + Site adapters for legacy course repositories use this graph to parse the + checked-in module/unit tree while retaining ownership of their existing + course headers and cohort policy. + """ + + parser_version: str + schema_version: int + source_path: str + modules: tuple[ModuleGraph, ...] = field(default=()) + projects: tuple[ProjectGraph, ...] = field(default=()) @dataclass(frozen=True, slots=True) @@ -115,9 +203,9 @@ class CourseGraph: hashtag: str = "" visible: bool = True instructors: tuple[InstructorGraph, ...] = field(default=()) - # The one module tree, owned by the course. Top-level modules in display - # order; each may carry ``children`` (submodules, max depth two) or - # ``units`` directly, never both. + projects: tuple[ProjectGraph, ...] = field(default=()) + # The one module tree, owned by the course. Top-level modules and every + # module's `items` preserve the physical mixed sibling order. modules: tuple[ModuleGraph, ...] = field(default=()) cohorts: tuple[CohortGraph, ...] = field(default=()) @@ -137,31 +225,101 @@ class CurriculumParseError(ValueError): def validate_module_tree(modules: tuple[ModuleGraph, ...], *, where: str, depth: int = 1) -> None: """Validate a course's module tree once, for the one course parser. - Enforces: a module has either ``children`` or ``units``, never both, naming the - offending directory (``where``); nesting does not exceed two module levels; sibling - module slugs (and sibling unit slugs) are unique within their own parent, not globally - -- so two submodules under different parents may each contain a unit slugged the same. + Enforces unique sibling slugs and orders across the mixed module/unit sequence, + explicit ordering for every sibling, and the existing two-level module bound. """ seen_module_slugs: set[str] = set() + seen_module_orders: set[int] = set() for module in modules: if module.slug in seen_module_slugs: raise CurriculumParseError(f"{where}: duplicate module slug {module.slug!r}") seen_module_slugs.add(module.slug) module_where = f"{module.source_path or where}" - if module.children and module.units: + if not module.has_order: raise CurriculumParseError( - f"{module_where}: has both child modules and direct units; " - "a module must have only one" + f"{module_where}: missing sibling order; declare sort_order or use an NN- prefix" ) - if module.children: + if module.sort_order in seen_module_orders: + raise CurriculumParseError( + f"{module_where}: duplicate sibling order {module.sort_order}" + ) + seen_module_orders.add(module.sort_order) + seen_sibling_slugs: set[str] = set() + seen_orders: set[int] = set() + for sibling in module.items: + sibling_where = sibling.source_path or module_where + if sibling.slug in seen_sibling_slugs: + raise CurriculumParseError( + f"{sibling_where}: duplicate sibling slug {sibling.slug!r}" + ) + seen_sibling_slugs.add(sibling.slug) + if not sibling.has_order: + raise CurriculumParseError( + f"{sibling_where}: missing sibling order; declare sort_order or use an " + "NN- prefix" + ) + if sibling.sort_order in seen_orders: + raise CurriculumParseError( + f"{sibling_where}: duplicate sibling order {sibling.sort_order}" + ) + seen_orders.add(sibling.sort_order) + if isinstance(sibling, UnitGraph) and sibling.homework_unit: + validate_homework_unit(sibling.homework_unit, where=sibling_where) + children = module.children + if children: if depth >= 2: raise CurriculumParseError( f"{module_where}: exceeds the maximum module depth of two levels" ) - validate_module_tree(module.children, where=module_where, depth=depth + 1) - seen_unit_slugs: set[str] = set() - for unit in module.units: - if unit.slug in seen_unit_slugs: - raise CurriculumParseError(f"{module_where}: duplicate unit slug {unit.slug!r}") - seen_unit_slugs.add(unit.slug) + validate_module_tree(children, where=module_where, depth=depth + 1) + + +def validate_homework_unit(homework: HomeworkUnitGraph, *, where: str) -> None: + """Validate question identities, choice shapes and legacy scoring values.""" + + seen_question_ids: set[str] = set() + seen_content_ids: set[str] = set() + for index, question in enumerate(homework.questions): + pointer = f"{where}:/questions/{index}" + if question.stable_id in seen_question_ids: + raise CurriculumParseError( + f"{pointer}/id: duplicate question id {question.stable_id!r}" + ) + if question.content_id in seen_content_ids: + raise CurriculumParseError( + f"{pointer}/content_id: duplicate question content_id {question.content_id!r}" + ) + seen_question_ids.add(question.stable_id) + seen_content_ids.add(question.content_id) + option_ids = [option.id for option in question.options] + if len(option_ids) != len(set(option_ids)): + raise CurriculumParseError(f"{pointer}/options: duplicate option id") + choice = question.type in {"multiple_choice", "checkboxes"} + if choice and not question.options: + raise CurriculumParseError(f"{pointer}/options: choice questions need ordered options") + if not choice and question.options: + raise CurriculumParseError( + f"{pointer}/options: free-form questions do not take options" + ) + if question.correct is not None and choice: + try: + indexes = [int(item.strip()) for item in question.correct.split(",")] + except (TypeError, ValueError): + raise CurriculumParseError( + f"{pointer}/correct: choice answers use 1-based option indexes" + ) from None + if ( + not indexes + or any( + index_value < 1 or index_value > len(question.options) + for index_value in indexes + ) + or len(set(indexes)) != len(indexes) + or (question.type == "multiple_choice" and len(indexes) != 1) + ): + raise CurriculumParseError( + f"{pointer}/correct: answer index must identify an authored option" + ) + if question.points < 0: + raise CurriculumParseError(f"{pointer}/points: must be zero or a positive integer") diff --git a/docs/plan/STATUS.md b/docs/plan/STATUS.md index c056d7cf..de97c6cc 100644 --- a/docs/plan/STATUS.md +++ b/docs/plan/STATUS.md @@ -147,7 +147,7 @@ issues that can start now. | `C5.2i` | community-base | Shared inline homework steps and resumable drafts | C5.1c | no | done | https://github.com/DataTalksClub/community-base/releases/tag/v0.5.5 | | `C5.2j` | community-base | Shared learner homework state and accepted-submission snapshot | C5.2i | no | in-progress | https://github.com/DataTalksClub/community-base/pull/305 | | `C5.3` | community-base | Release 0.6.0 | C3.7, C4.3, C5.2e, C5.1e, C5.2h | no | todo | https://github.com/DataTalksClub/community-base/issues/273 | -| `C5.4` | community-base | Repository-derived curriculum hierarchy and YAML-backed homework units | C5.1e, C5.2i, C7.10, C7.11 | no | todo | https://github.com/DataTalksClub/community-base/issues/306 | +| `C5.4` | community-base | Repository-derived curriculum hierarchy and YAML-backed homework units | C5.1e, C5.2i, C7.10, C7.11 | no | in-progress | https://github.com/DataTalksClub/community-base/issues/306 | | `A5.3` | AI-Shipping-Labs/website | AISL: render course hierarchy from repository structure | C5.4 | no | todo | https://github.com/AI-Shipping-Labs/website/issues/1830 | | `D5.3` | DataTalksClub/website | DTC: adopt repository-derived course hierarchy and homework units | C5.4 | no | todo | https://github.com/DataTalksClub/website/issues/436 | | `A5.1` | AI-Shipping-Labs/website | Map AISL courses to the shared apps | C5.3, A5.3 | no | todo | https://github.com/AI-Shipping-Labs/website/issues/1696 | diff --git a/tests/content_sync/fixtures/invalid/course_mixed_module/01-module/01-unit.md b/tests/content_sync/fixtures/valid_mixed_course/01-module/01-unit.md similarity index 100% rename from tests/content_sync/fixtures/invalid/course_mixed_module/01-module/01-unit.md rename to tests/content_sync/fixtures/valid_mixed_course/01-module/01-unit.md diff --git a/tests/content_sync/fixtures/invalid/course_mixed_module/01-module/02-submodule/module.yaml b/tests/content_sync/fixtures/valid_mixed_course/01-module/02-submodule/module.yaml similarity index 100% rename from tests/content_sync/fixtures/invalid/course_mixed_module/01-module/02-submodule/module.yaml rename to tests/content_sync/fixtures/valid_mixed_course/01-module/02-submodule/module.yaml diff --git a/tests/content_sync/fixtures/invalid/course_mixed_module/01-module/module.yaml b/tests/content_sync/fixtures/valid_mixed_course/01-module/module.yaml similarity index 100% rename from tests/content_sync/fixtures/invalid/course_mixed_module/01-module/module.yaml rename to tests/content_sync/fixtures/valid_mixed_course/01-module/module.yaml diff --git a/tests/content_sync/fixtures/invalid/course_mixed_module/content.yaml b/tests/content_sync/fixtures/valid_mixed_course/content.yaml similarity index 100% rename from tests/content_sync/fixtures/invalid/course_mixed_module/content.yaml rename to tests/content_sync/fixtures/valid_mixed_course/content.yaml diff --git a/tests/content_sync/fixtures/invalid/course_mixed_module/course.yaml b/tests/content_sync/fixtures/valid_mixed_course/course.yaml similarity index 100% rename from tests/content_sync/fixtures/invalid/course_mixed_module/course.yaml rename to tests/content_sync/fixtures/valid_mixed_course/course.yaml diff --git a/tests/content_sync/test_documents.py b/tests/content_sync/test_documents.py index 5cd0693d..3e5455c6 100644 --- a/tests/content_sync/test_documents.py +++ b/tests/content_sync/test_documents.py @@ -13,7 +13,13 @@ from community_base.content_sync.resolution import resolve_repository FIXTURES = Path(__file__).parent / "fixtures" -VALID = ("valid_course", "valid_multi", "valid_docs", "lenient_references") +VALID = ( + "valid_course", + "valid_mixed_course", + "valid_multi", + "valid_docs", + "lenient_references", +) WIKI_MANIFEST = "schema_version: 1\ncollections:\n - kind: wiki\n path: wiki\n" PAGE = ( @@ -384,7 +390,7 @@ def test_a_site_kind_module_can_be_imported_before_reading(tmp_path): ("invalid/bad_schema_version", "reject", "reject", ""), ("invalid/bad_slug_name", "reject", "reject", ""), ("invalid/bad_uuid", "reject", "reject", ""), - ("invalid/course_mixed_module", "reject", "reject", ""), + ("valid_mixed_course", "accept", "accept", ""), ("invalid/date_prefix", "reject", "reject", ""), ("invalid/docs_missing_index", "reject", "reject", ""), ("invalid/docs_too_deep", "reject", "reject", ""), diff --git a/tests/curriculum/test_parsers.py b/tests/curriculum/test_parsers.py index 851b2bba..e35f03b4 100644 --- a/tests/curriculum/test_parsers.py +++ b/tests/curriculum/test_parsers.py @@ -329,7 +329,7 @@ def test_a_published_live_cohort_declares_its_dates(tmp_path): assert "start_date" in str(error.value) -def test_a_module_holding_both_submodules_and_units_is_rejected(tmp_path): +def test_a_module_holding_both_submodules_and_units_keeps_one_sibling_order(tmp_path): root = copy(DTC_NESTED, tmp_path, "mixed") stray = root / "01-week-one" / "99-stray-unit.md" stray.write_text( @@ -337,10 +337,77 @@ def test_a_module_holding_both_submodules_and_units_is_rejected(tmp_path): "title: Stray\n---\nShould not be allowed here.\n" ) - with pytest.raises(CurriculumParseError) as error: - parse(root) + week = parse(root).course.modules[0] + + assert [(item.slug, item.sort_order) for item in week.items] == [ + ("topic-a", 1), + ("topic-b", 2), + ("stray-unit", 99), + ] + + +def test_a_yaml_homework_unit_pairs_structured_fields_with_companion_prose(tmp_path): + root = copy(DTC_NESTED, tmp_path, "homework-tree") + week = root / "01-week-one" + (week / "02-topic-b").rename(week / "04-topic-b") + lesson = week / "02-session.md" + lesson.write_text( + "---\ncontent_id: 2b3c4d5e-000b-4000-8000-000000000001\n" + "title: Session\n---\nSession notes.\n" + ) + homework = week / "03-homework" + homework.mkdir() + (homework / "homework.yaml").write_text( + "content_id: 2b3c4d5e-000c-4000-8000-000000000001\n" + "title: Homework\nslug: homework\nsort_order: 3\n" + "due_at: '2026-10-05T23:59:59+00:00'\n" + "form:\n homework_url: true\nquestions:\n" + "- content_id: 2b3c4d5e-000d-4000-8000-000000000001\n" + " id: q1-sample\n type: multiple_choice\n prompt: Choose one.\n" + " points: 2\n step_label: First step\n options:\n" + " - id: option-one\n label: One\n" + " - id: option-two\n label: Two\n correct: '2'\n" + ) + (homework / "homework.md").write_text("Instructions from Markdown.\n") + + course = parse(root).course + week_graph = course.modules[0] + homework_graph = week_graph.items[2] + + assert [(item.slug, item.sort_order) for item in week_graph.items] == [ + ("topic-a", 1), + ("session", 2), + ("homework", 3), + ("topic-b", 4), + ] + assert homework_graph.kind == "homework" + assert homework_graph.body.strip() == "Instructions from Markdown." + assert homework_graph.body_source_path == "01-week-one/03-homework/homework.md" + assert homework_graph.homework_unit.due_at.isoformat() == "2026-10-05T23:59:59+00:00" + question = homework_graph.homework_unit.questions[0] + assert (question.content_id, question.stable_id, question.step_label) == ( + "2b3c4d5e-000d-4000-8000-000000000001", + "q1-sample", + "First step", + ) + assert [option.label for option in question.options] == ["One", "Two"] + assert question.correct == "2" + + +def test_a_tree_only_parse_ignores_legacy_course_and_cohort_metadata(tmp_path): + from community_base.curriculum.parsers import parse_course_tree + + root = copy(DTC_NESTED, tmp_path, "legacy-tree") + (root / "content.yaml").unlink() + (root / "course.yaml").write_text( + "legacy_access_policy: keep-at-site\ncohorts: [not, a, shared, schema]\n" + ) + (root / "cohorts" / "2026" / "cohort.yaml").write_text("not: package metadata\n") + + tree = parse_course_tree(root) - assert "01-week-one" in str(error.value) + assert [module.slug for module in tree.modules] == ["week-one", "week-two"] + assert tree.modules[0].items[0].slug == "topic-a" def test_three_module_levels_are_rejected(tmp_path): From cfbde844e8f32ee5f4fca6aa1cde72a1b12475df Mon Sep 17 00:00:00 2001 From: Alexey Grigorev Date: Sat, 26 Sep 2026 22:10:50 +0200 Subject: [PATCH 2/7] Import and project recursive mixed course trees --- community_base/content_sync/kinds/course.py | 3 + community_base/content_sync/kinds/layouts.py | 25 ++- community_base/curriculum/importing.py | 152 +++++++++++----- community_base/curriculum/models.py | 98 +++++----- community_base/curriculum/parsers.py | 38 +++- community_base/curriculum/services.py | 167 ++++++++++++++---- community_base/curriculum/source.py | 11 +- .../templates/curriculum/_syllabus_item.html | 32 ++++ .../templates/curriculum/course_detail.html | 22 +-- .../templates/curriculum/module_overview.html | 11 +- .../templates/curriculum/unit_detail.html | 25 ++- community_base/curriculum/urls.py | 16 +- community_base/curriculum/views.py | 134 ++++++++++---- tests/curriculum/test_import.py | 134 +++++++++++++- tests/curriculum/test_models.py | 31 ++-- tests/curriculum/test_parsers.py | 38 +++- tests/curriculum/test_services.py | 46 ++++- tests/curriculum/test_views.py | 57 ++++++ 18 files changed, 811 insertions(+), 229 deletions(-) create mode 100644 community_base/curriculum/templates/curriculum/_syllabus_item.html diff --git a/community_base/content_sync/kinds/course.py b/community_base/content_sync/kinds/course.py index dddf2ead..7396c6c2 100644 --- a/community_base/content_sync/kinds/course.py +++ b/community_base/content_sync/kinds/course.py @@ -67,6 +67,9 @@ keys={ "syllabus_section": KeySpec("string", max_length=255), "is_bonus": KeySpec("boolean", default=False), + # Existing course sources call the same generic module flag `bonus`. + # The graph and persistence API use the clearer `is_bonus` name. + "bonus": KeySpec("boolean"), "available_after_days": KeySpec("integer"), }, ) diff --git a/community_base/content_sync/kinds/layouts.py b/community_base/content_sync/kinds/layouts.py index c97fa72f..bd15790c 100644 --- a/community_base/content_sync/kinds/layouts.py +++ b/community_base/content_sync/kinds/layouts.py @@ -225,7 +225,7 @@ class CourseLayout(Layout): named. """ - def __init__(self, max_module_levels: int = 2) -> None: + def __init__(self, max_module_levels: int | None = None) -> None: self.max_module_levels = max_module_levels def walk(self, root: DirNode) -> Found: @@ -255,6 +255,23 @@ def walk(self, root: DirNode) -> Found: continue if name == CODE_DIR or is_asset_dir(child): continue + if child.has(HOMEWORK_MANIFEST) and not child.has(MODULE_MANIFEST): + problems.append( + ( + child.path, + Problem( + "", + "3.5", + "a homework unit must be inside a module directory", + ), + ) + ) + continue + if not child.has(MODULE_MANIFEST) and not child.has(HOMEWORK_MANIFEST): + # Directories without course manifests are supporting source + # material (for example scripts, solutions or assets), not + # curriculum nodes. A module is declared by module.yaml. + continue self._walk_module(child, root.path, course_path, 1, items, problems) return items, problems @@ -275,7 +292,7 @@ def _walk_module(self, node, container, parent, level, items, problems) -> None: ) ) return - if level > self.max_module_levels: + if self.max_module_levels is not None and level > self.max_module_levels: problems.append( ( node.path, @@ -310,7 +327,9 @@ def _walk_module(self, node, container, parent, level, items, problems) -> None: submodules = [ child for child in node.dirs - if _base_name(child.path) != CODE_DIR and not is_asset_dir(child) + if _base_name(child.path) != CODE_DIR + and not is_asset_dir(child) + and (child.has(MODULE_MANIFEST) or child.has(HOMEWORK_MANIFEST)) ] for name in units: items.append( diff --git a/community_base/curriculum/importing.py b/community_base/curriculum/importing.py index 1492eaaf..f70b227d 100644 --- a/community_base/curriculum/importing.py +++ b/community_base/curriculum/importing.py @@ -27,7 +27,14 @@ Module, Unit, ) -from community_base.curriculum.source import CurriculumParseError, InstructorGraph, ParsedCurriculum +from community_base.curriculum.source import ( + CourseTreeGraph, + CurriculumParseError, + InstructorGraph, + ModuleGraph, + ParsedCurriculum, + UnitGraph, +) ACTION_CREATED = "created" ACTION_UPDATED = "updated" @@ -116,6 +123,47 @@ def apply_curriculum_graph(parsed: ParsedCurriculum, source, checkout) -> tuple[ return course, counts +def apply_curriculum_tree( + course: Course, + tree: CourseTreeGraph, + *, + commit: str, + checkout, +) -> dict: + """Upsert only module and unit rows for an already-owned shared course. + + This is the adapter seam for a site that still owns its course header, + cohorts, access and routes. Source unit IDs are looked up across the whole + course before a row is reparented, so a physical move retains the Unit + primary key and every learner/coursework foreign key to it. + """ + + counts = {"created": 0, "updated": 0, "unchanged": 0, "deleted": 0} + seen_modules: set[str] = set() + seen_units: set[str] = set() + with transaction.atomic(): + _apply_module_tree( + course, + tree.modules, + parent=None, + commit=commit, + checkout=checkout, + counts=counts, + seen=seen_modules, + seen_units=seen_units, + top_level_by_ref={}, + ) + counts["deleted"] += delete_stale( + Unit.objects.filter(module__course=course).exclude(source_content_id__isnull=True), + seen_units, + ) + counts["deleted"] += delete_stale( + Module.objects.filter(course=course).exclude(source_content_id__isnull=True), + seen_modules, + ) + return counts + + def _manifest_checksum(parsed) -> str: canonical = repr(sorted(_canonical(_graph_summary(parsed.course)).items())) return hashlib.sha256(canonical.encode("utf-8")).hexdigest() @@ -161,6 +209,7 @@ def _apply(parsed, source, checkout, commit) -> tuple[Course, dict]: # resolve their placements by the same identifier the source graph uses # (content_id, or slug when the source has none). seen_module_ids: set[str] = set() + seen_unit_ids: set[str] = set() top_level_by_ref: dict[str, Module] = {} _apply_module_tree( course, @@ -170,8 +219,13 @@ def _apply(parsed, source, checkout, commit) -> tuple[Course, dict]: checkout=checkout, counts=counts, seen=seen_module_ids, + seen_units=seen_unit_ids, top_level_by_ref=top_level_by_ref, ) + counts["deleted"] += delete_stale( + Unit.objects.filter(module__course=course).exclude(source_content_id__isnull=True), + seen_unit_ids, + ) counts["deleted"] += delete_stale( Module.objects.filter(course=course).exclude(source_content_id__isnull=True), seen_module_ids, @@ -199,38 +253,36 @@ def _apply_module_tree( checkout, counts: dict, seen: set, + seen_units: set, top_level_by_ref: dict, depth: int = 0, ) -> None: - for position, module_graph in enumerate(module_graphs): + for module_graph in module_graphs: module = _module(course, parent, module_graph) seen.add(module_graph.content_id) - values = _module_values(module_graph, position, commit, checkout) + values = _module_values(module_graph, parent, commit, checkout) counts[write_values(module, values)] += 1 if depth == 0: top_level_by_ref[module_graph.content_id or module_graph.slug] = module - seen_unit_ids: set = set() - for unit_graph in module_graph.units: - unit = _unit(module, unit_graph) - seen_unit_ids.add(unit_graph.content_id) - counts[write_values(unit, _unit_values(unit_graph, commit, checkout))] += 1 - counts["deleted"] += delete_stale( - Unit.objects.filter(module=module).exclude(source_content_id__isnull=True), - seen_unit_ids, - ) - - _apply_module_tree( - course, - module_graph.children, - parent=module, - commit=commit, - checkout=checkout, - counts=counts, - seen=seen, - top_level_by_ref=top_level_by_ref, - depth=depth + 1, - ) + for item in module_graph.items: + if isinstance(item, UnitGraph): + unit = _unit(course, module, item) + seen_units.add(item.content_id) + counts[write_values(unit, _unit_values(item, module, commit, checkout))] += 1 + elif isinstance(item, ModuleGraph): + _apply_module_tree( + course, + (item,), + parent=module, + commit=commit, + checkout=checkout, + counts=counts, + seen=seen, + seen_units=seen_units, + top_level_by_ref=top_level_by_ref, + depth=depth + 1, + ) def _apply_placements(cohort: Cohort, cohort_graph, top_level_by_ref: dict) -> int: @@ -298,21 +350,23 @@ def _module(course: Course, parent: Module | None, graph) -> Module: if module is None: module = Module.objects.filter(course=course, parent=parent, slug=graph.slug).first() if module is None: - module = Module(course=course, slug=graph.slug) - module.parent = parent - module.source_content_id = graph.content_id + module = Module(course=course) return module -def _unit(module: Module, graph) -> Unit: +def _unit(course: Course, module: Module, graph: UnitGraph) -> Unit: unit = None if graph.content_id: - unit = Unit.objects.filter(module=module, source_content_id=graph.content_id).first() + # Content IDs belong to the course tree, not to a module. Look up across + # the whole course so moving a physical source file reuses its row and + # preserves learner progress and coursework foreign keys. + unit = Unit.objects.filter( + module__course=course, source_content_id=graph.content_id + ).first() if unit is None: unit = Unit.objects.filter(module=module, slug=graph.slug).first() if unit is None: - unit = Unit(module=module, slug=graph.slug) - unit.source_content_id = graph.content_id + unit = Unit() return unit @@ -349,11 +403,13 @@ def _cohort_values(graph, commit, checkout) -> dict: } -def _module_values(graph, position, commit, checkout) -> dict: - sort_order = graph.sort_order or position +def _module_values(graph, parent, commit, checkout) -> dict: return { + "parent": parent, + "slug": graph.slug, + "source_content_id": graph.content_id, "title": graph.title, - "sort_order": sort_order, + "sort_order": graph.sort_order, "syllabus_section": graph.syllabus_section, "overview": graph.overview, "is_bonus": graph.is_bonus, @@ -362,8 +418,11 @@ def _module_values(graph, position, commit, checkout) -> dict: } -def _unit_values(graph, commit, checkout) -> dict: +def _unit_values(graph, module, commit, checkout) -> dict: return { + "module": module, + "slug": graph.slug, + "source_content_id": graph.content_id, "title": graph.title, "sort_order": graph.sort_order, "kind": graph.kind, @@ -375,7 +434,9 @@ def _unit_values(graph, commit, checkout) -> dict: "timestamps": list(graph.timestamps), "is_preview": graph.is_preview, "required_level": graph.required_level, - "content_hash": _body_hash(graph), + "content_hash": ( + graph.content_hash if graph.content_hash is not None else _body_hash(graph) + ), **provenance(graph.source_path, commit, file_checksum(checkout, graph.source_path)), } @@ -403,13 +464,13 @@ def write_values(instance, values) -> str: was_new = instance.pk is None content_keys = [key for key in values if key not in _PROVENANCE_KEYS] if not was_new: - changed = any( - _canonical(getattr(instance, key)) != _canonical(values[key]) for key in content_keys - ) + changed = any(not _same_field_value(instance, key, values[key]) for key in content_keys) if not changed: + for key, value in values.items(): + setattr(instance, key, value) for key in _PROVENANCE_KEYS: setattr(instance, key, values[key]) - fields = [*_PROVENANCE_KEYS, "source_content_id"] + fields = list(dict.fromkeys([*values, *_PROVENANCE_KEYS])) instance.save(update_fields=fields) return ACTION_UNCHANGED for key, value in values.items(): @@ -421,6 +482,17 @@ def write_values(instance, values) -> str: _PROVENANCE_KEYS = ("source_path", "source_commit_sha", "source_checksum") +def _same_field_value(instance, key, value) -> bool: + if key == "source_content_id": + old = getattr(instance, key) + return (str(old) if old is not None else None) == ( + str(value) if value is not None else None + ) + if key in {"module", "parent"}: + return getattr(instance, f"{key}_id") == getattr(value, "pk", None) + return _canonical(getattr(instance, key)) == _canonical(value) + + def delete_stale(queryset, seen_ids: set) -> int: seen = {str(value) for value in seen_ids if value is not None} stale = [ diff --git a/community_base/curriculum/models.py b/community_base/curriculum/models.py index dae5e7fe..af2ff2d7 100644 --- a/community_base/curriculum/models.py +++ b/community_base/curriculum/models.py @@ -167,24 +167,35 @@ def primary_instructor(self): def _countable_units(self): """Units that count toward the progress denominator. - Every unit counts, including ``kind=event`` units, except a unit (or its module, or - that module's parent module) marked ``is_bonus`` -- tracked and displayed, but + Every unit counts, including ``kind=event`` units, except a unit or any + ancestor module marked ``is_bonus`` -- tracked and displayed, but excluded from the denominator (owner decision, community-base#252). ``kind=checklist_item`` units are excluded outright: a pre-work checklist is a separate readiness track, not lesson/homework/event course progress, regardless of whether an individual item is marked required (``is_bonus=False``) or optional (``is_bonus=True``). """ - return ( + module_rows = list( + Module.objects.filter(course=self).values_list("pk", "parent_id", "is_bonus") + ) + bonus_module_ids = {pk for pk, _parent_id, is_bonus in module_rows if is_bonus} + while True: + inherited = { + pk for pk, parent_id, _is_bonus in module_rows if parent_id in bonus_module_ids + } + added = inherited - bonus_module_ids + if not added: + break + bonus_module_ids.update(added) + return list( Unit.objects.filter(module__course=self) .exclude(kind=UNIT_KIND_CHECKLIST_ITEM) .exclude(is_bonus=True) - .exclude(module__is_bonus=True) - .exclude(module__parent__is_bonus=True) + .exclude(module_id__in=bonus_module_ids) ) def total_units(self): - return self._countable_units().count() + return len(self._countable_units()) def completed_units(self, user): if user is None or not user.is_authenticated: @@ -200,8 +211,8 @@ def get_syllabus(self): A cohort's effective modules are its :class:`CohortModule` placements when it has any, otherwise the course's full top-level module tree (see - :meth:`Cohort.effective_modules`) -- computed here in two queries total rather than - one query per cohort. + :meth:`Cohort.effective_modules`). The shared persisted tree projection is built + once and sliced to each cohort's selected top-level modules. """ cohorts = list(self.cohorts.order_by("start_date", "pk")) @@ -221,8 +232,15 @@ def get_syllabus(self): ) for placement in placements: placements_by_cohort.setdefault(placement.cohort_id, []).append(placement.module) + from community_base.curriculum.services import get_curriculum_tree + + full_tree = get_curriculum_tree(self) + tree_by_module_id = {node.module.pk: node for node in full_tree} for cohort in cohorts: cohort.syllabus_modules = placements_by_cohort.get(cohort.pk) or default_modules + cohort.syllabus_tree = tuple( + tree_by_module_id[module.pk] for module in cohort.syllabus_modules + ) return cohorts def get_next_unit_for(self, user): @@ -331,10 +349,8 @@ def effective_modules(self): class Module(SourceProvenanceMixin, models.Model): """An ordered module of a course. A submodule is a module with ``parent`` set. - A module holds either child modules or direct units, never both (enforced in - :meth:`clean`, not a database constraint, because the rule spans two related - tables -- ``children`` and ``units`` -- which a ``CheckConstraint`` cannot express). - Nesting is capped at two module levels: a submodule cannot itself have children. + A module's direct units and child modules share the same physical sibling + order, so a node may hold both and modules may nest to any repository depth. """ course = models.ForeignKey(Course, on_delete=models.CASCADE, related_name="modules") @@ -344,7 +360,7 @@ class Module(SourceProvenanceMixin, models.Model): blank=True, on_delete=models.CASCADE, related_name="children", - help_text="Set to make this module a submodule of another. Maximum two levels.", + help_text="Set to make this module a child of another module.", ) slug = models.SlugField(max_length=300, default="") title = models.CharField(max_length=300) @@ -407,33 +423,29 @@ def clean(self): if self.pk is not None and self.parent_id == self.pk: errors["parent"] = "A module cannot be its own parent." elif self.parent is not None: - if self.parent.parent_id is not None: - errors["parent"] = "A submodule cannot itself have children (max two levels)." if self.course_id and self.parent.course_id != self.course_id: errors["parent"] = "A parent module must belong to the same course." - if self.parent.units.exists(): - errors["parent"] = ( - f"Module {self.parent.title!r} already has direct units; " - "it cannot also have child modules." - ) - if self.pk is not None: - has_children = self.children.exists() - has_units = self.units.exists() - if has_children and has_units: - errors["parent"] = ( - f"Module {self.title!r} has both child modules and direct units; " - "it must have only one." - ) + ancestor = self.parent + seen_ancestors = set() + while ancestor is not None and ancestor.pk not in seen_ancestors: + if self.pk is not None and ancestor.pk == self.pk: + errors["parent"] = "A module cannot be nested beneath itself." + break + seen_ancestors.add(ancestor.pk) + ancestor = ancestor.parent if errors: raise ValidationError(errors) @property def effective_is_bonus(self) -> bool: - """Bonus cascades from an ancestor: a bonus week's submodules are bonus too.""" + """Bonus cascades from any ancestor, however deep the module is.""" - if self.is_bonus: - return True - return bool(self.parent_id and self.parent.is_bonus) + module = self + while module is not None: + if module.is_bonus: + return True + module = module.parent if module.parent_id else None + return False class CohortModule(models.Model): @@ -584,18 +596,6 @@ def set_rendered_html(self, rendered_html: str) -> None: self.body_html_source = BODY_HTML_SITE self.body_html = rendered_html - def clean(self): - super().clean() - if self.module_id and self.module.children.exists(): - raise ValidationError( - { - "module": ( - f"Module {self.module.title!r} has child modules; " - "it cannot also have direct units." - ) - } - ) - @property def course(self): return self.module.course @@ -621,15 +621,15 @@ def effective_is_bonus(self) -> bool: @property def effective_available_after_days(self): - """Resolve the drip offset: unit override, its module's, then its parent module's.""" + """Resolve the nearest drip offset: unit override then module ancestors.""" if self.available_after_days is not None: return self.available_after_days module = self.module - if module.available_after_days is not None: - return module.available_after_days - if module.parent_id: - return module.parent.available_after_days + while module is not None: + if module.available_after_days is not None: + return module.available_after_days + module = module.parent if module.parent_id else None return None diff --git a/community_base/curriculum/parsers.py b/community_base/curriculum/parsers.py index bc3c06e1..cdb51696 100644 --- a/community_base/curriculum/parsers.py +++ b/community_base/curriculum/parsers.py @@ -36,6 +36,7 @@ from __future__ import annotations import datetime as dt +import hashlib from collections.abc import Iterable, Mapping from pathlib import Path, PurePosixPath from typing import Any @@ -127,6 +128,7 @@ def parse_course_tree( source: Any, *, path: str = ".", + ignore: Iterable[str] = (), project_specs: Iterable[Mapping[str, Any]] = (), ) -> CourseTreeGraph: """Parse one physical module/unit tree without taking ownership of its header. @@ -142,9 +144,13 @@ def parse_course_tree( ``project_specs`` lets an adapter pass project fields it owns through the generic source-relative module resolver without passing unrelated header metadata into the package parser. + + ``ignore`` is an explicit adapter boundary for legacy source headers that + owned ignore globs before ``content.yaml``. Patterns are relative to the + selected course root and hide files before any module/unit parsing. """ - result, collection, course_manifest_path = _read_course_tree(source, path) + result, collection, course_manifest_path = _read_course_tree(source, path, ignore) by_parent: dict[str | None, list[ParsedDocument]] = {} for document in result.documents: by_parent.setdefault(document.raw.parent, []).append(document) @@ -165,7 +171,9 @@ def parse_course_tree( ) -def _read_course_tree(source: Any, path: str) -> tuple[ReadResult, Collection, str]: +def _read_course_tree( + source: Any, path: str, ignore: Iterable[str] +) -> tuple[ReadResult, Collection, str]: """Read the course layout under ``path`` without a repository manifest.""" if isinstance(source, str | Path): @@ -181,7 +189,11 @@ def _read_course_tree(source: Any, path: str) -> tuple[ReadResult, Collection, s raise CurriculumParseError(f"{root}: course tree source is not a directory") collection_path = _course_tree_relative_path(source, path) content_checkout = checkout if checkout is not None and hasattr(checkout, "read_text") else None - repository = _read_repository(root, (), content_checkout) + patterns = tuple( + f"{collection_path}/{pattern.lstrip('/')}" if collection_path else pattern + for pattern in ignore + ) + repository = _read_repository(root, patterns, content_checkout) node = _node_at(repository.tree, collection_path) if node is None: raise CurriculumParseError( @@ -395,13 +407,23 @@ def _module_graph( overview=_overview(result, document), syllabus_section=values.get("syllabus_section") or "", sort_order=document.sort_order, - is_bonus=bool(values.get("is_bonus")), + is_bonus=_module_bonus(document), available_after_days=values.get("available_after_days"), has_order=_has_order(document), items=tuple(items), ) +def _module_bonus(document: ParsedDocument) -> bool: + """Map the source's ``bonus`` spelling onto the shared ``is_bonus`` flag.""" + + if "is_bonus" in document.data and "bonus" in document.data: + raise CurriculumParseError( + f"{document.raw.path}: declare either is_bonus or bonus, not both" + ) + return bool(document.values.get("is_bonus") or document.values.get("bonus")) + + def _overview(result: ReadResult, document: ParsedDocument) -> str: """A module's `README.md`, the one place section 3.2 reads one for a course.""" @@ -423,6 +445,7 @@ def _unit_graph(document: ParsedDocument, inherited: int | None) -> UnitGraph: title=document.title, source_path=document.raw.path, body=body, + content_hash=_unit_content_hash(body), video_url=values.get("video_url") or "", timestamps=tuple(values.get("timestamps") or ()), required_level=_declared_level(document, inherited), @@ -473,7 +496,8 @@ def _homework_unit_graph(document: ParsedDocument, inherited: int | None) -> Uni slug=document.slug, title=document.title, source_path=document.raw.path, - body=document.body, + homework=document.body, + content_hash=_unit_content_hash(document.body), required_level=_declared_level(document, inherited), sort_order=document.sort_order, kind="homework", @@ -485,6 +509,10 @@ def _homework_unit_graph(document: ParsedDocument, inherited: int | None) -> Uni ) +def _unit_content_hash(body: str) -> str: + return hashlib.md5(body.encode("utf-8")).hexdigest() if body else "" + + def _homework_form(values: Mapping[str, Any]) -> HomeworkFormGraph: return HomeworkFormGraph( homework_url=values.get("homework_url"), diff --git a/community_base/curriculum/services.py b/community_base/curriculum/services.py index e051db04..927ca72c 100644 --- a/community_base/curriculum/services.py +++ b/community_base/curriculum/services.py @@ -1,9 +1,10 @@ """Domain services for enrollment, progress and cohort drip scheduling.""" +from __future__ import annotations + import datetime from dataclasses import dataclass -from django.db.models import Prefetch from django.utils import timezone from community_base.curriculum.models import ( @@ -125,6 +126,136 @@ def completed_unit_ids(user, units) -> set[int]: ) +@dataclass(frozen=True, slots=True) +class UnitProjection: + """A persisted unit at its source-derived position in the physical tree.""" + + unit: Unit + path: str + module_path: str + depth: int + kind: str = "unit" + + +@dataclass(frozen=True, slots=True) +class ModuleProjection: + """A persisted module with mixed, ordered module/unit child projections. + + ``descendant_unit_count`` includes direct units as well as units below child + modules. ``descendant_module_count`` counts child modules, not this module. + Child module items are recursively projected ModuleProjection objects. + """ + + module: Module + items: tuple[ModuleProjection | UnitProjection, ...] + path: str + depth: int + direct_unit_count: int + descendant_unit_count: int + descendant_module_count: int + kind: str = "module" + + @property + def level(self) -> int: + """The human-facing level, with top-level modules at level one.""" + + return self.depth + 1 + + @property + def all_units(self) -> tuple[UnitProjection, ...]: + """Return every unit below this module in reading order.""" + + return tuple( + nested + for item in self.items + for nested in (item.all_units if isinstance(item, ModuleProjection) else (item,)) + ) + + +def get_curriculum_tree( + course: Course, + modules=None, +) -> tuple[ModuleProjection, ...]: + """Project a course's physical module tree with every mixed level preserved. + + Pass ``modules`` to project a cohort's already ordered top-level selection; + child modules and direct units remain in the shared source order. This + projection contains learner-facing curriculum fields only and never carries + homework scoring metadata. + """ + + all_modules = list(Module.objects.filter(course=course).order_by("sort_order", "pk")) + all_units = list( + Unit.objects.filter(module__course=course) + .select_related("module") + .order_by("sort_order", "pk") + ) + modules_by_parent: dict[int | None, list[Module]] = {} + modules_by_id: dict[int, Module] = {} + for module in all_modules: + modules_by_parent.setdefault(module.parent_id, []).append(module) + modules_by_id[module.pk] = module + units_by_module: dict[int, list[Unit]] = {} + for unit in all_units: + units_by_module.setdefault(unit.module_id, []).append(unit) + + def item_key(item): + # Unit and Module tables have independent primary-key sequences. The + # type tag closes the rare tie while normal source siblings use one + # unique sort_order across both types. + return (item.sort_order, item.pk, 0 if isinstance(item, Unit) else 1) + + def project(module: Module, parent_path: str, depth: int) -> ModuleProjection: + path = f"{parent_path}/{module.slug}" if parent_path else module.slug + direct_units = units_by_module.get(module.pk, []) + child_modules = modules_by_parent.get(module.pk, []) + projected_children = {child.pk: project(child, path, depth + 1) for child in child_modules} + physical_items = [*direct_units, *child_modules] + physical_items.sort(key=item_key) + items: list[ModuleProjection | UnitProjection] = [] + descendant_unit_count = 0 + descendant_module_count = len(child_modules) + for item in physical_items: + if isinstance(item, Unit): + items.append( + UnitProjection( + unit=item, + path=f"{path}/{item.slug}", + module_path=path, + depth=depth, + ) + ) + descendant_unit_count += 1 + else: + child = projected_children[item.pk] + items.append(child) + descendant_unit_count += child.descendant_unit_count + descendant_module_count += child.descendant_module_count + return ModuleProjection( + module=module, + items=tuple(items), + path=path, + depth=depth, + direct_unit_count=len(direct_units), + descendant_unit_count=descendant_unit_count, + descendant_module_count=descendant_module_count, + ) + + selected_modules = list(modules) if modules is not None else modules_by_parent.get(None, []) + + def parent_context(module: Module) -> tuple[str, int]: + ancestors = [] + parent_id = module.parent_id + while parent_id is not None: + parent = modules_by_id[parent_id] + ancestors.append(parent.slug) + parent_id = parent.parent_id + ancestors.reverse() + return "/".join(ancestors), len(ancestors) + + return tuple(project(module, *parent_context(module)) for module in selected_modules) + + @dataclass(frozen=True) class DripDecision: """Drip-schedule decision after tier/unit access has been granted.""" @@ -164,39 +295,9 @@ def decide_unit_drip( def get_all_units_ordered(course: Course) -> list[Unit]: - """Return every unit of the course in depth-first reading order. - - For each top-level module, in ``sort_order``: if it has submodules, each submodule's - units in order; otherwise the module's own units directly -- a module holds either - children or units, never both (community-base#252). ``id`` is an explicit tiebreaker - after ``sort_order``, which is not unique. - """ + """Return every unit in depth-first, mixed sibling source order.""" - top_modules = list( - Module.objects.filter(course=course, parent__isnull=True) - .prefetch_related( - Prefetch("children", queryset=Module.objects.order_by("sort_order", "pk")), - Prefetch("units", queryset=Unit.objects.order_by("sort_order", "pk")), - ) - .order_by("sort_order", "pk") - ) - child_ids = [child.pk for module in top_modules for child in module.children.all()] - units_by_module: dict[int, list[Unit]] = {} - if child_ids: - for unit in Unit.objects.filter(module_id__in=child_ids).order_by( - "module_id", "sort_order", "pk" - ): - units_by_module.setdefault(unit.module_id, []).append(unit) - - ordered: list[Unit] = [] - for module in top_modules: - children = list(module.children.all()) - if children: - for child in children: - ordered.extend(units_by_module.get(child.pk, [])) - else: - ordered.extend(module.units.all()) - return ordered + return [item.unit for module in get_curriculum_tree(course) for item in module.all_units] def get_next_unit(course: Course, current_unit: Unit): diff --git a/community_base/curriculum/source.py b/community_base/curriculum/source.py index e7e3f3a7..362684d3 100644 --- a/community_base/curriculum/source.py +++ b/community_base/curriculum/source.py @@ -51,6 +51,7 @@ class UnitGraph: is_bonus: bool = False available_after_days: int | None = None body_source_path: str | None = None + content_hash: str | None = None has_order: bool = False homework_unit: HomeworkUnitGraph | None = None @@ -222,11 +223,11 @@ class CurriculumParseError(ValueError): """A repository layout does not satisfy the curriculum source contract.""" -def validate_module_tree(modules: tuple[ModuleGraph, ...], *, where: str, depth: int = 1) -> None: +def validate_module_tree(modules: tuple[ModuleGraph, ...], *, where: str) -> None: """Validate a course's module tree once, for the one course parser. Enforces unique sibling slugs and orders across the mixed module/unit sequence, - explicit ordering for every sibling, and the existing two-level module bound. + and explicit ordering for every sibling at every physical level. """ seen_module_slugs: set[str] = set() @@ -268,11 +269,7 @@ def validate_module_tree(modules: tuple[ModuleGraph, ...], *, where: str, depth: validate_homework_unit(sibling.homework_unit, where=sibling_where) children = module.children if children: - if depth >= 2: - raise CurriculumParseError( - f"{module_where}: exceeds the maximum module depth of two levels" - ) - validate_module_tree(children, where=module_where, depth=depth + 1) + validate_module_tree(children, where=module_where) def validate_homework_unit(homework: HomeworkUnitGraph, *, where: str) -> None: diff --git a/community_base/curriculum/templates/curriculum/_syllabus_item.html b/community_base/curriculum/templates/curriculum/_syllabus_item.html new file mode 100644 index 00000000..81cab60b --- /dev/null +++ b/community_base/curriculum/templates/curriculum/_syllabus_item.html @@ -0,0 +1,32 @@ +
  • + {% if item.kind == "module" %} +

    + {% if item.depth %} + {{ item.module.title }} + {% else %} + {{ item.module.title }} + {% endif %} +

    + {% if item.items %} +
      + {% for child in item.items %} + {% include "curriculum/_syllabus_item.html" with item=child course=course cohort=cohort has_access=has_access completed_unit_ids=completed_unit_ids %} + {% endfor %} +
    + {% endif %} + {% else %} + {% with unit=item.unit %} + {% if has_access or unit.is_preview %} + {% if item.depth %} + {{ unit.title }} + {% else %} + {{ unit.title }} + {% endif %} + {% else %} + {{ unit.title }} + {% endif %} + {% if unit.is_preview %}Preview{% endif %} + {% if unit.pk in completed_unit_ids %}Completed{% endif %} + {% endwith %} + {% endif %} +
  • diff --git a/community_base/curriculum/templates/curriculum/course_detail.html b/community_base/curriculum/templates/curriculum/course_detail.html index fee8597c..59beee2b 100644 --- a/community_base/curriculum/templates/curriculum/course_detail.html +++ b/community_base/curriculum/templates/curriculum/course_detail.html @@ -23,7 +23,13 @@

    {{ course.title }}

    {% csrf_token %} - {% if next_unit and default_cohort %}Continue{% endif %} + {% if next_unit and default_cohort %} + {% if next_unit.module_depth %} + Continue + {% else %} + Continue + {% endif %} + {% endif %} {% elif has_access %}
    {% csrf_token %} @@ -55,19 +61,9 @@

    {{ cohort.title }}

    {% elif cohort.registration_url %} Register {% endif %} - {% for module in cohort.syllabus_modules %} -

    {{ module.title }}

    + {% for module in cohort.syllabus_tree %}
      - {% for unit in module.units.all %} -
    • - {% if has_access or unit.is_preview %} - {{ unit.title }} - {% else %} - {{ unit.title }} - {% endif %} - {% if unit.is_preview %}Preview{% endif %} -
    • - {% endfor %} + {% include "curriculum/_syllabus_item.html" with item=module course=course cohort=cohort has_access=has_access %}
    {% endfor %} diff --git a/community_base/curriculum/templates/curriculum/module_overview.html b/community_base/curriculum/templates/curriculum/module_overview.html index 2fdd3966..2d888acf 100644 --- a/community_base/curriculum/templates/curriculum/module_overview.html +++ b/community_base/curriculum/templates/curriculum/module_overview.html @@ -10,15 +10,8 @@

    {{ module.title }}

    {% if module.overview_html %}
    {{ module.overview_html|safe }}
    {% endif %}
      - {% for unit in units %} -
    1. - {% if has_access or unit.is_preview %} - {{ unit.title }} - {% else %} - {{ unit.title }} - {% endif %} - {% if unit.pk in completed_unit_ids %}Completed{% endif %} -
    2. + {% for item in module_projection.items %} + {% include "curriculum/_syllabus_item.html" with item=item course=course cohort=cohort has_access=has_access completed_unit_ids=completed_unit_ids %} {% empty %}
    3. No lessons in this module yet.
    4. {% endfor %} diff --git a/community_base/curriculum/templates/curriculum/unit_detail.html b/community_base/curriculum/templates/curriculum/unit_detail.html index 42c811e9..ea24d499 100644 --- a/community_base/curriculum/templates/curriculum/unit_detail.html +++ b/community_base/curriculum/templates/curriculum/unit_detail.html @@ -6,7 +6,11 @@

      {{ course.title }} · - {{ module.title }} + {% if module_depth %} + {{ module.title }} + {% else %} + {{ module.title }} + {% endif %}

      {{ unit.title }}

      @@ -57,15 +61,28 @@

      {{ homework.title }}

      {% endif %}