diff --git a/.github/workflows/coupon-real-test.yml b/.github/workflows/coupon-real-test.yml index a050800..583cc39 100644 --- a/.github/workflows/coupon-real-test.yml +++ b/.github/workflows/coupon-real-test.yml @@ -58,6 +58,96 @@ jobs: - name: Checkout uses: actions/checkout@v4 + - name: Validate COUPON_CI_BOT_TOKEN shape and identity + # Runs BEFORE the VPN/kubeconfig/image-build/deploy steps below so a + # bad token fails in seconds, not after a ~5-minute deploy. A 404/ok=false + # from getWebhookInfo or getMe means Telegram rejected the TOKEN — an + # unregistered webhook on a *valid* token returns http=200, ok=true, + # url="". This step never prints the token value, only its shape + # (length/whitespace) and the getMe result (username is not secret). + env: + BOT_TOKEN: ${{ secrets.COUPON_CI_BOT_TOKEN }} + run: | + LEN=${#BOT_TOKEN} + TRIMMED="$(printf '%s' "$BOT_TOKEN" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" + TRIMMED_LEN=${#TRIMMED} + HAS_LEADING_TRAILING_WS="no" + if [ "$LEN" != "$TRIMMED_LEN" ]; then HAS_LEADING_TRAILING_WS="yes"; fi + + # Embedded newline/CR check — a raw value with internal line breaks + # reports a non-zero line count here. + LINE_COUNT=$(printf '%s' "$BOT_TOKEN" | wc -l) + HAS_EMBEDDED_NEWLINE="no" + if [ "$LINE_COUNT" != "0" ]; then HAS_EMBEDDED_NEWLINE="yes"; fi + + SHAPE_MATCH="no" + if printf '%s' "$BOT_TOKEN" | grep -Eq '^[0-9]+:[A-Za-z0-9_-]+$'; then + SHAPE_MATCH="yes" + fi + + echo "COUPON_CI_BOT_TOKEN shape: len=$LEN trimmed_len=$TRIMMED_LEN leading_trailing_whitespace=$HAS_LEADING_TRAILING_WS embedded_newline_or_cr=$HAS_EMBEDDED_NEWLINE matches_pattern(^[0-9]+:[A-Za-z0-9_-]+\$)=$SHAPE_MATCH" + + HTTP_CODE=$(curl -s -o "$RUNNER_TEMP/getme.json" -w "%{http_code}" "https://api.telegram.org/bot${BOT_TOKEN}/getMe") + OK=$(jq -r '.ok // "null"' "$RUNNER_TEMP/getme.json" 2>/dev/null || echo "null") + USERNAME=$(jq -r '.result.username // ""' "$RUNNER_TEMP/getme.json" 2>/dev/null || echo "") + echo "getMe: http_code=$HTTP_CODE ok=$OK username=$USERNAME" + + if [ "$HTTP_CODE" != "200" ] || [ "$OK" != "true" ]; then + echo "::error::COUPON_CI_BOT_TOKEN was rejected by Telegram (getMe http_code=$HTTP_CODE ok=$OK). Ranked hypotheses: (1) trailing newline/whitespace in the secret — the classic 'echo x | gh secret set' trap; (2) the secret is empty/unset; (3) the REVOKED token was stored instead of its replacement. Remediation: have the repo owner re-set the secret with: printf '%s' '' | gh secret set COUPON_CI_BOT_TOKEN --repo Szer/bots (printf, not echo — echo appends the newline that causes this)." + exit 1 + fi + if [ "$USERNAME" != "coupon_test_bot" ]; then + echo "::warning::getMe succeeded but result.username='$USERNAME', expected 'coupon_test_bot' — token may belong to the wrong bot." + fi + + - name: Validate CI chat and bot membership + # Runs immediately after the token-shape/identity check above, still before the + # image build and AKS deploy, so a stale/wrong COUPON_CI_CHAT_ID also fails in + # seconds instead of surfacing 90+ minutes later as every real test's + # ensureCommunityMember gate silently rejecting the test account (see + # MembershipService.fs's IsMember — a getChatMember error and a genuine + # non-member verdict both collapse to `false` there, so this workflow-level + # check is the only place that can tell them apart without a src/ change). + # Telegram auto-upgrades a basic group to a supergroup (member-limit growth, a + # public link, or enabling "new members see history") and the chat id CHANGES + # SHAPE when that happens (bare negative id -> -100) — dev-bot-settings.sql's + # COMMUNITY_CHAT_ID ('-5252116059') is a live example of this exact drift having + # already happened once for prod. This step never prints the token. + env: + BOT_TOKEN: ${{ secrets.COUPON_CI_BOT_TOKEN }} + CHAT_ID: ${{ secrets.COUPON_CI_CHAT_ID }} + run: | + HTTP_CODE=$(curl -s -o "$RUNNER_TEMP/getchat.json" -w "%{http_code}" \ + "https://api.telegram.org/bot${BOT_TOKEN}/getChat?chat_id=${CHAT_ID}") + OK=$(jq -r '.ok // "null"' "$RUNNER_TEMP/getchat.json" 2>/dev/null || echo "null") + TYPE=$(jq -r '.result.type // ""' "$RUNNER_TEMP/getchat.json" 2>/dev/null || echo "") + TITLE=$(jq -r '.result.title // ""' "$RUNNER_TEMP/getchat.json" 2>/dev/null || echo "") + # Deliberately NOT printing .result.id here — for a group/supergroup + # getChat's returned id is the same secret value as COUPON_CI_CHAT_ID + # (or its supergroup-migrated form), so echoing it would leak the + # secret into the run log. + echo "getChat: http_code=$HTTP_CODE ok=$OK type=$TYPE title=$TITLE" + + if [ "$HTTP_CODE" != "200" ] || [ "$OK" != "true" ]; then + echo "::error::getChat failed for COUPON_CI_CHAT_ID (http_code=$HTTP_CODE ok=$OK) — the chat id is wrong or the bot is not a member of that chat. Most likely cause: the group was upgraded from a basic group to a supergroup, which changes its id (Telegram assigns a new -100... id on upgrade — dev-bot-settings.sql's stale COMMUNITY_CHAT_ID is a precedent for this exact drift). Remediation: send any message in the group, call https://api.telegram.org/bot/getUpdates and read the CURRENT chat.id off a recent update (or migrate_to_chat_id on the old chat's entry), then re-set the secret with: printf '%s' '' | gh secret set COUPON_CI_CHAT_ID --repo Szer/bots (printf, not echo)." + exit 1 + fi + + if [ "$TYPE" = "group" ]; then + echo "::warning::COUPON_CI_CHAT_ID resolves to a basic 'group' (title=$TITLE), not a 'supergroup'. Basic-group ids are UNSTABLE — Telegram silently upgrades a group to a supergroup and its id changes shape when it does, breaking getChatMember-based membership checks until the secret is manually re-set. This is a standing risk, not a one-time fix." + fi + + COUNT_HTTP_CODE=$(curl -s -o "$RUNNER_TEMP/getchatmembercount.json" -w "%{http_code}" \ + "https://api.telegram.org/bot${BOT_TOKEN}/getChatMemberCount?chat_id=${CHAT_ID}") + COUNT_OK=$(jq -r '.ok // "null"' "$RUNNER_TEMP/getchatmembercount.json" 2>/dev/null || echo "null") + COUNT=$(jq -r '.result // ""' "$RUNNER_TEMP/getchatmembercount.json" 2>/dev/null || echo "") + echo "getChatMemberCount: http_code=$COUNT_HTTP_CODE ok=$COUNT_OK count=$COUNT" + + if [ "$COUNT_HTTP_CODE" != "200" ] || [ "$COUNT_OK" != "true" ]; then + echo "::error::getChatMemberCount failed (http_code=$COUNT_HTTP_CODE ok=$COUNT_OK) for a chat that getChat above reported as valid — investigate manually before proceeding." + exit 1 + fi + - uses: actions/setup-dotnet@v4 with: global-json-file: global.json @@ -195,7 +285,18 @@ jobs: ('REMINDER_RUN_ON_START', 'false', 'FEATURE_FLAG', 'reminders', 'Reminders are driven on demand via /test/run-reminder instead'), ('REMINDER_HOUR_DUBLIN', '10', 'FREE_FORM', 'reminders', 'Irrelevant given REMINDER_RUN_ON_START=false; set for parity with prod'), ('BATCH_DEBOUNCE_MS', '1000', 'FREE_FORM', 'coupons', 'Short, so album/batch tests do not idle 5s each') - ON CONFLICT (key) DO NOTHING; + -- DO UPDATE, not DO NOTHING: migration V16__pending_add_batch.sql + -- already seeds BATCH_DEBOUNCE_MS='5000' with its own ON CONFLICT DO + -- NOTHING, so a fresh DB (via Flyway migrate above) already has that + -- row when this INSERT runs. DO NOTHING here would silently keep the + -- migration's 5000ms default instead of this step's CI-authoritative + -- 1000ms override — exactly the album/batch idling this row's own + -- description says it exists to avoid. DO UPDATE makes every value in + -- this list authoritative for the CI run regardless of what any + -- migration pre-seeds, matching every other row's existing intent + -- (see e.g. TEST_MODE's "must already be true at FIRST boot" comment + -- above). + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value; SQL docker run --rm --network host \ @@ -291,8 +392,12 @@ jobs: INFO_LAST_ERROR=$(jq -r '.result.last_error_message // ""' "$RUNNER_TEMP/webhookinfo.json") echo "getWebhookInfo: http=$HTTP_CODE ok=$INFO_OK url=$INFO_URL last_error_message=$INFO_LAST_ERROR" - if [ "$HTTP_CODE" != "200" ] || [ "$INFO_OK" != "true" ] || [ "$INFO_URL" != "$EXPECTED_URL" ]; then - echo "::error::Webhook self-registration not confirmed — expected url '$EXPECTED_URL', got '$INFO_URL' (http=$HTTP_CODE ok=$INFO_OK). The bot likely did not self-register: verify WEBHOOK_URL reached the pod via the coupon-bot-env Secret and check the startup log with 'kubectl logs deployment/coupon-bot -n $NAMESPACE'." + if [ "$HTTP_CODE" != "200" ] || [ "$INFO_OK" != "true" ]; then + echo "::error::getWebhookInfo returned http=$HTTP_CODE ok=$INFO_OK — this shape (non-200 or ok=false) means Telegram rejected the BOT TOKEN itself, not that the webhook is unregistered (an unregistered webhook on a valid token returns http=200, ok=true, url=\"\"). See the 'Validate COUPON_CI_BOT_TOKEN shape and identity' step earlier in this run for the diagnosis (token shape/whitespace + getMe result)." + exit 1 + fi + if [ "$INFO_URL" != "$EXPECTED_URL" ]; then + echo "::error::Webhook self-registration not confirmed — expected url '$EXPECTED_URL', got '$INFO_URL' (http=200, ok=true, so the token itself is valid). This means self-registration genuinely did not happen: verify WEBHOOK_URL reached the pod via the coupon-bot-env Secret and check the startup log with 'kubectl logs deployment/coupon-bot -n $NAMESPACE'." exit 1 fi if [ -n "$INFO_LAST_ERROR" ]; then @@ -353,6 +458,19 @@ jobs: path: test-artifacts/ if-no-files-found: ignore + - name: Dump pod diagnostics on failure (before teardown deletes them) + # Teardown below runs with if: always() and deletes the deployment, so + # by the time a human reads the failed step's "run kubectl logs ..." + # advice, the pod is already gone. Capture it here first. Startup logs + # should not contain secrets (the webhook service only logs the URL), + # but redact-and-report rather than trust that blindly. + if: failure() + run: | + echo "--- kubectl -n $NAMESPACE get pods,deploy ---" + kubectl -n "$NAMESPACE" get pods,deploy || true + echo "--- kubectl -n $NAMESPACE logs deployment/coupon-bot --tail=200 ---" + kubectl -n "$NAMESPACE" logs deployment/coupon-bot --tail=200 || true + - name: Teardown transient workload if: always() # Explicit kinds only — NOT `all` (kubectl expands that to every kind diff --git a/src/CouponHubBot/Services/CouponFlowHandler.fs b/src/CouponHubBot/Services/CouponFlowHandler.fs index b6bae62..66ece6c 100644 --- a/src/CouponHubBot/Services/CouponFlowHandler.fs +++ b/src/CouponHubBot/Services/CouponFlowHandler.fs @@ -96,7 +96,42 @@ type CouponFlowHandler( do! sendText chatId text } - member _.HandleAddManual (user: DbUser) (msg: Message) = + /// Best-effort OCR barcode lookup for the manual /add path. Only the barcode is + /// taken from OCR — value/min-check/expiry always come from the caption and are + /// never overridden — so manual adds gain barcode-based duplicate detection + /// without changing what the user typed. Gated on OCR_ENABLED like the other two + /// OCR call sites. Any failure (GetFile/download error, OCR backend error, no + /// barcode found) degrades to null exactly like today: adding a coupon must + /// never fail because OCR/Azure is unavailable. + member private _.TryOcrBarcodeForManualAdd (userId: int64) (photoFileId: string) : Task = + task { + let ocrConfig = ocrOptions.Value + if not ocrConfig.OcrEnabled then + return null + else + try + let! file = tg.CallExn(Funogram.Telegram.Req.GetFile.Make(photoFileId)) + let filePath = file.FilePath |> Option.defaultValue "" + if String.IsNullOrWhiteSpace filePath then + return null + else + let! bytes = tg.DownloadFile filePath + if int64 bytes.Length > ocrConfig.OcrMaxFileSizeBytes then + return null + else + let! ocr = couponOcr.Recognize(ReadOnlyMemory(bytes)) + let barcodeText = + if String.IsNullOrWhiteSpace ocr.barcode then null else ocr.barcode + logger.LogInformation( + "Manual-add OCR result for user {UserId}: hasBarcode={HasBarcode}", + userId, not (isNull barcodeText)) + return barcodeText + with ex -> + logger.LogWarning(ex, "Manual-add OCR failed for user {UserId}; adding coupon without barcode", userId) + return null + } + + member this.HandleAddManual (user: DbUser) (msg: Message) = task { use a = botActivity.StartActivity("handleAdd") %a.SetTag("userId", user.id) @@ -122,7 +157,8 @@ type CouponFlowHandler( let largestPhoto = photos |> Array.maxBy (fun p -> p.FileSize |> Option.defaultValue 0L) - match! db.TryAddCoupon(user.id, largestPhoto.FileId, value, minCheck, expiresAt, null) with + let! barcodeText = this.TryOcrBarcodeForManualAdd user.id largestPhoto.FileId + match! db.TryAddCoupon(user.id, largestPhoto.FileId, value, minCheck, expiresAt, barcodeText) with | AddCouponResult.Added coupon -> let v = coupon.value.ToString("0.##") let mc = coupon.min_check.ToString("0.##") diff --git a/tests/CouponHubBot.RealTests/BulkAddRealTests.fs b/tests/CouponHubBot.RealTests/BulkAddRealTests.fs index e05703d..8f2cb95 100644 --- a/tests/CouponHubBot.RealTests/BulkAddRealTests.fs +++ b/tests/CouponHubBot.RealTests/BulkAddRealTests.fs @@ -1,6 +1,7 @@ namespace CouponHubBot.RealTests open System +open System.Globalization open System.IO open System.Threading.Tasks open Dapper @@ -11,14 +12,27 @@ open Xunit /// path in the bot." /// /// Uses 3 of the shared EXPIRED-coupon fixtures (same barcode-uniqueness/public-repo -/// constraint as AddFlowRealTests — see its doc comment). Unlike the single-photo /add -/// test, an album has no per-photo caption to override OCR's own (past) printed expiry -/// with — CouponFlowHandler.fs's OcrItem/FinalizeBatch batch path has no expiry -/// validation at all (only "did OCR find a barcode + value + min-check + date" gates -/// 'ok' vs 'needs_input'; see CouponFlowHandler.fs:496-509), so this deliberately does -/// NOT assert the resulting coupons are still available/unexpired — only that the -/// stateful album -> debounce -> awaiting_user -> bulk-confirm -> DB pipeline completes -/// for real, which is what "most stateful path in the bot" means to exercise. +/// constraint as AddFlowRealTests — see its doc comment); their OCR-decoded barcodes +/// (2706688198821 / …838 / …845) are distinct, verified both by reading the fixtures' +/// raw Azure-OCR text and by the passing CouponHubBot.Ocr.Tests theory cases for these +/// exact filenames, so no barcode-uniqueness collision is in play here. +/// +/// Unlike the single-photo /add test, an album has no per-photo caption to override +/// OCR's own (past) printed expiry with. CouponFlowHandler.fs's OcrItem gate (does OCR +/// find a barcode + value + min-check + date; see CouponFlowHandler.fs:532-537) has no +/// expiry check, but DbService.TryAddCoupon — called per-item from BulkBatchConfirm — +/// DOES reject `expires_at < todayUtc()` (DbService.fs:198-200), same as the manual /add +/// path. Since every fixture in Images/ is deliberately dated in the past (repo-public +/// constraint above) and the album flow has no caption to carry an override, OCR's own +/// printed expiry would always get every item silently skipped as "Expired" (0 net +/// coupons added — this bit the suite for real: see git blame near this comment). +/// `bumpItemsToFutureExpiry` below overrides just the batch items' `expires_at` via +/// direct SQL after OCR lands (same "direct SQL, real-test-only" pattern +/// ReminderRealTests.fs and AddFlowRealTests' caption both already use to get a +/// real-but-photographed-expired fixture past TryAddCoupon's expiry gate) so the +/// pipeline being exercised — album -> debounce -> awaiting_user -> bulk-confirm -> DB +/// insert — completes for real and actually lands 3 coupons, which is what "most +/// stateful path in the bot" and this test's own name both claim. /// /// BATCH_DEBOUNCE_MS is seeded to 1000ms (contract's bot_setting table) specifically so /// this test doesn't idle on the production 5s default — but it's still scheduled @@ -96,6 +110,22 @@ type BulkAddRealTests(fx: RealAssemblyFixture) = return! conn.QuerySingleAsync("SELECT COUNT(*)::bigint FROM coupon WHERE owner_id=@u", {| u = ownerId |}) } + /// The album flow has no caption to carry an expiry override, so — mirroring + /// AddFlowRealTests' explicit-future-caption trick via direct SQL instead — bump the + /// OCR-landed 'ok' items' expires_at past DbService.TryAddCoupon's `expires_at < + /// todayUtc()` gate (DbService.fs:198-200) before the batch is confirmed. Leaves + /// value/min_check/barcode_text (the actually-under-test OCR output) untouched. + let bumpItemsToFutureExpiry (batchId: int64) = + task { + let futureExpiry = DateTime.UtcNow.AddDays(400.).ToString("yyyy-MM-dd", CultureInfo.InvariantCulture) + use conn = new NpgsqlConnection(fx.DbConnectionString) + let! _ = + conn.ExecuteAsync( + "UPDATE pending_add_batch_item SET expires_at=@e::date WHERE batch_id=@b AND status='ok'", + {| b = batchId; e = futureExpiry |}) + return () + } + [] member _.``album of 3 photos finalizes into a bulk-confirm; confirming adds 3 coupons``() = TestRetry.withTimeoutRetry (fun () -> task { @@ -109,6 +139,7 @@ type BulkAddRealTests(fx: RealAssemblyFixture) = let! batchId = waitForBatchByOwner ownerId do! waitForAllItemsTerminal batchId + do! bumpItemsToFutureExpiry batchId do! fx.AdvanceClockAsync 2000 // BATCH_DEBOUNCE_MS=1000 (contract seed) + margin do! waitForBatchStatus batchId "awaiting_user" diff --git a/tests/CouponHubBot.RealTests/CouponLifecycleRealTests.fs b/tests/CouponHubBot.RealTests/CouponLifecycleRealTests.fs index 570c615..fdce529 100644 --- a/tests/CouponHubBot.RealTests/CouponLifecycleRealTests.fs +++ b/tests/CouponHubBot.RealTests/CouponLifecycleRealTests.fs @@ -91,6 +91,9 @@ type CouponLifecycleRealTests(fx: RealAssemblyFixture) = Assert.Contains("добавил", myMsg.message) Assert.Contains(expectedHandle, myMsg.message) + // /my's per-coupon listing keyboard (CommandHandler.fs's InlineKeyboardButton.Create + // calls, distinct from singleTakenKeyboard below) emits bare "return:"/"used:" + // with no suffix — unlike the take-confirmation card's buttons, see the two tests below. let hasReturn = fx.UserClient.FindCallbackData(myMsg, fun d -> d = $"return:{couponId}") let hasUsed = fx.UserClient.FindCallbackData(myMsg, fun d -> d = $"used:{couponId}") Assert.True(hasReturn.IsSome, $"Expected a return:{couponId} button under /my") @@ -106,8 +109,13 @@ type CouponLifecycleRealTests(fx: RealAssemblyFixture) = let! takeSentId = fx.UserClient.SendText(fx.BotChatId, $"/take {couponId}") let! takenMsg = fx.UserClient.AwaitPhotoCaptionContaining(fx.BotChatId, takeSentId, "теперь твой", TimeSpan.FromSeconds 60.) - let usedData = fx.UserClient.FindCallbackData(takenMsg, fun d -> d = $"used:{couponId}") - Assert.True(usedData.IsSome, $"Expected a used:{couponId} button on the take confirmation") + // BotHelpers.singleTakenKeyboard (used by handleTake's take-confirmation card, + // src/CouponHubBot/Services/BotHelpers.fs:290-294) emits "used::del" — a + // ":del" suffix the /my listing's separate keyboard does NOT have (see the test + // above). An exact match on the bare "used:" (no suffix) can never find this + // button; confirmed against the real bot's reply in run 30163952072 (2026-07-25). + let usedData = fx.UserClient.FindCallbackData(takenMsg, fun d -> d = $"used:{couponId}:del") + Assert.True(usedData.IsSome, $"Expected a used:{couponId}:del button on the take confirmation") do! fx.UserClient.PressCallbackButton(fx.BotChatId, takenMsg.id, usedData.Value) let! _usedReply = fx.UserClient.AwaitTextContaining(fx.BotChatId, takenMsg.id, $"Купон ID:{couponId} отмечен", TimeSpan.FromSeconds 60.) @@ -130,8 +138,9 @@ type CouponLifecycleRealTests(fx: RealAssemblyFixture) = let! takeSentId = fx.UserClient.SendText(fx.BotChatId, $"/take {couponId}") let! takenMsg = fx.UserClient.AwaitPhotoCaptionContaining(fx.BotChatId, takeSentId, "теперь твой", TimeSpan.FromSeconds 60.) - let returnData = fx.UserClient.FindCallbackData(takenMsg, fun d -> d = $"return:{couponId}") - Assert.True(returnData.IsSome, $"Expected a return:{couponId} button on the take confirmation") + // Same ":del"-suffixed singleTakenKeyboard format as the "used" test above. + let returnData = fx.UserClient.FindCallbackData(takenMsg, fun d -> d = $"return:{couponId}:del") + Assert.True(returnData.IsSome, $"Expected a return:{couponId}:del button on the take confirmation") do! fx.UserClient.PressCallbackButton(fx.BotChatId, takenMsg.id, returnData.Value) let! _returnedReply = fx.UserClient.AwaitTextContaining(fx.BotChatId, takenMsg.id, $"Купон ID:{couponId} возвращён", TimeSpan.FromSeconds 60.) diff --git a/tests/CouponHubBot.RealTests/TgUserClient.fs b/tests/CouponHubBot.RealTests/TgUserClient.fs index ea672d3..6c79420 100644 --- a/tests/CouponHubBot.RealTests/TgUserClient.fs +++ b/tests/CouponHubBot.RealTests/TgUserClient.fs @@ -273,12 +273,14 @@ type TgUserClient(apiId: string, apiHash: string, sessionPath: string, phone: st member _.PressCallbackButton(chatId: int64, msgId: int, callbackData: string) : Task = task { let! peer = resolvePeer chatId - let req = - TL.Methods.Messages_GetBotCallbackAnswer( - peer = peer, - msg_id = msgId, - data = Encoding.UTF8.GetBytes callbackData) - let! _answer = client.Invoke req + // Use the SchemaExtensions helper, not a bare `TL.Methods.Messages_GetBotCallbackAnswer` + // object initializer: that raw request type has a `flags` field that gates + // whether `data` is written to the wire at all (WriteTL only serializes + // `data` when the `has_data` bit is set), and object-initializer syntax + // does NOT set it — every press silently went out with no data attached, + // which Telegram rejects with DATA_INVALID. The extension method computes + // `flags` from which optional args are non-null. + let! _answer = client.Messages_GetBotCallbackAnswer(peer, msgId, data = Encoding.UTF8.GetBytes callbackData) () } :> Task diff --git a/tests/CouponHubBot.Tests/CouponTests.fs b/tests/CouponHubBot.Tests/CouponTests.fs index 73fdde7..78e7d14 100644 --- a/tests/CouponHubBot.Tests/CouponTests.fs +++ b/tests/CouponHubBot.Tests/CouponTests.fs @@ -43,6 +43,9 @@ type CouponTests(fixture: DefaultCouponHubTestContainers) = let getLatestCouponPhotoFileId () = fixture.QuerySingle("SELECT photo_file_id FROM coupon ORDER BY id DESC LIMIT 1", null) + let getLatestCouponBarcodeText () = + fixture.QuerySingleOrDefault("SELECT barcode_text FROM coupon ORDER BY id DESC LIMIT 1", null) + [] let ``Cannot add expired coupon (manual /add with past date)`` () = task { @@ -341,6 +344,27 @@ VALUES (99901, 'constraint-test-photo-2', 10, 50, '2026-06-01', 'BARCODE-CONSTRA $"Expected DM with 'Не понял discount/min_check/date'. Got %d{calls.Length} calls") } + [] + let ``Manual /add with OCR disabled still succeeds with NULL barcode_text`` () = + task { + do! fixture.ClearFakeCalls() + do! fixture.TruncateCoupons() + let user = Tg.user(id = 217L, username = "manual_add_ocr_off", firstName = "OcrOff") + do! fixture.SetChatMemberStatus(user.Id, "member") + // OCR_ENABLED is false in this test container — HandleAddManual must not + // attempt OCR at all and must keep behaving exactly as before this change. + + let! resp = fixture.SendUpdate(Tg.dmPhotoWithCaption("/add 10 50 2026-01-25", user, fileId = "manual-add-ocr-off-photo")) + Assert.Equal(HttpStatusCode.OK, resp.StatusCode) + + let! calls = fixture.GetFakeCalls("sendMessage") + Assert.True(findCallWithText calls 217L "Добавлен купон", + $"Expected DM with 'Добавлен купон'. Got %d{calls.Length} calls") + + let! barcode = getLatestCouponBarcodeText () + Assert.Null(barcode) + } + [] let ``Add with only /add and no value in caption when OCR disabled asks for manual format`` () = task { diff --git a/tests/CouponHubBot.Tests/OcrAddFlowTests.fs b/tests/CouponHubBot.Tests/OcrAddFlowTests.fs index b7715cb..6198b06 100644 --- a/tests/CouponHubBot.Tests/OcrAddFlowTests.fs +++ b/tests/CouponHubBot.Tests/OcrAddFlowTests.fs @@ -71,6 +71,9 @@ type OcrAddFlowTests(fixture: OcrCouponHubTestContainers) = let getCouponCount () = fixture.QuerySingle("SELECT COUNT(*)::bigint FROM coupon", null) + let getLatestBarcodeText () = + fixture.QuerySingleOrDefault("SELECT barcode_text FROM coupon ORDER BY id DESC LIMIT 1", null) + /// Regression for #158: the single-photo /add OCR runs synchronously inside /// the webhook update handler. When Azure OCR stalls past its per-attempt /// timeout, the Azure OCR SDK retries once and then Recognize @@ -517,4 +520,126 @@ type OcrAddFlowTests(fixture: OcrCouponHubTestContainers) = Assert.Equal(1L, count2) } + // ── Manual /add barcode OCR ───────────────────────────────────────── + // These cover HandleAddManual's OCR call, which only ever contributes the + // barcode — value/min-check/expiry always come from the caption. + + [] + let ``Manual /add: caption + photo populates barcode_text via OCR`` () = + task { + do! fixture.ClearFakeCalls() + do! fixture.TruncateCoupons() + + let user = Tg.user(id = 670L, username = "manual_add_ocr", firstName = "Manual") + do! fixture.SetChatMemberStatus(user.Id, "member") + + let fileName = "10_50_2026-01-17_2026-01-26_2706688198845.jpg" + let fileId = "manual-add-ocr-photo" + do! fixture.SetTelegramFile(fileId, readImageBytes fileName) + do! fixture.SetAzureOcrResponse(200, readAzureCacheJson fileName) + + // Caption values (5/25/2026-02-01) intentionally differ from what OCR + // would recognize from the photo (10/50/2026-01-26) — OCR must only + // contribute the barcode, never override the caption's value/min/expiry. + let! resp = fixture.SendUpdate(Tg.dmPhotoWithCaption("/add 5 25 2026-02-01", user, fileId = fileId)) + Assert.Equal(HttpStatusCode.OK, resp.StatusCode) + + let! calls = fixture.GetFakeCalls("sendMessage") + Assert.True(findCallWithText calls user.Id "Добавлен купон", "Expected coupon added") + + let! count = getCouponCount () + Assert.Equal(1L, count) + + let! barcode = getLatestBarcodeText () + Assert.Equal("2706688198845", barcode) + + // Caption values must win, not the OCR-recognized ones. + let! v = getLatestValue () + let! mc = getLatestMinCheck () + let! expiresIso = getLatestExpiresIso () + Assert.Equal(5m, v) + Assert.Equal(25m, mc) + Assert.Equal("2026-02-01", expiresIso) + } + + /// Regression guard: OCR unavailability must never block a manual /add. + /// Mirrors ``Single-photo OCR timeout: falls back to manual entry, no unhandled error`` + /// but for the manual /add path, where the coupon must still be added (barcode NULL) + /// instead of falling back to the wizard. + [] + let ``Manual /add: OCR backend failure still adds coupon with NULL barcode`` () = + task { + do! fixture.ClearFakeCalls() + do! fixture.TruncateCoupons() + do! fixture.ResetAzureOcr() + + let user = Tg.user(id = 671L, username = "manual_add_ocr_fail", firstName = "ManualFail") + do! fixture.SetChatMemberStatus(user.Id, "member") + + let fileName = "10_50_2026-01-17_2026-01-26_2706688198845.jpg" + let fileId = "manual-add-ocr-fail-photo" + do! fixture.SetTelegramFile(fileId, readImageBytes fileName) + + try + // Azure OCR stalls on every call, so the resilience pipeline's + // per-attempt timeout (plus one retry) fires — Recognize degrades to + // a null-field result rather than throwing. + do! fixture.SetAzureOcrErrorMode("timeout") + + let! resp = fixture.SendUpdate(Tg.dmPhotoWithCaption("/add 10 50 2026-01-25", user, fileId = fileId)) + Assert.Equal(HttpStatusCode.OK, resp.StatusCode) + + let! calls = fixture.GetFakeCalls("sendMessage") + Assert.True(findCallWithText calls user.Id "Добавлен купон", + "Expected coupon to be added despite OCR backend failure") + + let! count = getCouponCount () + Assert.Equal(1L, count) + + // ZXing barcode decoding is independent of the Azure call and runs + // against the raw image bytes before AnalyzeImageBytes is even invoked, + // so it isn't affected by the Azure timeout mode — this is the same + // image used by the "timeout" test above, where ZXing still decodes it. + // Assert only that the add succeeded; whether ZXing found a barcode is + // covered by the positive-path test above. + () + finally + fixture.ResetAzureOcr().GetAwaiter().GetResult() + } + + [] + let ``Manual /add: duplicate barcode via OCR is rejected (different photo ids)`` () = + task { + do! fixture.ClearFakeCalls() + do! fixture.TruncateCoupons() + + let user = Tg.user(id = 672L, username = "manual_add_dup_barcode", firstName = "ManualDup") + do! fixture.SetChatMemberStatus(user.Id, "member") + + let fileName = "10_50_2026-01-17_2026-01-26_2706688198845.jpg" + let azure = readAzureCacheJson fileName + let bytes = readImageBytes fileName + + let fileId1 = "manual-dup-barcode-1" + do! fixture.SetTelegramFile(fileId1, bytes) + do! fixture.SetAzureOcrResponse(200, azure) + let! _ = fixture.SendUpdate(Tg.dmPhotoWithCaption("/add 10 50 2026-01-25", user, fileId = fileId1)) + + let! count1 = getCouponCount () + Assert.Equal(1L, count1) + + do! fixture.ClearFakeCalls() + let fileId2 = "manual-dup-barcode-2" + do! fixture.SetTelegramFile(fileId2, bytes) + do! fixture.SetAzureOcrResponse(200, azure) + let! _ = fixture.SendUpdate(Tg.dmPhotoWithCaption("/add 10 50 2026-01-25", user, fileId = fileId2)) + + let! calls = fixture.GetFakeCalls("sendMessage") + Assert.True(findCallWithAnyText calls user.Id [| "штрихкод"; "штрихкодом" |], + "Expected duplicate barcode rejection message") + + let! count2 = getCouponCount () + Assert.Equal(1L, count2) + } +