Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,7 @@ For a single course with no grade items, human output says

---

### `lighthouse submit -f FILE COURSE_ID FOLDER_ID [--yes] [--json]`
### `lighthouse submit -f FILE COURSE_ID FOLDER_ID [--yes] [--dry-run] [--json]`

Submit a file to a D2L dropbox folder.

Expand All @@ -909,6 +909,7 @@ courses` affect local state only.
|------|-------------|
| `-f`, `--file` | Path to the file to submit (required) |
| `--yes` | Skip confirmation prompt; required in non-TTY mode |
| `--dry-run` | Resolve the course and folder read-only and print the destination; never reads the file body or uploads, and needs no `--yes` |
| `--json` | Output structured JSON result |

**API call:** `POST /d2l/api/le/1.93/{orgId}/dropbox/folders/{folderId}/submissions/mysubmissions/`
Expand Down Expand Up @@ -952,6 +953,22 @@ Submitted successfully. Submission ID: 5001
}
```

**JSON output (`--json`, `--dry-run`):**
```json
{
"dry_run": true,
"course_id": 1001,
"course_name": "Introduction to CS",
"folder_id": 101,
"folder_name": "Homework 1",
"folder_verified": true,
"file": {"name": "homework.pdf", "size_bytes": 24576}
}
```

`folder_verified` is `false`, with a `warning`, when the folder's name
could not be read; check the folder ID before submitting.

---

### `lighthouse announcements [COURSE_ID] [--json]`
Expand Down
13 changes: 11 additions & 2 deletions lighthouse_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,10 @@ def assignments(course_id: str | None, json_output: bool) -> None:
@click.argument("folder_id")
@click.option("-f", "--file", "file_path", required=True, help="Path to the file to submit.")
@click.option("--yes", "yes", is_flag=True, default=False, help="Skip confirmation prompt and submit immediately.")
@click.option("--dry-run", "dry_run", is_flag=True, default=False,
help="Resolve and print the destination without reading or uploading the file.")
@click.option("--json", "json_output", is_flag=True, help="Output this command's JSON result.")
def submit(course_id: str, folder_id: str, file_path: str, yes: bool, json_output: bool) -> None:
def submit(course_id: str, folder_id: str, file_path: str, yes: bool, dry_run: bool, json_output: bool) -> None:
"""REMOTE WRITE: submit a file to a D2L dropbox folder.

COURSE_ID is the course identifier (numeric OrgUnitId or name substring).
Expand All @@ -518,18 +520,25 @@ def submit(course_id: str, folder_id: str, file_path: str, yes: bool, json_outpu
Example:
lighthouse submit "signals" "Assignment 1" --file solution.pdf
lighthouse submit signals "Assignment 1" --file solution.pdf --yes
lighthouse submit signals "Assignment 1" --file solution.pdf --dry-run --json

This command changes remote LMS state. The command prompts
for confirmation before submitting (course name, folder name, file path).
Use --yes to skip the prompt (required for agent/automation use).
--dry-run only resolves the course and folder (read-only) and prints the
destination; it needs no --yes and never uploads.
Comment thread
rabesss marked this conversation as resolved.

On success, prints a JSON object with submission_id, folder_id, folder_name,
course_id, course_name, file info, and submitted_at timestamp.
course_id, course_name, file info, and submitted_at timestamp. A --dry-run
Comment thread
rabesss marked this conversation as resolved.
instead prints dry_run, course_id, course_name, folder_id, folder_name,
folder_verified and file info (plus a warning when the folder name
could not be read).
"""
raise SystemExit(cmd_submit(
course_id=course_id,
folder_id=folder_id,
file_path=file_path,
yes=yes,
json_output=json_output,
dry_run=dry_run,
))
58 changes: 50 additions & 8 deletions lighthouse_cli/submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
_DEFAULT_FOLDER_NAME = "Unknown folder"
_DEFAULT_FILE_NAME = "Unknown file"
_CLIENT_INIT_ERROR = "Could not initialize Lighthouse client."
_DRY_RUN_UNVERIFIED_WARNING = (
"The folder name could not be read; check the folder ID. No submission was sent."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Warning says the folder name "could not be read", but the unverified path now also fires when the name was read yet unusable for display

Since 40b827f, _get_folder_name returns (_DEFAULT_FOLDER_NAME, False) whenever safe_display_text rejects Name — including a successfully read name that only fails the display projection: longer than _MAX_DISPLAY_NAME_LENGTH (256), whitespace-only, or caught by the secret/object heuristics. The new {"Name": "x" * 300} test case pins this. For such input the emitted warning "The folder name could not be read; check the folder ID" is misleading: the name was read, only its display projection was rejected, and folder resolution itself succeeded, so "check the folder ID" sends the user down the wrong path. Consider wording that covers both cases. The same "could not be read" phrasing also appears in the README note (README.md:969-970) and the submit docstring (cli.py:534), and the parametrized test name asserts "could not be read" for the over-long case.

Suggested change
"The folder name could not be read; check the folder ID. No submission was sent."
"The folder name could not be confirmed; check the folder ID. No submission was sent."

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

)


def cmd_submit(
Expand All @@ -31,6 +34,7 @@ def cmd_submit(
file_path: str,
yes: bool = False,
json_output: bool = False,
dry_run: bool = False,
) -> int:
"""Submit a file to a dropbox folder.

Expand All @@ -45,6 +49,9 @@ def cmd_submit(
folder_name, course_id, course_name, file, submitted_at).

Non-interactive / agent-friendly: --yes + --json = only JSON on stdout.

``dry_run`` resolves the same destination with a read-only client and
prints the plan without reading the file body or uploading anything.
"""
# Validate the local input before constructing a client or resolving any
# remote identifiers. A declined submission should not read the file body,
Expand All @@ -64,14 +71,14 @@ def cmd_submit(

# Keep the explicit confirmation requirement for non-interactive callers.
# This check happens after local validation, but before any API work.
if not yes and not sys.stdin.isatty():
if not dry_run and not yes and not sys.stdin.isatty():
return _submit_error(
"Refusing to submit without --yes in non-interactive mode. Use --yes flag to confirm.",
json_output,
)

try:
client = LighthouseClient()
client = LighthouseClient(read_only_auth=dry_run)
except Exception:
return _submit_error(_CLIENT_INIT_ERROR, json_output)

Expand All @@ -82,7 +89,12 @@ def cmd_submit(
except Exception as e:
return _submit_error(e, json_output)

folder_name = _get_folder_name(client, org_id, folder_id_int)
folder_name, folder_verified = _get_folder_name(client, org_id, folder_id_int)
if dry_run:
return _submit_dry_run(
org_id, course_name, folder_id_int, folder_name, folder_verified,
file_path_obj, display_filename, json_output,
)

# Confirmation prompt (skip with --yes). JSON-mode prompts must not pollute
# stdout; ``input`` is called without a prompt because input() writes its
Expand Down Expand Up @@ -157,6 +169,35 @@ def cmd_submit(
return 0


def _submit_dry_run(
org_id: int, course_name: str, folder_id: int, folder_name: str, verified: bool,
file_path: Path, display_filename: str, json_output: bool,
) -> int:
"""Report the resolved destination; reads only the file's size."""
try:
file_size = file_path.stat().st_size
except OSError:
return _submit_error("Could not read file.", json_output)
if json_output:
payload: dict[str, object] = {
"dry_run": True,
"course_id": org_id,
"course_name": course_name,
"folder_id": folder_id,
"folder_name": folder_name,
"folder_verified": verified,
"file": {"name": display_filename, "size_bytes": file_size},
}
if not verified:
payload["warning"] = _DRY_RUN_UNVERIFIED_WARNING
_output_json(payload)
else:
print(f"Would submit to '{folder_name}' in '{course_name}'.\n File: {display_filename} ({file_size} bytes)")
if not verified:
print(f"Warning: {_DRY_RUN_UNVERIFIED_WARNING}")
return 0


def _submit_error(message: BaseException | str, json_output: bool) -> int:
"""Emit a safe submit failure without double-formatting its diagnostic.

Expand Down Expand Up @@ -309,15 +350,16 @@ def _positive_folder_id(value: object) -> int | None:
return None


def _get_folder_name(client: LighthouseClient, org_id: int, folder_id: int) -> str:
"""Get the name of a dropbox folder by ID."""
def _get_folder_name(client: LighthouseClient, org_id: int, folder_id: int) -> tuple[str, bool]:
"""Get a dropbox folder's display name and whether it was read and usable."""
try:
detail = client.get_dropbox_folder_detail(org_id, folder_id)
except Exception:
return _DEFAULT_FOLDER_NAME
return _DEFAULT_FOLDER_NAME, False
if not isinstance(detail, dict):
return _DEFAULT_FOLDER_NAME
return _safe_display_name(detail.get("Name"), _DEFAULT_FOLDER_NAME)
return _DEFAULT_FOLDER_NAME, False
name = _safe_display_name(detail.get("Name"), "")
return (name, True) if name else (_DEFAULT_FOLDER_NAME, False)


def _safe_display_name(value: object, fallback: str) -> str:
Expand Down
114 changes: 113 additions & 1 deletion tests/test_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def test_submit_client_constructor_failure_is_safe_json(
}
assert result.stdout.count('"error"') == 1
assert "CLIENT_SECRET_SENTINEL" not in result.output
mock_client_cls.assert_called_once_with()
mock_client_cls.assert_called_once_with(read_only_auth=False)
read_bytes_mock.assert_not_called()

def test_submit_success_with_yes_flag_json_output(
Expand Down Expand Up @@ -1890,3 +1890,115 @@ def test_multipart_boundary_is_unique(self, sample_submission_response: dict) ->
assert len(captured) == 2
boundaries = [request["headers"]["Content-Type"] for request in captured]
assert boundaries[0] != boundaries[1]


class TestSubmitDryRun:
"""`submit --dry-run` resolves the destination read-only and never uploads."""

@staticmethod
def _client(detail: object = None) -> MagicMock:
client = MagicMock()
client.get_courses.return_value = [{"OrgUnitId": 44347, "Name": "Signals & Systems"}]
client.get_dropbox_folders.return_value = [{"Id": 789, "Name": "Assignment 1 - Signals"}]
client.get_dropbox_folder_detail.return_value = (
{"Name": "Assignment 1 - Signals"} if detail is None else detail
)
return client

def test_dry_run_reports_destination_without_reading_or_uploading(
self, cli_runner: CliRunner, temp_pdf_file: Path,
) -> None:
from lighthouse_cli.cli import cli

client = self._client()
with patch("lighthouse_cli.submit.LighthouseClient", return_value=client) as client_cls, \
patch.object(Path, "read_bytes", side_effect=AssertionError("must not read the file")):
result = cli_runner.invoke(
cli, ["submit", "44347", "789", "--file", str(temp_pdf_file), "--dry-run", "--json"],
)
assert result.exit_code == 0, result.output
data = json_module.loads(result.stdout)
assert data == {
"dry_run": True, "course_id": 44347, "course_name": "Signals & Systems",
"folder_id": 789, "folder_name": "Assignment 1 - Signals", "folder_verified": True,
"file": {"name": "test.pdf", "size_bytes": temp_pdf_file.stat().st_size},
}
client_cls.assert_called_once_with(read_only_auth=True)
client.submit_file.assert_not_called()

def test_dry_run_needs_no_yes_in_non_interactive_mode(
self, cli_runner: CliRunner, temp_pdf_file: Path,
) -> None:
from lighthouse_cli.cli import cli

client = self._client()
with patch("lighthouse_cli.submit.LighthouseClient", return_value=client):
result = cli_runner.invoke(
cli, ["submit", "44347", "789", "--file", str(temp_pdf_file), "--dry-run"],
)
assert result.exit_code == 0, result.output
assert "Would submit to 'Assignment 1 - Signals' in 'Signals & Systems'" in result.output
client.submit_file.assert_not_called()

@pytest.mark.parametrize("detail", [RuntimeError("lookup failed"), {"Name": ""}, {}, {"Name": "x" * 300}])
def test_dry_run_flags_a_folder_whose_name_could_not_be_read(
self, cli_runner: CliRunner, temp_pdf_file: Path, detail: object,
) -> None:
from lighthouse_cli.cli import cli

client = self._client()
if isinstance(detail, Exception):
client.get_dropbox_folder_detail.side_effect = detail
else:
client.get_dropbox_folder_detail.return_value = detail
with patch("lighthouse_cli.submit.LighthouseClient", return_value=client):
result = cli_runner.invoke(
cli, ["submit", "44347", "789", "--file", str(temp_pdf_file), "--dry-run", "--json"],
)
assert result.exit_code == 0
data = json_module.loads(result.stdout)
assert data["folder_verified"] is False
assert data["folder_name"] == "Unknown folder"
assert "No submission was sent" in data["warning"]
client.submit_file.assert_not_called()

def test_dry_run_verifies_a_folder_literally_named_like_the_fallback(
self, cli_runner: CliRunner, temp_pdf_file: Path,
) -> None:
from lighthouse_cli.cli import cli

client = self._client(detail={"Name": "Unknown folder"})
with patch("lighthouse_cli.submit.LighthouseClient", return_value=client):
result = cli_runner.invoke(
cli, ["submit", "44347", "789", "--file", str(temp_pdf_file), "--dry-run", "--json"],
)
assert result.exit_code == 0, result.output
data = json_module.loads(result.stdout)
assert data["folder_verified"] is True
assert "warning" not in data

def test_dry_run_still_reports_resolution_errors(
self, cli_runner: CliRunner, temp_pdf_file: Path,
) -> None:
from lighthouse_cli.cli import cli

client = self._client()
client.get_courses.return_value = []
with patch("lighthouse_cli.submit.LighthouseClient", return_value=client):
result = cli_runner.invoke(
cli, ["submit", "nope", "789", "--file", str(temp_pdf_file), "--dry-run", "--json"],
)
assert result.exit_code == 1
assert json_module.loads(result.stdout)["error"]
client.submit_file.assert_not_called()

def test_real_submit_still_requires_yes_when_non_interactive(
self, cli_runner: CliRunner, temp_pdf_file: Path,
) -> None:
from lighthouse_cli.cli import cli

with patch("lighthouse_cli.submit.LighthouseClient") as client_cls:
result = cli_runner.invoke(cli, ["submit", "44347", "789", "--file", str(temp_pdf_file)])
assert result.exit_code == 1
assert "--yes" in result.output
client_cls.assert_not_called()
Loading