From 67e8d5b558f976a391d89beec854520854aab745 Mon Sep 17 00:00:00 2001 From: Toutsu Date: Mon, 8 Jun 2026 22:34:18 +0300 Subject: [PATCH] fix(bot): keep capacity and club wizard steps consistent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix two wizard FSM bugs reported after v3.9.4: 1. Capacity waitlist buttons could still advance the draft without a numeric MaxPlayers value. The final submit validation then rejected the draft with 'Не заполнены поля: лимит мест'. Now waitlist:on/off stay on Capacity until MaxPlayers is set; users must either enter a numeric limit or explicitly choose '♾ Без лимита'. 2. PickClub computed NextAfterVisibility before SetClubId, so the first club click left the wizard on PickClub and the second click advanced. Now ClubId is saved first and NextAfterVisibility is evaluated after that mutation, so a valid club click advances on the first try. TDD: - WaitlistChoiceWithoutCapacity_StaysOnCapacityStep covers waitlist:on/off. - PickClub_ValidGuid_AdvancesToPublishOnFirstClick covers the single-click club path. - Stale Capacity waitlist callback test updated to the safer no-advance contract. Closes #127 --- .../Wizard/GameCreationWizard.cs | 36 ++++++++++++++----- .../GameCreationWizardStepTransitionsTests.cs | 31 +++++++--------- .../GameCreationWizardValidationTests.cs | 23 ++++++++++++ 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs b/src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs index 2b523de..cea0e52 100644 --- a/src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs +++ b/src/GmRelay.Shared/Features/Sessions/CreateSession/Wizard/GameCreationWizard.cs @@ -298,13 +298,25 @@ public sealed class GameCreationWizard : (null, "Неверная длительность"), }; - private static (string?, string?) ApplyCapacityChoice(WizardPayload p, string choice) => choice switch + private static (string?, string?) ApplyCapacityChoice(WizardPayload p, string choice) { - "waitlist:on" => (WizardStepNames.Visibility, SetWaitlist(p, true)), - "waitlist:off" => (WizardStepNames.Visibility, SetWaitlist(p, false)), - "no_limit" => (WizardStepNames.Visibility, SetMaxPlayers(p, null)), - _ => (null, "Неизвестный выбор"), - }; + if (choice is "no_limit") + { + return (WizardStepNames.Visibility, SetMaxPlayers(p, null)); + } + + if (choice is "waitlist:on" or "waitlist:off" && p.Single?.MaxPlayers is null) + { + return (null, "Сначала введите лимит мест или нажмите «♾ Без лимита»"); + } + + return choice switch + { + "waitlist:on" => (WizardStepNames.Visibility, SetWaitlist(p, true)), + "waitlist:off" => (WizardStepNames.Visibility, SetWaitlist(p, false)), + _ => (null, "Неизвестный выбор"), + }; + } private static (string?, string?) ApplyVisibilityChoice(WizardPayload p, string choice) => choice switch { @@ -316,9 +328,15 @@ public sealed class GameCreationWizard }; private static (string?, string?) ApplyPickClubChoice(WizardPayload p, string choice) - => Guid.TryParse(choice, out var id) - ? (NextAfterVisibility(p), SetClubId(p, id)) - : (null, "Неверный идентификатор клуба"); + { + if (!Guid.TryParse(choice, out var id)) + { + return (null, "Неверный идентификатор клуба"); + } + + var error = SetClubId(p, id); + return (NextAfterVisibility(p), error); + } private static (string?, string?) ApplyPublishChoice(WizardPayload p, string choice) => choice switch { diff --git a/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardStepTransitionsTests.cs b/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardStepTransitionsTests.cs index 6064da5..bf9b549 100644 --- a/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardStepTransitionsTests.cs +++ b/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardStepTransitionsTests.cs @@ -20,9 +20,7 @@ public sealed class GameCreationWizardStepTransitionsTests [InlineData(WizardStepNames.System, "Dnd5e", WizardStepNames.Duration)] // Duration → DateTime (single, no maxPlayers yet) [InlineData(WizardStepNames.Duration, "240", WizardStepNames.DateTime)] - // Capacity → Visibility - [InlineData(WizardStepNames.Capacity, "waitlist:on", WizardStepNames.Visibility)] - [InlineData(WizardStepNames.Capacity, "waitlist:off", WizardStepNames.Visibility)] + // Capacity → Visibility (only explicit no-limit can skip numeric capacity) [InlineData(WizardStepNames.Capacity, "no_limit", WizardStepNames.Visibility)] // Visibility → Publish (public, no club) [InlineData(WizardStepNames.Visibility, "public", WizardStepNames.Publish)] @@ -98,13 +96,11 @@ public sealed class GameCreationWizardStepTransitionsTests } [Fact] - public async Task ChoiceCallback_FromMismatchedStep_AdvancesBasedOnCallbackStep() + public async Task StaleCapacityWaitlistCallback_WithoutCapacity_StaysOnCurrentStep() { - // The wizard's callback parser uses the step encoded in the callback - // (not the draft's current step) to drive transitions. So a stale - // "Capacity" button pressed while the user is on System will in fact - // move the draft forward as if they had pressed it on Capacity. We - // lock that behaviour in. + // A stale waitlist button from Capacity must not move a draft forward + // unless MaxPlayers is already set. Otherwise users can reach Confirm + // with a missing capacity and get "Не заполнены поля: лимит мест". var wizard = BuildWizard(out var drafts, out _); var draft = NewDraft(WizardStepNames.System); drafts.Seed(draft); @@ -112,17 +108,13 @@ public sealed class GameCreationWizardStepTransitionsTests var data = WizardCallbackData.Choice(WizardStepNames.Capacity, "waitlist:on"); await wizard.HandleInteractionAsync(CallbackInteraction(data, ownerId: draft.OwnerId), draft, CancellationToken.None); - Assert.Equal(WizardStepNames.Visibility, draft.Step); + Assert.Equal(WizardStepNames.System, draft.Step); } [Fact] - public async Task PickClub_ValidGuid_ReachesStableStep() + public async Task PickClub_ValidGuid_AdvancesToPublishOnFirstClick() { - // The wizard has a quirk: NextAfterVisibility is evaluated before - // SetClubId, so a single click leaves the draft still on PickClub. - // We assert that the wizard does NOT throw and the messenger is asked - // to re-render (i.e. the handler ran end-to-end). - var wizard = BuildWizard(out var drafts, out var messenger); + var wizard = BuildWizard(out var drafts, out _); var clubId = Guid.NewGuid(); var payload = new WizardPayload { @@ -138,8 +130,11 @@ public sealed class GameCreationWizardStepTransitionsTests var data = WizardCallbackData.Choice(WizardStepNames.PickClub, clubId.ToString()); await wizard.HandleInteractionAsync(CallbackInteraction(data, ownerId: draft.OwnerId), draft, CancellationToken.None); - // Wizard acknowledged the callback and re-rendered the (still PickClub) step. - Assert.NotEmpty(messenger.Edits); + Assert.Equal(WizardStepNames.Publish, draft.Step); + using var doc = JsonDocument.Parse(draft.PayloadJson); + var root = doc.RootElement; + Assert.True(root.TryGetProperty("clubId", out var clubIdJson)); + Assert.Equal(clubId, clubIdJson.GetGuid()); } [Fact] diff --git a/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardValidationTests.cs b/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardValidationTests.cs index d33ec9f..0a55d25 100644 --- a/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardValidationTests.cs +++ b/tests/GmRelay.Bot.Tests/Features/Sessions/CreateSession/Wizard/GameCreationWizardValidationTests.cs @@ -136,6 +136,29 @@ public sealed class GameCreationWizardValidationTests Assert.Equal(WizardStepNames.Capacity, draft.Step); } + [Theory] + [InlineData("waitlist:on")] + [InlineData("waitlist:off")] + public async Task WaitlistChoiceWithoutCapacity_StaysOnCapacityStep(string choice) + { + var wizard = BuildWizard(out var drafts, out _); + var draft = NewDraft(WizardStepNames.Capacity, + new WizardPayload + { + Type = WizardCreationType.Single, + Title = "T", + System = "Dnd5e", + DurationMinutes = 240, + Single = new WizardSingleInput { ScheduledAt = DateTimeOffset.UtcNow.AddDays(1) }, + }); + drafts.Seed(draft); + + var data = WizardCallbackData.Choice(WizardStepNames.Capacity, choice); + await wizard.HandleInteractionAsync(CallbackInteraction(data, ownerId: draft.OwnerId), draft, CancellationToken.None); + + Assert.Equal(WizardStepNames.Capacity, draft.Step); + } + [Theory] [InlineData("0")] [InlineData("13")]