From 2fcd28c1bce59f6cbfcdd079addff23bcef29ee7 Mon Sep 17 00:00:00 2001 From: BlindMaster24 <375291171150z@gmail.com> Date: Sun, 13 Sep 2026 20:57:46 +0300 Subject: [PATCH] refactor(tgbot): make the add-client expiry presets say what they do (#6503) * refactor(tgbot): make the add-client expiry presets say what they do The wizard's "Add N days" buttons were a copy of the renewal handler, whose accumulate branch they cleared two lines later: the branch tested a value the line above had just set to zero, so it was dead and only the "set N days from first use" path was reachable. That reads as an accident, and the automated review of #6499 flagged it twice. The wizard keeps the term it sets, which is now the code: a create flow has no expiry to add to, the custom keypad lands in this same case, and a corrected number has to replace the one it follows. 0 stays the Unlimited button. The renewal handler (reset_exp_c) genuinely adds to the client's remaining time and is unchanged. * refactor(tgbot): fold in the review of the expiry-preset change The test now starts every row from a term a preset could have left, so each row fails on its own under the accumulate semantics rather than depending on the row before it, and it reuses the package's draft helpers instead of a second copy. The wizard presets drop the "Add" verb they never honoured; the renewal keyboard keeps it, where reset_exp_c really does add to the remaining time. --- .../tgbot/tgbot_add_client_expiry_test.go | 61 +++++++++++++++++++ .../service/tgbot/tgbot_draft_render_test.go | 53 +++------------- internal/web/service/tgbot/tgbot_router.go | 33 ++++------ 3 files changed, 83 insertions(+), 64 deletions(-) create mode 100644 internal/web/service/tgbot/tgbot_add_client_expiry_test.go diff --git a/internal/web/service/tgbot/tgbot_add_client_expiry_test.go b/internal/web/service/tgbot/tgbot_add_client_expiry_test.go new file mode 100644 index 000000000..d3bc66162 --- /dev/null +++ b/internal/web/service/tgbot/tgbot_add_client_expiry_test.go @@ -0,0 +1,61 @@ +package tgbot + +import ( + "strings" + "testing" + + "github.com/mymmrac/telego" + "github.com/nicksnyder/go-i18n/v2/i18n" +) + +// Pins the decision the dead accumulate branch hid: a preset tap chooses the term +// instead of adding to it, and 0 is Unlimited. The custom keypad shares this case. +func TestAddClientExpiryPresetReplacesTheTerm(t *testing.T) { + const chatID = int64(7505) + draftLocalizer(t, + &i18n.Message{ID: "tgbot.days", Other: "Days"}, + &i18n.Message{ID: "tgbot.unlimited", Other: "Unlimited"}, + ) + url, textsFor := draftTexts(t) + swapTestBot(t, url) + + // A fresh draft attaches no inbound, so the card never looks one up by remark. + draft := addClientDrafts.forChat(chatID) + origRunning := isRunning + t.Cleanup(func() { + addClientDrafts.reset(chatID) + isRunning = origRunning + }) + isRunning = true + + tb := &Tgbot{} + for _, tc := range []struct { + days string + want string + }{ + {"30", "Expire: 30 Days"}, // not 37: a second tap replaces the first + {"90", "Expire: 90 Days"}, // not 97, which accumulating would show + {"0", "Expire: Unlimited"}, // the Unlimited button clears the term + } { + t.Run(tc.days, func(t *testing.T) { + // A term left by an earlier preset; every row has to fail on its own + // under the accumulate semantics this change rejected. + draft.expiryTime = -7 * 86400000 + + tb.answerCallback(&telego.CallbackQuery{ + ID: "q1", + From: telego.User{ID: 1}, + Data: "add_client_reset_exp_c " + tc.days, + Message: &telego.Message{MessageID: 7, Chat: telego.Chat{ID: chatID}}, + }, true) // admin + + sent := textsFor(chatID) + if len(sent) == 0 { + t.Fatalf("add_client_reset_exp_c %s rendered no card", tc.days) + } + if got := sent[len(sent)-1]; !strings.Contains(got, tc.want) { + t.Errorf("card after add_client_reset_exp_c %s = %q, want it to contain %q", tc.days, got, tc.want) + } + }) + } +} diff --git a/internal/web/service/tgbot/tgbot_draft_render_test.go b/internal/web/service/tgbot/tgbot_draft_render_test.go index 6c6eb00d9..1ec6464c2 100644 --- a/internal/web/service/tgbot/tgbot_draft_render_test.go +++ b/internal/web/service/tgbot/tgbot_draft_render_test.go @@ -3,11 +3,7 @@ package tgbot import ( "encoding/json" "html" - "io" - "net/http" - "net/http/httptest" "strings" - "sync" "testing" "github.com/mhsanaei/3x-ui/v3/internal/web/locale" @@ -49,57 +45,26 @@ func TestClientDraftMessageRendersHTML(t *testing.T) { } } -// botPromptLocalizer renders the two prompts the callback tests drive, with the +// draftLocalizer registers only the messages a wizard test drives, with the // templates the translation files carry; without it I18n returns the bare key. -func botPromptLocalizer(t *testing.T) { +func draftLocalizer(t *testing.T, msgs ...*i18n.Message) { t.Helper() bundle := i18n.NewBundle(language.MustParse("en-US")) bundle.RegisterUnmarshalFunc("json", json.Unmarshal) - _ = bundle.AddMessages(language.MustParse("en-US"), - &i18n.Message{ID: "tgbot.messages.email_prompt", Other: "📧 Default Email: {{ .ClientEmail }}\n\nEnter your email."}, - &i18n.Message{ID: "tgbot.messages.comment_prompt", Other: "💬 Default Comment: {{ .ClientComment }}\n\nEnter your comment."}, - ) + _ = bundle.AddMessages(language.MustParse("en-US"), msgs...) orig := locale.LocalizerBot t.Cleanup(func() { locale.LocalizerBot = orig }) locale.LocalizerBot = i18n.NewLocalizer(bundle, "en-US") } -// promptTexts serves the methods these prompts touch and returns the text of -// every sendMessage, so a test can check what Telegram would actually parse. -func promptTexts(t *testing.T) (string, func() []string) { - t.Helper() - var mu sync.Mutex - var texts []string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - result := any(true) - if r.URL.Path == "/bot"+testBotToken+"/sendMessage" { - var payload struct { - Text string `json:"text"` - } - _ = json.Unmarshal(body, &payload) - mu.Lock() - texts = append(texts, payload.Text) - mu.Unlock() - result = map[string]any{"message_id": 1, "date": 0, "chat": map[string]any{"id": 1, "type": "private"}} - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": result}) - })) - t.Cleanup(srv.Close) - - return srv.URL, func() []string { - mu.Lock() - defer mu.Unlock() - return append([]string(nil), texts...) - } -} - // Regression test: the wizard's own prompts are HTML-parsed as well, so the // draft value they echo has to be escaped exactly like the draft card. func TestAddClientPromptsEscapeDraftValues(t *testing.T) { - botPromptLocalizer(t) - url, texts := promptTexts(t) + draftLocalizer(t, + &i18n.Message{ID: "tgbot.messages.email_prompt", Other: "📧 Default Email: {{ .ClientEmail }}\n\nEnter your email."}, + &i18n.Message{ID: "tgbot.messages.comment_prompt", Other: "💬 Default Comment: {{ .ClientComment }}\n\nEnter your comment."}, + ) + url, texts := draftTexts(t) swapTestBot(t, url) draft := addClientDrafts.forChat(1) @@ -130,7 +95,7 @@ func TestAddClientPromptsEscapeDraftValues(t *testing.T) { Message: &telego.Message{Chat: telego.Chat{ID: 1}}, }, true) // admin - sent := texts() + sent := texts(1) if len(sent) == 0 { t.Fatalf("no prompt was sent for %s", tc.data) } diff --git a/internal/web/service/tgbot/tgbot_router.go b/internal/web/service/tgbot/tgbot_router.go index fc0839c64..1664b2049 100644 --- a/internal/web/service/tgbot/tgbot_router.go +++ b/internal/web/service/tgbot/tgbot_router.go @@ -612,19 +612,10 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.answers.errorOperation")) t.searchClient(chatId, email, callbackQuery.Message.GetMessageID()) case "add_client_reset_exp_c": - draft.expiryTime = 0 + // The wizard's presets and its custom keypad land in this one case, so a + // second tap replaces the term it set; 0 is the Unlimited button. days, _ := strconv.ParseInt(dataArray[1], 10, 64) - var date int64 - if draft.expiryTime > 0 { - if draft.expiryTime-time.Now().Unix()*1000 < 0 { - date = -(days * 24 * 60 * 60000) - } else { - date = draft.expiryTime + days*24*60*60000 - } - } else { - date = draft.expiryTime - days*24*60*60000 - } - draft.expiryTime = date + draft.expiryTime = -days * 24 * 60 * 60000 messageId := callbackQuery.Message.GetMessageID() message_text := t.BuildClientDraftMessage(draft) @@ -1146,21 +1137,23 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool tu.InlineKeyboardButton(t.I18nBot("tgbot.unlimited")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 0")), tu.InlineKeyboardButton(t.I18nBot("tgbot.buttons.custom")).WithCallbackData(t.encodeQuery("add_client_reset_exp_in 0")), ), + // No "Add" verb: these replace the term the draft carries, unlike the + // renewal keyboard, whose reset_exp_c handler really does add to it. tu.InlineKeyboardRow( - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 7 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 7")), - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 10 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 10")), + tu.InlineKeyboardButton("7 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 7")), + tu.InlineKeyboardButton("10 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 10")), ), tu.InlineKeyboardRow( - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 14 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 14")), - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 20 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 20")), + tu.InlineKeyboardButton("14 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 14")), + tu.InlineKeyboardButton("20 "+t.I18nBot("tgbot.days")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 20")), ), tu.InlineKeyboardRow( - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 1 "+t.I18nBot("tgbot.month")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 30")), - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 3 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 90")), + tu.InlineKeyboardButton("1 "+t.I18nBot("tgbot.month")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 30")), + tu.InlineKeyboardButton("3 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 90")), ), tu.InlineKeyboardRow( - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 6 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 180")), - tu.InlineKeyboardButton(t.I18nBot("tgbot.add")+" 12 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 365")), + tu.InlineKeyboardButton("6 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 180")), + tu.InlineKeyboardButton("12 "+t.I18nBot("tgbot.months")).WithCallbackData(t.encodeQuery("add_client_reset_exp_c 365")), ), ) t.editMessageCallbackTgBot(chatId, callbackQuery.Message.GetMessageID(), inlineKeyboard)