Portfolios: a plugin theme that sets only portfolios is offered in the picker, then refused on save #412

New issue

Closed

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

tiagoagueda commented

2026-10-01 10:46:31 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the theme picker and the theme field's validator, and confirmed by running CVForm against the snapshot with a portfolio-only theme registered.

Related to #132.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at documents/models.py:213 (theme = theme_field(themes.Kind.CV)) and documents/models.py:52-67 (theme_field); documents/themes.py and documents/forms.py have not moved.

The CV model holds both CVs and portfolios, but its theme column is validated as if every row were a CV. A theme from a plugin that has a template for portfolios and not for CVs is offered on a portfolio's form and then refused by the model validator when the form is saved. The plugin contract explicitly allows such a theme ("a theme sets what it has a template for, and nothing else"), so a portfolio can never use it.

Where: documents/models.py:173 (theme = theme_field(themes.Kind.CV)), documents/models.py:47-62 (theme_field, which attaches themes.SetsThisKind(kind)), documents/themes.py:236-258 (SetsThisKind.__call__ refuses when not theme.sets(self.kind)), documents/forms.py:120-130 (CVForm.theme_kind, which offers themes.Kind.PORTFOLIO themes for a portfolio).
What happens: an administrator installs a plugin whose theme declares only templates={ThemeKind.PORTFOLIO: ...}. A person creating a portfolio sees "Gallery, from Acme" in the theme menu, chooses it and saves. ModelForm validation runs the field validator fixed to cv, and the form comes back with "The Gallery theme does not set this kind of document." — about a pair the menu itself offered. Reproduced on the snapshot: CVForm(data={"name": "Work", "kind": "portfolio", "theme": "gallery", ...}) lists ('gallery', 'Gallery, from Acme') among its choices and is_valid() is False with that error on theme.
Why it matters: the picker and the model disagree about what a portfolio may be set in, so a legitimate plugin theme is unusable for the only kind it was written for, and the person gets an error they could not have avoided. Postulo's own themes set every kind, which is why the test suite never meets this: tests/test_document_themes.py registers only letter-only and CV-only themes.
Fix: validate the theme against the row's own kind. Keep a field validator that checks only that the theme exists and sets a CV or a portfolio (or drop the kind from it for this column), and add CV.clean() that, when themes.find(self.theme) returns a theme, raises a ValidationError on theme with code wrong_kind unless theme.sets(self.theme_kind). An unknown theme name must keep falling back to plain as it does now. The validator change alters the field's deconstruction, so it needs a migration.

Done when

A test in tests/test_document_themes.py registers a portfolio-only theme, saves a portfolio in it through documents:cv_create, and asserts the portfolio exists with that theme; a second test asserts that a CV (kind cv) set in that theme is refused by full_clean() with code wrong_kind.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the theme picker and the theme field's validator, and confirmed by running `CVForm` against the snapshot with a portfolio-only theme registered. Related to #132. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `documents/models.py:213` (`theme = theme_field(themes.Kind.CV)`) and `documents/models.py:52-67` (`theme_field`); `documents/themes.py` and `documents/forms.py` have not moved. The `CV` model holds both CVs and portfolios, but its `theme` column is validated as if every row were a CV. A theme from a plugin that has a template for portfolios and not for CVs is offered on a portfolio's form and then refused by the model validator when the form is saved. The plugin contract explicitly allows such a theme ("a theme sets what it has a template for, and nothing else"), so a portfolio can never use it. - **Where:** `documents/models.py:173` (`theme = theme_field(themes.Kind.CV)`), `documents/models.py:47-62` (`theme_field`, which attaches `themes.SetsThisKind(kind)`), `documents/themes.py:236-258` (`SetsThisKind.__call__` refuses when `not theme.sets(self.kind)`), `documents/forms.py:120-130` (`CVForm.theme_kind`, which offers `themes.Kind.PORTFOLIO` themes for a portfolio). - **What happens:** an administrator installs a plugin whose theme declares only `templates={ThemeKind.PORTFOLIO: ...}`. A person creating a portfolio sees "Gallery, from Acme" in the theme menu, chooses it and saves. `ModelForm` validation runs the field validator fixed to `cv`, and the form comes back with "The Gallery theme does not set this kind of document." — about a pair the menu itself offered. Reproduced on the snapshot: `CVForm(data={"name": "Work", "kind": "portfolio", "theme": "gallery", ...})` lists `('gallery', 'Gallery, from Acme')` among its choices and `is_valid()` is `False` with that error on `theme`. - **Why it matters:** the picker and the model disagree about what a portfolio may be set in, so a legitimate plugin theme is unusable for the only kind it was written for, and the person gets an error they could not have avoided. Postulo's own themes set every kind, which is why the test suite never meets this: `tests/test_document_themes.py` registers only letter-only and CV-only themes. - **Fix:** validate the theme against the row's own kind. Keep a field validator that checks only that the theme exists and sets a CV or a portfolio (or drop the kind from it for this column), and add `CV.clean()` that, when `themes.find(self.theme)` returns a theme, raises a `ValidationError` on `theme` with code `wrong_kind` unless `theme.sets(self.theme_kind)`. An unknown theme name must keep falling back to `plain` as it does now. The validator change alters the field's deconstruction, so it needs a migration. ## Done when - A test in `tests/test_document_themes.py` registers a portfolio-only theme, saves a portfolio in it through `documents:cv_create`, and asserts the portfolio exists with that theme; a second test asserts that a CV (kind `cv`) set in that theme is refused by `full_clean()` with code `wrong_kind`.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:46:31 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:46:31 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 09:26:54 +00:00

Validate a CV's theme against the row's own kind, not always "cv"

tiagoagueda closed this issue

2026-10-04 09:26:54 +00:00