Add counterparty requirements customization#794
Open
Luquitasjeffrey wants to merge 6 commits intomainfrom
Open
Add counterparty requirements customization#794Luquitasjeffrey wants to merge 6 commits intomainfrom
Luquitasjeffrey wants to merge 6 commits intomainfrom
Conversation
…ers to restrict other users to take their orders
…s for counterparty requirements
Contributor
WalkthroughAdds a configurable "counterparty requirements" feature: user model fields, a wizard for configuring minimum account age and completed orders, order-time validation to reject non‑compliant takers, new i18n strings across languages, and env limits in ChangesCounterparty Requirements Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bot/modules/user/scenes/requirements.ts`:
- Around line 33-35: The error heading is hardcoded; move it into i18n by adding
a locale key (e.g. "error.heading" or include the heading in each error key) and
change the errorText generator to use ctx.i18n.t to fetch that heading instead
of the literal '<strong>⚠️ ERROR</strong>' — modify the anonymous function
assigned to errorText (error => { ... }) to return ctx.i18n.t('error.heading') +
'\n' + ctx.i18n.t(error.i18n, error) or update error.i18n to a combined key and
call ctx.i18n.t(error.i18n, error), leaving ctx.i18n.t usage and error.i18n
intact.
- Around line 62-73: Both empty catch blocks (including the one in initHandler)
currently swallow Telegram/API failures; replace them with error handling that
logs the error and gives the user a recovery/fallback. Specifically, in
initHandler and the other try/catch: catch the thrown error, call your logger
(or console.error) with the error and context, notify the user with a short
fallback message via ctx.reply or ctx.answerCallbackQuery, and if the failure
was editing or sending the tracked message, attempt a safe fallback by sending a
new message (ctx.reply) and update state.message and state.message.text
accordingly so the wizard can continue; alternatively rethrow after logging if
you want global middleware to handle retries. Ensure you reference initHandler
and the state.message mutations when making the changes.
- Around line 89-91: The scene.command declarations are using leading slashes
which prevents Telegraf from matching them; update the scene.command calls for
the commands currently written as '/counterpartyage', '/counterpartyorders', and
'/reset' to use the plain command names without the slash (e.g.,
'counterpartyage', 'counterpartyorders', 'reset') where they are registered (the
scene.command(...) calls around the handlers that reference resetMessage and the
corresponding handler functions), leaving the rest of the handler logic
unchanged.
In `@locales/pt.yaml`:
- Around line 715-716: Update the Portuguese help strings for the keys that
start with "/counterpartyage" and "/counterpartyorders" to replace the Spanish
phrase "la contraparte" with the correct Portuguese "a contraparte" so the copy
is fully Portuguese (i.e., change "la contraparte" → "a contraparte" in both
lines).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8773ab1c-b912-4c1a-85d2-a9c2e8f7ef0d
📒 Files selected for processing (18)
.env-samplebot/messages.tsbot/middleware/stage.tsbot/modules/orders/takeOrder.tsbot/modules/user/index.tsbot/modules/user/scenes/index.tsbot/modules/user/scenes/requirements.tslocales/de.yamllocales/en.yamllocales/es.yamllocales/fa.yamllocales/fr.yamllocales/it.yamllocales/ko.yamllocales/pt.yamllocales/ru.yamllocales/uk.yamlmodels/user.ts
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I did a strict pass on the full PR and I can't approve it in the current state.
- This PR is not scoped to issue #648 or even to the feature described in the title/body.
The advertised change is counterparty requirements, but this branch also includes unrelated Golden Honey Badger changes and pending payments messaging changes:
bot/commands.tsbot/messages.tsbot/ordersActions.tsutil/imageCache.tsutil/index.tsmodels/order.tsimages/Honeybadger.pngjobs/pending_payments.ts.env-sample(GOLDEN_HONEY_BADGER_PROBABILITY)- locale additions/removals for
golden_honey_badgerandpending_payment_failed_earnings
Those changes are behaviorally unrelated to counterparty requirements and make the review/risk surface much larger than necessary. Please split them into a separate PR.
jobs/pending_payments.tsregresses withdrawal UX.
This PR removes the withdraw_earnings inline button from both the expired-invoice path and the failed-payment path, and also swaps pending_payment_failed_earnings for the generic pending_payment_failed message. That is not a neutral refactor. It changes the recovery path for community earnings withdrawals and should not ride along inside this feature PR.
- The linked issue is still not implemented.
The PR says Fixes #648, but issue #648 asks for a reject flow to immediately expire invoices and repost the order. This branch adds requirement-based filtering instead. Even if the filtering is useful, it does not satisfy the issue being auto-closed by this PR.
CI is green and the command-registration fixes from the review thread are in place, but the scope problem is enough for me to block this.
Please cut this down to the counterparty requirements feature only, or open separate PRs for the Honey Badger / pending-payments work.
…quirements-customization
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bot/modules/user/scenes/requirements.ts (1)
104-105: ⚡ Quick winUse camelCase for local variables in this TypeScript scene.
min_daysandmin_ordersshould be renamed to camelCase for consistency with repository rules.As per coding guidelines, `**/*.{ts,tsx}`: Use camelCase for functions and variables.Proposed fix
- const min_days = parseInt(days); - if (isNaN(min_days) || min_days < 0) throw new Error('NotValidNumber'); + const minDays = parseInt(days); + if (isNaN(minDays) || minDays < 0) throw new Error('NotValidNumber'); @@ - if (min_days > maxAge) { + if (minDays > maxAge) { @@ - user.counterparty_requirements.min_days_using_bot = min_days; + user.counterparty_requirements.min_days_using_bot = minDays; @@ - days: min_days, + days: minDays, @@ - const min_orders = parseInt(orders); - if (isNaN(min_orders) || min_orders < 0) + const minOrders = parseInt(orders); + if (isNaN(minOrders) || minOrders < 0) throw new Error('NotValidNumber'); @@ - if (min_orders > maxOrders) { + if (minOrders > maxOrders) { @@ - user.counterparty_requirements.min_completed_orders = min_orders; + user.counterparty_requirements.min_completed_orders = minOrders; @@ - orders: min_orders, + orders: minOrders,Also applies to: 151-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/modules/user/scenes/requirements.ts` around lines 104 - 105, Rename the snake_case local variables min_days and min_orders to camelCase (minDays and minOrders) throughout the requirements scene so they follow project naming rules; update their declarations (e.g., the parseInt assignment) and every usage (validation checks like isNaN/min < 0, error throws, and any later references around the other mentioned block at lines ~151-153) to use minDays/minOrders to avoid compile errors and keep naming consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bot/modules/user/scenes/requirements.ts`:
- Around line 131-137: The catch block that sets state.error = { i18n:
'invalid_number' } in the handler using ctx.scene.state (CommunityWizardState)
should only mark validation/parsing errors as 'invalid_number'; change the catch
to inspect the caught err (e.g., instanceof ValidationError, isAxiosError,
err.name/message or a custom validation check) and: for validation failures set
state.error.i18n = 'invalid_number' and call updateMessage(ctx); for all other
errors log the full error (using your logger) and set a different state.error
(e.g., i18n: 'internal_error' or a generic error key) or rethrow so
DB/Telegram/API failures aren’t presented as user input errors; apply the same
fix to the other catch block referenced around the second occurrence (the block
at lines 179-185) and ensure updateMessage(ctx) is called with the appropriate
state in each branch.
---
Nitpick comments:
In `@bot/modules/user/scenes/requirements.ts`:
- Around line 104-105: Rename the snake_case local variables min_days and
min_orders to camelCase (minDays and minOrders) throughout the requirements
scene so they follow project naming rules; update their declarations (e.g., the
parseInt assignment) and every usage (validation checks like isNaN/min < 0,
error throws, and any later references around the other mentioned block at lines
~151-153) to use minDays/minOrders to avoid compile errors and keep naming
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91a6125f-89c7-4b79-8e0c-a939fb19f297
📒 Files selected for processing (14)
.env-samplebot/messages.tsbot/modules/orders/takeOrder.tsbot/modules/user/scenes/requirements.tslocales/de.yamllocales/en.yamllocales/es.yamllocales/fa.yamllocales/fr.yamllocales/it.yamllocales/ko.yamllocales/pt.yamllocales/ru.yamllocales/uk.yaml
✅ Files skipped from review due to trivial changes (8)
- .env-sample
- locales/pt.yaml
- locales/ru.yaml
- locales/fa.yaml
- locales/ko.yaml
- locales/en.yaml
- locales/uk.yaml
- locales/de.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- locales/fr.yaml
- locales/it.yaml
- bot/modules/orders/takeOrder.ts
Comment on lines
+131
to
+137
| } catch (err) { | ||
| const state = ctx.scene.state as CommunityWizardState; | ||
| state.error = { | ||
| i18n: 'invalid_number', | ||
| }; | ||
| await updateMessage(ctx); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid classifying all exceptions as invalid_number.
Right now, non-validation failures (e.g., DB save or Telegram API issues) are surfaced to users as input errors, which is misleading and makes incidents harder to diagnose.
Proposed fix
} catch (err) {
const state = ctx.scene.state as CommunityWizardState;
- state.error = {
- i18n: 'invalid_number',
- };
+ if (err instanceof Error && err.message === 'NotValidNumber') {
+ state.error = { i18n: 'invalid_number' };
+ } else {
+ logger.error(err);
+ state.error = { i18n: 'generic_error' };
+ }
await updateMessage(ctx);
}
@@
} catch (err) {
const state = ctx.scene.state as CommunityWizardState;
- state.error = {
- i18n: 'invalid_number',
- };
+ if (err instanceof Error && err.message === 'NotValidNumber') {
+ state.error = { i18n: 'invalid_number' };
+ } else {
+ logger.error(err);
+ state.error = { i18n: 'generic_error' };
+ }
await updateMessage(ctx);
}Also applies to: 179-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bot/modules/user/scenes/requirements.ts` around lines 131 - 137, The catch
block that sets state.error = { i18n: 'invalid_number' } in the handler using
ctx.scene.state (CommunityWizardState) should only mark validation/parsing
errors as 'invalid_number'; change the catch to inspect the caught err (e.g.,
instanceof ValidationError, isAxiosError, err.name/message or a custom
validation check) and: for validation failures set state.error.i18n =
'invalid_number' and call updateMessage(ctx); for all other errors log the full
error (using your logger) and set a different state.error (e.g., i18n:
'internal_error' or a generic error key) or rethrow so DB/Telegram/API failures
aren’t presented as user input errors; apply the same fix to the other catch
block referenced around the second occurrence (the block at lines 179-185) and
ensure updateMessage(ctx) is called with the appropriate state in each branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #648
It adds a /requirements command to configure counteparty requirements for other users to take published orders.
Why someone would want to do this?
Suppose I want to trade some sats and I want the counterparty to be an experienced user, this filtering allows me to find a better counterpart and reduce the fraud risk, or the risk of wasting time with a counterparty that doesn't wants to trade.
Summary by CodeRabbit
New Features
Documentation