fix(value-source): preserve store contract for game_event + error precedence
Two HIGH issues surfaced by review of 3b8f00e:
1. ``_build_game_event`` was newly succeeding where the old store
raised ``ValueError("Invalid source type: game_event")``. The
coverage-assertion-symmetry comment was honest about it being
a path that didn't exist before, but silent broadening of the
create contract is a real behaviour delta — any internal caller
that previously caught the error would now succeed.
Make ``_build_game_event`` raise NotImplementedError. The
coverage assertion still passes (the entry exists), but the
historical "you can't create game_event sources through the
store" contract is preserved. game_event instances continue to
be wired up by the game-integration setup path.
2. The new ``create_source`` ran ``_check_name_unique`` BEFORE
``build_source``. When both ``source_type`` is invalid AND
``name`` collides with an existing source, the old code raised
``"Invalid source type: …"`` first; the new code raised the
name-collision error. Swap the order: build first (which
validates source_type), then check name uniqueness, then
persist. Bonus: a uuid is no longer minted for a source we end
up rejecting on type.
New test pins the game_event NotImplementedError so a future
refactor doesn't accidentally re-open the create path.
38 value-source-store + factory tests stay green; ruff clean.
This commit is contained in:
@@ -180,6 +180,26 @@ def test_build_source_unknown_type_raises():
|
||||
)
|
||||
|
||||
|
||||
def test_build_source_game_event_is_not_creatable_via_store():
|
||||
"""game_event sources were never creatable through the store API.
|
||||
|
||||
The builder exists for coverage-assertion symmetry but raises so the
|
||||
historical contract holds (game-event value sources are wired up by
|
||||
the game-integration setup path, not user CRUD).
|
||||
"""
|
||||
with pytest.raises(NotImplementedError, match="game_event"):
|
||||
vsf.build_source(
|
||||
source_type="game_event",
|
||||
sid="vs_t",
|
||||
name="Test",
|
||||
now=_now(),
|
||||
description=None,
|
||||
tags=None,
|
||||
icon=None,
|
||||
icon_color=None,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# apply_update — representative paths
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user