mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-07-23 04:14:55 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ff0ce4d123 | ||
|
|
8b3665b32d | ||
|
|
9cce6d1b68 | ||
|
|
bb77e65d52 | ||
|
|
eb5bf53476 |
@@ -1,662 +0,0 @@
|
||||
# Mail Fetch Re-download Fix Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Stop `paperless_mail` from re-downloading full IMAP message bodies for mail it has already handled, and make sure attachment-less mail under attachments-only rules is recorded so it stops matching the search forever.
|
||||
|
||||
**Architecture:** In `_handle_mail_rule`, replace the single "fetch every matching message's full body" call with a cheap `UID SEARCH` first, subtract UIDs already in `ProcessedMail`, and fetch bodies only for the remainder (batched). Separately, extract the existing "record `PROCESSED_WO_CONSUMPTION`" block into a shared helper and call it from the no-attachments-under-attachments-only-scope early return in `_handle_message`, which currently skips it entirely.
|
||||
|
||||
**Tech Stack:** Django (pytest-django), `imap_tools` (`MailBox.uids()`, `AND` query builder), existing `ProcessedMail` model — no new dependencies, no migrations.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- No new required settings and no DB migrations for existing installs (spec: "No new required settings/migrations for existing installs").
|
||||
- Batch size for body fetches is a hardcoded module constant (`MAIL_FETCH_BATCH_SIZE = 500`), not user-configurable.
|
||||
- No mailbox action (mark-read/flag/tag/move/delete) is applied for the no-attachment/no-consumption case — only a `ProcessedMail` row is written, matching the existing precedent for other no-consumption outcomes.
|
||||
- Every path that currently produces a document or applies a mail action must behave exactly as before (no change to `_process_attachments`'/`_process_eml`'s consuming behavior, action application, or the existing per-message `already_processed` dedup check, which stays in place as a safety net).
|
||||
- Follow the existing per-message `uid_validity` matching semantics exactly: when `self._current_uid_validity is not None`, only match `ProcessedMail` rows with the same `uid_validity` or `uid_validity IS NULL`; when it is `None` (server didn't report UIDVALIDITY), fall back to matching on `(rule, uid, folder)` alone, ignoring `uid_validity`. (This exact conditional already exists at `mail.py:722-726` for the per-message check — the new UID-diff query must replicate it or several existing uidvalidity tests will break.)
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add UID search support to the `BogusMailBox` test double
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_mail/tests/test_mail.py:137-171` (`BogusMailBox.fetch`)
|
||||
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Produces: `BogusMailBox.uids(self, criteria, charset="") -> list[str]`, filtering with the same rules as `fetch()`, returning only UIDs (no message bodies). `BogusMailBox.fetch()` gains support for a `UID <comma-list>` criteria token (used by real code's `AND(uid=[...])` queries).
|
||||
- Consumes: nothing new — this is test infrastructure only, no production code changes in this task.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py` (near the other `BogusMailBox`-adjacent tests, e.g. after `test_handle_message`):
|
||||
|
||||
```python
|
||||
def test_bogus_mailbox_uids_and_uid_criteria(self) -> None:
|
||||
mailbox = self.mailMocker.bogus_mailbox
|
||||
all_messages = list(mailbox.messages)
|
||||
|
||||
# uids() returns the UIDs of unseen messages, no bodies needed to call it
|
||||
unseen_uids = mailbox.uids("(UNSEEN)")
|
||||
self.assertEqual(
|
||||
set(unseen_uids),
|
||||
{m.uid for m in all_messages if not m.seen},
|
||||
)
|
||||
|
||||
# fetch() with an explicit UID criteria returns only the matching messages
|
||||
target_uid = all_messages[0].uid
|
||||
from imap_tools import AND
|
||||
|
||||
fetched = mailbox.fetch(AND(uid=[target_uid]), mark_seen=False)
|
||||
self.assertEqual([m.uid for m in fetched], [target_uid])
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_bogus_mailbox_uids_and_uid_criteria -v`
|
||||
Expected: FAIL with `AttributeError: 'BogusMailBox' object has no attribute 'uids'` (and/or the UID-criteria fetch returning all 3 default messages instead of 1, since `fetch()` doesn't understand a `UID` token yet).
|
||||
|
||||
- [ ] **Step 3: Implement `uids()` and `UID` criteria support**
|
||||
|
||||
Replace `src/paperless_mail/tests/test_mail.py:137-171`:
|
||||
|
||||
```python
|
||||
def fetch(self, criteria, mark_seen, charset="", *, bulk=True):
|
||||
return self._filter_messages(criteria)
|
||||
|
||||
def uids(self, criteria, charset="") -> list[str]:
|
||||
return [m.uid for m in self._filter_messages(criteria)]
|
||||
|
||||
def _filter_messages(self, criteria):
|
||||
msg = self.messages
|
||||
|
||||
criteria = str(criteria).strip("()").split(" ")
|
||||
|
||||
if "UNSEEN" in criteria:
|
||||
msg = filter(lambda m: not m.seen, msg)
|
||||
|
||||
if "SUBJECT" in criteria:
|
||||
subject = criteria[criteria.index("SUBJECT") + 1].strip('"')
|
||||
msg = filter(lambda m: subject in m.subject, msg)
|
||||
|
||||
if "BODY" in criteria:
|
||||
body = criteria[criteria.index("BODY") + 1].strip('"')
|
||||
msg = filter(lambda m: body in m.text, msg)
|
||||
|
||||
if "FROM" in criteria:
|
||||
from_ = criteria[criteria.index("FROM") + 1].strip('"')
|
||||
msg = filter(lambda m: from_ in m.from_, msg)
|
||||
|
||||
if "TO" in criteria:
|
||||
to_ = criteria[criteria.index("TO") + 1].strip('"')
|
||||
msg = filter(lambda m: any(to_ in to_addr for to_addr in m.to), msg)
|
||||
|
||||
if "UNFLAGGED" in criteria:
|
||||
msg = filter(lambda m: not m.flagged, msg)
|
||||
|
||||
if "UNKEYWORD" in criteria:
|
||||
tag = criteria[criteria.index("UNKEYWORD") + 1].strip("'")
|
||||
msg = filter(lambda m: tag not in m.flags, msg)
|
||||
|
||||
if "(X-GM-LABELS" in criteria: # ['NOT', '(X-GM-LABELS', '"processed"']
|
||||
msg = filter(lambda m: "processed" not in m.flags, msg)
|
||||
|
||||
if "UID" in criteria:
|
||||
uid_list = criteria[criteria.index("UID") + 1].split(",")
|
||||
msg = filter(lambda m: m.uid in uid_list, msg)
|
||||
|
||||
return list(msg)
|
||||
```
|
||||
|
||||
This is a pure refactor of the existing filtering logic into `_filter_messages`, reused by both `fetch()` (unchanged behavior) and the new `uids()`, plus one new `if "UID" in criteria` branch.
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_bogus_mailbox_uids_and_uid_criteria -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full existing `test_mail.py` suite to confirm the refactor didn't change `fetch()` behavior**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
|
||||
Expected: all tests PASS (same pass count as before this task)
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_mail/tests/test_mail.py
|
||||
git commit -m "test: add BogusMailBox.uids() and UID criteria support"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Record `ProcessedMail` for attachment-less mail under attachments-only rules
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_mail/mail.py:749-793` (`_handle_message`)
|
||||
- Modify: `src/paperless_mail/mail.py:952-979` (`_process_attachments`, tail)
|
||||
- Test: `src/paperless_mail/tests/test_mail.py:540-548` (`test_handle_empty_message`, rewritten)
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Produces: `MailAccountHandler._record_processed_without_consumption(self, message: MailMessage, rule: MailRule) -> None` — idempotently writes a `ProcessedMail(status="PROCESSED_WO_CONSUMPTION")` row for `(rule, message.uid, rule.folder)` if one doesn't already exist for the current `self._current_uid_validity`. No mailbox action is applied.
|
||||
- Consumes: `self._current_uid_validity` (already set by `_handle_mail_rule` before `_handle_message` is called; `None` when called directly, e.g. in unit tests).
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Replace `src/paperless_mail/tests/test_mail.py:540-548` (`test_handle_empty_message`):
|
||||
|
||||
```python
|
||||
def test_handle_empty_message(self) -> None:
|
||||
message = self.mailMocker.messageBuilder.create_message(
|
||||
subject="No attachments here",
|
||||
attachments=[],
|
||||
)
|
||||
|
||||
account = MailAccount.objects.create()
|
||||
rule = MailRule.objects.create(
|
||||
account=account,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
|
||||
result = self.mail_account_handler._handle_message(message, rule)
|
||||
|
||||
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
|
||||
self.assertEqual(result, 0)
|
||||
|
||||
processed = ProcessedMail.objects.get(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
)
|
||||
self.assertEqual(processed.status, "PROCESSED_WO_CONSUMPTION")
|
||||
|
||||
# Calling it again must not create a second row
|
||||
self.mail_account_handler._handle_message(message, rule)
|
||||
self.assertEqual(
|
||||
ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
).count(),
|
||||
1,
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_empty_message -v`
|
||||
Expected: FAIL — `ProcessedMail.DoesNotExist` (no row is created by the current early return).
|
||||
|
||||
- [ ] **Step 3: Add the shared helper and call it from both places**
|
||||
|
||||
Insert a new method right after `_handle_message` ends, i.e. after `src/paperless_mail/mail.py:793` (`return processed_elements`), before `def filename_inclusion_matches`:
|
||||
|
||||
```python
|
||||
def _record_processed_without_consumption(
|
||||
self,
|
||||
message: MailMessage,
|
||||
rule: MailRule,
|
||||
) -> None:
|
||||
if not ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
uid_validity=self._current_uid_validity,
|
||||
).exists():
|
||||
ProcessedMail.objects.create(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid=message.uid,
|
||||
uid_validity=self._current_uid_validity,
|
||||
subject=message.subject,
|
||||
received=make_aware(message.date)
|
||||
if is_naive(message.date)
|
||||
else message.date,
|
||||
status="PROCESSED_WO_CONSUMPTION",
|
||||
)
|
||||
```
|
||||
|
||||
Then modify `_handle_message`'s early return at `src/paperless_mail/mail.py:756-760`:
|
||||
|
||||
```python
|
||||
# Skip Message handling when only attachments are to be processed but
|
||||
# message doesn't have any.
|
||||
if (
|
||||
not message.attachments
|
||||
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
|
||||
):
|
||||
return processed_elements
|
||||
```
|
||||
|
||||
becomes:
|
||||
|
||||
```python
|
||||
# Skip Message handling when only attachments are to be processed but
|
||||
# message doesn't have any.
|
||||
if (
|
||||
not message.attachments
|
||||
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
|
||||
):
|
||||
self._record_processed_without_consumption(message, rule)
|
||||
return processed_elements
|
||||
```
|
||||
|
||||
Then replace the tail of `_process_attachments` at `src/paperless_mail/mail.py:952-979`:
|
||||
|
||||
```python
|
||||
if len(consume_tasks) > 0:
|
||||
queue_consumption_tasks(
|
||||
consume_tasks=consume_tasks,
|
||||
rule=rule,
|
||||
message=message,
|
||||
uid_validity=self._current_uid_validity,
|
||||
)
|
||||
else:
|
||||
# No files to consume, just mark as processed if it wasn't by .eml processing
|
||||
if not ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
uid_validity=self._current_uid_validity,
|
||||
).exists():
|
||||
ProcessedMail.objects.create(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid=message.uid,
|
||||
uid_validity=self._current_uid_validity,
|
||||
subject=message.subject,
|
||||
received=make_aware(message.date)
|
||||
if is_naive(message.date)
|
||||
else message.date,
|
||||
status="PROCESSED_WO_CONSUMPTION",
|
||||
)
|
||||
|
||||
return processed_attachments
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
if len(consume_tasks) > 0:
|
||||
queue_consumption_tasks(
|
||||
consume_tasks=consume_tasks,
|
||||
rule=rule,
|
||||
message=message,
|
||||
uid_validity=self._current_uid_validity,
|
||||
)
|
||||
else:
|
||||
# No files to consume, just mark as processed if it wasn't by .eml processing
|
||||
self._record_processed_without_consumption(message, rule)
|
||||
|
||||
return processed_attachments
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_empty_message -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full `test_mail.py` suite (regression check for the `_process_attachments` refactor)**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
|
||||
Expected: all PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
|
||||
git commit -m "fix: record ProcessedMail for attachment-less mail under attachments-only rules"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Diff UIDs against `ProcessedMail` before fetching message bodies
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_mail/mail.py:641-693` (`_handle_mail_rule`, the fetch section)
|
||||
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Consumes: `MailAccountHandler._record_processed_without_consumption` (Task 2), `ProcessedMail` model, `imap_tools.AND`, `M.uids()` (Task 1's `BogusMailBox.uids()` in tests; `imap_tools.MailBox.uids()` in production).
|
||||
- Produces: no new public interface — `_handle_mail_rule`'s external behavior (return value, exceptions raised) is unchanged; only its internal fetch strategy changes. `_handle_mail_rule` now returns `0` immediately, without calling `M.fetch()` at all, when every UID matching the search criteria already has a `ProcessedMail` row.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py`:
|
||||
|
||||
```python
|
||||
def test_handle_mail_account_skips_body_fetch_for_already_processed_mail(
|
||||
self,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- An attachment-less mail under an attachments-only mark-read rule,
|
||||
already recorded as PROCESSED_WO_CONSUMPTION
|
||||
WHEN:
|
||||
- The mail account is processed again and the mail still matches the
|
||||
search criteria (it was never marked read, since no mail action is
|
||||
applied for the no-consumption case)
|
||||
THEN:
|
||||
- No IMAP body fetch happens for that mail; only the cheap UID search runs.
|
||||
"""
|
||||
account = MailAccount.objects.create(
|
||||
name="test",
|
||||
imap_server="",
|
||||
username="admin",
|
||||
password="secret",
|
||||
)
|
||||
rule = MailRule.objects.create(
|
||||
name="testrule",
|
||||
account=account,
|
||||
action=MailRule.MailAction.MARK_READ,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
|
||||
message = self.mailMocker.messageBuilder.create_message(
|
||||
subject="No attachment",
|
||||
attachments=[],
|
||||
)
|
||||
self.mailMocker.bogus_mailbox.messages = [message]
|
||||
self.mailMocker.bogus_mailbox.updateClient()
|
||||
|
||||
# First run: records ProcessedMail without consuming anything.
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
self.assertTrue(
|
||||
ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
).exists(),
|
||||
)
|
||||
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
|
||||
|
||||
# Second run: message still matches UNSEEN (mark-read action never ran),
|
||||
# but its body must not be downloaded again.
|
||||
with mock.patch.object(
|
||||
self.mailMocker.bogus_mailbox,
|
||||
"fetch",
|
||||
wraps=self.mailMocker.bogus_mailbox.fetch,
|
||||
) as fetch_spy:
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
|
||||
fetch_spy.assert_not_called()
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_skips_body_fetch_for_already_processed_mail -v`
|
||||
Expected: FAIL — `fetch_spy.assert_not_called()` fails because the current code always calls `M.fetch()` for every message matching the search criteria, regardless of `ProcessedMail`.
|
||||
|
||||
- [ ] **Step 3: Implement the UID diff**
|
||||
|
||||
Replace `src/paperless_mail/mail.py:683-693`:
|
||||
|
||||
```python
|
||||
try:
|
||||
messages = M.fetch(
|
||||
criteria=criterias,
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
try:
|
||||
all_uids = set(
|
||||
M.uids(criteria=criterias, charset=rule.account.character_set),
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while searching folder {rule.folder}",
|
||||
) from err
|
||||
|
||||
processed_uids_qs = ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid__in=all_uids,
|
||||
)
|
||||
if self._current_uid_validity is not None:
|
||||
processed_uids_qs = processed_uids_qs.filter(
|
||||
Q(uid_validity=self._current_uid_validity)
|
||||
| Q(uid_validity__isnull=True),
|
||||
)
|
||||
processed_uids = set(processed_uids_qs.values_list("uid", flat=True))
|
||||
|
||||
new_uids = all_uids - processed_uids
|
||||
|
||||
if not new_uids:
|
||||
self.log.debug(
|
||||
f"Rule {rule}: No new mail matching criteria {criterias}",
|
||||
)
|
||||
return 0
|
||||
|
||||
try:
|
||||
messages = M.fetch(
|
||||
criteria=AND(uid=list(new_uids)),
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
```
|
||||
|
||||
Note the `uid_validity` handling here deliberately mirrors the existing per-message check at `mail.py:722-726` exactly (see Global Constraints) — the extra `Q(...)` filter is applied only when `self._current_uid_validity is not None`.
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_skips_body_fetch_for_already_processed_mail -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full `test_mail.py` suite (regression check, especially the uidvalidity tests)**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
|
||||
Expected: all PASS, including:
|
||||
|
||||
- `test_handle_mail_account_skip_duplicate_uids_from_fetch`
|
||||
- `test_handle_mail_account_skips_mail_already_processed_in_same_uidvalidity`
|
||||
- `test_handle_mail_account_processes_mail_after_uidvalidity_change`
|
||||
- `test_handle_mail_account_skips_mail_processed_before_uidvalidity_tracking`
|
||||
- `test_handle_mail_account_processes_mail_when_uidvalidity_unavailable`
|
||||
- `test_handle_mail_account_skips_mail_when_uidvalidity_unavailable_but_prior_record_exists`
|
||||
- `test_handle_mail_account_overlapping_rules_only_first_consumes`
|
||||
|
||||
If any of these fail, the `uid_validity` branching in Step 3 does not match `mail.py:722-726` closely enough — re-check against the Global Constraints note before changing test expectations.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
|
||||
git commit -m "fix: diff UIDs against ProcessedMail before fetching message bodies"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Batch the body fetch for large backlogs
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_mail/mail.py:74-76` (module-level constant)
|
||||
- Modify: `src/paperless_mail/mail.py` (the `M.fetch(...)` block added in Task 3)
|
||||
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Produces: module-level constant `paperless_mail.mail.MAIL_FETCH_BATCH_SIZE: int = 500`.
|
||||
- Consumes: `itertools` (already imported at `mail.py:2`), `sorted_new_uids` derived from Task 3's `new_uids` set.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py`. Add `from paperless_mail.mail import MAIL_FETCH_BATCH_SIZE` to the imports at the top of the file (alongside the other `from paperless_mail.mail import ...` lines at `test_mail.py:35-38`).
|
||||
|
||||
```python
|
||||
@mock.patch("paperless_mail.mail.MAIL_FETCH_BATCH_SIZE", 5)
|
||||
def test_handle_mail_account_batches_body_fetch_for_large_backlog(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- More new/unprocessed mail than MAIL_FETCH_BATCH_SIZE
|
||||
WHEN:
|
||||
- The mail account is processed
|
||||
THEN:
|
||||
- The body fetch is issued in multiple batches
|
||||
- Every message is still processed (none dropped at a batch boundary)
|
||||
"""
|
||||
account = MailAccount.objects.create(
|
||||
name="test",
|
||||
imap_server="",
|
||||
username="admin",
|
||||
password="secret",
|
||||
)
|
||||
rule = MailRule.objects.create(
|
||||
name="testrule",
|
||||
account=account,
|
||||
action=MailRule.MailAction.MARK_READ,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
|
||||
message_count = 12 # more than the patched batch size of 5
|
||||
self.mailMocker.bogus_mailbox.messages = [
|
||||
self.mailMocker.messageBuilder.create_message(
|
||||
subject=f"No attachment {i}",
|
||||
attachments=[],
|
||||
)
|
||||
for i in range(message_count)
|
||||
]
|
||||
self.mailMocker.bogus_mailbox.updateClient()
|
||||
|
||||
with mock.patch.object(
|
||||
self.mailMocker.bogus_mailbox,
|
||||
"fetch",
|
||||
wraps=self.mailMocker.bogus_mailbox.fetch,
|
||||
) as fetch_spy:
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
|
||||
# ceil(12 / 5) == 3 batches
|
||||
self.assertEqual(fetch_spy.call_count, 3)
|
||||
self.assertEqual(
|
||||
ProcessedMail.objects.filter(rule=rule).count(),
|
||||
message_count,
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_batches_body_fetch_for_large_backlog -v`
|
||||
Expected: FAIL — either an `ImportError` for `MAIL_FETCH_BATCH_SIZE` (doesn't exist yet) or, once that import is stubbed out, `fetch_spy.call_count == 1` instead of `3` (Task 3's implementation fetches all new UIDs in one call).
|
||||
|
||||
- [ ] **Step 3: Add the constant and batch the fetch**
|
||||
|
||||
Insert after `src/paperless_mail/mail.py:74` (right after the `APPLE_MAIL_TAG_COLORS` dict closes, before `class MailError`):
|
||||
|
||||
```python
|
||||
MAIL_FETCH_BATCH_SIZE = 500
|
||||
```
|
||||
|
||||
Replace the `M.fetch(...)` block added in Task 3 (Task 3 Step 3's final `try/except`):
|
||||
|
||||
```python
|
||||
try:
|
||||
messages = M.fetch(
|
||||
criteria=AND(uid=list(new_uids)),
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
sorted_new_uids = sorted(new_uids, key=int)
|
||||
message_batches = []
|
||||
for batch_start in range(0, len(sorted_new_uids), MAIL_FETCH_BATCH_SIZE):
|
||||
batch = sorted_new_uids[batch_start : batch_start + MAIL_FETCH_BATCH_SIZE]
|
||||
try:
|
||||
message_batches.append(
|
||||
M.fetch(
|
||||
criteria=AND(uid=batch),
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
),
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
|
||||
messages = itertools.chain(*message_batches)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_batches_body_fetch_for_large_backlog -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full `test_mail.py` suite**
|
||||
|
||||
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
|
||||
Expected: all PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
|
||||
git commit -m "perf: batch body fetches when many new UIDs are pending"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Full regression pass and lint
|
||||
|
||||
**Files:** none (verification only, plus any fixups this step surfaces)
|
||||
|
||||
- [ ] **Step 1: Run the full backend test suite with coverage/parallelism as configured**
|
||||
|
||||
Run: `uv run pytest src/paperless_mail/`
|
||||
Expected: all `paperless_mail` tests PASS
|
||||
|
||||
- [ ] **Step 2: Run the full project test suite**
|
||||
|
||||
Run: `uv run pytest`
|
||||
Expected: all tests PASS (no regressions outside `paperless_mail`)
|
||||
|
||||
- [ ] **Step 3: Lint and format**
|
||||
|
||||
Run: `uv run ruff check src/paperless_mail/` and `uv run ruff format --check src/paperless_mail/`
|
||||
Expected: no errors. If `ruff format --check` reports files needing formatting, run `uv run ruff format src/paperless_mail/` and re-check.
|
||||
|
||||
- [ ] **Step 4: Type check against the frozen baselines**
|
||||
|
||||
Run: `uv run mypy src/paperless_mail/mail.py` (or the project's standard mypy invocation) and confirm no new violations beyond `.mypy-baseline.txt`.
|
||||
Expected: no new errors introduced by this change.
|
||||
|
||||
- [ ] **Step 5: Commit any fixups**
|
||||
|
||||
If Steps 1-4 required changes:
|
||||
|
||||
```bash
|
||||
git add -u
|
||||
git commit -m "chore: fix lint/type fallout from mail fetch fix"
|
||||
```
|
||||
|
||||
If no changes were needed, skip this step — nothing to commit.
|
||||
@@ -1,216 +0,0 @@
|
||||
# Mail fetch fix: avoid re-downloading already-handled IMAP mail
|
||||
|
||||
## Problem
|
||||
|
||||
`paperless_mail`'s `_handle_mail_rule` (`src/paperless_mail/mail.py:641`) fetches
|
||||
the full RFC822 body of every message an IMAP `SEARCH` matches (via
|
||||
`M.fetch(criteria=criterias, ..., bulk=True)`), then does de-duplication in
|
||||
Python afterward. Two things compound into a real bug (reported upstream in
|
||||
[paperless-ngx#13175](https://github.com/paperless-ngx/paperless-ngx/issues/13175)):
|
||||
|
||||
1. **Full-body fetch happens before dedup.** The de-dup check against
|
||||
`ProcessedMail` (`mail.py:717-731`) happens per-message, after the body has
|
||||
already been downloaded for every matching message. Steady-state cost is
|
||||
_O(everything the search matches)_, not _O(new since last run)_.
|
||||
2. **Attachment-less mail under an attachments-only rule is never recorded.**
|
||||
`_handle_message` (`mail.py:749`) returns early, before any attachment
|
||||
processing, when a message has no attachments and
|
||||
`rule.consumption_scope == ATTACHMENTS_ONLY`:
|
||||
|
||||
```python
|
||||
if (
|
||||
not message.attachments
|
||||
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
|
||||
):
|
||||
return processed_elements # 0, no ProcessedMail row, no mail action applied
|
||||
```
|
||||
|
||||
No `ProcessedMail` row is written and no mail action (mark-read/flag/tag)
|
||||
is applied, since both only happen via `queue_consumption_tasks`, which is
|
||||
only reached from the attachment/eml processing paths. The message keeps
|
||||
matching the search (e.g. `UNSEEN`) forever.
|
||||
|
||||
Combined: an inbox where most mail has no attachments gets fully re-downloaded
|
||||
on every scheduled run (default every 10 minutes), indefinitely, for large
|
||||
mailboxes -- confirmed against the current codebase, not user
|
||||
misconfiguration.
|
||||
|
||||
## Goals
|
||||
|
||||
- Stop re-downloading full message bodies for mail that has already been
|
||||
handled (processed into a document, or determined to produce nothing).
|
||||
- Record `ProcessedMail` for the no-attachment / attachments-only early-return
|
||||
case, so it participates in dedup like every other terminal outcome.
|
||||
- No new required settings or DB migrations for existing installs.
|
||||
- Graceful behavior on first run against a large existing mailbox (no single
|
||||
giant IMAP command, no giant in-memory batch).
|
||||
- Preserve existing action/consumption semantics exactly for every path that
|
||||
currently produces a document or applies a mail action.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Changing what mail actions (mark-read/flag/tag/move/delete) get applied, or
|
||||
when.
|
||||
- Applying a mail action to the no-attachment early-return case (explicitly
|
||||
out of scope -- see Decisions).
|
||||
- Making batch size user-configurable.
|
||||
- Touching EML_ONLY / EVERYTHING consumption scopes' semantics (they already
|
||||
don't hit the early-return branch described above).
|
||||
|
||||
## Decisions
|
||||
|
||||
These were settled during brainstorming and are load-bearing for the design
|
||||
below:
|
||||
|
||||
- **Fix both halves together.** Recording `ProcessedMail` alone does not fix
|
||||
the bandwidth problem: without a mail action applied, an attachment-less
|
||||
message stays unseen and keeps matching the search criteria, so its body
|
||||
gets re-downloaded every run regardless of whether it gets reprocessed.
|
||||
Fixing only the bandwidth side without recording `ProcessedMail` would mean
|
||||
the UID-diff step never has anything to exclude for these messages. They
|
||||
must ship together.
|
||||
- **No mailbox action for the no-attachment case.** Only a `ProcessedMail`
|
||||
row is written; the message is not marked read/flagged/moved/deleted. This
|
||||
matches the existing precedent for other no-consumption outcomes
|
||||
(`mail.py:960-977`, e.g. an attachment present but filtered out or an
|
||||
unsupported mime type) and avoids changing the user's mailbox state for
|
||||
mail paperless previously never touched.
|
||||
- **UID-diff-before-fetch, not a BODYSTRUCTURE probe or SEARCH-side
|
||||
exclusion.** Considered and rejected:
|
||||
- BODYSTRUCTURE probing (fetch structure only, decide whether to fetch full
|
||||
body) only helps the attachments-only scope, not the general
|
||||
already-processed case, and `imap_tools` doesn't cleanly expose a
|
||||
structure-only fetch.
|
||||
- Excluding already-processed UIDs directly in the IMAP `SEARCH` criteria
|
||||
(`NOT UID (...)`) was rejected because for tens of thousands of
|
||||
already-processed UIDs the excluded-UID list itself blows up the command
|
||||
size -- worse than the two-step approach.
|
||||
- **Batch size is a hardcoded constant**, not a new setting, per the
|
||||
"no new required settings" goal.
|
||||
|
||||
## Design
|
||||
|
||||
### `_handle_mail_rule` (`mail.py:641`)
|
||||
|
||||
Replace the single `M.fetch(criteria=criterias, ...)` call with:
|
||||
|
||||
1. `self._current_uid_validity` computed as today (unchanged, already first).
|
||||
2. `criterias = make_criterias(...)` (unchanged).
|
||||
3. `all_uids = set(M.uids(criteria=criterias, charset=rule.account.character_set))`
|
||||
-- a `UID SEARCH`, no bodies.
|
||||
4. Query already-processed UIDs in one DB round trip, reusing the same
|
||||
uid_validity matching already used per-message:
|
||||
|
||||
```python
|
||||
processed_uids = set(
|
||||
ProcessedMail.objects.filter(
|
||||
rule=rule, folder=rule.folder, uid__in=all_uids,
|
||||
).filter(
|
||||
Q(uid_validity=self._current_uid_validity) | Q(uid_validity__isnull=True),
|
||||
).values_list("uid", flat=True)
|
||||
)
|
||||
```
|
||||
|
||||
5. `new_uids = all_uids - processed_uids`. If empty: log at debug level and
|
||||
`return 0` -- no body fetch at all. This is the steady-state case.
|
||||
6. Otherwise, iterate `new_uids` in fixed-size batches of
|
||||
`MAIL_FETCH_BATCH_SIZE = 500` (module-level constant), calling
|
||||
`M.fetch(criteria=AND(uid=batch), mark_seen=False, charset=rule.account.character_set, bulk=True)`
|
||||
per batch. Chain the resulting message iterators into the existing
|
||||
per-message loop (`mail.py:699+`) unchanged.
|
||||
7. The existing per-message `already_processed` DB check
|
||||
(`mail.py:717-731`) stays in place unchanged, as a safety net against
|
||||
races (e.g. concurrent rule runs against the same folder in this pass) and
|
||||
to keep behavior identical if steps 3/4 ever disagree with it.
|
||||
|
||||
### New helper: `_record_processed_without_consumption`
|
||||
|
||||
Extract the existing dedup-and-create block at `mail.py:960-977` into a
|
||||
method on the same class:
|
||||
|
||||
```python
|
||||
def _record_processed_without_consumption(self, message, rule) -> None:
|
||||
if not ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
uid_validity=self._current_uid_validity,
|
||||
).exists():
|
||||
ProcessedMail.objects.create(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid=message.uid,
|
||||
uid_validity=self._current_uid_validity,
|
||||
subject=message.subject,
|
||||
received=make_aware(message.date) if is_naive(message.date) else message.date,
|
||||
status="PROCESSED_WO_CONSUMPTION",
|
||||
)
|
||||
```
|
||||
|
||||
Call sites:
|
||||
|
||||
- `_process_attachments`'s existing no-consumables branch (replaces the
|
||||
inline block, same behavior).
|
||||
- `_handle_message`'s early return (`mail.py:756-760`), newly, for the
|
||||
no-attachments-under-attachments-only-scope case.
|
||||
|
||||
### Data flow (steady state, nothing new)
|
||||
|
||||
search criteria (unchanged) -> `UID SEARCH` for matching UIDs (cheap) ->
|
||||
subtract UIDs already in `ProcessedMail` (cheap, DB-only) -> zero new UIDs ->
|
||||
no fetch, no body download.
|
||||
|
||||
### Data flow (new mail present)
|
||||
|
||||
... same as above -> some UIDs remain -> fetch bodies only for those,
|
||||
batched -> existing per-message processing, action application, and
|
||||
`ProcessedMail` recording, unchanged except the no-attachment early return
|
||||
now also calls `_record_processed_without_consumption`.
|
||||
|
||||
## Error handling
|
||||
|
||||
- `M.uids(...)` raising is wrapped in the same `try/except` that currently
|
||||
wraps `M.fetch(...)` (`mail.py:683-693`), surfacing as `MailError`
|
||||
identically to today's search-failure behavior.
|
||||
- A batch's `M.fetch(...)` raising mid-loop is caught by the same
|
||||
`try/except`, applied per-batch. One failing batch fails the whole rule
|
||||
run for this pass -- matching today's all-or-nothing semantics (currently
|
||||
a single failed fetch already fails the whole rule).
|
||||
- Empty `new_uids` short-circuits before any fetch, so it can't hit fetch
|
||||
error paths at all for the common case.
|
||||
- `_record_processed_without_consumption` reuses the existing
|
||||
`ProcessedMail.objects.create` call already in production use; no new
|
||||
failure mode introduced.
|
||||
|
||||
## Testing
|
||||
|
||||
In `src/paperless_mail/tests/test_mail.py`:
|
||||
|
||||
- Extend the `BogusMailBox` test double with a `uids()` method that mirrors
|
||||
its existing `fetch()` criteria matching but returns only UIDs.
|
||||
- New test: an attachment-less message under an attachments-only mark-read
|
||||
rule is recorded as `PROCESSED_WO_CONSUMPTION` on the first run. On a
|
||||
second run, with the same message still unseen server-side, assert no
|
||||
body fetch happens (only `M.uids()` is called) -- i.e. steady state costs
|
||||
one search and zero body downloads.
|
||||
- New test: a folder with more new UIDs than `MAIL_FETCH_BATCH_SIZE` results
|
||||
in multiple batched `fetch` calls, and all messages are still processed
|
||||
(no messages dropped at a batch boundary).
|
||||
- Update existing `test_handle_empty_message` to assert a
|
||||
`PROCESSED_WO_CONSUMPTION` row now exists for the no-attachment case.
|
||||
- Regression: existing tests for `EML_ONLY`/`EVERYTHING` scopes and the
|
||||
multi-rule-same-folder dedup (`consumed_messages` set) should pass
|
||||
unchanged, since neither the per-message `already_processed` check nor the
|
||||
`consumed_messages` set logic is touched by this design.
|
||||
|
||||
## Open questions / risks
|
||||
|
||||
- `imap_tools`'s `AND(uid=batch)` criteria builder needs to be confirmed to
|
||||
produce a valid `UID FETCH <list>` command at the batch sizes used here;
|
||||
covered by the batching test above.
|
||||
- If a mail server doesn't support `UID SEARCH` the same way it supports the
|
||||
existing `fetch`'s implicit search, `M.uids()` could behave differently
|
||||
from today's `M.fetch()` on some edge-case server. Existing project test
|
||||
coverage (via `BogusMailBox`) won't catch server-specific quirks; this is
|
||||
the same class of risk as any IMAP-behavior assumption already baked into
|
||||
this module.
|
||||
@@ -1,5 +1,6 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import path from 'node:path'
|
||||
import { mockFilterSelectionData } from '../mock-filter-selection-data'
|
||||
|
||||
const REQUESTS_HAR = path.join(__dirname, 'requests/api-settings.har')
|
||||
|
||||
@@ -7,6 +8,7 @@ test('should activate / deactivate save button when settings change', async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/settings')
|
||||
await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled()
|
||||
await page.getByLabel('Use system setting').click()
|
||||
@@ -16,6 +18,7 @@ test('should activate / deactivate save button when settings change', async ({
|
||||
|
||||
test('should warn on unsaved changes', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/settings')
|
||||
await page.getByLabel('Use system setting').click()
|
||||
await page.getByRole('link', { name: 'Dashboard' }).click()
|
||||
@@ -28,6 +31,7 @@ test('should warn on unsaved changes', async ({ page }) => {
|
||||
|
||||
test('should apply appearance changes when set', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/settings')
|
||||
await expect(page.locator('html')).toHaveAttribute('data-bs-theme', /auto/)
|
||||
await page.getByLabel('Use system setting').click()
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import path from 'node:path'
|
||||
import { mockFilterSelectionData } from '../mock-filter-selection-data'
|
||||
|
||||
const REQUESTS_HAR1 = path.join(__dirname, 'requests/api-dashboard1.har')
|
||||
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-dashboard2.har')
|
||||
@@ -8,6 +9,7 @@ const REQUESTS_HAR4 = path.join(__dirname, 'requests/api-dashboard4.har')
|
||||
|
||||
test('dashboard inbox link', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await page.getByRole('link', { name: 'Documents in inbox' }).click()
|
||||
await expect(page).toHaveURL(/tags__id__in=9/)
|
||||
@@ -16,6 +18,7 @@ test('dashboard inbox link', async ({ page }) => {
|
||||
|
||||
test('dashboard total documents link', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await page.getByRole('link').filter({ hasText: 'Total documents' }).click()
|
||||
await expect(page).toHaveURL(/documents/)
|
||||
@@ -25,6 +28,7 @@ test('dashboard total documents link', async ({ page }) => {
|
||||
|
||||
test('dashboard saved view show all', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR3, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await page
|
||||
.locator('pngx-widget-frame')
|
||||
@@ -38,6 +42,7 @@ test('dashboard saved view show all', async ({ page }) => {
|
||||
|
||||
test('dashboard saved view document links', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR4, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await page
|
||||
.locator('pngx-widget-frame')
|
||||
@@ -51,6 +56,7 @@ test('dashboard saved view document links', async ({ page }) => {
|
||||
|
||||
test('test slim sidebar', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await page.locator('.sidebar-slim-toggler').click()
|
||||
await expect(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import path from 'node:path'
|
||||
import { mockFilterSelectionData } from '../mock-filter-selection-data'
|
||||
|
||||
const REQUESTS_HAR = path.join(__dirname, 'requests/api-document-detail.har')
|
||||
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-document-detail2.har')
|
||||
@@ -8,6 +9,7 @@ test('should activate / deactivate save button when changes are saved', async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/')
|
||||
await page.waitForSelector('pngx-document-detail pngx-input-text:first-child')
|
||||
await expect(page.getByTitle('Storage path', { exact: true })).toHaveText(
|
||||
@@ -20,6 +22,7 @@ test('should activate / deactivate save button when changes are saved', async ({
|
||||
|
||||
test('should warn on unsaved changes', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/')
|
||||
await expect(page.getByTitle('Correspondent', { exact: true })).toHaveText(
|
||||
/\w+/
|
||||
@@ -39,6 +42,7 @@ test('should warn on unsaved changes', async ({ page }) => {
|
||||
|
||||
test('should support tab direct navigation', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/details')
|
||||
await expect(page.getByRole('tab', { name: 'Details' })).toHaveAttribute(
|
||||
'aria-selected',
|
||||
@@ -68,6 +72,7 @@ test('should support tab direct navigation', async ({ page }) => {
|
||||
|
||||
test('should show a mobile preview', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/')
|
||||
await page.setViewportSize({ width: 400, height: 1000 })
|
||||
await expect(page.getByRole('tab', { name: 'Preview' })).toBeVisible()
|
||||
@@ -77,6 +82,7 @@ test('should show a mobile preview', async ({ page }) => {
|
||||
|
||||
test('should show a list of notes', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/notes')
|
||||
await expect(page.locator('pngx-document-notes')).toBeVisible()
|
||||
await expect(
|
||||
@@ -89,6 +95,7 @@ test('should show a list of notes', async ({ page }) => {
|
||||
|
||||
test('should support quick filters', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents/175/details')
|
||||
await page
|
||||
.getByRole('button', { name: 'Filter documents with these Tags' })
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import path from 'node:path'
|
||||
import { mockFilterSelectionData } from '../mock-filter-selection-data'
|
||||
|
||||
const REQUESTS_HAR1 = path.join(__dirname, 'requests/api-document-list1.har')
|
||||
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-document-list2.har')
|
||||
@@ -10,6 +11,7 @@ const REQUESTS_HAR6 = path.join(__dirname, 'requests/api-document-list6.har')
|
||||
|
||||
test('basic filtering', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
await page.getByRole('button', { name: 'Tags' }).click()
|
||||
await page.getByRole('menuitem', { name: 'Inbox' }).click()
|
||||
@@ -45,6 +47,7 @@ test('basic filtering', async ({ page }) => {
|
||||
|
||||
test('text filtering', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
await page.getByRole('main').getByRole('combobox').click()
|
||||
await page.getByRole('main').getByRole('combobox').fill('test')
|
||||
@@ -81,6 +84,7 @@ test('text filtering', async ({ page }) => {
|
||||
|
||||
test('date filtering', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR3, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
await page.getByRole('button', { name: 'Dates' }).click()
|
||||
await page.locator('.ng-arrow-wrapper').first().click()
|
||||
@@ -103,6 +107,7 @@ test('date filtering', async ({ page }) => {
|
||||
|
||||
test('sorting', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR4, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
await page.getByRole('button', { name: 'Sort' }).click()
|
||||
await page.getByRole('button', { name: 'ASN' }).click()
|
||||
@@ -141,6 +146,7 @@ test('sorting', async ({ page }) => {
|
||||
|
||||
test('change views', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR5, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
await page.locator('.btn-group > label').first().click()
|
||||
await expect(page.locator('pngx-document-list table')).toBeVisible()
|
||||
@@ -152,6 +158,7 @@ test('change views', async ({ page }) => {
|
||||
|
||||
test('bulk edit', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR6, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/documents')
|
||||
|
||||
await page.locator('pngx-document-card-small').nth(0).click()
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import { Page } from '@playwright/test'
|
||||
|
||||
const EMPTY_SELECTION_DATA = {
|
||||
selected_correspondents: [],
|
||||
selected_tags: [],
|
||||
selected_document_types: [],
|
||||
selected_storage_paths: [],
|
||||
selected_custom_fields: [],
|
||||
}
|
||||
|
||||
/**
|
||||
* The document list now fires a GET to filter_selection_data on every
|
||||
* non-search reload(), independent of and concurrent with the main list
|
||||
* request. It's not present in any of the recorded HAR fixtures, so with
|
||||
* `notFound: 'fallback'` it would otherwise fall through to the real
|
||||
* network (nothing listens there in e2e, since only the frontend dev
|
||||
* server is started) and fail every test that reloads the list.
|
||||
*
|
||||
* Playwright checks routes in reverse-registration order, so this must be
|
||||
* registered before a test's own page.routeFromHAR() call for the HAR
|
||||
* route's `notFound: 'fallback'` to defer back to this one.
|
||||
*/
|
||||
export async function mockFilterSelectionData(page: Page) {
|
||||
await page.route('**/api/documents/filter_selection_data/**', (route) =>
|
||||
route.fulfill({
|
||||
json: EMPTY_SELECTION_DATA,
|
||||
// The app calls the (cross-origin, from the e2e app's perspective)
|
||||
// backend at http://localhost:8000 while served from :4200, so a
|
||||
// fulfilled response needs the same CORS header the real backend
|
||||
// sends (and that recorded HAR responses already carry) or the
|
||||
// browser rejects it as a cross-origin failure.
|
||||
headers: { 'Access-Control-Allow-Origin': 'http://localhost:4200' },
|
||||
})
|
||||
)
|
||||
}
|
||||
@@ -1,10 +1,12 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import path from 'node:path'
|
||||
import { mockFilterSelectionData } from '../mock-filter-selection-data'
|
||||
|
||||
const REQUESTS_HAR = path.join(__dirname, 'requests/api-global-permissions.har')
|
||||
|
||||
test('should not allow user to edit settings', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(page.getByRole('link', { name: 'Settings' })).not.toBeAttached()
|
||||
await page.goto('/settings')
|
||||
@@ -15,6 +17,7 @@ test('should not allow user to edit settings', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view documents', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(
|
||||
page.locator('nav').getByRole('link', { name: 'Documents' })
|
||||
@@ -31,6 +34,7 @@ test('should not allow user to view documents', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view correspondents', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(
|
||||
page.getByRole('link', { name: 'Attributes' })
|
||||
@@ -43,6 +47,7 @@ test('should not allow user to view correspondents', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view tags', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(
|
||||
page.getByRole('link', { name: 'Attributes' })
|
||||
@@ -55,6 +60,7 @@ test('should not allow user to view tags', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view document types', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(
|
||||
page.getByRole('link', { name: 'Attributes' })
|
||||
@@ -67,6 +73,7 @@ test('should not allow user to view document types', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view storage paths', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(
|
||||
page.getByRole('link', { name: 'Attributes' })
|
||||
@@ -79,6 +86,7 @@ test('should not allow user to view storage paths', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view logs', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(page.getByRole('link', { name: 'Logs' })).not.toBeAttached()
|
||||
await page.goto('/logs')
|
||||
@@ -89,6 +97,7 @@ test('should not allow user to view logs', async ({ page }) => {
|
||||
|
||||
test('should not allow user to view tasks', async ({ page }) => {
|
||||
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
|
||||
await mockFilterSelectionData(page)
|
||||
await page.goto('/dashboard')
|
||||
await expect(page.getByRole('link', { name: 'Tasks' })).not.toBeAttached()
|
||||
await page.goto('/tasks')
|
||||
|
||||
@@ -191,6 +191,14 @@ describe('BulkEditorComponent', () => {
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
// A filter_selection_data request now fires concurrently with every
|
||||
// non-search reload(), independent of whether a given test flushes or
|
||||
// even inspects the primary list response. Drain any left unclaimed.
|
||||
httpTestingController.match(
|
||||
(request) =>
|
||||
request.url ===
|
||||
`${environment.apiBaseUrl}documents/filter_selection_data/`
|
||||
)
|
||||
httpTestingController.verify()
|
||||
})
|
||||
|
||||
@@ -386,7 +394,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { add_tags: [101], remove_tags: [] },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -432,7 +440,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { add_tags: [101], remove_tags: [] },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
})
|
||||
|
||||
@@ -461,7 +469,7 @@ describe('BulkEditorComponent', () => {
|
||||
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
|
||||
.flush(true)
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -552,7 +560,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { correspondent: 101 },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -584,7 +592,7 @@ describe('BulkEditorComponent', () => {
|
||||
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
|
||||
.flush(true)
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -650,7 +658,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { document_type: 101 },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -682,7 +690,7 @@ describe('BulkEditorComponent', () => {
|
||||
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
|
||||
.flush(true)
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -748,7 +756,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { storage_path: 101 },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -780,7 +788,7 @@ describe('BulkEditorComponent', () => {
|
||||
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
|
||||
.flush(true)
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -846,7 +854,7 @@ describe('BulkEditorComponent', () => {
|
||||
parameters: { add_custom_fields: [101], remove_custom_fields: [102] },
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -878,7 +886,7 @@ describe('BulkEditorComponent', () => {
|
||||
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
|
||||
.flush(true)
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -987,7 +995,7 @@ describe('BulkEditorComponent', () => {
|
||||
documents: [3, 4],
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1080,7 +1088,7 @@ describe('BulkEditorComponent', () => {
|
||||
documents: [3, 4],
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1115,7 +1123,7 @@ describe('BulkEditorComponent', () => {
|
||||
source_mode: 'latest_version',
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1156,7 +1164,7 @@ describe('BulkEditorComponent', () => {
|
||||
metadata_document_id: 3,
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1175,7 +1183,7 @@ describe('BulkEditorComponent', () => {
|
||||
delete_originals: true,
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1196,7 +1204,7 @@ describe('BulkEditorComponent', () => {
|
||||
archive_fallback: true,
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1299,7 +1307,7 @@ describe('BulkEditorComponent', () => {
|
||||
},
|
||||
})
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
@@ -1607,7 +1615,7 @@ describe('BulkEditorComponent', () => {
|
||||
expect(toastServiceShowInfoSpy).toHaveBeenCalled()
|
||||
expect(listReloadSpy).toHaveBeenCalled()
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
) // list reload
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
|
||||
|
||||
@@ -84,6 +84,28 @@ const view: SavedView = {
|
||||
filter_rules: filterRules,
|
||||
}
|
||||
|
||||
const emptySelectionData = {
|
||||
selected_correspondents: [],
|
||||
selected_tags: [],
|
||||
selected_document_types: [],
|
||||
selected_storage_paths: [],
|
||||
selected_custom_fields: [],
|
||||
}
|
||||
|
||||
// A successful (non-search) list response now triggers a separate,
|
||||
// non-blocking request for filter dropdown counts. Tests that flush a
|
||||
// successful list response need to also flush this follow-up request.
|
||||
function flushSelectionDataRequest(
|
||||
httpTestingController: HttpTestingController,
|
||||
querySuffix: string = ''
|
||||
) {
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/filter_selection_data/${querySuffix}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(emptySelectionData)
|
||||
}
|
||||
|
||||
describe('DocumentListViewService', () => {
|
||||
let httpTestingController: HttpTestingController
|
||||
let documentListViewService: DocumentListViewService
|
||||
@@ -105,6 +127,7 @@ describe('DocumentListViewService', () => {
|
||||
})
|
||||
|
||||
sessionStorage.clear()
|
||||
localStorage.clear()
|
||||
httpTestingController = TestBed.inject(HttpTestingController)
|
||||
documentListViewService = TestBed.inject(DocumentListViewService)
|
||||
settingsService = TestBed.inject(SettingsService)
|
||||
@@ -114,8 +137,19 @@ describe('DocumentListViewService', () => {
|
||||
|
||||
afterEach(() => {
|
||||
documentListViewService.cancelPending()
|
||||
// A filter_selection_data request now fires concurrently with every
|
||||
// non-search reload(), independent of whether the test cares about or
|
||||
// flushes the primary list response. Drain any that a test didn't
|
||||
// explicitly claim via flushSelectionDataRequest, so unrelated tests
|
||||
// don't have to know about this follow-up request to pass verify().
|
||||
httpTestingController.match(
|
||||
(request) =>
|
||||
request.url ===
|
||||
`${environment.apiBaseUrl}documents/filter_selection_data/`
|
||||
)
|
||||
httpTestingController.verify()
|
||||
sessionStorage.clear()
|
||||
localStorage.clear()
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
@@ -128,10 +162,11 @@ describe('DocumentListViewService', () => {
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
documentListViewService.reload()
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.isReloading).toBeFalsy()
|
||||
expect(documentListViewService.activeSavedViewId).toBeNull()
|
||||
@@ -143,12 +178,12 @@ describe('DocumentListViewService', () => {
|
||||
it('should handle error on page request out of range', () => {
|
||||
documentListViewService.currentPage = 50
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush([], { status: 404, statusText: 'Unexpected error' })
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
@@ -165,21 +200,20 @@ describe('DocumentListViewService', () => {
|
||||
]
|
||||
documentListViewService.setFilterRules(filterRulesAny)
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(
|
||||
{ archive_serial_number: 'hello' },
|
||||
{ status: 404, statusText: 'Unexpected error' }
|
||||
)
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
// the error is a plain field error (not a page-out-of-range or deleted
|
||||
// custom-field-sort case), so no automatic retry request is sent here
|
||||
expect(documentListViewService.error).toBeTruthy()
|
||||
// reset the list
|
||||
documentListViewService.setFilterRules([])
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -187,7 +221,7 @@ describe('DocumentListViewService', () => {
|
||||
documentListViewService.currentPage = 1
|
||||
documentListViewService.sortField = 'custom_field_999'
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(
|
||||
@@ -196,7 +230,7 @@ describe('DocumentListViewService', () => {
|
||||
)
|
||||
// resets itself
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -211,7 +245,7 @@ describe('DocumentListViewService', () => {
|
||||
]
|
||||
documentListViewService.setFilterRules(filterRulesAny)
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush('Generic error', { status: 404, statusText: 'Unexpected error' })
|
||||
@@ -219,7 +253,7 @@ describe('DocumentListViewService', () => {
|
||||
// reset the list
|
||||
documentListViewService.setFilterRules([])
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -228,7 +262,7 @@ describe('DocumentListViewService', () => {
|
||||
expect(documentListViewService.sortReverse).toBeTruthy()
|
||||
documentListViewService.setSort('added', false)
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.sortField).toEqual('added')
|
||||
@@ -236,12 +270,12 @@ describe('DocumentListViewService', () => {
|
||||
|
||||
documentListViewService.sortField = 'created'
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(documentListViewService.sortField).toEqual('created')
|
||||
documentListViewService.sortReverse = true
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.sortReverse).toBeTruthy()
|
||||
@@ -284,7 +318,7 @@ describe('DocumentListViewService', () => {
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${
|
||||
documentListViewService.pageSize
|
||||
}&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=true`
|
||||
}&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.currentPage).toEqual(page)
|
||||
@@ -301,7 +335,7 @@ describe('DocumentListViewService', () => {
|
||||
}
|
||||
documentListViewService.loadFromQueryParams(convertToParamMap(params))
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
expect(documentListViewService.filterRules).toEqual([
|
||||
@@ -311,12 +345,16 @@ describe('DocumentListViewService', () => {
|
||||
},
|
||||
])
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(
|
||||
httpTestingController,
|
||||
`?tags__id__all=${tags__id__all}`
|
||||
)
|
||||
})
|
||||
|
||||
it('should use filter rules to update query params', () => {
|
||||
documentListViewService.setFilterRules(filterRules)
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
})
|
||||
@@ -325,26 +363,31 @@ describe('DocumentListViewService', () => {
|
||||
documentListViewService.currentPage = 2
|
||||
let req = httpTestingController.expectOne((request) =>
|
||||
request.urlWithParams.startsWith(
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
|
||||
documentListViewService.setFilterRules(filterRules, true)
|
||||
|
||||
const filteredReqs = httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(filteredReqs).toHaveLength(1)
|
||||
filteredReqs[0].flush(full_results)
|
||||
flushSelectionDataRequest(
|
||||
httpTestingController,
|
||||
`?tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
})
|
||||
|
||||
it('should support quick filter', () => {
|
||||
documentListViewService.quickFilter(filterRules)
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
|
||||
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
})
|
||||
@@ -367,21 +410,21 @@ describe('DocumentListViewService', () => {
|
||||
convertToParamMap(params)
|
||||
)
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
|
||||
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
// reset the list
|
||||
documentListViewService.currentPage = 1
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=9`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=9`
|
||||
)
|
||||
documentListViewService.setFilterRules([])
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
documentListViewService.sortField = 'created'
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
documentListViewService.activateSavedView(null)
|
||||
})
|
||||
@@ -389,18 +432,22 @@ describe('DocumentListViewService', () => {
|
||||
it('should support navigating next / previous', () => {
|
||||
documentListViewService.setFilterRules([])
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
documentListViewService.pageSize = 3
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush({
|
||||
count: 3,
|
||||
results: documents.slice(0, 3),
|
||||
})
|
||||
// two reload()s ran above (setFilterRules, then pageSize), each firing
|
||||
// its own concurrent filter_selection_data request with an identical
|
||||
// (unfiltered) URL; this test doesn't assert on selectionData, so let
|
||||
// afterEach's drain step clean both up rather than disambiguating here.
|
||||
expect(documentListViewService.hasNext(documents[0].id)).toBeTruthy()
|
||||
expect(documentListViewService.hasPrevious(documents[0].id)).toBeFalsy()
|
||||
documentListViewService.getNext(documents[0].id).subscribe((docId) => {
|
||||
@@ -447,7 +494,7 @@ describe('DocumentListViewService', () => {
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
documentListViewService.pageSize = 3
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
jest
|
||||
.spyOn(documentListViewService, 'getLastPage')
|
||||
@@ -462,7 +509,7 @@ describe('DocumentListViewService', () => {
|
||||
expect(reloadSpy).toHaveBeenCalled()
|
||||
expect(documentListViewService.currentPage).toEqual(2)
|
||||
const reqs = httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(reqs.length).toBeGreaterThan(0)
|
||||
})
|
||||
@@ -497,11 +544,11 @@ describe('DocumentListViewService', () => {
|
||||
.mockReturnValue(documents)
|
||||
documentListViewService.currentPage = 2
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
documentListViewService.pageSize = 3
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
const reloadSpy = jest.spyOn(documentListViewService, 'reload')
|
||||
documentListViewService.getPrevious(1).subscribe({
|
||||
@@ -511,7 +558,7 @@ describe('DocumentListViewService', () => {
|
||||
expect(reloadSpy).toHaveBeenCalled()
|
||||
expect(documentListViewService.currentPage).toEqual(1)
|
||||
const reqs = httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(reqs.length).toBeGreaterThan(0)
|
||||
})
|
||||
@@ -524,10 +571,11 @@ describe('DocumentListViewService', () => {
|
||||
it('should support select a document', () => {
|
||||
documentListViewService.reload()
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
documentListViewService.toggleSelected(documents[0])
|
||||
expect(documentListViewService.isSelected(documents[0])).toBeTruthy()
|
||||
documentListViewService.toggleSelected(documents[0])
|
||||
@@ -537,10 +585,11 @@ describe('DocumentListViewService', () => {
|
||||
it('should support select all', () => {
|
||||
documentListViewService.reload()
|
||||
const reloadReq = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(reloadReq.request.method).toEqual('GET')
|
||||
reloadReq.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
|
||||
documentListViewService.selectAll()
|
||||
expect(documentListViewService.allSelected).toBeTruthy()
|
||||
@@ -553,13 +602,14 @@ describe('DocumentListViewService', () => {
|
||||
it('should support select page', () => {
|
||||
documentListViewService.pageSize = 3
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush({
|
||||
count: 3,
|
||||
results: documents.slice(0, 3),
|
||||
})
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
documentListViewService.selectPage()
|
||||
expect(documentListViewService.selected.size).toEqual(3)
|
||||
expect(documentListViewService.isSelected(documents[5])).toBeFalsy()
|
||||
@@ -568,10 +618,11 @@ describe('DocumentListViewService', () => {
|
||||
it('should support select range', () => {
|
||||
documentListViewService.reload()
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
documentListViewService.toggleSelected(documents[0])
|
||||
expect(documentListViewService.isSelected(documents[0])).toBeTruthy()
|
||||
documentListViewService.selectRangeTo(documents[2])
|
||||
@@ -583,9 +634,10 @@ describe('DocumentListViewService', () => {
|
||||
it('should clear all-selected mode when toggling a single document', () => {
|
||||
documentListViewService.reload()
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
|
||||
documentListViewService.selectAll()
|
||||
expect(documentListViewService.allSelected).toBeTruthy()
|
||||
@@ -599,9 +651,10 @@ describe('DocumentListViewService', () => {
|
||||
it('should clear all-selected mode when selecting a range', () => {
|
||||
documentListViewService.reload()
|
||||
const req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
|
||||
documentListViewService.selectAll()
|
||||
documentListViewService.toggleSelected(documents[1])
|
||||
@@ -619,22 +672,24 @@ describe('DocumentListViewService', () => {
|
||||
it('should support selection range reduction', () => {
|
||||
documentListViewService.reload()
|
||||
let req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(req.request.method).toEqual('GET')
|
||||
req.flush(full_results)
|
||||
flushSelectionDataRequest(httpTestingController)
|
||||
|
||||
documentListViewService.selectAll()
|
||||
expect(documentListViewService.selected.size).toEqual(6)
|
||||
|
||||
documentListViewService.setFilterRules(filterRules)
|
||||
req = httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=9`
|
||||
)
|
||||
req.flush({
|
||||
count: 3,
|
||||
results: documents.slice(0, 3),
|
||||
})
|
||||
flushSelectionDataRequest(httpTestingController, '?tags__id__all=9')
|
||||
expect(documentListViewService.allSelected).toBeTruthy()
|
||||
expect(documentListViewService.selected.size).toEqual(3)
|
||||
})
|
||||
@@ -643,7 +698,7 @@ describe('DocumentListViewService', () => {
|
||||
const cancelSpy = jest.spyOn(documentListViewService, 'cancelPending')
|
||||
documentListViewService.reload()
|
||||
httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(cancelSpy).toHaveBeenCalled()
|
||||
})
|
||||
@@ -662,7 +717,7 @@ describe('DocumentListViewService', () => {
|
||||
documentListViewService.setFilterRules([])
|
||||
expect(documentListViewService.sortField).toEqual('created')
|
||||
httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -689,11 +744,11 @@ describe('DocumentListViewService', () => {
|
||||
expect(localStorageSpy).toHaveBeenCalled()
|
||||
// reload triggered
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
documentListViewService.displayFields = null
|
||||
httpTestingController.match(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
expect(documentListViewService.displayFields).toEqual(
|
||||
DEFAULT_DISPLAY_FIELDS.filter((f) => f.id !== DisplayField.ADDED).map(
|
||||
@@ -738,7 +793,7 @@ describe('DocumentListViewService', () => {
|
||||
it('should generate quick filter URL preserving default state', () => {
|
||||
documentListViewService.reload()
|
||||
httpTestingController.expectOne(
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
|
||||
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
|
||||
)
|
||||
const urlTree = documentListViewService.getQuickFilterUrl(filterRules)
|
||||
expect(urlTree).toBeDefined()
|
||||
|
||||
@@ -314,12 +314,39 @@ export class DocumentListViewService {
|
||||
}
|
||||
}
|
||||
|
||||
private loadFilterSelectionData(filterRules: FilterRule[]) {
|
||||
this.documentService
|
||||
.getFilterSelectionData(filterRules)
|
||||
.pipe(takeUntil(this.unsubscribeNotifier))
|
||||
.subscribe({
|
||||
next: (selectionData) => {
|
||||
this.selectionData = selectionData
|
||||
this.markChanged()
|
||||
},
|
||||
error: () => {
|
||||
this.selectionData = null
|
||||
this.markChanged()
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
reload(onFinish?, updateQueryParams: boolean = true) {
|
||||
this.cancelPending()
|
||||
this.isReloading = true
|
||||
this.error = null
|
||||
this.markChanged()
|
||||
let activeListViewState = this.activeListViewState
|
||||
// Full-text search results are already narrowed by the search backend, so
|
||||
// computing selection data inline there is cheap. A plain (unfiltered or
|
||||
// ORM-filtered) browse can span the entire document set, so its selection
|
||||
// data is fetched separately -- concurrently with the list itself, rather
|
||||
// than blocking or waiting on it.
|
||||
const isFullTextSearch = isFullTextFilterRule(
|
||||
activeListViewState.filterRules
|
||||
)
|
||||
if (!isFullTextSearch) {
|
||||
this.loadFilterSelectionData(activeListViewState.filterRules)
|
||||
}
|
||||
this.documentService
|
||||
.listFiltered(
|
||||
activeListViewState.currentPage,
|
||||
@@ -327,17 +354,22 @@ export class DocumentListViewService {
|
||||
activeListViewState.sortField,
|
||||
activeListViewState.sortReverse,
|
||||
activeListViewState.filterRules,
|
||||
{ truncate_content: true, include_selection_data: true }
|
||||
{
|
||||
truncate_content: true,
|
||||
include_selection_data: isFullTextSearch,
|
||||
}
|
||||
)
|
||||
.pipe(takeUntil(this.unsubscribeNotifier))
|
||||
.subscribe({
|
||||
next: (result) => {
|
||||
const resultWithSelectionData = result as DocumentResults
|
||||
this.initialized = true
|
||||
this.isReloading = false
|
||||
activeListViewState.collectionSize = result.count
|
||||
activeListViewState.documents = result.results
|
||||
this.selectionData = resultWithSelectionData.selection_data ?? null
|
||||
if (isFullTextSearch) {
|
||||
this.selectionData =
|
||||
(result as DocumentResults).selection_data ?? null
|
||||
}
|
||||
this.syncSelectedToCurrentPage()
|
||||
this.markChanged()
|
||||
|
||||
@@ -376,6 +408,9 @@ export class DocumentListViewService {
|
||||
// e.g. field was deleted
|
||||
this.sortField = 'created'
|
||||
} else {
|
||||
// cancel the concurrently-fired selection-data request too, so it
|
||||
// can't resolve afterward and clobber this reset with stale data
|
||||
this.cancelPending()
|
||||
this.selectionData = null
|
||||
let errorMessage
|
||||
if (
|
||||
|
||||
@@ -41,6 +41,24 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges a plain params object into an HttpParams instance, skipping
|
||||
* null/undefined values so they aren't serialized as literal "null" /
|
||||
* "undefined" query string entries.
|
||||
*/
|
||||
protected withParams(
|
||||
params,
|
||||
base: HttpParams = new HttpParams()
|
||||
): HttpParams {
|
||||
let httpParams = base
|
||||
for (let key in params) {
|
||||
if (params[key] != null) {
|
||||
httpParams = httpParams.set(key, params[key])
|
||||
}
|
||||
}
|
||||
return httpParams
|
||||
}
|
||||
|
||||
list(
|
||||
page?: number,
|
||||
pageSize?: number,
|
||||
@@ -60,11 +78,7 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
|
||||
if (ordering) {
|
||||
httpParams = httpParams.set('ordering', ordering)
|
||||
}
|
||||
for (let extraParamKey in extraParams) {
|
||||
if (extraParams[extraParamKey] != null) {
|
||||
httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey])
|
||||
}
|
||||
}
|
||||
httpParams = this.withParams(extraParams, httpParams)
|
||||
return this.http
|
||||
.get<Results<T>>(this.getResourceUrl(), {
|
||||
params: httpParams,
|
||||
@@ -113,11 +127,7 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
|
||||
httpParams = httpParams.set('id__in', ids.join(','))
|
||||
httpParams = httpParams.set('ordering', '-id')
|
||||
httpParams = httpParams.set('page_size', 1000)
|
||||
for (let extraParamKey in extraParams) {
|
||||
if (extraParams[extraParamKey] != null) {
|
||||
httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey])
|
||||
}
|
||||
}
|
||||
httpParams = this.withParams(extraParams, httpParams)
|
||||
return this.http
|
||||
.get<Results<T>>(this.getResourceUrl(), {
|
||||
params: httpParams,
|
||||
|
||||
@@ -398,6 +398,13 @@ export class DocumentService extends AbstractPaperlessService<Document> {
|
||||
)
|
||||
}
|
||||
|
||||
getFilterSelectionData(filterRules: FilterRule[]): Observable<SelectionData> {
|
||||
return this.http.get<SelectionData>(
|
||||
this.getResourceUrl(null, 'filter_selection_data'),
|
||||
{ params: this.withParams(queryParamsFromFilterRules(filterRules)) }
|
||||
)
|
||||
}
|
||||
|
||||
getSuggestions(id: number): Observable<DocumentSuggestions> {
|
||||
return this.http.get<DocumentSuggestions>(
|
||||
this.getResourceUrl(id, 'suggestions')
|
||||
|
||||
@@ -1241,7 +1241,7 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
|
||||
],
|
||||
)
|
||||
|
||||
def test_list_with_include_selection_data(self) -> None:
|
||||
def test_selection_data_endpoint(self) -> None:
|
||||
correspondent = Correspondent.objects.create(name="c1")
|
||||
doc_type = DocumentType.objects.create(name="dt1")
|
||||
storage_path = StoragePath.objects.create(name="sp1")
|
||||
@@ -1259,30 +1259,28 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
|
||||
non_matching_doc.tags.add(Tag.objects.create(name="other"))
|
||||
|
||||
response = self.client.get(
|
||||
f"/api/documents/?tags__id__in={tag.id}&include_selection_data=true",
|
||||
f"/api/documents/filter_selection_data/?tags__id__in={tag.id}",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertIn("selection_data", response.data)
|
||||
self.assertNotIn("results", response.data)
|
||||
|
||||
selected_correspondent = next(
|
||||
item
|
||||
for item in response.data["selection_data"]["selected_correspondents"]
|
||||
for item in response.data["selected_correspondents"]
|
||||
if item["id"] == correspondent.id
|
||||
)
|
||||
selected_tag = next(
|
||||
item
|
||||
for item in response.data["selection_data"]["selected_tags"]
|
||||
if item["id"] == tag.id
|
||||
item for item in response.data["selected_tags"] if item["id"] == tag.id
|
||||
)
|
||||
selected_type = next(
|
||||
item
|
||||
for item in response.data["selection_data"]["selected_document_types"]
|
||||
for item in response.data["selected_document_types"]
|
||||
if item["id"] == doc_type.id
|
||||
)
|
||||
selected_storage_path = next(
|
||||
item
|
||||
for item in response.data["selection_data"]["selected_storage_paths"]
|
||||
for item in response.data["selected_storage_paths"]
|
||||
if item["id"] == storage_path.id
|
||||
)
|
||||
|
||||
@@ -1291,6 +1289,17 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
|
||||
self.assertEqual(selected_type["document_count"], 1)
|
||||
self.assertEqual(selected_storage_path["document_count"], 1)
|
||||
|
||||
def test_list_no_longer_supports_include_selection_data(self) -> None:
|
||||
"""
|
||||
include_selection_data was never part of a stable release (beta-only,
|
||||
introduced and removed within the 3.0.0-beta cycle) -- the plain list
|
||||
endpoint should just ignore the param now rather than compute it inline.
|
||||
"""
|
||||
response = self.client.get("/api/documents/?include_selection_data=true")
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertNotIn("selection_data", response.data)
|
||||
|
||||
def test_statistics(self) -> None:
|
||||
doc1 = Document.objects.create(
|
||||
title="none1",
|
||||
|
||||
+26
-31
@@ -1034,29 +1034,27 @@ class DocumentViewSet(
|
||||
],
|
||||
}
|
||||
|
||||
def get_queryset(self):
|
||||
def _base_document_queryset(self):
|
||||
# Root documents only, with the annotations that filter_backends rely
|
||||
# on (effective_content for SearchFilter, num_notes for ordering) --
|
||||
# but no select_related/prefetch_related, since those only matter for
|
||||
# serializing documents, not for filtering, ordering, or aggregating.
|
||||
latest_version_content = Subquery(
|
||||
Document.objects.filter(root_document=OuterRef("pk"))
|
||||
.order_by("-id")
|
||||
.values("content")[:1],
|
||||
)
|
||||
# A correlated subquery avoids the LEFT JOIN + Count() this used to
|
||||
# be, which forced a GROUP BY aggregate over every matching document
|
||||
# before the query could even be sorted or limited.
|
||||
note_count = Subquery(
|
||||
Note.objects.filter(document=OuterRef("pk"))
|
||||
.order_by()
|
||||
.values("document")
|
||||
.annotate(count=Count("pk"))
|
||||
.values("count"),
|
||||
output_field=IntegerField(),
|
||||
)
|
||||
return (
|
||||
Document.objects.filter(root_document__isnull=True)
|
||||
.distinct()
|
||||
.order_by("-created", "-id")
|
||||
.annotate(effective_content=Coalesce(latest_version_content, F("content")))
|
||||
.annotate(num_notes=Coalesce(note_count, 0))
|
||||
.annotate(num_notes=Count("notes"))
|
||||
)
|
||||
|
||||
def get_queryset(self):
|
||||
return (
|
||||
self._base_document_queryset()
|
||||
.select_related("correspondent", "storage_path", "document_type", "owner")
|
||||
.prefetch_related(
|
||||
Prefetch(
|
||||
@@ -1195,24 +1193,21 @@ class DocumentViewSet(
|
||||
|
||||
return response
|
||||
|
||||
def list(self, request, *args, **kwargs):
|
||||
if not get_boolean(
|
||||
str(request.query_params.get("include_selection_data", "false")),
|
||||
):
|
||||
return super().list(request, *args, **kwargs)
|
||||
|
||||
queryset = self.filter_queryset(self.get_queryset())
|
||||
selection_data = self._get_selection_data_for_queryset(queryset)
|
||||
|
||||
page = self.paginate_queryset(queryset)
|
||||
if page is not None:
|
||||
serializer = self.get_serializer(page, many=True)
|
||||
response = self.get_paginated_response(serializer.data)
|
||||
response.data["selection_data"] = selection_data
|
||||
return response
|
||||
|
||||
serializer = self.get_serializer(queryset, many=True)
|
||||
return Response({"results": serializer.data, "selection_data": selection_data})
|
||||
@extend_schema(
|
||||
operation_id="documents_filter_selection_data",
|
||||
description=(
|
||||
"Returns per-tag/correspondent/document-type/storage-path/custom-field "
|
||||
"document counts for the current filter, without paginating or "
|
||||
"serializing the matching documents themselves. Split out from the "
|
||||
"plain document list so that browsing the (potentially huge) unfiltered "
|
||||
"document list doesn't pay for this aggregation on every request."
|
||||
),
|
||||
responses={200: inline_serializer(name="SelectionData", fields={})},
|
||||
)
|
||||
@action(detail=False, methods=["get"], url_path="filter_selection_data")
|
||||
def filter_selection_data(self, request, *args, **kwargs):
|
||||
queryset = self.filter_queryset(self._base_document_queryset())
|
||||
return Response(self._get_selection_data_for_queryset(queryset))
|
||||
|
||||
def destroy(self, request, *args, **kwargs):
|
||||
from documents.search import get_backend
|
||||
|
||||
+23
-67
@@ -73,8 +73,6 @@ APPLE_MAIL_TAG_COLORS = {
|
||||
"grey": ["$MailFlagBit1", "$MailFlagBit2"],
|
||||
}
|
||||
|
||||
MAIL_FETCH_BATCH_SIZE = 500
|
||||
|
||||
|
||||
class MailError(Exception):
|
||||
pass
|
||||
@@ -683,55 +681,17 @@ class MailAccountHandler(LoggingMixin):
|
||||
)
|
||||
|
||||
try:
|
||||
all_uids = set(
|
||||
M.uids(criteria=criterias, charset=rule.account.character_set),
|
||||
messages = M.fetch(
|
||||
criteria=criterias,
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while searching folder {rule.folder}",
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
|
||||
processed_uids_qs = ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid__in=all_uids,
|
||||
)
|
||||
if self._current_uid_validity is not None:
|
||||
processed_uids_qs = processed_uids_qs.filter(
|
||||
Q(uid_validity=self._current_uid_validity)
|
||||
| Q(uid_validity__isnull=True),
|
||||
)
|
||||
processed_uids = set(processed_uids_qs.values_list("uid", flat=True))
|
||||
|
||||
new_uids = all_uids - processed_uids
|
||||
|
||||
if not new_uids:
|
||||
self.log.debug(
|
||||
f"Rule {rule}: No new mail matching criteria {criterias}",
|
||||
)
|
||||
return 0
|
||||
|
||||
sorted_new_uids = sorted(new_uids, key=int)
|
||||
message_batches = []
|
||||
# ikvk/imap_tools#268 requests a direct UID-list fetch that would let us drop this manual batching loop
|
||||
for batch_start in range(0, len(sorted_new_uids), MAIL_FETCH_BATCH_SIZE):
|
||||
batch = sorted_new_uids[batch_start : batch_start + MAIL_FETCH_BATCH_SIZE]
|
||||
try:
|
||||
message_batches.append(
|
||||
M.fetch(
|
||||
criteria=AND(uid=batch),
|
||||
mark_seen=False,
|
||||
charset=rule.account.character_set,
|
||||
bulk=True,
|
||||
),
|
||||
)
|
||||
except Exception as err:
|
||||
raise MailError(
|
||||
f"Rule {rule}: Error while fetching folder {rule.folder}",
|
||||
) from err
|
||||
|
||||
messages = itertools.chain(*message_batches)
|
||||
|
||||
mails_processed = 0
|
||||
total_processed_files = 0
|
||||
rule_seen_messages: set[tuple[str, str | None]] = set()
|
||||
@@ -797,7 +757,6 @@ class MailAccountHandler(LoggingMixin):
|
||||
not message.attachments
|
||||
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
|
||||
):
|
||||
self._record_processed_without_consumption(message, rule)
|
||||
return processed_elements
|
||||
|
||||
self.log.debug(
|
||||
@@ -833,25 +792,6 @@ class MailAccountHandler(LoggingMixin):
|
||||
|
||||
return processed_elements
|
||||
|
||||
def _record_processed_without_consumption(
|
||||
self,
|
||||
message: MailMessage,
|
||||
rule: MailRule,
|
||||
) -> None:
|
||||
ProcessedMail.objects.get_or_create(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
uid_validity=self._current_uid_validity,
|
||||
defaults={
|
||||
"subject": message.subject,
|
||||
"received": make_aware(message.date)
|
||||
if is_naive(message.date)
|
||||
else message.date,
|
||||
"status": "PROCESSED_WO_CONSUMPTION",
|
||||
},
|
||||
)
|
||||
|
||||
def filename_inclusion_matches(
|
||||
self,
|
||||
filter_attachment_filename_include: str | None,
|
||||
@@ -1018,7 +958,23 @@ class MailAccountHandler(LoggingMixin):
|
||||
)
|
||||
else:
|
||||
# No files to consume, just mark as processed if it wasn't by .eml processing
|
||||
self._record_processed_without_consumption(message, rule)
|
||||
if not ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
uid_validity=self._current_uid_validity,
|
||||
).exists():
|
||||
ProcessedMail.objects.create(
|
||||
rule=rule,
|
||||
folder=rule.folder,
|
||||
uid=message.uid,
|
||||
uid_validity=self._current_uid_validity,
|
||||
subject=message.subject,
|
||||
received=make_aware(message.date)
|
||||
if is_naive(message.date)
|
||||
else message.date,
|
||||
status="PROCESSED_WO_CONSUMPTION",
|
||||
)
|
||||
|
||||
return processed_attachments
|
||||
|
||||
|
||||
@@ -135,12 +135,6 @@ class BogusMailBox(AbstractContextManager):
|
||||
raise MailboxLoginError("BAD", "OK")
|
||||
|
||||
def fetch(self, criteria, mark_seen, charset="", *, bulk=True):
|
||||
return self._filter_messages(criteria)
|
||||
|
||||
def uids(self, criteria, charset="") -> list[str]:
|
||||
return [m.uid for m in self._filter_messages(criteria)]
|
||||
|
||||
def _filter_messages(self, criteria):
|
||||
msg = self.messages
|
||||
|
||||
criteria = str(criteria).strip("()").split(" ")
|
||||
@@ -174,10 +168,6 @@ class BogusMailBox(AbstractContextManager):
|
||||
if "(X-GM-LABELS" in criteria: # ['NOT', '(X-GM-LABELS', '"processed"']
|
||||
msg = filter(lambda m: "processed" not in m.flags, msg)
|
||||
|
||||
if "UID" in criteria:
|
||||
uid_list = criteria[criteria.index("UID") + 1].split(",")
|
||||
msg = filter(lambda m: m.uid in uid_list, msg)
|
||||
|
||||
return list(msg)
|
||||
|
||||
def delete(self, uid_list) -> None:
|
||||
@@ -435,54 +425,6 @@ class TestMail(
|
||||
|
||||
super().setUp()
|
||||
|
||||
@mock.patch("paperless_mail.mail.MAIL_FETCH_BATCH_SIZE", 5)
|
||||
def test_handle_mail_account_batches_body_fetch_for_large_backlog(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- More new/unprocessed mail than MAIL_FETCH_BATCH_SIZE
|
||||
WHEN:
|
||||
- The mail account is processed
|
||||
THEN:
|
||||
- The body fetch is issued in multiple batches
|
||||
- Every message is still processed (none dropped at a batch boundary)
|
||||
"""
|
||||
account = MailAccount.objects.create(
|
||||
name="test",
|
||||
imap_server="",
|
||||
username="admin",
|
||||
password="secret",
|
||||
)
|
||||
rule = MailRule.objects.create(
|
||||
name="testrule",
|
||||
account=account,
|
||||
action=MailRule.MailAction.MARK_READ,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
|
||||
message_count = 12 # more than the patched batch size of 5
|
||||
self.mailMocker.bogus_mailbox.messages = [
|
||||
self.mailMocker.messageBuilder.create_message(
|
||||
subject=f"No attachment {i}",
|
||||
attachments=[],
|
||||
)
|
||||
for i in range(message_count)
|
||||
]
|
||||
self.mailMocker.bogus_mailbox.updateClient()
|
||||
|
||||
with mock.patch.object(
|
||||
self.mailMocker.bogus_mailbox,
|
||||
"fetch",
|
||||
wraps=self.mailMocker.bogus_mailbox.fetch,
|
||||
) as fetch_spy:
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
|
||||
# ceil(12 / 5) == 3 batches
|
||||
self.assertEqual(fetch_spy.call_count, 3)
|
||||
self.assertEqual(
|
||||
ProcessedMail.objects.filter(rule=rule).count(),
|
||||
message_count,
|
||||
)
|
||||
|
||||
def test_get_correspondent(self) -> None:
|
||||
message = namedtuple("MailMessage", [])
|
||||
message.from_ = "someone@somewhere.com"
|
||||
@@ -595,59 +537,17 @@ class TestMail(
|
||||
],
|
||||
)
|
||||
|
||||
def test_bogus_mailbox_uids_and_uid_criteria(self) -> None:
|
||||
mailbox = self.mailMocker.bogus_mailbox
|
||||
all_messages = list(mailbox.messages)
|
||||
|
||||
# uids() returns the UIDs of unseen messages, no bodies needed to call it
|
||||
unseen_uids = mailbox.uids("(UNSEEN)")
|
||||
self.assertEqual(
|
||||
set(unseen_uids),
|
||||
{m.uid for m in all_messages if not m.seen},
|
||||
)
|
||||
|
||||
# fetch() with an explicit UID criteria returns only the matching messages
|
||||
target_uid = all_messages[0].uid
|
||||
from imap_tools import AND
|
||||
|
||||
fetched = mailbox.fetch(AND(uid=[target_uid]), mark_seen=False)
|
||||
self.assertEqual([m.uid for m in fetched], [target_uid])
|
||||
|
||||
def test_handle_empty_message(self) -> None:
|
||||
message = self.mailMocker.messageBuilder.create_message(
|
||||
subject="No attachments here",
|
||||
attachments=[],
|
||||
)
|
||||
message = namedtuple("MailMessage", [])
|
||||
|
||||
account = MailAccount.objects.create()
|
||||
rule = MailRule.objects.create(
|
||||
account=account,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
message.attachments = []
|
||||
rule = MailRule()
|
||||
|
||||
result = self.mail_account_handler._handle_message(message, rule)
|
||||
|
||||
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
|
||||
self.assertEqual(result, 0)
|
||||
|
||||
processed = ProcessedMail.objects.get(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
)
|
||||
self.assertEqual(processed.status, "PROCESSED_WO_CONSUMPTION")
|
||||
|
||||
# Calling it again must not create a second row
|
||||
self.mail_account_handler._handle_message(message, rule)
|
||||
self.assertEqual(
|
||||
ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
).count(),
|
||||
1,
|
||||
)
|
||||
|
||||
def test_handle_unknown_mime_type(self) -> None:
|
||||
message = self.mailMocker.messageBuilder.create_message(
|
||||
attachments=[
|
||||
@@ -1012,62 +912,6 @@ class TestMail(
|
||||
]
|
||||
self.assertEqual(queued_rule.id, first_rule.id)
|
||||
|
||||
def test_handle_mail_account_skips_body_fetch_for_already_processed_mail(
|
||||
self,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- An attachment-less mail under an attachments-only mark-read rule,
|
||||
already recorded as PROCESSED_WO_CONSUMPTION
|
||||
WHEN:
|
||||
- The mail account is processed again and the mail still matches the
|
||||
search criteria (it was never marked read, since no mail action is
|
||||
applied for the no-consumption case)
|
||||
THEN:
|
||||
- No IMAP body fetch happens for that mail; only the cheap UID search runs.
|
||||
"""
|
||||
account = MailAccount.objects.create(
|
||||
name="test",
|
||||
imap_server="",
|
||||
username="admin",
|
||||
password="secret",
|
||||
)
|
||||
rule = MailRule.objects.create(
|
||||
name="testrule",
|
||||
account=account,
|
||||
action=MailRule.MailAction.MARK_READ,
|
||||
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
|
||||
)
|
||||
|
||||
message = self.mailMocker.messageBuilder.create_message(
|
||||
subject="No attachment",
|
||||
attachments=[],
|
||||
)
|
||||
self.mailMocker.bogus_mailbox.messages = [message]
|
||||
self.mailMocker.bogus_mailbox.updateClient()
|
||||
|
||||
# First run: records ProcessedMail without consuming anything.
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
self.assertTrue(
|
||||
ProcessedMail.objects.filter(
|
||||
rule=rule,
|
||||
uid=message.uid,
|
||||
folder=rule.folder,
|
||||
).exists(),
|
||||
)
|
||||
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
|
||||
|
||||
# Second run: message still matches UNSEEN (mark-read action never ran),
|
||||
# but its body must not be downloaded again.
|
||||
with mock.patch.object(
|
||||
self.mailMocker.bogus_mailbox,
|
||||
"fetch",
|
||||
wraps=self.mailMocker.bogus_mailbox.fetch,
|
||||
) as fetch_spy:
|
||||
self.mail_account_handler.handle_mail_account(account)
|
||||
|
||||
fetch_spy.assert_not_called()
|
||||
|
||||
def test_handle_mail_account_skip_duplicate_uids_from_fetch(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
|
||||
Reference in New Issue
Block a user