From c1152a9e551582f26763b73d1d284bd9b32326b4 Mon Sep 17 00:00:00 2001 From: Kasper Hartwich Date: Sat, 26 Sep 2026 06:26:25 +0000 Subject: [PATCH 1/4] feat(messages): add mark-read to mark threads as read --- src/aula/api_client.py | 45 ++++++++++++++++++++++++++++ src/aula/cli.py | 33 ++++++++++++++++++++ tests/test_api_client.py | 65 ++++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 46 ++++++++++++++++++++++++++++ 4 files changed, 189 insertions(+) diff --git a/src/aula/api_client.py b/src/aula/api_client.py index 5d94776..8412b4f 100644 --- a/src/aula/api_client.py +++ b/src/aula/api_client.py @@ -994,6 +994,51 @@ async def get_messages_for_thread(self, thread_id: str, limit: int = 5) -> list[ return messages + async def mark_thread_read(self, thread_id: str) -> bool: + """Mark a message thread as read, up to and including its latest message. + + Aula keeps one read marker per thread: the ID of the last message read. This + finds the newest message and moves the marker there, as the web client does + when a thread is opened. + + Args: + thread_id: The thread to mark, as returned by ``get_message_threads()``. + + Returns: + ``True`` when the marker was set, ``False`` when the thread holds no message + that can be marked. + """ + resp = await self._request_with_version_retry( + "get", + f"{self.api_url}?method=messaging.getMessagesForThread&threadId={thread_id}&page=0&limit=5", + ) + resp.raise_for_status() + raw_messages = get_in(resp.json(), "data.messages", default=[]) + newest = next( + ( + m + for m in raw_messages + if m.get("messageType") + in ("Message", "MessageEdited", "MessageDeleted", "AutoReply") + ), + None, + ) + if newest is None or not newest.get("id"): + return False + + resp = await self._request_with_version_retry( + "post", + f"{self.api_url}?method=messaging.setLastReadMessage", + json={ + "threadId": thread_id, + "messageId": newest["id"], + "commonInboxId": None, + "otpInboxId": None, + }, + ) + resp.raise_for_status() + return True + async def get_calendar_events( self, institution_profile_ids: list[int], start: datetime, end: datetime ) -> list[CalendarEvent]: diff --git a/src/aula/cli.py b/src/aula/cli.py index bf5c4b8..02ae216 100644 --- a/src/aula/cli.py +++ b/src/aula/cli.py @@ -3320,6 +3320,39 @@ def _strip_relation(name: str | None) -> str | None: return name +@cli.command("mark-read") +@click.argument("thread_ids", nargs=-1, required=True) +@click.pass_context +@async_cmd +async def mark_read(ctx, thread_ids): + """Mark message threads as read. + + Writes to Aula: each thread is read up to and including its latest message, as if + it had been opened in the app. Thread IDs are the ones `aula messages` shows. + + Examples: + + aula mark-read 170189345 + + aula mark-read 170924263 170924078 + """ + results: list[dict] = [] + async with await _get_client(ctx) as client: + for thread_id in thread_ids: + try: + marked = await client.mark_thread_read(thread_id) + results.append({"thread_id": thread_id, "marked": marked}) + except Exception as e: + print_error(f"marking thread {thread_id} as read: {e}") + results.append({"thread_id": thread_id, "marked": False}) + + if output_json(ctx, results): + return + + for r in results: + click.echo(f" {'✓' if r['marked'] else '✗'} {r['thread_id']}") + + @cli.command("update-presence") @click.option( "--date", diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 0915b93..17d06af 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -3385,3 +3385,68 @@ async def test_empty_profile_ids_raises_before_the_request(self, mock_client): await mock_client.update_presence_status([], PresenceState.SICK) mock_client._request_with_version_retry.assert_not_awaited() + + +class TestMarkThreadRead: + """Tests for AulaApiClient.mark_thread_read method.""" + + @staticmethod + def _response(data): + response = HttpResponse(status_code=200, data=data) + response.raise_for_status = MagicMock() # type: ignore[assignment] + return response + + @staticmethod + def _client(messages): + client = AulaApiClient(http_client=AsyncMock(), access_token="test_token") + fetched = TestMarkThreadRead._response({"data": {"messages": messages}}) + written = TestMarkThreadRead._response({"status": {"code": 200}, "data": None}) + client._request_with_version_retry = AsyncMock(side_effect=[fetched, written]) + return client + + @pytest.mark.asyncio + async def test_sets_the_marker_on_the_newest_message(self): + """Aula lists the newest message first, and that is where the marker goes.""" + client = self._client( + [ + {"id": "new", "messageType": "Message"}, + {"id": "old", "messageType": "Message"}, + ] + ) + + assert await client.mark_thread_read("42") is True + + get_call, post_call = client._request_with_version_retry.await_args_list + assert get_call.args[0] == "get" + assert "method=messaging.getMessagesForThread&threadId=42" in get_call.args[1] + assert post_call.args[0] == "post" + assert post_call.args[1].endswith("?method=messaging.setLastReadMessage") + assert post_call.kwargs["json"] == { + "threadId": "42", + "messageId": "new", + "commonInboxId": None, + "otpInboxId": None, + } + + @pytest.mark.asyncio + async def test_skips_entries_that_are_not_messages(self): + """A thread event such as a recipient change is not a message to mark.""" + client = self._client( + [ + {"id": "event", "messageType": "RecipientsAdded"}, + {"id": "msg", "messageType": "MessageEdited"}, + ] + ) + + assert await client.mark_thread_read("42") is True + + assert client._request_with_version_retry.await_args.kwargs["json"]["messageId"] == "msg" + + @pytest.mark.asyncio + async def test_thread_without_messages_is_not_written(self): + """With nothing to mark, no write is sent.""" + client = self._client([{"id": "event", "messageType": "RecipientsAdded"}]) + + assert await client.mark_thread_read("42") is False + + assert client._request_with_version_retry.await_count == 1 diff --git a/tests/test_cli.py b/tests/test_cli.py index ebab315..c1795bf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1033,3 +1033,49 @@ def test_links_are_listed_under_the_table(self, capsys, monkeypatch): lines = capsys.readouterr().out.splitlines() assert lines[-1] == " [1] https://example.com/1" + + +class TestMarkRead: + @pytest.fixture + def fake_client(self): + client = MagicMock() + client.mark_thread_read = AsyncMock(return_value=True) + client.__aenter__ = AsyncMock(return_value=client) + client.__aexit__ = AsyncMock(return_value=False) + return client + + @pytest.fixture + def run(self, fake_client, monkeypatch): + monkeypatch.setattr("aula.cli._get_client", AsyncMock(return_value=fake_client)) + + def invoke(*args, output_format="text"): + return CliRunner().invoke( + cli.mark_read, list(args), obj={"OUTPUT_FORMAT": output_format} + ) + + return invoke + + def test_marks_every_thread_given(self, run, fake_client): + result = run("1", "2") + + assert result.exit_code == 0 + assert [c.args for c in fake_client.mark_thread_read.await_args_list] == [("1",), ("2",)] + assert "✓ 1" in result.output + assert "✓ 2" in result.output + + def test_a_failing_thread_does_not_stop_the_rest(self, run, fake_client): + fake_client.mark_thread_read = AsyncMock(side_effect=[RuntimeError("boom"), True]) + + result = run("1", "2") + + assert result.exit_code == 0 + assert "✗ 1" in result.output + assert "✓ 2" in result.output + + def test_json_output_lists_the_result_per_thread(self, run): + result = run("1", output_format="json") + + assert json.loads(result.output) == [{"thread_id": "1", "marked": True}] + + def test_needs_a_thread_id(self, run): + assert run().exit_code != 0 From c74817d48ac314b70ee7b005f66cb623f2baed63 Mon Sep 17 00:00:00 2001 From: Kasper Hartwich Date: Sat, 26 Sep 2026 06:33:37 +0000 Subject: [PATCH 2/4] fix(messages): check Aula's answer to mark-read and keep paging for a message --- src/aula/api_client.py | 83 +++++++++++++++---- src/aula/cli.py | 20 +++-- tests/test_api_client.py | 172 ++++++++++++++++++++++++++++++++------- tests/test_cli.py | 27 ++++-- 4 files changed, 244 insertions(+), 58 deletions(-) diff --git a/src/aula/api_client.py b/src/aula/api_client.py index 8412b4f..7654f40 100644 --- a/src/aula/api_client.py +++ b/src/aula/api_client.py @@ -118,6 +118,12 @@ def _teacher_names(lesson: dict[str, Any], role: str) -> list[str]: return list(dict.fromkeys(name for name in names if name)) +# Message types Aula's web client accepts as the target of a read marker. +_MARKABLE_MESSAGE_TYPES = ("Message", "MessageEdited", "MessageDeleted", "AutoReply") +# Upper bound on pages of 20 read while looking for a message in one thread. +_MAX_THREAD_PAGES = 50 + + class AulaApiClient: """Async client for Aula API endpoints. @@ -994,12 +1000,36 @@ async def get_messages_for_thread(self, thread_id: str, limit: int = 5) -> list[ return messages + @staticmethod + def _api_data(resp: HttpResponse, api_method: str) -> dict[str, Any]: + """Return the ``data`` object of an Aula answer, or raise when it is not a success. + + ``raise_for_status()`` only looks at the HTTP status, but Aula reports its own + errors inside an HTTP 200: ``{"status": {"code": N, ...}, "data": ...}``, where + ``code`` 0 is success. A body that is not that envelope is not trusted either. + """ + resp.raise_for_status() + body = resp.json() + status = body.get("status") if isinstance(body, dict) else None + if not isinstance(status, dict) or status.get("code") != 0: + code = status.get("code") if isinstance(status, dict) else None + raise HttpRequestError( + f"Aula rejected {api_method} (status code {code})", resp.status_code + ) + data = body.get("data") + return data if isinstance(data, dict) else {} + async def mark_thread_read(self, thread_id: str) -> bool: """Mark a message thread as read, up to and including its latest message. Aula keeps one read marker per thread: the ID of the last message read. This finds the newest message and moves the marker there, as the web client does - when a thread is opened. + when a thread is opened. The write is checked afterwards: the marker Aula + reports for the thread must be the message that was sent. + + Only threads in the guardian's own mailbox are handled. Common inboxes and OTP + inboxes need an extra inbox ID in the call, which this does not send, so such + a thread raises ``ValueError`` instead of being marked in the wrong place. Args: thread_id: The thread to mark, as returned by ``get_message_threads()``. @@ -1007,22 +1037,34 @@ async def mark_thread_read(self, thread_id: str) -> bool: Returns: ``True`` when the marker was set, ``False`` when the thread holds no message that can be marked. + + Raises: + HttpRequestError: When Aula rejects a call or does not confirm the write. + ValueError: When the thread is not in the guardian's own mailbox. """ - resp = await self._request_with_version_retry( - "get", - f"{self.api_url}?method=messaging.getMessagesForThread&threadId={thread_id}&page=0&limit=5", - ) - resp.raise_for_status() - raw_messages = get_in(resp.json(), "data.messages", default=[]) - newest = next( - ( - m - for m in raw_messages - if m.get("messageType") - in ("Message", "MessageEdited", "MessageDeleted", "AutoReply") - ), - None, - ) + get_url = f"{self.api_url}?method=messaging.getMessagesForThread&threadId={thread_id}" + newest: dict[str, Any] | None = None + # The newest entries come first, and a thread can start with events (recipients + # added or removed) that are not messages, so keep paging until one turns up. + for page in range(_MAX_THREAD_PAGES): + resp = await self._request_with_version_retry("get", f"{get_url}&page={page}&limit=20") + data = self._api_data(resp, "messaging.getMessagesForThread") + owner = data.get("mailBoxOwner") + if isinstance(owner, dict) and owner.get("mailBoxOwnerType") != "institutionProfile": + raise ValueError( + f"thread {thread_id} is in a common or OTP inbox, " + "which mark-read does not handle" + ) + newest = next( + ( + m + for m in data.get("messages") or [] + if isinstance(m, dict) and m.get("messageType") in _MARKABLE_MESSAGE_TYPES + ), + None, + ) + if newest is not None or not data.get("moreMessagesExist"): + break if newest is None or not newest.get("id"): return False @@ -1036,7 +1078,14 @@ async def mark_thread_read(self, thread_id: str) -> bool: "otpInboxId": None, }, ) - resp.raise_for_status() + self._api_data(resp, "messaging.setLastReadMessage") + + resp = await self._request_with_version_retry("get", f"{get_url}&page=0&limit=1") + marker = self._api_data(resp, "messaging.getMessagesForThread").get("lastReadMessageId") + if marker != newest["id"]: + raise HttpRequestError( + f"Aula did not confirm the read marker of thread {thread_id}", resp.status_code + ) return True async def get_calendar_events( diff --git a/src/aula/cli.py b/src/aula/cli.py index 02ae216..e2dde98 100644 --- a/src/aula/cli.py +++ b/src/aula/cli.py @@ -3328,7 +3328,9 @@ async def mark_read(ctx, thread_ids): """Mark message threads as read. Writes to Aula: each thread is read up to and including its latest message, as if - it had been opened in the app. Thread IDs are the ones `aula messages` shows. + it had been opened in the app. Thread IDs are the ones `aula messages` shows. Only + threads in your own mailbox are handled. Exits non-zero when any thread could not + be marked, after trying all of them. Examples: @@ -3336,21 +3338,23 @@ async def mark_read(ctx, thread_ids): aula mark-read 170924263 170924078 """ + is_json = bool(ctx.obj) and ctx.obj.get("OUTPUT_FORMAT") == "json" results: list[dict] = [] async with await _get_client(ctx) as client: for thread_id in thread_ids: try: marked = await client.mark_thread_read(thread_id) - results.append({"thread_id": thread_id, "marked": marked}) except Exception as e: - print_error(f"marking thread {thread_id} as read: {e}") - results.append({"thread_id": thread_id, "marked": False}) + print_error(f"marking thread {thread_id} as read: {e}", err=is_json) + marked = False + results.append({"thread_id": thread_id, "marked": marked}) - if output_json(ctx, results): - return + if not output_json(ctx, results): + for r in results: + click.echo(f" {'✓' if r['marked'] else '✗'} {r['thread_id']}") - for r in results: - click.echo(f" {'✓' if r['marked'] else '✗'} {r['thread_id']}") + if not all(r["marked"] for r in results): + ctx.exit(1) @cli.command("update-presence") diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 17d06af..569bbbb 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -11,6 +11,7 @@ from aula.const import API_URL, API_VERSION, CSRF_TOKEN_HEADER from aula.http import ( AulaAuthenticationError, + AulaConnectionError, AulaServerError, HttpRequestError, HttpResponse, @@ -3388,65 +3389,180 @@ async def test_empty_profile_ids_raises_before_the_request(self, mock_client): class TestMarkThreadRead: - """Tests for AulaApiClient.mark_thread_read method.""" + """Tests for AulaApiClient.mark_thread_read method. + + The responses are real ``HttpResponse`` objects, so the HTTP-status check and the + check of Aula's own status envelope both run. + """ + + OWNER = {"mailBoxOwnerType": "institutionProfile", "id": 1} + + @classmethod + def _page(cls, messages, *, more=False, last_read=None, owner=None, code=0, http=200): + data = { + "messages": messages, + "moreMessagesExist": more, + "lastReadMessageId": last_read, + "mailBoxOwner": owner or cls.OWNER, + } + return HttpResponse(status_code=http, data={"status": {"code": code}, "data": data}) @staticmethod - def _response(data): - response = HttpResponse(status_code=200, data=data) - response.raise_for_status = MagicMock() # type: ignore[assignment] - return response + def _ok(): + return HttpResponse(status_code=200, data={"status": {"code": 0}, "data": None}) @staticmethod - def _client(messages): + def _client(*responses): client = AulaApiClient(http_client=AsyncMock(), access_token="test_token") - fetched = TestMarkThreadRead._response({"data": {"messages": messages}}) - written = TestMarkThreadRead._response({"status": {"code": 200}, "data": None}) - client._request_with_version_retry = AsyncMock(side_effect=[fetched, written]) + client._request_with_version_retry = AsyncMock(side_effect=list(responses)) return client + @staticmethod + def _msg(msg_id, message_type="Message"): + return {"id": msg_id, "messageType": message_type} + + @staticmethod + def _calls(client): + return [ + (c.args[0], c.args[1], c.kwargs) + for c in client._request_with_version_retry.await_args_list + ] + @pytest.mark.asyncio - async def test_sets_the_marker_on_the_newest_message(self): + async def test_sets_the_marker_on_the_newest_message_and_checks_it(self): """Aula lists the newest message first, and that is where the marker goes.""" client = self._client( - [ - {"id": "new", "messageType": "Message"}, - {"id": "old", "messageType": "Message"}, - ] + self._page([self._msg("new"), self._msg("old")]), + self._ok(), + self._page([self._msg("new")], last_read="new"), ) assert await client.mark_thread_read("42") is True - get_call, post_call = client._request_with_version_retry.await_args_list - assert get_call.args[0] == "get" - assert "method=messaging.getMessagesForThread&threadId=42" in get_call.args[1] - assert post_call.args[0] == "post" - assert post_call.args[1].endswith("?method=messaging.setLastReadMessage") - assert post_call.kwargs["json"] == { + get_call, post_call, check_call = self._calls(client) + assert get_call[0] == "get" + assert "method=messaging.getMessagesForThread&threadId=42&page=0" in get_call[1] + assert post_call[0] == "post" + assert post_call[1].endswith("?method=messaging.setLastReadMessage") + assert post_call[2]["json"] == { "threadId": "42", "messageId": "new", "commonInboxId": None, "otpInboxId": None, } + assert check_call[0] == "get" @pytest.mark.asyncio async def test_skips_entries_that_are_not_messages(self): """A thread event such as a recipient change is not a message to mark.""" client = self._client( - [ - {"id": "event", "messageType": "RecipientsAdded"}, - {"id": "msg", "messageType": "MessageEdited"}, - ] + self._page( + [self._msg("event", "RecipientsRemoved"), self._msg("msg", "MessageEdited")] + ), + self._ok(), + self._page([], last_read="msg"), ) assert await client.mark_thread_read("42") is True - assert client._request_with_version_retry.await_args.kwargs["json"]["messageId"] == "msg" + assert self._calls(client)[1][2]["json"]["messageId"] == "msg" + + @pytest.mark.asyncio + async def test_keeps_paging_when_the_first_page_holds_only_events(self): + """Events on page 0 do not mean the thread has no message.""" + client = self._client( + self._page([self._msg("event", "RecipientsAdded")], more=True), + self._page([self._msg("msg")]), + self._ok(), + self._page([], last_read="msg"), + ) + + assert await client.mark_thread_read("42") is True + + calls = self._calls(client) + assert "page=0" in calls[0][1] + assert "page=1" in calls[1][1] + assert calls[2][2]["json"]["messageId"] == "msg" @pytest.mark.asyncio async def test_thread_without_messages_is_not_written(self): - """With nothing to mark, no write is sent.""" - client = self._client([{"id": "event", "messageType": "RecipientsAdded"}]) + """With nothing to mark once every page is read, no write is sent.""" + client = self._client(self._page([self._msg("event", "RecipientsAdded")])) assert await client.mark_thread_read("42") is False - assert client._request_with_version_retry.await_count == 1 + assert len(self._calls(client)) == 1 + + @pytest.mark.asyncio + async def test_api_error_in_a_200_answer_is_not_success(self): + """Aula reports its own errors inside an HTTP 200.""" + rejected = HttpResponse(status_code=200, data={"status": {"code": 5}, "data": None}) + client = self._client(self._page([self._msg("new")]), rejected) + + with pytest.raises(HttpRequestError, match="rejected messaging.setLastReadMessage"): + await client.mark_thread_read("42") + + @pytest.mark.asyncio + @pytest.mark.parametrize("body", [None, "", "", [], {}, {"status": "ok"}, {"data": {}}]) + async def test_malformed_answer_is_not_success(self, body): + """An empty or unexpected body is not read as a success.""" + client = self._client( + self._page([self._msg("new")]), HttpResponse(status_code=200, data=body) + ) + + with pytest.raises(HttpRequestError): + await client.mark_thread_read("42") + + @pytest.mark.asyncio + async def test_api_error_while_reading_the_thread_writes_nothing(self): + """A rejected read stops before any write.""" + client = self._client(self._page([self._msg("new")], code=1)) + + with pytest.raises(HttpRequestError): + await client.mark_thread_read("42") + + assert len(self._calls(client)) == 1 + + @pytest.mark.asyncio + async def test_http_error_raises(self): + """An HTTP error status raises through the response's own check.""" + client = self._client( + self._page([self._msg("new")]), HttpResponse(status_code=500, data=None) + ) + + with pytest.raises(AulaServerError): + await client.mark_thread_read("42") + + @pytest.mark.asyncio + async def test_network_error_propagates(self): + """A failed connection is not swallowed.""" + client = self._client( + self._page([self._msg("new")]), AulaConnectionError("down", status_code=0) + ) + + with pytest.raises(AulaConnectionError): + await client.mark_thread_read("42") + + @pytest.mark.asyncio + async def test_unconfirmed_marker_is_not_success(self): + """The write counts only when Aula reports the marker on that message.""" + client = self._client( + self._page([self._msg("new")]), + self._ok(), + self._page([], last_read="old"), + ) + + with pytest.raises(HttpRequestError, match="did not confirm"): + await client.mark_thread_read("42") + + @pytest.mark.asyncio + async def test_common_inbox_thread_is_refused_without_a_write(self): + """The inbox IDs are always sent as null, so other mailboxes are not handled.""" + client = self._client( + self._page([self._msg("new")], owner={"mailBoxOwnerType": "commonInbox", "id": 9}) + ) + + with pytest.raises(ValueError, match="common or OTP inbox"): + await client.mark_thread_read("42") + + assert len(self._calls(client)) == 1 diff --git a/tests/test_cli.py b/tests/test_cli.py index c1795bf..8f415df 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1063,19 +1063,36 @@ def test_marks_every_thread_given(self, run, fake_client): assert "✓ 1" in result.output assert "✓ 2" in result.output - def test_a_failing_thread_does_not_stop_the_rest(self, run, fake_client): + def test_a_failing_thread_is_reported_and_the_rest_still_run(self, run, fake_client): fake_client.mark_thread_read = AsyncMock(side_effect=[RuntimeError("boom"), True]) result = run("1", "2") - assert result.exit_code == 0 + assert result.exit_code == 1 assert "✗ 1" in result.output assert "✓ 2" in result.output + assert "boom" in result.output + + def test_a_thread_with_nothing_to_mark_fails_the_command(self, run, fake_client): + fake_client.mark_thread_read = AsyncMock(return_value=False) + + result = run("1") + + assert result.exit_code == 1 + assert "✗ 1" in result.output + + def test_json_output_stays_valid_when_a_thread_fails(self, run, fake_client): + fake_client.mark_thread_read = AsyncMock(side_effect=[RuntimeError("boom"), True]) - def test_json_output_lists_the_result_per_thread(self, run): - result = run("1", output_format="json") + result = run("1", "2", output_format="json") - assert json.loads(result.output) == [{"thread_id": "1", "marked": True}] + assert result.exit_code == 1 + assert json.loads(result.stdout) == [ + {"thread_id": "1", "marked": False}, + {"thread_id": "2", "marked": True}, + ] + assert "boom" in result.stderr + assert "boom" not in result.stdout def test_needs_a_thread_id(self, run): assert run().exit_code != 0 From edb1f9b7b046ed6a51f995fa270632b96fc754e7 Mon Sep 17 00:00:00 2001 From: Kasper Hartwich Date: Sat, 26 Sep 2026 06:38:12 +0000 Subject: [PATCH 3/4] fix(messages): refuse a mailbox not shown to be the guardian's own, follow API version changes --- README.md | 9 ++++ src/aula/api_client.py | 57 +++++++++++++++----- tests/test_api_client.py | 111 ++++++++++++++++++++++++++++++++++++--- tests/test_cli.py | 30 +++++++++++ 4 files changed, 187 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 37d5eed..a4393ac 100644 --- a/README.md +++ b/README.md @@ -196,12 +196,21 @@ The username can also be set via the `AULA_MITID_USERNAME` environment variable | Command | Description | |---|---| | `aula messages` | Recent message threads | +| `aula mark-read THREAD_ID...` | Mark threads as read (writes to Aula, own mailbox only; see below) | | `aula auto-reply` | View auto-reply status | | `aula contacts` | Contact list | | `aula notifications` | Recent notifications | | `aula notification-settings` | View notification preferences | | `aula search` | Search documents across Aula | +`aula mark-read` moves a thread's read marker to its newest message, as the app does +when a thread is opened, and checks that Aula took it. It reads the thread as it is +at that moment: a message that arrives during the call is not part of it. A thread +can begin with events (recipients added or removed) that are not messages, so it +reads up to 50 pages of 20, and stops with an error if none of them holds a message. +Only threads in your own mailbox are handled; common and OTP inboxes are refused. +It tries every thread given and exits with a non-zero code if any was not marked. + ### Calendar | Command | Description | diff --git a/src/aula/api_client.py b/src/aula/api_client.py index 7654f40..5ffd17a 100644 --- a/src/aula/api_client.py +++ b/src/aula/api_client.py @@ -124,6 +124,10 @@ def _teacher_names(lesson: dict[str, Any], role: str) -> list[str]: _MAX_THREAD_PAGES = 50 +class ThreadScanLimitError(Exception): + """Raised when a thread's first pages hold no message, but Aula reports more pages.""" + + class AulaApiClient: """Async client for Aula API endpoints. @@ -1019,6 +1023,17 @@ def _api_data(resp: HttpResponse, api_method: str) -> dict[str, Any]: data = body.get("data") return data if isinstance(data, dict) else {} + def _thread_messages_url(self, thread_id: str, page: int, limit: int) -> str: + """URL for one page of a thread's messages, on the API version in use now. + + Built on every call: a request can negotiate a newer API version, and a URL + kept from before that would go to the old one. + """ + return ( + f"{self.api_url}?method=messaging.getMessagesForThread" + f"&threadId={thread_id}&page={page}&limit={limit}" + ) + async def mark_thread_read(self, thread_id: str) -> bool: """Mark a message thread as read, up to and including its latest message. @@ -1027,9 +1042,18 @@ async def mark_thread_read(self, thread_id: str) -> bool: when a thread is opened. The write is checked afterwards: the marker Aula reports for the thread must be the message that was sent. + The newest message is taken from the first page that holds one. A message that + arrives while this runs is not seen, so the marker goes on the newest message + as it was when the thread was read, and that thread may still show as unread. + + A thread can begin with events (recipients added or removed) that are not + messages, so pages of 20 are read until a message turns up, at most + ``_MAX_THREAD_PAGES`` of them. + Only threads in the guardian's own mailbox are handled. Common inboxes and OTP - inboxes need an extra inbox ID in the call, which this does not send, so such - a thread raises ``ValueError`` instead of being marked in the wrong place. + inboxes need an extra inbox ID in the call, which this does not send, so a + thread whose mailbox is not shown to be the guardian's own is refused with + ``ValueError`` instead of being marked in the wrong place. Args: thread_id: The thread to mark, as returned by ``get_message_threads()``. @@ -1040,20 +1064,22 @@ async def mark_thread_read(self, thread_id: str) -> bool: Raises: HttpRequestError: When Aula rejects a call or does not confirm the write. - ValueError: When the thread is not in the guardian's own mailbox. + ValueError: When the thread is not shown to be in the guardian's own mailbox. + ThreadScanLimitError: When ``_MAX_THREAD_PAGES`` pages held no message and + Aula says there are more. """ - get_url = f"{self.api_url}?method=messaging.getMessagesForThread&threadId={thread_id}" newest: dict[str, Any] | None = None - # The newest entries come first, and a thread can start with events (recipients - # added or removed) that are not messages, so keep paging until one turns up. + more = False for page in range(_MAX_THREAD_PAGES): - resp = await self._request_with_version_retry("get", f"{get_url}&page={page}&limit=20") + resp = await self._request_with_version_retry( + "get", self._thread_messages_url(thread_id, page, 20) + ) data = self._api_data(resp, "messaging.getMessagesForThread") owner = data.get("mailBoxOwner") - if isinstance(owner, dict) and owner.get("mailBoxOwnerType") != "institutionProfile": + if not isinstance(owner, dict) or owner.get("mailBoxOwnerType") != "institutionProfile": raise ValueError( - f"thread {thread_id} is in a common or OTP inbox, " - "which mark-read does not handle" + f"thread {thread_id} is not shown to be in your own mailbox, " + "and mark-read only handles those" ) newest = next( ( @@ -1063,8 +1089,13 @@ async def mark_thread_read(self, thread_id: str) -> bool: ), None, ) - if newest is not None or not data.get("moreMessagesExist"): + more = bool(data.get("moreMessagesExist")) + if newest is not None or not more: break + if newest is None and more: + raise ThreadScanLimitError( + f"thread {thread_id} has no message in its first {_MAX_THREAD_PAGES} pages" + ) if newest is None or not newest.get("id"): return False @@ -1080,7 +1111,9 @@ async def mark_thread_read(self, thread_id: str) -> bool: ) self._api_data(resp, "messaging.setLastReadMessage") - resp = await self._request_with_version_retry("get", f"{get_url}&page=0&limit=1") + resp = await self._request_with_version_retry( + "get", self._thread_messages_url(thread_id, 0, 1) + ) marker = self._api_data(resp, "messaging.getMessagesForThread").get("lastReadMessageId") if marker != newest["id"]: raise HttpRequestError( diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 569bbbb..7c50246 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -7,7 +7,7 @@ import pytest -from aula.api_client import AulaApiClient +from aula.api_client import AulaApiClient, ThreadScanLimitError from aula.const import API_URL, API_VERSION, CSRF_TOKEN_HEADER from aula.http import ( AulaAuthenticationError, @@ -3397,14 +3397,17 @@ class TestMarkThreadRead: OWNER = {"mailBoxOwnerType": "institutionProfile", "id": 1} + MISSING = object() + @classmethod - def _page(cls, messages, *, more=False, last_read=None, owner=None, code=0, http=200): + def _page(cls, messages, *, more=False, last_read=None, owner=OWNER, code=0, http=200): data = { "messages": messages, "moreMessagesExist": more, "lastReadMessageId": last_read, - "mailBoxOwner": owner or cls.OWNER, } + if owner is not cls.MISSING: + data["mailBoxOwner"] = owner return HttpResponse(status_code=http, data={"status": {"code": code}, "data": data}) @staticmethod @@ -3556,13 +3559,105 @@ async def test_unconfirmed_marker_is_not_success(self): await client.mark_thread_read("42") @pytest.mark.asyncio - async def test_common_inbox_thread_is_refused_without_a_write(self): - """The inbox IDs are always sent as null, so other mailboxes are not handled.""" + @pytest.mark.parametrize( + "owner", + [ + {"mailBoxOwnerType": "commonInbox", "id": 9}, + {"mailBoxOwnerType": "otpInbox", "id": 9}, + {"mailBoxOwnerType": None}, + {"id": 1}, + {}, + None, + "institutionProfile", + [], + MISSING, + ], + ) + async def test_mailbox_not_shown_to_be_the_guardians_own_is_refused(self, owner): + """The inbox IDs are sent as null, so a mailbox not shown to be the own one is refused.""" + client = self._client(self._page([self._msg("new")], owner=owner)) + + with pytest.raises(ValueError, match="not shown to be in your own mailbox"): + await client.mark_thread_read("42") + + assert len(self._calls(client)) == 1 + + @pytest.mark.asyncio + async def test_urls_follow_an_api_version_bump_between_calls(self): + """A version negotiated by the first request is used by paging and the check.""" + client = AulaApiClient(http_client=AsyncMock(), access_token="test_token") + old_url = client.api_url + new_url = old_url.replace(old_url.split("/v")[-1], str(int(old_url.split("/v")[-1]) + 1)) + answers = iter( + [ + self._page([self._msg("event", "RecipientsAdded")], more=True), + self._page([self._msg("msg")]), + self._ok(), + self._page([], last_read="msg"), + ] + ) + seen: list[str] = [] + + async def request(method, url, **kwargs): + seen.append(url) + if len(seen) == 1: + client.api_url = new_url # what a 410 answer does inside the real request + return next(answers) + + client._request_with_version_retry = request + + assert await client.mark_thread_read("42") is True + + assert seen[0].startswith(old_url + "?") + assert all(url.startswith(new_url + "?") for url in seen[1:]) + + @pytest.mark.asyncio + async def test_marks_deleted_and_auto_reply_messages(self): + """The web client accepts these types as the newest message too.""" + for message_type in ("MessageDeleted", "AutoReply"): + client = self._client( + self._page([self._msg("last", message_type)]), + self._ok(), + self._page([], last_read="last"), + ) + + assert await client.mark_thread_read("42") is True + assert self._calls(client)[1][2]["json"]["messageId"] == "last" + + @pytest.mark.asyncio + async def test_page_limit_is_an_error_not_no_messages(self, monkeypatch): + """Events on every permitted page, with more to come, are not 'nothing to mark'.""" + monkeypatch.setattr("aula.api_client._MAX_THREAD_PAGES", 3) client = self._client( - self._page([self._msg("new")], owner={"mailBoxOwnerType": "commonInbox", "id": 9}) + *[self._page([self._msg(f"e{i}", "RecipientsAdded")], more=True) for i in range(3)] ) - with pytest.raises(ValueError, match="common or OTP inbox"): + with pytest.raises(ThreadScanLimitError, match="first 3 pages"): await client.mark_thread_read("42") - assert len(self._calls(client)) == 1 + assert len(self._calls(client)) == 3 + + @pytest.mark.asyncio + async def test_message_on_the_last_permitted_page_is_found(self, monkeypatch): + """The cap is inclusive: the last page allowed is still read.""" + monkeypatch.setattr("aula.api_client._MAX_THREAD_PAGES", 3) + client = self._client( + self._page([self._msg("e0", "RecipientsAdded")], more=True), + self._page([self._msg("e1", "RecipientsAdded")], more=True), + self._page([self._msg("msg")], more=True), + self._ok(), + self._page([], last_read="msg"), + ) + + assert await client.mark_thread_read("42") is True + + @pytest.mark.asyncio + async def test_exhausted_event_only_thread_is_false(self, monkeypatch): + """Only events, and Aula says there are no more pages: nothing to mark.""" + monkeypatch.setattr("aula.api_client._MAX_THREAD_PAGES", 3) + client = self._client( + self._page([self._msg("e0", "RecipientsAdded")], more=True), + self._page([self._msg("e1", "RecipientsAdded")], more=False), + ) + + assert await client.mark_thread_read("42") is False diff --git a/tests/test_cli.py b/tests/test_cli.py index 8f415df..f5141bc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1094,5 +1094,35 @@ def test_json_output_stays_valid_when_a_thread_fails(self, run, fake_client): assert "boom" in result.stderr assert "boom" not in result.stdout + def test_a_refused_thread_is_followed_by_a_marked_one(self, run, fake_client): + fake_client.mark_thread_read = AsyncMock(side_effect=[ValueError("common inbox"), True]) + + result = run("1", "2") + + assert result.exit_code == 1 + assert "✗ 1" in result.output + assert "✓ 2" in result.output + + def test_json_output_when_every_thread_is_marked(self, run): + result = run("1", "2", output_format="json") + + assert result.exit_code == 0 + assert json.loads(result.stdout) == [ + {"thread_id": "1", "marked": True}, + {"thread_id": "2", "marked": True}, + ] + + def test_json_output_when_every_thread_fails(self, run, fake_client): + fake_client.mark_thread_read = AsyncMock(side_effect=RuntimeError("boom")) + + result = run("1", "2", output_format="json") + + assert result.exit_code == 1 + assert json.loads(result.stdout) == [ + {"thread_id": "1", "marked": False}, + {"thread_id": "2", "marked": False}, + ] + assert result.stderr.count("boom") == 2 + def test_needs_a_thread_id(self, run): assert run().exit_code != 0 From 77b4f584414375f921b7e583dfcbca198bd5ffae Mon Sep 17 00:00:00 2001 From: Kasper Hartwich Date: Sat, 26 Sep 2026 06:40:54 +0000 Subject: [PATCH 4/4] docs(messages): describe mark-read's snapshot and scan limit precisely --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a4393ac..17ff566 100644 --- a/README.md +++ b/README.md @@ -204,10 +204,11 @@ The username can also be set via the `AULA_MITID_USERNAME` environment variable | `aula search` | Search documents across Aula | `aula mark-read` moves a thread's read marker to its newest message, as the app does -when a thread is opened, and checks that Aula took it. It reads the thread as it is -at that moment: a message that arrives during the call is not part of it. A thread +when a thread is opened, and checks that Aula took it. The marker targets the newest +message found while reading; messages arriving afterward may remain unread. A thread can begin with events (recipients added or removed) that are not messages, so it -reads up to 50 pages of 20, and stops with an error if none of them holds a message. +reads up to 50 pages of 20. Running out of that scan limit raises an error, while a +thread that holds only events is returned unmarked and makes the command exit 1. Only threads in your own mailbox are handled; common and OTP inboxes are refused. It tries every thread given and exits with a non-zero code if any was not marked.