From df6178febb76dacdc0e0064f82f1ede555d6381a Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 21 Mar 2026 02:04:05 +0000
Subject: [PATCH 1/4] Initial plan
From 9e518327b7cb6faff960e09e38ae95816fe738d3 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 21 Mar 2026 02:10:27 +0000
Subject: [PATCH 2/4] fix: preserve Moodle add-form defaults for module
creation
Co-authored-by: erseco <1876752+erseco@users.noreply.github.com>
Agent-Logs-Url: https://github.com/erseco/python-moodle/sessions/b581ebb4-5f74-4506-9f2a-ab90ef9afa30
---
src/py_moodle/module.py | 136 +++++++++++++++++-----------
tests/unit/test_module_addition.py | 139 +++++++++++++++++++++++++++++
2 files changed, 221 insertions(+), 54 deletions(-)
create mode 100644 tests/unit/test_module_addition.py
diff --git a/src/py_moodle/module.py b/src/py_moodle/module.py
index b8726dc..48c6d36 100644
--- a/src/py_moodle/module.py
+++ b/src/py_moodle/module.py
@@ -62,6 +62,65 @@ def _get_base_modedit_payload(
}
+def _extract_modedit_form_data(form: BeautifulSoup) -> Dict[str, Any]:
+ """Extract the current values from a Moodle modedit form."""
+ form_data: Dict[str, Any] = {}
+
+ for field in form.find_all(["input", "textarea", "select"]):
+ name = field.get("name")
+ if not name or field.get("type") in ["submit", "button", "reset", "file"]:
+ continue
+
+ if field.name == "textarea":
+ form_data[name] = field.text or ""
+ elif field.name == "select":
+ selected_option = field.find("option", selected=True)
+ if selected_option and selected_option.has_attr("value"):
+ form_data[name] = selected_option["value"]
+ else:
+ first_option = field.find("option", value=True)
+ if first_option:
+ form_data[name] = first_option["value"]
+ elif field.get("type") in ("checkbox", "radio"):
+ if field.has_attr("checked"):
+ form_data[name] = field.get("value", "1")
+ else:
+ form_data[name] = field.get("value", "")
+
+ return form_data
+
+
+def _load_modedit_form_data(
+ session: requests.Session,
+ url: str,
+ action_description: str,
+) -> tuple[Dict[str, Any], Any]:
+ """Load a Moodle modedit form page and return its current form values."""
+ compatibility = get_session_compatibility(session)
+
+ try:
+ resp = session.get(url)
+ resp.raise_for_status()
+ soup = BeautifulSoup(resp.text, "lxml")
+ except requests.RequestException as e:
+ raise MoodleModuleError(
+ f"Failed to load module form while {action_description}: {e}"
+ )
+
+ form = compatibility.find_modedit_form(soup)
+ if form is None:
+ error_message = compatibility.extract_error_message(soup)
+ if error_message:
+ raise MoodleModuleError(
+ f"Failed to load module form while {action_description}: {error_message}"
+ )
+ raise MoodleModuleError(
+ f"Could not find the module form while {action_description}."
+ )
+
+ return _extract_modedit_form_data(form), compatibility
+
+
# --- Public Generic Functions ---
@@ -111,39 +170,44 @@ def add_generic_module(
except MoodleCourseError as e:
raise MoodleModuleError(f"Failed to get initial section state: {e}")
- # 2. Build the full payload
+ # 2. Load the real add form so Moodle-specific defaults and hidden fields are
+ # preserved when creating the module.
+ add_form_url = (
+ f"{base_url}/course/modedit.php?"
+ f"add={urllib.parse.quote(module_name)}&type=&course={course_id}"
+ f"§ion={section_number}&return=0&sr=-1"
+ )
+ form_payload, compatibility = _load_modedit_form_data(
+ session, add_form_url, f"adding {module_name}"
+ )
+
+ # 3. Build the full payload
base_payload = _get_base_modedit_payload(
course_id, section_number, sesskey, module_name, module_id, mode="add"
)
- full_payload = {**base_payload, **specific_payload}
+ full_payload = {**base_payload, **form_payload, **specific_payload}
- # 3. POST to modedit.php
+ # 4. POST to modedit.php
url = f"{base_url}/course/modedit.php"
headers = {"Content-Type": "application/x-www-form-urlencoded"}
- encoded_payload = urllib.parse.urlencode(full_payload)
- compatibility = get_session_compatibility(session)
+ encoded_payload = urllib.parse.urlencode(full_payload, doseq=True)
resp = session.post(
url, data=encoded_payload, headers=headers, allow_redirects=False
)
# A clear success is a redirect (302 or 303) to the course page.
- if resp.status_code not in [200, 302, 303]:
- # A 200 OK status almost always means a silent failure.
- # Parse the HTML to find Moodle's error message.
+ if resp.status_code not in [302, 303]:
soup = BeautifulSoup(resp.text, "lxml")
error_message = compatibility.extract_error_message(soup)
if error_message:
- # Found a specific error message.
raise MoodleModuleError(
f"Form submission failed. Moodle error: {error_message}"
)
- else:
- # No clear error message, but creation failed.
- raise MoodleModuleError(
- f"Failed to create module. Status: {resp.status_code}. Moodle returned the edit form, indicating a silent failure. Check permissions or required fields."
- )
+ raise MoodleModuleError(
+ f"Failed to create module. Status: {resp.status_code}. Moodle returned the edit form instead of redirecting, so the activity may not have been created correctly."
+ )
- # 4. Get final state and determine the new cmid
+ # 5. Get final state and determine the new cmid
time.sleep(1) # Give Moodle a moment to process the change
try:
course_data_after = get_course_with_sections_and_modules(
@@ -185,45 +249,9 @@ def update_generic_module(
edit_url = f"{base_url}/course/modedit.php?update={cmid}"
# 1. Fetch the edit page to get the current state of the form
- try:
- resp = session.get(edit_url)
- resp.raise_for_status()
- soup = BeautifulSoup(resp.text, "lxml")
- except requests.RequestException as e:
- raise MoodleModuleError(f"Failed to load module edit page for cmid {cmid}: {e}")
-
- # 2. Parse the form and extract all input, textarea, and select fields
- compatibility = get_session_compatibility(session)
- form = compatibility.find_modedit_form(soup)
- if not form:
- raise MoodleModuleError("Could not find the edit form on the page.")
-
- form_data = {}
-
- for field in form.find_all(["input", "textarea", "select"]):
- name = field.get("name")
- # Ignore fields without a name or buttons
- if not name or field.get("type") in ["submit", "button", "reset"]:
- continue
-
- # Logic for different tag/type combinations
- if field.name == "textarea":
- form_data[name] = field.text or ""
- elif field.name == "select":
- selected_option = field.find("option", selected=True)
- if selected_option and selected_option.has_attr("value"):
- form_data[name] = selected_option["value"]
- else:
- # If no option is selected, browsers usually submit the first one.
- first_option = field.find("option", value=True)
- if first_option:
- form_data[name] = first_option["value"]
-
- elif field.get("type") in ("checkbox", "radio"):
- if field.has_attr("checked"):
- form_data[name] = field.get("value", "1")
- else: # Handles text, hidden, password, etc.
- form_data[name] = field.get("value", "")
+ form_data, compatibility = _load_modedit_form_data(
+ session, edit_url, f"updating module {cmid}"
+ )
# 3. Modify the form data with the user's changes
form_data.update(specific_payload)
diff --git a/tests/unit/test_module_addition.py b/tests/unit/test_module_addition.py
new file mode 100644
index 0000000..9b38f01
--- /dev/null
+++ b/tests/unit/test_module_addition.py
@@ -0,0 +1,139 @@
+"""Unit tests for generic module creation helpers."""
+
+from __future__ import annotations
+
+from urllib.parse import parse_qs
+
+import pytest
+
+from py_moodle.module import MoodleModuleError, add_generic_module
+
+
+class _FakeResponse:
+ """Minimal response object for module unit tests."""
+
+ def __init__(self, status_code: int, text: str = ""):
+ self.status_code = status_code
+ self.text = text
+
+ def raise_for_status(self) -> None:
+ """Mimic ``requests.Response.raise_for_status``."""
+ if self.status_code >= 400:
+ raise RuntimeError(f"HTTP {self.status_code}")
+
+
+class _FakeSession:
+ """Minimal session object that records add-module requests."""
+
+ def __init__(self, post_response: _FakeResponse):
+ self.post_response = post_response
+ self.get_calls: list[str] = []
+ self.post_calls: list[dict[str, object]] = []
+
+ def get(self, url: str):
+ """Return a fixed add-form HTML page."""
+ self.get_calls.append(url)
+ return _FakeResponse(
+ 200,
+ text="""
+
+ """,
+ )
+
+ def post(self, url: str, data=None, headers=None, allow_redirects=False):
+ """Record the outgoing POST request."""
+ self.post_calls.append(
+ {
+ "url": url,
+ "data": data,
+ "headers": headers,
+ "allow_redirects": allow_redirects,
+ }
+ )
+ return self.post_response
+
+
+def test_add_generic_module_preserves_form_defaults(monkeypatch):
+ """Module creation should submit the fetched add form defaults."""
+ session = _FakeSession(_FakeResponse(303))
+ course_states = iter(
+ [
+ {"sections": [{"id": 10, "section": 1, "modules": []}]},
+ {"sections": [{"id": 10, "section": 1, "modules": [{"id": 98}]}]},
+ ]
+ )
+
+ monkeypatch.setattr(
+ "py_moodle.module._get_module_id_from_name",
+ lambda *args, **kwargs: 123,
+ )
+ monkeypatch.setattr(
+ "py_moodle.module.get_course_with_sections_and_modules",
+ lambda *args, **kwargs: next(course_states),
+ )
+ monkeypatch.setattr("py_moodle.module.time.sleep", lambda *_args: None)
+
+ new_cmid = add_generic_module(
+ session=session,
+ base_url="https://moodle.example.test",
+ sesskey="sesskey123",
+ module_name="assign",
+ course_id=5,
+ section_id=10,
+ specific_payload={
+ "name": "Essay 1",
+ "activityeditor[text]": "Write an essay.
",
+ },
+ )
+
+ assert new_cmid == 98
+ assert session.get_calls == [
+ "https://moodle.example.test/course/modedit.php?add=assign&type=&course=5§ion=1&return=0&sr=-1"
+ ]
+
+ posted_data = parse_qs(session.post_calls[0]["data"])
+ assert posted_data["gradecat"] == ["7"]
+ assert posted_data["assignsubmission_file_enabled"] == ["1"]
+ assert posted_data["name"] == ["Essay 1"]
+ assert posted_data["activityeditor[text]"] == ["Write an essay.
"]
+
+
+def test_add_generic_module_rejects_non_redirect_responses(monkeypatch):
+ """A 200 response should be treated as a failed module creation."""
+ session = _FakeSession(
+ _FakeResponse(200, 'Validation failed
')
+ )
+
+ monkeypatch.setattr(
+ "py_moodle.module._get_module_id_from_name",
+ lambda *args, **kwargs: 123,
+ )
+ monkeypatch.setattr(
+ "py_moodle.module.get_course_with_sections_and_modules",
+ lambda *args, **kwargs: {"sections": [{"id": 10, "section": 1, "modules": []}]},
+ )
+
+ with pytest.raises(MoodleModuleError, match="Validation failed"):
+ add_generic_module(
+ session=session,
+ base_url="https://moodle.example.test",
+ sesskey="sesskey123",
+ module_name="assign",
+ course_id=5,
+ section_id=10,
+ specific_payload={"name": "Essay 1"},
+ )
From a0637e9817e729ac360eadb90139291e209d317b Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 21 Mar 2026 02:14:25 +0000
Subject: [PATCH 3/4] test: cover safe module add-form submission
Co-authored-by: erseco <1876752+erseco@users.noreply.github.com>
Agent-Logs-Url: https://github.com/erseco/python-moodle/sessions/b581ebb4-5f74-4506-9f2a-ab90ef9afa30
---
src/py_moodle/module.py | 42 ++++++++++++++++++++++++-----
tests/unit/test_module_addition.py | 43 +++++++++++++++++++++++++-----
2 files changed, 72 insertions(+), 13 deletions(-)
diff --git a/src/py_moodle/module.py b/src/py_moodle/module.py
index 48c6d36..afd8ccc 100644
--- a/src/py_moodle/module.py
+++ b/src/py_moodle/module.py
@@ -63,7 +63,16 @@ def _get_base_modedit_payload(
def _extract_modedit_form_data(form: BeautifulSoup) -> Dict[str, Any]:
- """Extract the current values from a Moodle modedit form."""
+ """Extract the current values from a Moodle modedit form.
+
+ Args:
+ form: A parsed Moodle ``modedit.php`` form element.
+
+ Returns:
+ A mapping of form field names to their current values for ``input``,
+ ``textarea``, and ``select`` elements. File inputs are skipped because
+ browsers do not submit their existing values back to Moodle.
+ """
form_data: Dict[str, Any] = {}
for field in form.find_all(["input", "textarea", "select"]):
@@ -74,11 +83,21 @@ def _extract_modedit_form_data(form: BeautifulSoup) -> Dict[str, Any]:
if field.name == "textarea":
form_data[name] = field.text or ""
elif field.name == "select":
- selected_option = field.find("option", selected=True)
- if selected_option and selected_option.has_attr("value"):
- form_data[name] = selected_option["value"]
+ selected_options = [
+ option["value"]
+ for option in field.find_all("option", selected=True)
+ if option.has_attr("value")
+ ]
+ if field.has_attr("multiple"):
+ if selected_options:
+ form_data[name] = selected_options
+ elif selected_options:
+ form_data[name] = selected_options[0]
else:
- first_option = field.find("option", value=True)
+ first_option = next(
+ (option for option in field.find_all("option") if option.has_attr("value")),
+ None,
+ )
if first_option:
form_data[name] = first_option["value"]
elif field.get("type") in ("checkbox", "radio"):
@@ -95,7 +114,17 @@ def _load_modedit_form_data(
url: str,
action_description: str,
) -> tuple[Dict[str, Any], Any]:
- """Load a Moodle modedit form page and return its current form values."""
+ """Load a Moodle modedit form page and return its current form values.
+
+ Args:
+ session: Authenticated Moodle session.
+ url: The ``modedit.php`` URL to fetch.
+ action_description: Human-readable action used in error messages.
+
+ Returns:
+ A tuple containing the extracted form data and the compatibility
+ strategy used to locate the form and parse errors.
+ """
compatibility = get_session_compatibility(session)
try:
@@ -190,6 +219,7 @@ def add_generic_module(
# 4. POST to modedit.php
url = f"{base_url}/course/modedit.php"
headers = {"Content-Type": "application/x-www-form-urlencoded"}
+ # ``doseq=True`` preserves repeated values for multi-select fields.
encoded_payload = urllib.parse.urlencode(full_payload, doseq=True)
resp = session.post(
url, data=encoded_payload, headers=headers, allow_redirects=False
diff --git a/tests/unit/test_module_addition.py b/tests/unit/test_module_addition.py
index 9b38f01..1646712 100644
--- a/tests/unit/test_module_addition.py
+++ b/tests/unit/test_module_addition.py
@@ -6,7 +6,13 @@
import pytest
-from py_moodle.module import MoodleModuleError, add_generic_module
+from bs4 import BeautifulSoup
+
+from py_moodle.module import (
+ MoodleModuleError,
+ _extract_modedit_form_data,
+ add_generic_module,
+)
class _FakeResponse:
@@ -46,7 +52,7 @@ def get(self, url: str):
name="assignsubmission_file_enabled"
value="1"
checked
- >
+ />
@@ -67,8 +73,28 @@ def post(self, url: str, data=None, headers=None, allow_redirects=False):
return self.post_response
-def test_add_generic_module_preserves_form_defaults(monkeypatch):
- """Module creation should submit the fetched add form defaults."""
+def test_extract_modedit_form_data_preserves_multi_select_values():
+ """Multi-select fields should keep all selected values."""
+ soup = BeautifulSoup(
+ """
+
+ """,
+ "lxml",
+ )
+
+ form_data = _extract_modedit_form_data(soup.form)
+
+ assert form_data["tags[]"] == ["alpha", "beta"]
+
+
+def test_add_generic_module_merges_fetched_form_defaults(monkeypatch):
+ """Module creation should merge the fetched add form defaults."""
session = _FakeSession(_FakeResponse(303))
course_states = iter(
[
@@ -112,10 +138,13 @@ def test_add_generic_module_preserves_form_defaults(monkeypatch):
assert posted_data["activityeditor[text]"] == ["Write an essay.
"]
-def test_add_generic_module_rejects_non_redirect_responses(monkeypatch):
- """A 200 response should be treated as a failed module creation."""
+@pytest.mark.parametrize("status_code", [200, 400, 404, 500, 502])
+def test_add_generic_module_rejects_non_redirect_responses(
+ monkeypatch, status_code
+):
+ """Any non-redirect response should be treated as a failed module creation."""
session = _FakeSession(
- _FakeResponse(200, 'Validation failed
')
+ _FakeResponse(status_code, 'Validation failed
')
)
monkeypatch.setattr(
From ceba8788a3404634f03ae01917006d07153abb3c Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 21 Mar 2026 02:28:34 +0000
Subject: [PATCH 4/4] style: apply black formatting to module add tests
Co-authored-by: erseco <1876752+erseco@users.noreply.github.com>
Agent-Logs-Url: https://github.com/erseco/python-moodle/sessions/c778c28f-5b52-4d45-9b74-b91b20604c6b
---
src/py_moodle/module.py | 6 +++++-
tests/unit/test_module_addition.py | 5 +----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/py_moodle/module.py b/src/py_moodle/module.py
index afd8ccc..6ab3868 100644
--- a/src/py_moodle/module.py
+++ b/src/py_moodle/module.py
@@ -95,7 +95,11 @@ def _extract_modedit_form_data(form: BeautifulSoup) -> Dict[str, Any]:
form_data[name] = selected_options[0]
else:
first_option = next(
- (option for option in field.find_all("option") if option.has_attr("value")),
+ (
+ option
+ for option in field.find_all("option")
+ if option.has_attr("value")
+ ),
None,
)
if first_option:
diff --git a/tests/unit/test_module_addition.py b/tests/unit/test_module_addition.py
index 1646712..0db7ef4 100644
--- a/tests/unit/test_module_addition.py
+++ b/tests/unit/test_module_addition.py
@@ -5,7 +5,6 @@
from urllib.parse import parse_qs
import pytest
-
from bs4 import BeautifulSoup
from py_moodle.module import (
@@ -139,9 +138,7 @@ def test_add_generic_module_merges_fetched_form_defaults(monkeypatch):
@pytest.mark.parametrize("status_code", [200, 400, 404, 500, 502])
-def test_add_generic_module_rejects_non_redirect_responses(
- monkeypatch, status_code
-):
+def test_add_generic_module_rejects_non_redirect_responses(monkeypatch, status_code):
"""Any non-redirect response should be treated as a failed module creation."""
session = _FakeSession(
_FakeResponse(status_code, 'Validation failed
')