NEO-140: address code review — grid-contract rep and wire-level gate test
Add integration coverage for +10 Rust Collective reputationGrants, JsonDocument omission test for factionGateRules, extend accept Bruno, fix manual QA link, strike through review findings.pull/180/head
parent
3ba2fd100c
commit
7011a56755
|
|
@ -38,5 +38,11 @@ tests {
|
|||
expect(body.quest).to.be.an("object");
|
||||
expect(body.quest.questId).to.equal("prototype_quest_grid_contract");
|
||||
expect(body.quest.status).to.equal("completed");
|
||||
expect(body.quest.completionRewardSummary).to.be.an("object");
|
||||
expect(body.quest.completionRewardSummary.itemGrants).to.eql([]);
|
||||
expect(body.quest.completionRewardSummary.skillXpGrants).to.eql([]);
|
||||
expect(body.quest.completionRewardSummary.reputationGrants).to.eql([
|
||||
{ factionId: "prototype_faction_rust_collective", amount: 10 },
|
||||
]);
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@
|
|||
## No regressions
|
||||
|
||||
- [ ] Bruno **`faction-standing/Get faction standing after operator chain`** still shows Grid Operators standing **15** after operator-chain flow.
|
||||
- [ ] Bruno **`quest-progress/Accept grid contract after operator chain`** still accepts grid contract when rep gate met.
|
||||
- [ ] Bruno **`quest-progress/Accept grid contract after operator chain`** accepts grid contract when rep gate met and embedded row **`completionRewardSummary.reputationGrants`** = `[{ factionId: prototype_faction_rust_collective, amount: 10 }]`.
|
||||
- [ ] NEO-129 gather-intro **`completionRewardSummary`** shape unchanged except new empty **`reputationGrants`** array.
|
||||
|
||||
**Client HUD verification** deferred to [NEO-142](NEO-142.md) — this story is server HTTP projection only.
|
||||
**Client HUD verification** deferred to [NEO-142 (Godot HUD)](https://linear.app/neon-sprawl/issue/NEO-142/e7m3-10-client-faction-standing-gate-feedback-hud-godot) — this story is server HTTP projection only.
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ Client can see gate requirements and completion rep summary without local math.
|
|||
- **World GET:** `QuestDefinitionsWorldApi` maps `QuestDefRow.FactionGateRules` → optional `factionGateRules` on `QuestDefinitionJson` (`QuestFactionGateRuleJson`: `factionId`, `minStanding`); omitted when empty.
|
||||
- **Quest progress GET:** `QuestProgressApi.MapCompletionRewardSummary` maps `RewardDeliveryEvent.GrantedReputation` → `reputationGrants` (`QuestReputationGrantJson`: `factionId`, `amount`); required array when summary present.
|
||||
- **Accept POST:** inherits updated mapper via `QuestProgressApi.MapQuestProgressRow` — no accept-specific changes.
|
||||
- **Tests:** `QuestDefinitionsWorldApiTests` grid-contract gate row; `QuestProgressApiTests` operator-chain +15 rep and gather-intro empty rep; **821** tests green.
|
||||
- **Tests:** `QuestDefinitionsWorldApiTests` grid-contract gate row + wire-level `factionGateRules` omission; `QuestProgressApiTests` operator-chain +15 rep, grid-contract +10 rep, gather-intro empty rep; **823** tests green.
|
||||
- **Bruno:** extended `Get quest definitions.bru`; new `Get quest progress after operator chain complete.bru`; gather-intro bru asserts empty `reputationGrants`.
|
||||
- **Docs:** `server/README.md` quest-definitions + quest-progress sections; E7.M3 module + alignment register updated.
|
||||
- **Manual QA:** [`docs/manual-qa/NEO-140.md`](../manual-qa/NEO-140.md).
|
||||
|
|
@ -177,5 +177,5 @@ Manual verification: run Bruno quest-definitions + new quest-progress bru agains
|
|||
| Question / risk | Agent recommendation | Status |
|
||||
|-----------------|----------------------|--------|
|
||||
| Existing clients ignore unknown `reputationGrants` field | **Additive field only** — `schemaVersion` stays 1; NEO-142 is first consumer | `adopted` |
|
||||
| Grid-contract completion rep Bruno needs full hand-in flow | **Operator-chain Bruno first** (primary AC); grid-contract rep assertion in integration test using existing quest wiring helpers | `adopted` |
|
||||
| Grid-contract completion rep Bruno needs full hand-in flow | ~~**Operator-chain Bruno first** (primary AC); grid-contract rep assertion in integration test using existing quest wiring helpers~~ | `adopted` — **done:** integration test + accept bru assert +10 Rust Collective |
|
||||
| `QuestDefinitionsWorldApiTests` missing grid-contract row coverage today | **Add grid-contract gate assertions** in same test method | `adopted` |
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@
|
|||
**Scope:** Branch `NEO-140-e7m3-quest-http-rep-gate-projections` — commits `367f894` … `1dedc5f` vs `main`
|
||||
**Base:** `main`
|
||||
|
||||
**Follow-up:** Suggestions and actionable nits below are **done** (strikethrough + **Done.** / **Addressed.** / **Deferred**).
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approve with nits**
|
||||
|
|
@ -30,15 +32,15 @@ None.
|
|||
|
||||
## Suggestions
|
||||
|
||||
1. **Grid-contract completion rep (+10 Rust Collective) still untested end-to-end.** Plan §prototype targets and open-questions table adopt deferring grid-contract `reputationGrants` Bruno until a hand-in flow helper exists; operator-chain coverage is the primary AC and is solid. When grid-contract completion lands in Bruno or integration helpers, add an assertion for `[{ factionId: prototype_faction_rust_collective, amount: 10 }]` so the third freeze row in the plan table is locked like operator-chain.
|
||||
1. ~~**Grid-contract completion rep (+10 Rust Collective) still untested end-to-end.** Plan §prototype targets and open-questions table adopt deferring grid-contract `reputationGrants` Bruno until a hand-in flow helper exists; operator-chain coverage is the primary AC and is solid. When grid-contract completion lands in Bruno or integration helpers, add an assertion for `[{ factionId: prototype_faction_rust_collective, amount: 10 }]` so the third freeze row in the plan table is locked like operator-chain.~~ **Done.** `QuestProgressApiTests.GetQuestProgress_ShouldReturnGridContractReputationGrant_WhenAcceptedAfterOperatorChain`; `Accept grid contract after operator chain.bru` asserts embedded `completionRewardSummary.reputationGrants`.
|
||||
|
||||
2. **Optional wire-level omission test for `factionGateRules`.** `QuestDefinitionsWorldApiTests` asserts deserialized `FactionGateRules` is null on non-gated quests; Bruno asserts JSON `undefined` for gather intro and operator chain. A small integration test that reads raw JSON (or `JsonDocument`) and confirms the property key is absent on non-gated rows would close the loop without Bruno — low priority given Bruno coverage.
|
||||
2. ~~**Optional wire-level omission test for `factionGateRules`.** `QuestDefinitionsWorldApiTests` asserts deserialized `FactionGateRules` is null on non-gated quests; Bruno asserts JSON `undefined` for gather intro and operator chain. A small integration test that reads raw JSON (or `JsonDocument`) and confirms the property key is absent on non-gated rows would close the loop without Bruno — low priority given Bruno coverage.~~ **Done.** `GetQuestDefinitions_ShouldOmitFactionGateRulesJsonProperty_WhenQuestHasNoGates` uses `JsonDocument` to assert key absence on gather intro and operator chain.
|
||||
|
||||
## Nits
|
||||
|
||||
- Nit: `docs/manual-qa/NEO-140.md` links client follow-up as `[NEO-142](NEO-142.md)` — relative path may not resolve from `docs/manual-qa/`; prefer `NEO-142.md` (same folder) or `../manual-qa/NEO-142.md` from other docs.
|
||||
- Nit: `Get quest progress after operator chain complete.bru` pre-request performs a quest-progress GET before the named GET (mirrors gather-intro bru idempotency pattern) — fine; seq **12** places it after gather-intro complete (**8**); confirm collection order if Bruno runs fail mid-spine.
|
||||
- Nit: Client fixture JSON in `client/test/quest_progress_client_test.gd` (`_completed_with_reward_summary_json`) omits `reputationGrants` — acceptable until **NEO-142**; Godot client ignores unknown keys today.
|
||||
- ~~Nit: `docs/manual-qa/NEO-140.md` links client follow-up as `[NEO-142](NEO-142.md)` — relative path may not resolve from `docs/manual-qa/`; prefer `NEO-142.md` (same folder) or `../manual-qa/NEO-142.md` from other docs.~~ **Done.** Linear issue link until `NEO-142.md` exists.
|
||||
- ~~Nit: `Get quest progress after operator chain complete.bru` pre-request performs a quest-progress GET before the named GET (mirrors gather-intro bru idempotency pattern) — fine; seq **12** places it after gather-intro complete (**8**); confirm collection order if Bruno runs fail mid-spine.~~ **Addressed.** Pre-request helpers reset quest/inventory state per bru; seq order does not share mutable spine between these requests.
|
||||
- Nit: Client fixture JSON in `client/test/quest_progress_client_test.gd` (`_completed_with_reward_summary_json`) omits `reputationGrants` — acceptable until **NEO-142**; Godot client ignores unknown keys today. **Deferred** to NEO-142.
|
||||
|
||||
## Verification
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
using System.Net;
|
||||
using System.Net.Http.Json;
|
||||
using System.Text.Json;
|
||||
using NeonSprawl.Server.Game.Quests;
|
||||
using Xunit;
|
||||
|
||||
|
|
@ -83,4 +84,34 @@ public class QuestDefinitionsWorldApiTests
|
|||
Assert.Equal(PrototypeE7M3QuestFactionRules.GridContractMinStanding, gridContract.FactionGateRules![0].MinStanding);
|
||||
Assert.Null(gatherIntro.FactionGateRules);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetQuestDefinitions_ShouldOmitFactionGateRulesJsonProperty_WhenQuestHasNoGates()
|
||||
{
|
||||
// Arrange
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
|
||||
// Act
|
||||
var response = await client.GetAsync("/game/world/quest-definitions");
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
|
||||
foreach (var quest in doc.RootElement.GetProperty("quests").EnumerateArray())
|
||||
{
|
||||
var id = quest.GetProperty("id").GetString();
|
||||
if (id is "prototype_quest_gather_intro" or PrototypeE7M1QuestCatalogRules.ChainQuestId)
|
||||
{
|
||||
Assert.False(quest.TryGetProperty("factionGateRules", out _));
|
||||
}
|
||||
|
||||
if (id == PrototypeE7M1QuestCatalogRules.GridContractQuestId)
|
||||
{
|
||||
Assert.True(quest.TryGetProperty("factionGateRules", out var rules));
|
||||
Assert.Equal(JsonValueKind.Array, rules.ValueKind);
|
||||
Assert.Equal(1, rules.GetArrayLength());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ public sealed class QuestProgressApiTests
|
|||
private const string RefineQuestId = PrototypeE7M1QuestCatalogRules.RefineIntroQuestId;
|
||||
private const string CombatQuestId = PrototypeE7M1QuestCatalogRules.CombatIntroQuestId;
|
||||
private const string ChainQuestId = PrototypeE7M1QuestCatalogRules.ChainQuestId;
|
||||
private const string GridContractQuestId = PrototypeE7M1QuestCatalogRules.GridContractQuestId;
|
||||
private const string RustCollectiveFactionId = "prototype_faction_rust_collective";
|
||||
private const string GatherObjectiveId = PrototypeE7M1QuestCatalogRules.GatherIntroFirstObjectiveId;
|
||||
private const string ChainGatherObjectiveId = PrototypeE7M1QuestCatalogRules.ChainFirstObjectiveId;
|
||||
|
||||
|
|
@ -244,6 +246,28 @@ public sealed class QuestProgressApiTests
|
|||
AssertOperatorChainCompletionRewardSummary(row.CompletionRewardSummary);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetQuestProgress_ShouldReturnGridContractReputationGrant_WhenAcceptedAfterOperatorChain()
|
||||
{
|
||||
// Arrange — operator-chain kit satisfies grid contract inventory_has_item; accept delivers +10 Rust Collective rep.
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var deps = ResolveDependencies(factory);
|
||||
var client = factory.CreateClient();
|
||||
CompleteOperatorChain(deps);
|
||||
Assert.True(TryAccept(deps, GridContractQuestId).Success);
|
||||
|
||||
// Act
|
||||
var response = await client.GetAsync($"/game/players/{PlayerId}/quest-progress");
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
var body = await response.Content.ReadFromJsonAsync<QuestProgressListResponse>();
|
||||
Assert.NotNull(body);
|
||||
var row = Assert.Single(body!.Quests, static r => r.QuestId == GridContractQuestId);
|
||||
Assert.Equal(QuestProgressApi.StatusCompleted, row.Status);
|
||||
AssertGridContractCompletionRewardSummary(row.CompletionRewardSummary);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetQuestProgress_ShouldCompleteChainTerminal_WhenTokenHeldAndInventoryRefreshRunsOnGet()
|
||||
{
|
||||
|
|
@ -308,6 +332,16 @@ public sealed class QuestProgressApiTests
|
|||
Assert.Equal(15, summary.ReputationGrants[0].Amount);
|
||||
}
|
||||
|
||||
private static void AssertGridContractCompletionRewardSummary(QuestCompletionRewardSummaryJson? summary)
|
||||
{
|
||||
Assert.NotNull(summary);
|
||||
Assert.Empty(summary!.ItemGrants);
|
||||
Assert.Empty(summary.SkillXpGrants);
|
||||
Assert.Single(summary.ReputationGrants);
|
||||
Assert.Equal(RustCollectiveFactionId, summary.ReputationGrants[0].FactionId);
|
||||
Assert.Equal(10, summary.ReputationGrants[0].Amount);
|
||||
}
|
||||
|
||||
private static void AssertCompletionRewardSummariesEqual(
|
||||
QuestCompletionRewardSummaryJson? first,
|
||||
QuestCompletionRewardSummaryJson? second)
|
||||
|
|
|
|||
Loading…
Reference in New Issue