feat(domain,rules): give a unit a product-life status - #226
Open
LKSNDRTMLKV wants to merge 1 commit into
Open
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 28 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #218. Phase 5 of
docs/architecture/PRODUCT-LINEAGE.md, and the last of#208's plan.
Three pieces, per the issue: the
LifeStatusfield, theTransferReasonvariant for the waste handover, and the
dpp-rulesconsistency rule that bindsthe status to the derivation edges.
Non-breaking.
life_statusisOptionwithskip_serializing_if, andTransferReasonwas already#[non_exhaustive].Three things worth your eyes
Everything else follows the issue. These three are calls I had to make, and I'd
rather flag them than have them merge unnoticed.
1. This PR corrects a contradiction in the design note
§4.2 of
PRODUCT-LINEAGE.mdsaidlife_statuswhile the issue says it
I read these as reconciling rather than conflicting: the note demanded that
Phase 5 specify a mutation path, the issue specifies it (waste transition =
new passport version), and being in
PROTECTED_PATCH_FIELDSis precisely whatforces a caller onto that path rather than an alternative to it. §4.2 is updated
to say so, and to record that it is correcting itself — leaving the note arguing
with the code would be the same failure as leaving a doc self-contradictory
after a merge.
If you meant the note's reading instead, this is the commit to push back on.
2. The wire forms are hyphenated, against house style
Every other wire vocabulary here is camelCase. These five are not:
Annex XIII point 4(c) does not name concepts for us to spell as we like — it
enumerates the literal values the status is "defined as". Serde's camelCase
would produce
reused, which is not a value the instrument contains, so thevariants carry explicit
#[serde(rename)]and a test pins all five against thequoted text. The alternative — camelCase everywhere for internal consistency —
is defensible, but it invents a value, and a citation a reader cannot check
against the primary source is worth less than the consistency it buys.
3.
wasteis exempt from the consistency ruleThe issue asks that
life_statusagree withderived_from's operations. Takenliterally that breaks a lawful record, so I carved out
waste.A waste battery's derivation edges describe how it was manufactured; they say
nothing about whether it is now waste. A repurposed unit that later became waste
carries a
repurposingedge and awastestatus, and both are correct —checking one against the other would report an entirely ordinary record as
inconsistent. This follows from the issue's own reasoning that
'waste'is "theone transition that happens to a record that continues", but the issue does not
spell out the consequence for the rule, so I am naming it here.
The rule is some, not every
A claimed status must be supported by at least one edge, not by all of them.
Requiring agreement would make the plural mixed-predecessor case
unrepresentable — Art. 77(7) permits several predecessors and nothing forces
them to share an operation — and that case is exactly why the issue chose to
store the status rather than derive it. Requiring every edge to agree would
reintroduce the flaw by the back door.
repurposedis supported by either repurposing operation, since Art. 3(30) andArt. 3(31) differ by the waste status of the input rather than the outcome.
Disclosure
lifeStatusisDisclosure::IndividualinPASSPORT_FIELD_DISCLOSURE, andthere is a test asserting it. That entry is the only thing between an individual
unit's life status and an anonymous reader:
passport_default()'sdefault_disclosureisPublic, so an unclassified envelope field is served toeveryone. Its absence is the silent failure, which is why it gets a test of
its own rather than relying on review.
No strip-list needed — the public view applies
PASSPORT_FIELD_DISCLOSUREgenerically through the scope-aware filter.
RETENTION_MUTABLE_FIELDSdeliberately does not gain the field: a waste transition is a new version, not
a mutation of a retention-locked record.
Citations
Art. 77(7) and its second subparagraph at OJ L 191, 28.7.2023 p. 73;
Annex XIII point 4(c) at p. 109; Art. 3(29)–(32) at p. 27. These match the pins
re-verified during #208's Phase 1 rather than being freshly asserted.
There is no sixth value: "approaching end of life" appears nowhere in
Regulation (EU) 2023/1542, and a test would now fail if it came back.
Checks
just checkgreen: fmt, clippy, nextest (1289 tests), doctests, plugins,rustdoc, audit. New tests: 4 on
LifeStatus, 10 on the consistency rule.Two pre-existing tripwires fired during this work and both were right — the
PROTECTED_PATCH_FIELDSkey-reality check (its fixture did not populate the newfield) and
TransferReason::ALL's exhaustive match. Fixed rather than workedaround.