feat(events): support single-activity rolls and automatic winners - #35
Open
mwg-bender[bot] wants to merge 5 commits into
Open
mwg-bender[bot] wants to merge 5 commits into
mwg-bender[bot] wants to merge 5 commits into
Conversation
Enforce one-to-five valid selections, normalize winners across edits, and retain dialog state on failed saves. Cover API persistence and UI save and reroll regressions for issue #27.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three unresolved moderate findings affect malformed data handling, import normalization, and refresh error handling.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds support for 1–5 activity events, automatic single-activity winners, stronger validation, and improved save handling.
Changes:
- Added winner normalization and selection validation.
- Improved UI slot management and save/refresh error handling.
- Expanded API/UI tests and repository documentation.
| File | Summary | Final review notes |
|---|---|---|
MW-GC.EventManager.Web/Pages/Events.razor |
Supports single-slot events and save errors. | Moderate issue (2 votes): refresh JsonException is not handled. |
MW-GC.EventManager.Tests/SlotRerollTests.cs |
Adds slot and save-flow coverage. | No final comments. |
MW-GC.EventManager.Tests/SingleActivityApiTests.cs |
Adds API and persistence coverage. | No final comments. |
MW-GC.EventManager.Tests/MW-GC.EventManager.Tests.csproj |
Adds test dependencies. | No final comments. |
MW-GC.EventManager.Shared/Entities/EventEntity.cs |
Adds selection validation and winner rules. | Two moderate issues (1 vote each): null nested collections are accepted, and imported single-activity events may lack normalized winners. |
MW-GC.EventManager.API/Services/EventGenerator.cs |
Enforces selection bounds. | No final comments. |
MW-GC.EventManager.API/Properties/AssemblyInfo.cs |
Exposes internals to tests. | No final comments. |
MW-GC.EventManager.API/Functions/EventFunctions.cs |
Validates requests and normalizes winners. | No final comments. |
docs/agents/triage-labels.md |
Documents triage labels. | No final comments. |
docs/agents/issue-tracker.md |
Documents issue workflow. | No final comments. |
docs/agents/domain.md |
Documents domain context. | No final comments. |
AGENTS.md |
Documents repository guidance. | No final comments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Retrying an ambiguous create failure can submit a duplicate event because creates are not idempotent.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (1)
Retain a per-dialog request key and use atomic table inserts so a lost response cannot create a second event. Reconcile matching retries and reject changed payloads without overwriting saved data.
Comment on lines
+1
to
+7
| # Customized event create retries | ||
|
|
||
| `POST /api/events` accepts an optional `Idempotency-Key` header containing one non-empty GUID in `D` format. The key becomes the event ID; body IDs remain ignored. The UI generates a key for each create dialog and retains it across failed saves, including lost responses and timeouts. Updates still use PUT. | ||
|
|
||
| The API uses Azure Table Storage's atomic insert, not upsert or a read-before-write check. An existing row cannot be replaced by a retry. An identical normalized payload returns the existing event (200); a new insert returns 201. If the stored details differ, the API returns 409 with instructions to reload and edit the saved event. Invalid keys return 400. Requests without a key retain legacy server-generated IDs and are not retry-safe. | ||
|
|
||
| The event row itself is the deduplication record, so protection lasts while that row exists. This is not a durable request ledger: reopening the dialog or reloading the browser starts a new create, and replay after deletion can recreate the same ID. The generated-event endpoint is unchanged. Deploy the API support before the updated UI; an older API ignores the header. |
This branch has not been deployed
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.
Summary
Validation
Using DOTNET_ROOT=/opt/data/profiles/development/cache/scratch/dotnet-issue23-complete and DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1:
dotnet test MW-GC.EventManager.Tests/MW-GC.EventManager.Tests.csproj: 71 passed, 0 failed/skipped.dotnet build MW-GC.EventManager.slnx: succeeded, 0 warnings/errors.dotnet test MW-GC.EventManager.Tests/MW-GC.EventManager.Tests.csproj --filter FullyQualifiedName~SingleActivityApiTests: 39 passed.dotnet test MW-GC.EventManager.Tests/MW-GC.EventManager.Tests.csproj --filter FullyQualifiedName~SlotRerollTests: 32 passed.git diff --check: clean.UI tests exercise component logic with mocked HTTP, not a live browser. Storage tests use mocked Azure Tables. No merge or deployment performed.
Publication audit
New publication supersedes closed, unmerged #34; #34 is not approval evidence. Final validation commit is authored by MWG Bender[bot]. Prior implementation commit 363d20e retains its existing Hermes Agent identity; published history was not rewritten. Base is explicitly dev, whose fetched tip is an ancestor of this branch.
Closes #27