Server settings → Data protection: the dry run ignores the retention typed beside its button and reports on the saved one #483

New issue

Closed

opened 2026-10-01 10:47:01 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:01 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading DataProtectionView.post and pressing the dry run in a test.

Related to #297.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at core/server_views.py:614-624 (DataProtectionView.post); core/gdpr.py, the template and tests/test_gdpr.py have not changed, and the dry run answers the same.

The "Run the dry run" button sits in the Retention card directly under the retention field, in the same form, which reads as "show me what this number would touch". The view never looks at the posted number: the report is drawn from the value already saved, while the re-rendered field keeps showing what was typed.

Where: core/server_views.py:613-623, DataProtectionView.post, which binds the form and then calls gdpr.retention_dry_run() with no argument; core/gdpr.py:317-344, where retention_dry_run() takes its cutoff from retention_cutoff() and so from SiteSettings.get().retention_days; the button is in templates/server/data_protection.html:19-30.
What happens: with nothing saved, an administrator types 365 and presses the dry run. Checked: the page answers "No retention limit is set, so there is no policy to run. Set one above and the dry run will say what it would touch", while the field above still shows value="365". With 30 saved and 365 typed, the report lists everything older than 30 days; only the "(30 days)" in the report's first line shows that it is not the number on screen. tests/test_gdpr.py:323-341 posts retention_days=1 with the dry run and passes because the report follows the stored 30, so the suite does not notice the mismatch.
Why it matters: the dry run exists so an administrator can see what a policy would touch before adopting it. As built, the preview answers a different question from the one on screen, and an administrator may save a limit believing they previewed it.
Fix: give retention_dry_run() a days argument, validate the bound form in the dry-run branch and pass its cleaned retention_days when it is valid and set, falling back to the stored value otherwise, and still save nothing. If the dry run is instead meant to report on the saved policy only, move the button out of the form beside the saved value and say so; either way the report and the field must not disagree.

Done when

tests/test_gdpr.py has a test that, with nothing stored, posts retention_days=30 with dry_run and finds a 45-day-old contact in the report, and finds SiteSettings.get().retention_days still None afterwards. It fails today.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `DataProtectionView.post` and pressing the dry run in a test. Related to #297. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `core/server_views.py:614-624` (`DataProtectionView.post`); `core/gdpr.py`, the template and `tests/test_gdpr.py` have not changed, and the dry run answers the same. The "Run the dry run" button sits in the Retention card directly under the retention field, in the same form, which reads as "show me what this number would touch". The view never looks at the posted number: the report is drawn from the value already saved, while the re-rendered field keeps showing what was typed. - **Where:** `core/server_views.py:613-623`, `DataProtectionView.post`, which binds the form and then calls `gdpr.retention_dry_run()` with no argument; `core/gdpr.py:317-344`, where `retention_dry_run()` takes its cutoff from `retention_cutoff()` and so from `SiteSettings.get().retention_days`; the button is in `templates/server/data_protection.html:19-30`. - **What happens:** with nothing saved, an administrator types 365 and presses the dry run. Checked: the page answers "No retention limit is set, so there is no policy to run. Set one above and the dry run will say what it would touch", while the field above still shows `value="365"`. With 30 saved and 365 typed, the report lists everything older than 30 days; only the "(30 days)" in the report's first line shows that it is not the number on screen. `tests/test_gdpr.py:323-341` posts `retention_days=1` with the dry run and passes because the report follows the stored 30, so the suite does not notice the mismatch. - **Why it matters:** the dry run exists so an administrator can see what a policy would touch before adopting it. As built, the preview answers a different question from the one on screen, and an administrator may save a limit believing they previewed it. - **Fix:** give `retention_dry_run()` a `days` argument, validate the bound form in the dry-run branch and pass its cleaned `retention_days` when it is valid and set, falling back to the stored value otherwise, and still save nothing. If the dry run is instead meant to report on the saved policy only, move the button out of the form beside the saved value and say so; either way the report and the field must not disagree. ## Done when - `tests/test_gdpr.py` has a test that, with nothing stored, posts `retention_days=30` with `dry_run` and finds a 45-day-old contact in the report, and finds `SiteSettings.get().retention_days` still `None` afterwards. It fails today.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:01 +00:00

tiagoagueda added the

interface

tier

bug

labels

2026-10-01 10:47:01 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 01:59:02 +00:00

Make the retention dry run answer for the number typed beside it

tiagoagueda closed this issue