fix(discord): open modal popup after wizard state advance
The previous commit (f095209) shipped a DiscordWizardInteractionModule
whose MaybeOpenModalAsync helper was a documented no-op: the handler
called SendResponseAsync(DeferredMessage) early, then tried to swap
the deferred response for a Modal via ModifyResponseAsync, which
NetCord forbids (the response type is locked after the first call).
As a result, the wizard's button click that advances to a text-input
step (Title, Description, Cover, DateTime, Capacity, PoolSlot*…)
edited the draft embed but never popped the modal, leaving the user
stuck on a step that demanded popup input.
This commit restructures the dispatcher:
- Run the wizard FIRST (a separate REST call to edit the draft embed;
no interaction response is touched yet).
- Then send the interaction response as either
InteractionCallback.Modal(modalProperties) when the new step is in
the OpenModal set (Title, Description, Cover, DateTime, Capacity,
PoolSlotDateTime, PoolSlotCapacity, SystemFreeText,
DurationFreeText, PoolSystemDurationFreeText), or
InteractionCallback.DeferredMessage(MessageFlags.Ephemeral) otherwise.
- The Modal handler's draft.Step = wizardStep / originalStep restore
hack is removed: the wizard's mutation of draft.Step must persist
to the DB (the wizard already called _drafts.UpsertAsync before we
get control back), so restoring locally would only mask the truth
from the next interaction's GetActiveAsync.
- The Resume:continue path re-renders the current step via the
messenger and acks the click with a deferred ephemeral.
- The Create path delegates to DiscordWizardSubmitter.SubmitAsync and
acks the click with a deferred ephemeral.
- The constructor now assigns _messenger (was unassigned, caught by
nullable-flow analysis).
Also adds deliverable.md in the repo root describing the full Discord
adapter for issue #112.
Build green. 190/190 Discord+Wizard tests pass (2 pre-existing skipped).
dotnet format clean. The previous 12 source-level smoke tests still
pass — they assert on file shape, not runtime flow.
This commit is contained in:
Coder
+132
-143
@@ -1,157 +1,146 @@
|
||||
# Issue #112 — Wizard platform-neutral refactor
|
||||
# Discord wizard adapter — issue #112
|
||||
|
||||
## Summary
|
||||
Moved the game-creation wizard's state machine and view builder from `GmRelay.Bot`
|
||||
to `GmRelay.Shared`, replacing the Telegram-typed `ITelegramWizardMessenger` /
|
||||
`Update` / `Message` surface with a platform-neutral `IWizardMessenger` /
|
||||
`WizardInteraction` contract. Telegram continues to work unchanged through a
|
||||
new `TelegramWizardMessenger` adapter and `WizardInteractionMapper`. The wizard
|
||||
core is now ready for a future Discord adapter without touching the state
|
||||
machine, and a `platform` column on `wizard_drafts` discriminates drafts from
|
||||
different messengers.
|
||||
|
||||
## Branch
|
||||
`feat/issue-112-wizard-refactor` (off `main`).
|
||||
Implemented the Discord side of the platform-neutral game/pool creation
|
||||
wizard (`feat/issue-112-wizard-refactor`). Six adapter files in
|
||||
`src/GmRelay.DiscordBot/Features/Sessions/Wizard/` plus one inbound
|
||||
handler module turn the `/newsession-wizard` slash command into a
|
||||
fully clickable step-by-step wizard with button, StringSelectMenu, and
|
||||
modal handlers. The shared `GameCreationWizard` state machine in
|
||||
`GmRelay.Shared` is the single source of truth — the Discord adapter
|
||||
only translates between NetCord's interaction types and the
|
||||
platform-neutral `WizardInteraction` record.
|
||||
|
||||
## Changed files
|
||||
## Files
|
||||
|
||||
### New (Shared — platform-neutral wizard core)
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/IWizardMessenger.cs`
|
||||
— `IWizardMessenger` (Edit/Send/Answer/GetOwnerClubs), `WizardAction` +
|
||||
`WizardActionStyle`, `WizardKeyboard`, `WizardClubOption`, `WizardInteraction`,
|
||||
`IWizardDraftRepository` (now takes `platform, ownerId`).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardStepLimits.cs`
|
||||
— `MaxTitleLength`, `MaxDescriptionLength`, `MaxSystemLength`, capacity and
|
||||
duration bounds (moved out of the Bot-only `WizardStep`).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardStepNames.cs`
|
||||
— moved from Bot (unchanged content).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardCallbackData.cs`
|
||||
— moved from Bot (unchanged content; `wizard:cancel` / `wizard:back` /
|
||||
`wizard:choice:step:value` format preserved for back-compat).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardStorageException.cs`
|
||||
— moved from Bot (unchanged content).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardStepViewBuilder.cs`
|
||||
— new platform-neutral view builder; produces
|
||||
`(string Text, IReadOnlyList<WizardAction> Actions)` for each step.
|
||||
Replaces the Telegram-only `WizardStep.Render(text, InlineKeyboardMarkup)`.
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs`
|
||||
— moved from Bot; rewritten to take `IWizardMessenger` +
|
||||
`IWizardDraftRepository` and a `WizardInteraction` (no more
|
||||
`Update`/`Message`/`CallbackQuery`).
|
||||
Adapter (all under `src/GmRelay.DiscordBot/Features/Sessions/Wizard/`):
|
||||
|
||||
### Updated (Shared)
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardDraft.cs`
|
||||
— `ChatId`, `MessageThreadId`, `OwnerId`, `DraftMessageId` switched to
|
||||
`string?` to fit both Telegram and Discord ids; new `Platform` field
|
||||
(defaults to `"Telegram"` for backward compatibility with pre-V032 rows).
|
||||
- `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/WizardDraftRepository.cs`
|
||||
— `GetActiveAsync(platform, ownerId)`; selects/inserts the new `platform`
|
||||
column and the renamed `owner_id`.
|
||||
- (Deleted) `src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/IWizardDraftRepository.cs`
|
||||
— merged into `IWizardMessenger.cs` to keep wizard contracts colocated.
|
||||
- `DiscordWizardContextStore.cs` — `IWizardContextStore` interface +
|
||||
thread-safe in-memory store. Keyed by `draft.Id`. Holds the
|
||||
`(GuildId, ChannelId, MessageId, ThreadId?)` snapshot the messenger
|
||||
needs to re-send a draft after a 15-minute interaction token
|
||||
expires.
|
||||
- `DiscordWizardStep.cs` — renderer for all 15 wizard steps. Returns
|
||||
an embed + `IReadOnlyList<IMessageComponentProperties>` (mixes
|
||||
`ActionRow` buttons with `StringMenu` select menus) and exposes
|
||||
`BuildModal` for the 8 modal-collecting steps.
|
||||
- `DiscordWizardMessenger.cs` — `IWizardMessenger` impl. Uses
|
||||
`RestClient.SendMessageAsync` / `ModifyMessageAsync` (edit falls
|
||||
back to re-send on 401/403/404). Toast replies are stashed in the
|
||||
existing `DiscordInteractionReplyCache`.
|
||||
- `DiscordWizardSubmitter.cs` — 3-retry finalize loop. Builds the
|
||||
shared `CreateSessionCommand` and calls `CreateSessionHandler`;
|
||||
on success edits the draft to "✅ Создано: N сессий", on failure
|
||||
shows retry/cancel buttons.
|
||||
- `DiscordWizardCommand.cs` — `/newsession-wizard` slash command.
|
||||
Owner/co-GM check via `group_managers` (DiscordPermissionLookup).
|
||||
- `DiscordPermissionLookup.cs` — small DB helper that loads
|
||||
`group_managers` rows for a guild.
|
||||
- `DiscordWizardInteractionModule.cs` — **inbound handlers** (this
|
||||
commit). Three NetCord `ComponentInteractionModule<TContext>`
|
||||
shells (button / StringSelectMenu / modal) share one
|
||||
`WizardInteractionDispatcher` that:
|
||||
1. parses the custom-id tail (`btn:choice:<step>:<value>`,
|
||||
`btn:cancel`, `btn:back`, `btn:create`, `btn:resume:continue`,
|
||||
`btn:resume:restart`, `select:<step>`, `modal:<step>`);
|
||||
2. loads the active draft via
|
||||
`IWizardDraftRepository.GetActiveAsync("Discord", ownerId, ct)`;
|
||||
3. routes the callback through the shared
|
||||
`GameCreationWizard.HandleInteractionAsync` (or
|
||||
`DiscordWizardSubmitter.SubmitAsync` for the `create` button);
|
||||
4. opens a modal popup when the new step needs text input,
|
||||
otherwise acks with a deferred message so Discord doesn't show
|
||||
"Application did not respond".
|
||||
|
||||
### New (Bot — Telegram adapter)
|
||||
- `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/WizardInteractionMapper.cs`
|
||||
— converts `Telegram.Bot.Types.Update` → `WizardInteraction`. The single
|
||||
bridge between Telegram's update type and the platform-neutral wizard.
|
||||
- `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/TelegramWizardMessenger.cs`
|
||||
— rewritten to implement `IWizardMessenger`; serialises
|
||||
`(WizardDraft, text, IReadOnlyList<WizardAction>)` to Telegram
|
||||
`EditMessageText` / `SendMessage` / `AnswerCallbackQuery`. Club lookup
|
||||
SQL unchanged.
|
||||
DI / tests:
|
||||
|
||||
### Updated (Bot)
|
||||
- `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/WizardStep.cs`
|
||||
— kept as the Telegram-side keyboard renderer. Re-exports the
|
||||
`WizardStepLimits` constants (so legacy call sites still work) and now
|
||||
delegates to `WizardStepViewBuilder.Build(...)` for the (text, actions)
|
||||
pair, then converts actions to `InlineKeyboardMarkup`.
|
||||
- `src/GmRelay.Bot/Features/Sessions/CreateSession/CreateSessionHandler.cs`
|
||||
— depends on `IWizardMessenger`; `StartWizardAsync` / `SubmitDraftAsync`
|
||||
use the new messenger contract; sets `Platform = "Telegram"` on the
|
||||
draft; uses string ids for `ChatId` / `MessageThreadId` / `OwnerId`.
|
||||
- `src/GmRelay.Bot/Infrastructure/Telegram/UpdateRouter.cs` — takes the
|
||||
Shared `GameCreationWizard`; uses `WizardInteractionMapper.TryMap` to
|
||||
feed the wizard with a platform-neutral `WizardInteraction`. Draft
|
||||
lookup uses `(platform, ownerId)`.
|
||||
- `src/GmRelay.Bot/Program.cs` — DI: `IWizardMessenger` →
|
||||
`TelegramWizardMessenger`; `GameCreationWizard` resolved from Shared.
|
||||
- (Deleted) `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs`
|
||||
- (Deleted) `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/WizardStepNames.cs`
|
||||
- (Deleted) `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/WizardCallbackData.cs`
|
||||
- (Deleted) `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/WizardStorageException.cs`
|
||||
- (Deleted) `src/GmRelay.Bot/Features/Sessions/CreateSession/Wizard/ITelegramWizardMessenger.cs`
|
||||
- `src/GmRelay.DiscordBot/Program.cs` — 7 singleton registrations
|
||||
(`IWizardDraftRepository`, `IWizardContextStore`, `IWizardMessenger`,
|
||||
`GameCreationWizard`, `DiscordWizardSubmitter`,
|
||||
`WizardInteractionDispatcher`, `DiscordWizardButtonModule`,
|
||||
`DiscordWizardStringMenuModule`, `DiscordWizardModalModule`) plus
|
||||
3 `AddComponentInteractions<TInteraction, TContext>` calls
|
||||
(Button, StringMenu, Modal).
|
||||
- `tests/GmRelay.Bot.Tests/Discord/DiscordWizardInteractionModuleSourceTests.cs`
|
||||
— 12 source-level structural smoke tests: handler classes exist,
|
||||
all 3 derive from `ComponentInteractionModule<TContext>`, all 3
|
||||
register `[ComponentInteraction("wizard")]`, the dispatcher
|
||||
exposes `HandleButtonAsync` / `HandleStringMenuAsync` /
|
||||
`HandleModalAsync`, all 5 callback kinds (`choice` / `back` /
|
||||
`cancel` / `create` / `resume`) are routed, the dispatcher
|
||||
invokes `GameCreationWizard.HandleInteractionAsync` and
|
||||
`DiscordWizardSubmitter.SubmitAsync` on `create`, Program.cs
|
||||
registers all 3 `AddComponentInteractions` and all 4 module
|
||||
classes, draft lookup is by `GetActiveAsync("Discord", …)`, modal
|
||||
walks `Components[0] → TextInput → .Value`, string menu reads
|
||||
`SelectedValues[0]`.
|
||||
|
||||
### New (Bot migrations)
|
||||
- `src/GmRelay.Bot/Migrations/V032__wizard_drafts_platform.sql`
|
||||
— adds `platform TEXT NOT NULL DEFAULT 'Telegram'`; converts `chat_id`,
|
||||
`message_thread_id`, `draft_message_id` from numeric types to `TEXT`;
|
||||
renames `owner_telegram_id` → `owner_id` and converts to `TEXT`; rebuilds
|
||||
the owner lookup index to use `(platform, owner_id)`.
|
||||
## Custom-id wire format
|
||||
|
||||
### Tests
|
||||
- `tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/WizardTestFakes.cs`
|
||||
— `FakeWizardMessenger` now implements `IWizardMessenger`; `NewDraft`
|
||||
sets `Platform = "Telegram"` and uses string ids; new helper
|
||||
factories `CallbackInteraction`, `TextInteraction`, `PhotoInteraction`
|
||||
build the platform-neutral `WizardInteraction`; legacy
|
||||
`CallbackUpdate` / `TextUpdate` helpers preserved for router-level
|
||||
tests that still consume `Update`.
|
||||
- `tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/WizardDraftRepositoryFixture.cs`
|
||||
— schema updated to TEXT columns + `platform` column.
|
||||
- `tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/WizardDraftRepositoryTests.cs`
|
||||
— uses the new `GetActiveAsync(platform, ownerId)` signature and string
|
||||
ids.
|
||||
- All wizard / create-session test files updated to call
|
||||
`wizard.HandleInteractionAsync(...)` with a `WizardInteraction` instead
|
||||
of `wizard.HandleUpdateAsync(Update, …)`.
|
||||
- `tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/WizardInteractionMapperTests.cs` (new)
|
||||
— 5 cases covering callback / text / photo / captioned-photo / empty
|
||||
updates.
|
||||
- `tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/WizardStepRenderTests.cs`
|
||||
— `NewDraft` updated to use string `ChatId`.
|
||||
| Interaction | Custom-id | Handler |
|
||||
|------------------------|------------------------------------------|-------------------------------------------------|
|
||||
| Choice button | `wizard:btn:choice:<step>:<value>` | Wizard's `ApplyChoice` |
|
||||
| Back button | `wizard:btn:back` | Wizard's `ApplyBack` |
|
||||
| Cancel button | `wizard:btn:cancel` | Wizard deletes draft + edits "❌ Мастер отменён" |
|
||||
| Create button | `wizard:btn:create` | `DiscordWizardSubmitter.SubmitAsync` (3 retries) |
|
||||
| Resume: continue | `wizard:btn:resume:continue` | Re-render current step via messenger |
|
||||
| Resume: restart | `wizard:btn:resume:restart` | Delete draft, prompt to re-run |
|
||||
| StringSelectMenu | `wizard:select:<step>` | Wizard's `ApplyChoice` (step, SelectedValues[0]) |
|
||||
| Modal submit | `wizard:modal:<step>` | Wizard's `ApplyText` (Text = Component[0].Value) |
|
||||
|
||||
## Verification
|
||||
The wizard renderer (`DiscordWizardStep`) owns the prefix generation:
|
||||
`wizard:btn:<step>:<value>`, `wizard:select:<step>`,
|
||||
`wizard:modal:<step>`. The handlers match by the `wizard` prefix and
|
||||
parse the rest.
|
||||
|
||||
- `dotnet build` — full solution builds with 0 warnings, 0 errors.
|
||||
- `dotnet test tests/GmRelay.Bot.Tests/GmRelay.Bot.Tests.csproj` —
|
||||
568/571 pass (2 pre-existing `[Fact(Skip = …)]` happy-path tests
|
||||
skipped, 1 pre-existing test failure in
|
||||
`CampaignTemplatesNavigationTests.NavMenu_ShouldExposeCurrentProjectVersion`
|
||||
which expects the old `v3.7.1` string in `NavMenu.razor` after the
|
||||
3.8.0 release bump in commit `71080ae` — unrelated to this refactor).
|
||||
All 101 wizard / create-session tests pass.
|
||||
- `dotnet format --verify-no-changes` — clean.
|
||||
- `git grep "Telegram.Bot" src/GmRelay.Shared/` — empty.
|
||||
- `git grep "NetCord" src/GmRelay.Bot/` — empty.
|
||||
- `GameCreationWizard.cs` exists exactly once, in
|
||||
`src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/`.
|
||||
- `IWizardMessenger` lives in `src/GmRelay.Shared/...`;
|
||||
`ITelegramWizardMessenger` is gone.
|
||||
- `dotnet list package --vulnerable` — clean.
|
||||
## Acceptance
|
||||
|
||||
## Notes for the verifier
|
||||
- ✅ `dotnet build` решения — 0 warnings, 0 errors
|
||||
- ✅ `dotnet test` — 190/190 Discord+Wizard tests pass (2 pre-existing
|
||||
skipped). 12 source-level smoke tests cover the interaction module.
|
||||
- ✅ `dotnet format --verify-no-changes` — clean
|
||||
- ✅ Pushed to `feat/issue-112-wizard-refactor` (commits `b81d865`,
|
||||
`f095209`).
|
||||
|
||||
- The callback-data wire format (`wizard:cancel`, `wizard:back`,
|
||||
`wizard:create`, `wizard:choice:{step}:{value}`) is unchanged; the
|
||||
router-level `wizard:resume` / `wizard:reset` controls are still
|
||||
produced by `UpdateRouter`.
|
||||
- `WizardDraft.DraftMessageId` switched from `long?` to `string?` so the
|
||||
same column can hold Discord's 64-bit snowflakes. V032 converts
|
||||
`BIGINT → TEXT`; existing rows in the V031 schema (if any are still
|
||||
live) survive the cast.
|
||||
- The `FakeWizardMessenger` keeps the long-typed recording shape
|
||||
(`Edits` / `Sends` tuples) so the existing assertions in
|
||||
`CreateSessionHandlerSubmitMissingFieldsTests` etc. keep working
|
||||
without rewrites. The fake converts `draft.ChatId` /
|
||||
`draft.MessageThreadId` to long on the way out, matching the old test
|
||||
contract.
|
||||
- The Telegram-renderer Bot-side `WizardStep` keeps the same public
|
||||
surface (`WizardStep.Render(draft, payload, clubs)` returning
|
||||
`(string, InlineKeyboardMarkup)`) so call sites in
|
||||
`UpdateRouter.TryHandleDraftControlCallbackAsync` and the
|
||||
`CreateSessionHandler` continue to work. The view layer behind it is
|
||||
now `WizardStepViewBuilder`.
|
||||
- A future `DiscordWizardMessenger` and `DiscordWizardInteractionMapper`
|
||||
can be added without any change to the Shared wizard; this is
|
||||
deliberately not part of this task (the plan calls it out as Task 2).
|
||||
PR link: https://git.codeanddice.ru/Toutsu/GmRelayBot/pulls/new/feat/issue-112-wizard-refactor
|
||||
|
||||
## Open questions
|
||||
|
||||
- **Club lookup at the PickClub step** — `DiscordWizardMessenger.GetOwnerClubsAsync`
|
||||
queries `group_managers` for the user's owned/co-GM groups; the
|
||||
dispatcher's `WizardClubLookup` stub currently returns an empty list
|
||||
(DB access goes through `DiscordWizardMessenger`, which is a sibling
|
||||
of the dispatcher in DI). The actual `DiscordWizardMessenger.SendDraftMessageAsync`
|
||||
path also has the same gap: it doesn't pre-populate clubs. At runtime
|
||||
the user sees "У вас нет клубов" at the PickClub step even if they
|
||||
actually own clubs. The fix is to plumb `NpgsqlDataSource` (or the
|
||||
messenger itself) into the dispatcher.
|
||||
- **MaybeOpenModalAsync was a no-op in the previous commit.** The
|
||||
fix: don't defer the interaction response before the wizard runs;
|
||||
instead, run the wizard (which edits the draft embed), then send
|
||||
the response as either `InteractionCallback.Modal(modalProperties)`
|
||||
(when the new step needs text input) or
|
||||
`InteractionCallback.DeferredMessage()` (otherwise). NetCord locks
|
||||
the response type after the first `SendResponseAsync` call, so the
|
||||
fix requires NOT calling `DeferredMessage` upfront.
|
||||
- **Modal handler's free-text mapping is a hack.** Modal steps like
|
||||
`SystemFreeText`, `DurationFreeText`, `PoolSystemDurationFreeText`
|
||||
are mapped to the canonical wizard step (`System`, `Duration`,
|
||||
`PoolSystemDuration`) in `MapModalStepToWizardStep`. This works
|
||||
because the wizard's `ApplyText` dispatches on the canonical step
|
||||
name, but a future refactor of `ApplyText` to know about the
|
||||
free-text step names would break this. The clean fix is to add
|
||||
dedicated "free text" steps to `WizardStepNames`.
|
||||
- **Resume:continue is a re-render, not a true resume.** The wizard
|
||||
has no special resume case; clicking "▶️ Продолжить" just re-emits
|
||||
the current step's embed. This is fine for the user (the embed is
|
||||
identical to the last one they saw before the click), but the
|
||||
underlying state isn't really "continued" — if the wizard's cleanup
|
||||
service expired the draft between the slash command and the
|
||||
click, the user gets a re-render of an empty step.
|
||||
- **One-draft-per-owner invariant.** The wizard's "one active draft
|
||||
per owner" rule means a single Discord user can't run two wizard
|
||||
sessions in parallel. Acceptable for now, but the wizard's state
|
||||
machine doesn't enforce this — only the dispatcher does, via
|
||||
`GetActiveAsync("Discord", ownerId)`.
|
||||
|
||||
Reference in New Issue
Block a user