Merge pull request #252 from fiddlerwoaroof/fix/pr8t-nacd-item-coordinates
Refuse a wire write unless every coordinate names an item (improvise-nacd, improvise-pr8t)
This commit is contained in:
@@ -5929,8 +5929,10 @@ is stored, so the stored arm accepted it without asking the projection. The test
|
||||
runs with `Money` collapsed, where the host draws that cell as the fold, and
|
||||
expanded, where the host draws no such cell. It asserts `MutationRefused` and an
|
||||
unchanged document in both, so a rule that asks the host's layout cannot pass
|
||||
the expanded half. Red at `5eaafbae`. The fix is `App::group_refusal`, which asks
|
||||
the document whether any coordinate names a group rather than an item. Rival:
|
||||
the expanded half. Red at `5eaafbae`. The fix was `App::group_refusal`, which
|
||||
asked the document whether any coordinate names a group rather than an item.
|
||||
Since improvise-nacd it is `App::non_item_refusal`, which asks whether every
|
||||
coordinate names an item. Rival:
|
||||
the group rule removed. Injected into a full workspace run, it fails this test
|
||||
and nothing else.
|
||||
|
||||
@@ -5951,6 +5953,72 @@ improvise-b114 rule is: no coordinate names a group, asked of the document.
|
||||
Rival: the group rule removed. Injected into a full workspace run, it fails this
|
||||
test and the b114 test, and nothing else.
|
||||
|
||||
### a_wire_write_to_an_item_named_like_its_categorys_group_is_accepted
|
||||
|
||||
improvise-pr8t, found in the adversarial review of PR #235 at `5c19fdd4`.
|
||||
`Category::add_item` checks only the items, so a category may hold an item
|
||||
`Money` and a group `Money` at once. The b114 rule exempted that case with
|
||||
`!category.items.contains_key(name)`, and nothing pinned the exemption: with it
|
||||
removed, the whole workspace stayed green while a caught-up write to
|
||||
`{Month=Feb, _Measure=Money}`, a real item's cell, was refused.
|
||||
|
||||
The test builds exactly that case on the stored record `{Month=Feb}`. It asserts
|
||||
`Accepted` and the stored value, so a rule that refuses and a rule that accepts
|
||||
but drops the write both fail.
|
||||
|
||||
The test passed on the unfixed code, as a pin should. Two rivals were run against it:
|
||||
|
||||
- On the b114 code, the exemption removed (`let group = true && …`): red,
|
||||
`a real item was refused: … Rejected(MutationRefused)`.
|
||||
- On the nacd code, a rule that also refuses any name that is a group, even
|
||||
when it is an item: red, and in a full `improvise-command` run this test is
|
||||
the only failure.
|
||||
|
||||
### a_wire_write_naming_no_item_of_its_category_is_rejected
|
||||
|
||||
improvise-nacd, found in the same review. The b114 rule refused a coordinate
|
||||
that *is a group*. The invariant it protects is wider: every coordinate names an
|
||||
item (improvise-gw18). Two writes fell in the gap. The fixture puts Revenue and
|
||||
Cost in `Money`, puts `Money` under the parent group `Totals`, and stores
|
||||
`{Month=Feb}`.
|
||||
|
||||
- `_Measure=Bogus` is no item and no group.
|
||||
- `_Measure=Totals` is a parent group that holds no item directly, so
|
||||
`items_in_group("Totals")` is empty and the group test did not see it.
|
||||
|
||||
Both were answered `Accepted` and stored the coordinate. The test asserts
|
||||
`Rejected(MutationRefused)` and an unchanged document for each. Both were
|
||||
confirmed red separately at `202e7eac`: the loop stops at its first failure, so
|
||||
the second name was checked by swapping the order.
|
||||
|
||||
The fix is `App::non_item_refusal`: every coordinate must be one of
|
||||
`Model::effective_item_names` of its category. The argument for a positive rule
|
||||
is in `context/code/command.md`, in the section on wire writes.
|
||||
|
||||
Rival: restrict the rule to `_Measure`, the ticket's first suggested fix shape.
|
||||
It fails `a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected`,
|
||||
because `Month=Q2` is a group of a regular category.
|
||||
|
||||
The fix also moved two fixtures in `ui/reduce.rs`. `model_app` and
|
||||
`a_wire_model_command_marks_the_document_dirty` stored or wrote
|
||||
`_Measure=Revenue` without adding `Revenue` as an item. So the cell they wrote
|
||||
was exactly the non-item coordinate this rule refuses. Both now add the item.
|
||||
That changes nothing they assert. It is the fixture being made a document that
|
||||
could exist.
|
||||
|
||||
### a_write_under_a_formula_is_refused_for_the_formula_not_as_a_non_item
|
||||
|
||||
improvise-nacd. A formula target is a `_Measure` name but not an item of the
|
||||
category, so the non-item rule could refuse a write under a formula before the
|
||||
`Derived` rule saw it. The write would still be refused, but for a reason that
|
||||
is false. The rule uses `Model::effective_item_names`, which includes formula
|
||||
targets, so the formula rule gives the reason.
|
||||
|
||||
Rival: the rule written with the category's raw `items`. Before this test, that
|
||||
rival passed the whole workspace, because every other formula test asserts only
|
||||
that the write is refused. It fails this test with
|
||||
`refused for another reason: Profit is no item of _Measure, …`.
|
||||
|
||||
### a_stored_record_the_hosts_view_aggregates_is_writable
|
||||
|
||||
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
|
||||
|
||||
Reference in New Issue
Block a user