NEO-55: address code review suggestions for inventory HTTP
Add remove round-trip and invalid_quantity API tests, Bruno remove request, dependency register NEO-55 footnote, and strike-through review follow-ups.pull/90/head
parent
d127de7bbb
commit
fddbb95a45
|
|
@ -0,0 +1,41 @@
|
|||
meta {
|
||||
name: POST inventory remove
|
||||
type: http
|
||||
seq: 4
|
||||
}
|
||||
|
||||
docs {
|
||||
NEO-55: remove stack via mutationKind remove. Run Post inventory add first on a fresh server,
|
||||
or expect applied true when dev-local-1 already holds scrap_metal_bulk.
|
||||
}
|
||||
|
||||
post {
|
||||
url: {{baseUrl}}/game/players/dev-local-1/inventory
|
||||
body: json
|
||||
auth: none
|
||||
}
|
||||
|
||||
body:json {
|
||||
{
|
||||
"schemaVersion": 1,
|
||||
"mutationKind": "remove",
|
||||
"itemId": "scrap_metal_bulk",
|
||||
"quantity": 2
|
||||
}
|
||||
}
|
||||
|
||||
tests {
|
||||
test("remove returns 200 with applied true and reduced quantity", function () {
|
||||
expect(res.getStatus()).to.equal(200);
|
||||
const body = res.getBody();
|
||||
expect(body.schemaVersion).to.equal(1);
|
||||
expect(body.applied).to.equal(true);
|
||||
expect(body.reasonCode).to.equal(null);
|
||||
expect(body.inventory).to.be.an("object");
|
||||
const scrap = body.inventory.bagSlots.find((s) => s.itemId === "scrap_metal_bulk");
|
||||
if (scrap) {
|
||||
expect(scrap.quantity).to.be.a("number");
|
||||
expect(scrap.quantity).to.be.at.least(0);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -1,3 +1,10 @@
|
|||
meta {
|
||||
name: inventory
|
||||
}
|
||||
|
||||
docs {
|
||||
NEO-55 inventory HTTP. Run GET first on a fresh dev server for empty 24+1 grid.
|
||||
POST add/remove mutate dev-local-1; cumulative stacks from prior manual runs can make
|
||||
Post inventory add assertions use at.least(5). Restart server (or truncate DB in Postgres
|
||||
mode) for a clean inventory baseline.
|
||||
}
|
||||
|
|
|
|||
|
|
@ -48,7 +48,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
|
|||
| E3.M4 | SinkAndDurabilityLifecycle | E3.M3, E8.M3 | DurabilityState, ItemSinkEvent, RepairCostRule | Pre-production | Planned |
|
||||
| E3.M5 | EconomyBalancePolicy | E3.M4, E9.M2 | EconomyPolicy, PriceBandRule, FaucetSinkRatio | Pre-production | Planned |
|
||||
|
||||
**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), **NEO-53** (`GET /game/world/item-definitions`), and **NEO-54** (inventory store + stack/slot rules engine) moved the register row to **In Progress**; later slices update the alignment table as they land.
|
||||
**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), **NEO-53** (`GET /game/world/item-definitions`), **NEO-54** (inventory store + stack/slot rules engine), and **NEO-55** (`GET`/`POST /game/players/{id}/inventory`) moved the register row to **In Progress**; later slices update the alignment table as they land.
|
||||
|
||||
### Epic 4 — World Topology
|
||||
|
||||
|
|
|
|||
|
|
@ -19,11 +19,11 @@ NEO-55 adds versioned **`GET` / `POST /game/players/{id}/inventory`** with DTOs,
|
|||
| [`docs/plans/NEO-55-implementation-plan.md`](../plans/NEO-55-implementation-plan.md) | **Matches** — single POST + `mutationKind`, fixed 24+1 slot arrays, `applied`/`reasonCode`/snapshot echo, status mapping, Bruno + README, no manual QA doc per kickoff; acceptance checklist complete. |
|
||||
| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Matches** — NEO-55 HTTP bullet under **Related implementation slices**. |
|
||||
| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M3 row notes NEO-55 landed with README + Bruno pointers; NEO-56 telemetry still planned. |
|
||||
| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E3.M3 **In Progress** is correct; **E3.M3 note** footnote still lists through **NEO-54** only (NEO-55 not mentioned). |
|
||||
| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E3.M3 **In Progress**; footnote lists NEO-50–NEO-55. |
|
||||
| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — inventory mutations remain server-authoritative; HTTP is read/apply surface only. |
|
||||
| [`server/README.md`](../../server/README.md) | **Matches** — Player inventory section documents GET/POST shapes, curl examples, reason codes, persistence; NEO-54 deferral removed. |
|
||||
|
||||
Register/tracking: alignment table updated; optional footnote extension for NEO-55 in dependency register.
|
||||
Register/tracking: alignment table and dependency register footnote updated for NEO-55.
|
||||
|
||||
## Blocking issues
|
||||
|
||||
|
|
@ -31,19 +31,19 @@ None.
|
|||
|
||||
## Suggestions
|
||||
|
||||
1. **Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-55** (inventory HTTP) alongside NEO-50–NEO-54, matching the alignment table and prior NEO-54 review pattern.
|
||||
1. ~~**Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-55** (inventory HTTP) alongside NEO-50–NEO-54, matching the alignment table and prior NEO-54 review pattern.~~ **Done.**
|
||||
|
||||
2. **POST remove happy path (optional)** — Integration tests cover add success and remove deny (`insufficient_quantity`) but not add-then-remove success. A short round-trip test would lock the full POST contract for both `mutationKind` values.
|
||||
2. ~~**POST remove happy path (optional)** — Integration tests cover add success and remove deny (`insufficient_quantity`) but not add-then-remove success. A short round-trip test would lock the full POST contract for both `mutationKind` values.~~ **Done.** — `PostInventory_ShouldApplyRemove_AfterAdd_WithUpdatedSnapshot` in `PlayerInventoryApiTests.cs`.
|
||||
|
||||
3. **`invalid_quantity` HTTP smoke (optional)** — Engine returns `invalid_quantity` for non-positive quantity; no HTTP test asserts **200** + `applied: false` + that code. Low risk (engine tested in NEO-54) but would complete the documented reason-code surface at the API layer.
|
||||
3. ~~**`invalid_quantity` HTTP smoke (optional)** — Engine returns `invalid_quantity` for non-positive quantity; no HTTP test asserts **200** + `applied: false` + that code. Low risk (engine tested in NEO-54) but would complete the documented reason-code surface at the API layer.~~ **Done.** — `PostInventory_ShouldDenyNonPositiveQuantity_WithInvalidQuantity` in `PlayerInventoryApiTests.cs`.
|
||||
|
||||
## Nits
|
||||
|
||||
- Nit: **`Post inventory add.bru`** uses `expect(scrap.quantity).to.be.at.least(5)` — correct for a cumulative dev server, but an exact `=== 5` assertion would fail if prior manual runs left stacks. Acceptable for prototype Bruno; document “fresh dev player” in folder docs if flakes appear.
|
||||
- ~~Nit: **`Post inventory add.bru`** uses `expect(scrap.quantity).to.be.at.least(5)` — correct for a cumulative dev server, but an exact `=== 5` assertion would fail if prior manual runs left stacks. Acceptable for prototype Bruno; document “fresh dev player” in folder docs if flakes appear.~~ **Done.** — `folder.bru` docs note fresh-server baseline and cumulative stacks.
|
||||
|
||||
- Nit: Plan referenced a **`BuildSnapshot`** helper name (NEO-37 mirror); implementation uses **`MapSnapshot`** — behavior is equivalent, naming only.
|
||||
|
||||
- Nit: No Bruno request for **`remove`** happy path — plan minimum (one deny) is met via `invalid_item`; a remove-after-add Bruno would complement manual QA.
|
||||
- ~~Nit: No Bruno request for **`remove`** happy path — plan minimum (one deny) is met via `invalid_item`; a remove-after-add Bruno would complement manual QA.~~ **Done.** — `Post inventory remove.bru` (seq 4).
|
||||
|
||||
## Verification
|
||||
|
||||
|
|
|
|||
|
|
@ -139,6 +139,54 @@ public sealed class PlayerInventoryApiTests
|
|||
Assert.Equal(10, occupied[0].Quantity);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PostInventory_ShouldApplyRemove_AfterAdd_WithUpdatedSnapshot()
|
||||
{
|
||||
// Arrange
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
|
||||
// Act
|
||||
_ = await client.PostAsJsonAsync(
|
||||
"/game/players/dev-local-1/inventory",
|
||||
Mutation("add", "scrap_metal_bulk", quantity: 10));
|
||||
var response = await client.PostAsJsonAsync(
|
||||
"/game/players/dev-local-1/inventory",
|
||||
Mutation("remove", "scrap_metal_bulk", quantity: 4));
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
var body = await response.Content.ReadFromJsonAsync<PlayerInventoryMutationResponse>();
|
||||
Assert.NotNull(body);
|
||||
Assert.True(body!.Applied);
|
||||
Assert.Null(body.ReasonCode);
|
||||
var occupied = body.Inventory.BagSlots.Where(static s => s.Quantity > 0).ToList();
|
||||
Assert.Single(occupied);
|
||||
Assert.Equal("scrap_metal_bulk", occupied[0].ItemId);
|
||||
Assert.Equal(6, occupied[0].Quantity);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PostInventory_ShouldDenyNonPositiveQuantity_WithInvalidQuantity()
|
||||
{
|
||||
// Arrange
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
|
||||
// Act
|
||||
var response = await client.PostAsJsonAsync(
|
||||
"/game/players/dev-local-1/inventory",
|
||||
Mutation("add", "scrap_metal_bulk", quantity: 0));
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
var body = await response.Content.ReadFromJsonAsync<PlayerInventoryMutationResponse>();
|
||||
Assert.NotNull(body);
|
||||
Assert.False(body!.Applied);
|
||||
Assert.Equal(PlayerInventoryReasonCodes.InvalidQuantity, body.ReasonCode);
|
||||
Assert.All(body.Inventory.BagSlots, static slot => Assert.Equal(0, slot.Quantity));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PostInventory_ShouldDenyUnknownItem_WithInvalidItem_AndLeaveInventoryEmpty()
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue