Postulo/postulo

Fork 0

A rendered document points at whatever made it, not at one of two columns #130

New issue

Closed

opened 2026-09-09 10:25:19 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-09-09 10:25:19 +00:00

Owner

Observation

on the documents, general conceived for one kid CV's, shoould in fact extended to other
kinds of documents

First prerequisite. Before a third kind of document can exist, the documents app has to stop
naming its two kinds in foreign keys.

What exists

The app already holds two authored kinds — CV and CoverLetter — and the second carries
four shapes of its own in LetterKind. So it is not quite "conceived for CVs". What is
conceived for exactly two kinds is the plumbing around them, and it says so in three places:

RenderedDocument, the frozen PDF, records what it came from with a pair of nullable
foreign keys:

cv = models.ForeignKey(CV, null=True, blank=True, related_name="renders", ...)
cover_letter = models.ForeignKey(CoverLetter, null=True, blank=True, related_name="renders", ...)

DocumentCopy, the row saying where a copy went, does the same for the other axis:

rendered = models.ForeignKey(RenderedDocument, null=True, blank=True, ...)
upload = models.ForeignKey(UploadedDocument, null=True, blank=True, ...)

archiving.py then asks in code what the columns could not say:

def _lookup(document) -> dict:
    if isinstance(document, RenderedDocument):
        ...

Add portfolios, emails and reports and each of those grows: two more nullable columns on
two models, an isinstance branch wherever one is read, and a migration each time.

The app already contains the argument against this. CVItem uses a generic relation,
and the docstring says why:

A generic relation is used because a CV is an ordered list of heterogeneous things. Six
nullable foreign keys with a check constraint would say the same thing less clearly, and
would need widening every time a new kind of item is added.

That is this problem, one model over, decided the other way.

What this asks for

One polymorphic link from a rendered document to whatever produced it, and one from a copy
to whatever was copied — so that a new kind of document is a new package rather than a new
column.

Worth being careful about

Two migrations of live data, not one. RenderedDocument.cv and .cover_letter hold
real rows on every instance, and so does DocumentCopy. Moving them to a generic link means
writing the content type and object id across, keeping the old columns until the new ones
are proven, and a reverse that works — core/migrations/0008 is the recent precedent for
the shape.

on_delete semantics are not the same and are load-bearing. RenderedDocument.cv is
SET_NULL: deleting a CV must not delete the PDF an employer received, and that rule is the
whole point of the model. DocumentCopy.rendered is CASCADE. A generic foreign key has no
on_delete at all, so both behaviours have to be re-created deliberately — a
GenericRelation for the cascade, and something explicit for the one that must not
cascade. Getting this wrong deletes somebody's record of what they sent.

Queries get worse before they get better. related_name="renders" and
select_related("cv") are cheap; a generic link is two columns and no join. Where the
interface lists documents with what made them, this wants a prefetch rather than a loop.

The export carries these links. core/export.py writes documents with their
relationships and the importer reconnects them by local id. Both move with this, and the
importer still has to read an archive written yesterday.

A kind that produces nothing is still a kind. An UploadedDocument came from a file, not
from an authored thing; the generic link has to be legitimately empty and every reader has to
cope, exactly as keys_for() copes with a widget key it does not recognise.

## Observation > on the documents, general conceived for one kid CV's, shoould in fact extended to other > kinds of documents First prerequisite. Before a third kind of document can exist, the documents app has to stop naming its two kinds in foreign keys. ## What exists The app already holds two authored kinds — `CV` and `CoverLetter` — and the second carries four shapes of its own in `LetterKind`. So it is not quite "conceived for CVs". What *is* conceived for exactly two kinds is the plumbing around them, and it says so in three places: **`RenderedDocument`**, the frozen PDF, records what it came from with a pair of nullable foreign keys: ```python cv = models.ForeignKey(CV, null=True, blank=True, related_name="renders", ...) cover_letter = models.ForeignKey(CoverLetter, null=True, blank=True, related_name="renders", ...) ``` **`DocumentCopy`**, the row saying where a copy went, does the same for the other axis: ```python rendered = models.ForeignKey(RenderedDocument, null=True, blank=True, ...) upload = models.ForeignKey(UploadedDocument, null=True, blank=True, ...) ``` **`archiving.py`** then asks in code what the columns could not say: ```python def _lookup(document) -> dict: if isinstance(document, RenderedDocument): ... ``` Add portfolios, emails and reports and each of those grows: two more nullable columns on two models, an `isinstance` branch wherever one is read, and a migration each time. **The app already contains the argument against this.** `CVItem` uses a generic relation, and the docstring says why: > A generic relation is used because a CV is an ordered list of heterogeneous things. Six > nullable foreign keys with a check constraint would say the same thing less clearly, and > would need widening every time a new kind of item is added. That is this problem, one model over, decided the other way. ## What this asks for One polymorphic link from a rendered document to whatever produced it, and one from a copy to whatever was copied — so that a new kind of document is a new package rather than a new column. ## Worth being careful about **Two migrations of live data, not one.** `RenderedDocument.cv` and `.cover_letter` hold real rows on every instance, and so does `DocumentCopy`. Moving them to a generic link means writing the content type and object id across, keeping the old columns until the new ones are proven, and a reverse that works — `core/migrations/0008` is the recent precedent for the shape. **`on_delete` semantics are not the same and are load-bearing.** `RenderedDocument.cv` is `SET_NULL`: deleting a CV must not delete the PDF an employer received, and that rule is the whole point of the model. `DocumentCopy.rendered` is `CASCADE`. A generic foreign key has no `on_delete` at all, so both behaviours have to be re-created deliberately — a `GenericRelation` for the cascade, and something explicit for the one that must *not* cascade. Getting this wrong deletes somebody's record of what they sent. **Queries get worse before they get better.** `related_name="renders"` and `select_related("cv")` are cheap; a generic link is two columns and no join. Where the interface lists documents with what made them, this wants a prefetch rather than a loop. **The export carries these links.** `core/export.py` writes documents with their relationships and the importer reconnects them by local id. Both move with this, and the importer still has to read an archive written yesterday. **A kind that produces nothing is still a kind.** An `UploadedDocument` came from a file, not from an authored thing; the generic link has to be legitimately empty and every reader has to cope, exactly as `keys_for()` copes with a widget key it does not recognise.

tiagoagueda added this to the 0.3.0 milestone

2026-09-09 10:25:19 +00:00

tiagoagueda added the

tier

enhancement

labels

2026-09-09 10:25:19 +00:00

tiagoagueda added a new dependency

2026-09-09 10:25:21 +00:00

#133 A document is a kind, and a kind is a plugin

tiagoagueda added

tier

and removed

tier

labels

2026-09-09 11:49:08 +00:00

tiagoagueda referenced this issue from a commit

2026-09-09 21:42:33 +00:00

Point a render at whatever made it, not at one of two columns

tiagoagueda commented

2026-09-09 21:42:58 +00:00

Author

Owner

One link from a render to whatever made it, one from a copy to whatever was copied. The
app's own argument, applied one model over:

A generic relation is used because a CV is an ordered list of heterogeneous things. Six
nullable foreign keys with a check constraint would say the same thing less clearly, and
would need widening every time a new kind of item is added.

RenderedDocument.cv/.cover_letter → source. DocumentCopy.rendered/.upload →
document. archiving._lookup's isinstance → a content type. The next kind of document is
a package.

The two `on_delete` behaviours

The part with the most care in it, because they are not the same one and a generic
foreign key has neither:

RenderedDocument.cv was SET_NULL. Deleting a CV must not delete the PDF an employer
received — that is what the model is for. So a receiver clears the link, and clears it
rather than leaving it dangling: a GenericForeignKey reads a missing row as None on its
own, but the columns would keep pointing at an id another row could later take.
DocumentCopy.rendered was CASCADE. So the cascade lives on a GenericRelation at
each document, and deleting a PDF still takes the rows saying where its copies went.

Each direction is a test, because getting either backwards loses somebody's record of what
they sent.

The other four warnings

Two migrations of live data, not one. Add, carry across, then remove — with a reverse
that carries back, following core/0008. The autodetector wanted to drop the columns before
anything had read them, which is a one-way door on somebody's instance.

Queries get worse before they get better. They did not get worse. A generic link has no
join to select_related, so copies_for is one batch keyed on content type: a page listing
documents with where their copies went pays two queries rather than one per row, and
django_assert_num_queries(1) holds it there.

The export carries these links. Format 10 writes a source_kind and a source_ref
instead of two id columns, and the importer reads both shapes — an archive made last month is
still an archive. That reading is a named function now rather than eleven lines inside a long
one, which is what let the old shape be tested at all.

A kind that produces nothing is still a kind. An uploaded document came from a file, and
a render whose source was deleted has none either; both are ordinary states. source_label
is what a reader asks instead of isinstance, so a kind arriving later is one it already
describes.

The check constraint saying "exactly one of these two" is gone with the two: a generic link
is exactly one thing by construction, which is one fewer rule to widen when the third kind
arrives.

14 tests. Shipped in b609df1 on 0.3.0, with main kept level. #132 is next in that
chain, and now has somewhere to stand.

One link from a render to whatever made it, one from a copy to whatever was copied. The app's own argument, applied one model over: > A generic relation is used because a CV is an ordered list of heterogeneous things. Six > nullable foreign keys with a check constraint would say the same thing less clearly, and > would need widening every time a new kind of item is added. `RenderedDocument.cv`/`.cover_letter` → `source`. `DocumentCopy.rendered`/`.upload` → `document`. `archiving._lookup`'s `isinstance` → a content type. The next kind of document is a package. ## The two `on_delete` behaviours The part with the most care in it, because they are **not the same one** and a generic foreign key has neither: - **`RenderedDocument.cv` was `SET_NULL`.** Deleting a CV must not delete the PDF an employer received — that is what the model is for. So a receiver clears the link, and *clears* it rather than leaving it dangling: a `GenericForeignKey` reads a missing row as `None` on its own, but the columns would keep pointing at an id another row could later take. - **`DocumentCopy.rendered` was `CASCADE`.** So the cascade lives on a `GenericRelation` at each document, and deleting a PDF still takes the rows saying where its copies went. Each direction is a test, because getting either backwards loses somebody's record of what they sent. ## The other four warnings **Two migrations of live data, not one.** Add, carry across, then remove — with a reverse that carries back, following `core/0008`. The autodetector wanted to drop the columns before anything had read them, which is a one-way door on somebody's instance. **Queries get worse before they get better.** They did not get worse. A generic link has no join to `select_related`, so `copies_for` is one batch keyed on content type: a page listing documents with where their copies went pays two queries rather than one per row, and `django_assert_num_queries(1)` holds it there. **The export carries these links.** Format **10** writes a `source_kind` and a `source_ref` instead of two id columns, and the importer reads both shapes — an archive made last month is still an archive. That reading is a named function now rather than eleven lines inside a long one, which is what let the old shape be tested at all. **A kind that produces nothing is still a kind.** An uploaded document came from a file, and a render whose source was deleted has none either; both are ordinary states. `source_label` is what a reader asks instead of `isinstance`, so a kind arriving later is one it already describes. The check constraint saying "exactly one of these two" is gone with the two: a generic link is exactly one thing by construction, which is one fewer rule to widen when the third kind arrives. 14 tests. Shipped in `b609df1` on `0.3.0`, with `main` kept level. #132 is next in that chain, and now has somewhere to stand.

tiagoagueda closed this issue

2026-09-09 21:42:58 +00:00

tiagoagueda referenced this issue from a commit

2026-09-09 22:30:47 +00:00

Let a CV and a letter list the PDFs made from them again

tiagoagueda referenced this issue

2026-09-09 22:56:02 +00:00

A career entry says the same thing in more than one language #131

tiagoagueda referenced this issue from a commit

2026-09-10 06:13:52 +00:00

Make a portfolio a document Postulo writes, and say what a kind is once

tiagoagueda referenced this issue