Skip to content

UX: keep withdrawal session active after failed community earnings payment - #784

Merged
grunch merged 2 commits into
lnp2pBot:mainfrom
Matobi98:issue768
Apr 21, 2026
Merged

UX: keep withdrawal session active after failed community earnings payment#784
grunch merged 2 commits into
lnp2pBot:mainfrom
Matobi98:issue768

Conversation

@Matobi98

@Matobi98 Matobi98 commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Related issue

Closes #768

Problem

When a community admin's earnings withdrawal fails (payment exhausts all attempts or invoice expires), the bot sends an error message but leaves the admin with no way to retry without navigating back to the menu. If the admin tries to
send a new invoice directly, the bot responds with "I don't understand what you're saying."

Solution

Instead of leaving the admin stranded after a failure, the bot now sends an inline button alongside the error message that re-enters the withdrawal wizard directly. The admin can retry with a new invoice in one click, without going
back to the community menu.

Additionally, a related bug was fixed: if a withdrawal failed due to an expired invoice, the wizard would incorrectly block a new submission with "I'm already trying to pay this" because the expired record was still being matched by
the pending payment query.

Changes

  • jobs/pending_payments.ts: both terminal failure cases (expired invoice and max attempts reached) now include an inline "Withdraw Earnings" button alongside the error message, and use a new dedicated message key instead of the
    generic order payment failure message
  • bot/modules/community/scenes.ts: the pending payment check now excludes expired invoice records so they don't block retries
  • locales/*.yaml (10 files): new pending_payment_failed_earnings key with messaging specific to community earnings withdrawals

Summary by CodeRabbit

  • Bug Fixes

    • Improved payment invoice expiration handling for more accurate payment status tracking
  • New Features

    • Added "withdraw earnings" button in payment failure notifications for easier user recovery
  • Documentation

    • Enhanced multilingual failure messages for earnings payment attempts

@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The changes add a filter condition to detect expired invoices in the earnings wizard, incorporate "withdraw earnings" buttons into payment failure notifications, introduce a new localization key for earnings-specific payment failures across ten languages, and update a development dependency version.

Changes

Cohort / File(s) Summary
Earnings Wizard Logic
bot/modules/community/scenes.ts
Added is_invoice_expired: false condition to the MongoDB query to filter out already-expired invoices when checking for pending payments.
Payment Failure Notifications
jobs/pending_payments.ts
Added inline keyboard with "withdraw earnings" button to expired invoice and max-retry failure messages; changed failure notification key from pending_payment_failed to pending_payment_failed_earnings.
Localization Strings
locales/de.yaml, locales/en.yaml, locales/es.yaml, locales/fa.yaml, locales/fr.yaml, locales/it.yaml, locales/ko.yaml, locales/pt.yaml, locales/ru.yaml, locales/uk.yaml
Added new pending_payment_failed_earnings translation key across ten languages describing payment failure recovery with attempt count and retry instructions.
Development Dependencies
package.json
Updated telegram-test-api from ^2.5.0 to ^4.2.1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PR #782: Updates telegram-test-api to version 4.2.1 in package.json, addressing the same dependency upgrade.
  • PR #667: Modifies attemptCommunitiesPendingPayments function with retry and backoff logic, overlapping with the notification enhancements in this PR.

Suggested reviewers

  • Catrya
  • grunch
  • Luquitasjeffrey

Poem

🐰 Hop through invoices, old and new,
Expired ones we filter through,
Buttons bloom for earnings withdrawn,
Ten tongues echo from dusk to dawn,
A global garden, strong and true! 🌍

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes an unrelated dependency update (telegram-test-api from ^2.5.0 to ^4.2.1) that is not necessary for addressing issue #768. Remove the telegram-test-api version update from this PR and address it in a separate pull request dedicated to dependency updates.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR implements Option A from issue #768 by improving the failure message with an inline retry button and localized messaging, allowing admins to re-enter the withdrawal wizard instead of going back to the menu.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately summarizes the main change: adding an inline button to keep the withdrawal session active after failed community earnings payments, which directly addresses the UX improvement described in the PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
jobs/pending_payments.ts (1)

198-216: ⚠️ Potential issue | 🟡 Minor

Stop flow after marking an earnings invoice as expired.

After sending invoice_expired_earnings, execution continues into the generic failure path. On a max-attempt run, users can receive both “expired invoice” and terminal “failed” messages in the same cycle. Short-circuit this branch.

Suggested fix
       if (!!payment && payment.is_expired) {
         pending.is_invoice_expired = true;
         await bot.telegram.sendMessage(
           user.tg_id,
           i18nCtx.t('invoice_expired_earnings'),
           {
             reply_markup: {
               inline_keyboard: [
                 [
                   {
                     text: i18nCtx.t('withdraw_earnings'),
                     callback_data: `withdrawEarnings_${pending.community_id}`,
                   },
                 ],
               ],
             },
           },
         );
+        continue;
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@jobs/pending_payments.ts` around lines 198 - 216, This branch marks an
invoice expired and sends invoice_expired_earnings via bot.telegram.sendMessage
but then falls through into the generic failure handling; after setting
pending.is_invoice_expired = true and sending the message, short-circuit further
processing for that pending (e.g., add a continue to the enclosing loop or
return from the per-pending handler) so it does not also trigger the terminal
"failed" path; locate the code around payment, pending, bot.telegram.sendMessage
and the invoice_expired_earnings/i18nCtx usage and stop the flow immediately
after the sendMessage call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@jobs/pending_payments.ts`:
- Around line 198-216: This branch marks an invoice expired and sends
invoice_expired_earnings via bot.telegram.sendMessage but then falls through
into the generic failure handling; after setting pending.is_invoice_expired =
true and sending the message, short-circuit further processing for that pending
(e.g., add a continue to the enclosing loop or return from the per-pending
handler) so it does not also trigger the terminal "failed" path; locate the code
around payment, pending, bot.telegram.sendMessage and the
invoice_expired_earnings/i18nCtx usage and stop the flow immediately after the
sendMessage call.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 08d21974-d62b-4c0c-8bab-794a4db41827

📥 Commits

Reviewing files that changed from the base of the PR and between b897d8e and fd0119d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • bot/modules/community/scenes.ts
  • jobs/pending_payments.ts
  • locales/de.yaml
  • locales/en.yaml
  • locales/es.yaml
  • locales/fa.yaml
  • locales/fr.yaml
  • locales/it.yaml
  • locales/ko.yaml
  • locales/pt.yaml
  • locales/ru.yaml
  • locales/uk.yaml
  • package.json

@Luquitasjeffrey Luquitasjeffrey changed the title fix: keep withdrawal session active after failed community earnings payment UX: keep withdrawal session active after failed community earnings payment Apr 19, 2026

@Luquitasjeffrey Luquitasjeffrey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@grunch
grunch merged commit 064ded1 into lnp2pBot:main Apr 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve UX when community earnings withdrawal payment fails

3 participants