Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bot, jobs,tests/bot: convert requires to imports #638

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

webwarrior-ws
Copy link
Contributor

@webwarrior-ws webwarrior-ws commented Mar 4, 2025

Converted requires to imports where applicable and fixed
type errors resulting from it.

Also removed @ts-check annotations. They are used to typecheck .js files, but make no sense in .ts files as they are already type-checked.

Summary by CodeRabbit

  • Refactor

    • Enhanced the bot’s internal architecture by standardizing module imports and refining context handling for community interactions.
    • Improved error handling and type safety across commands, messaging, and payment processing, leading to more consistent and reliable operations.
  • Chore

    • Streamlined code structure and updated module syntax to boost maintainability and performance.
    • Transitioned from CommonJS to ES module syntax for imports across multiple files, enhancing consistency and modernizing the codebase.

Converted requires to imports where applicable and fixed
type errors resulting from it.
They are used to typecheck .js files, but make no sense in .ts
files as they are already type-checked.
Copy link

coderabbitai bot commented Mar 4, 2025

Walkthrough

This update transitions the codebase from using the legacy MainContext to the refined CommunityContext and modernizes the module syntax throughout the project. Multiple files in core bot functionalities, middleware, modules, and jobs have undergone changes to replace CommonJS require with ES6 import syntax. Function signatures across commands, messages, and middleware now use updated context types (including the introduction of HasTelegram) and, in some cases, include default parameters. Minor adjustments such as removing TypeScript checking directives (// @ts-check) are also present.

Changes

File(s) Change Summaries
app.ts Removed MainContext import and added CommunityContext; changed options type to use CommunityContext; updated delay import from CommonJS to ES6 syntax.
bot/commands.ts, bot/messages.ts Converted various CommonJS imports to ES6; modified multiple function signatures by replacing MainContext with HasTelegram or CommunityContext; introduced default parameter values for improved handling.
bot/index.ts, bot/start.ts, bot/ordersActions.ts, bot/scenes.ts Transitioned module imports and exports from CommonJS to ES6; updated bot initialization functions and export statements; changed context types in start functions.
bot/middleware/stage.ts, bot/middleware/user.ts Replaced CommonJS import syntax with ES6; updated middleware function signatures from MainContext to CommunityContext.
bot/modules/community/* Standardized imports to ES6 (including user middleware and CommunityEvents); removed // @ts-check from commands.ts; updated import for isPendingPayment in scenes.ts.
bot/modules/dispute/* Updated module imports from CommonJS to ES6; in index.ts, changed bot parameter type from Telegraf<MainContext> to Telegraf<CommunityContext>; added new OrderEvents import in commands.ts.
bot/modules/events/* Removed the // @ts-check directives in both community.ts and orders.ts.
bot/modules/language/index.ts, bot/modules/nostr/index.ts Replaced require with ES6 import; updated the configure function’s bot parameter type to CommunityContext.
bot/modules/orders/*, bot/modules/user/index.ts Removed // @ts-check from orders/commands.ts; added new import for MainContext in orders/index.ts; updated imports in takeOrder.ts and user/index.ts to ES6 syntax.
jobs/* (cancel_orders.ts, communities.ts, delete_published_orders.ts, node_info.ts, pending_payments.ts) Modified bot parameter types from MainContext to CommunityContext or HasTelegram; switched from CommonJS to ES6 imports; refined type assertion in delete_published_orders.ts for order objects.

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch

Poem

Hoppin’ through the code, I sing with glee,
From CommonJS to ES6, now I’m fancy and free!
I swapped my MainContext for a community vibe,
With HasTelegram and defaults, the changes truly jive.
(_/)
Code rabbit here, cheers to our clean spree!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08b1333 and 0c2ace1.

📒 Files selected for processing (1)
  • tests/bot/bot.spec.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • tests/bot/bot.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: ci_to_main
  • GitHub Check: Analyze (javascript)

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (4)
bot/start.ts (4)

220-237: 🛠️ Refactor suggestion

Non-null assertion and context type inconsistency.

This handler has two issues:

  1. It uses a non-null assertion on the result of validateParams without explicitly checking for null.
  2. It still uses MainContext instead of CommunityContext.
-  bot.command('maintenance', superAdminMiddleware, async (ctx: MainContext): Promise<void> => {
+  bot.command('maintenance', superAdminMiddleware, async (ctx: CommunityContext): Promise<void> => {
     try {
-      const [val] = (await validateParams(ctx, 2, '\\<_on/off_\\>'))!;
-      if (!val) return;
+      const validatedParams = await validateParams(ctx, 2, '\\<_on/off_\\>');
+      if (!validatedParams) return;
+      const [val] = validatedParams;
+      if (!val) return;

252-259: 🛠️ Refactor suggestion

Context type inconsistency and CommonJS require usage.

This handler has two issues:

  1. It still uses MainContext instead of CommunityContext.
  2. It uses CommonJS require instead of ES6 import.
-  bot.command('version', async (ctx: MainContext) => {
+  bot.command('version', async (ctx: CommunityContext) => {
     try {
-      const pckg = require('../../package.json');
+      // Consider importing package.json at the top of the file
+      // but since it's a one-time use, this approach is acceptable
+      const pckg = require('../../package.json');
       await ctx.reply(pckg.version);

291-324: 🛠️ Refactor suggestion

Non-null assertion and context type inconsistency.

Similar issues as observed in previous handlers:

  1. Non-null assertion without explicit null check.
  2. MainContext instead of CommunityContext.
-  bot.command('freezeorder', adminMiddleware, async (ctx: MainContext) => {
+  bot.command('freezeorder', adminMiddleware, async (ctx: CommunityContext) => {
     try {
-      const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!;
+      const validatedParams = await validateParams(ctx, 2, '\\<_order id_\\>');
+      if (!validatedParams) return;
+      const [orderId] = validatedParams;

531-570: 🛠️ Refactor suggestion

Context type inconsistency and non-null assertion.

Similar to previous comments, this handler:

  1. Uses MainContext instead of CommunityContext.
  2. Uses non-null assertion without explicit null check.
-  bot.command('checkorder', superAdminMiddleware, async (ctx: MainContext) => {
+  bot.command('checkorder', superAdminMiddleware, async (ctx: CommunityContext) => {
     try {
-      const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!;
+      const validatedParams = await validateParams(ctx, 2, '\\<_order id_\\>');
+      if (!validatedParams) return;
+      const [orderId] = validatedParams;
🧹 Nitpick comments (17)
app.ts (1)

5-5: Consider converting the remaining require statement.

There's still a CommonJS require statement for resubscribeInvoices that could be converted to an ES module import for consistency.

-const { resubscribeInvoices } = require('./ln');
+import { resubscribeInvoices } from './ln';
bot/modules/nostr/index.ts (2)

1-1: Consider converting the remaining require statement.

There's still a CommonJS require statement for websocket-polyfill that could be converted to an ES module import for consistency.

-require('websocket-polyfill');
+import 'websocket-polyfill';

24-24: Consider converting the remaining require statement.

There's still a CommonJS require statement for OrderEvents that could be converted to an ES module import for consistency.

-const OrderEvents = require('../events/orders');
+import * as OrderEvents from '../events/orders';
jobs/cancel_orders.ts (4)

2-2: Redundant MainContext import.

The MainContext is imported but never used in this file. You should remove it from the imports to maintain clean code.

-import { HasTelegram, MainContext } from "../bot/start";
+import { HasTelegram } from "../bot/start";

11-11: Consider updating function signature to match usage.

The function now takes a HasTelegram type but immediately casts to CommunityContext in the implementation. Consider updating the parameter type to directly accept Telegraf<CommunityContext> for better type safety and to avoid type assertions.

-const cancelOrders = async (bot: HasTelegram) => {
+const cancelOrders = async (bot: Telegraf<CommunityContext>) => {

34-34: Type casting could be avoided with proper parameter typing.

Using type assertions (as CommunityContext) indicates potential type incompatibility. This could be avoided by properly typing the function parameter.

-        await cancelShowHoldInvoice(bot as CommunityContext, order, true);
+        await cancelShowHoldInvoice(bot, order, true);

Note: This change should be made after updating the function signature as suggested earlier.


36-36: Type casting could be avoided with proper parameter typing.

Similar to the previous comment, this type assertion could be avoided with proper typing of the function parameter.

-        await cancelAddInvoice(bot as CommunityContext, order, true);
+        await cancelAddInvoice(bot, order, true);

Note: This change should be made after updating the function signature as suggested earlier.

jobs/pending_payments.ts (1)

6-9: Unused MainContext import.

The MainContext is imported but not used in the file. Consider removing it to maintain clean imports.

-import { MainContext } from '../bot/start';
 import { payRequest, isPendingPayment } from '../ln';
 import { getUserI18nContext } from '../util';
 import { CommunityContext } from '../bot/modules/community/communityContext';
bot/start.ts (3)

207-218: Context type inconsistency.

While many handlers have been updated to use CommunityContext, this handler still uses MainContext. Consider updating this handler to use CommunityContext for consistency.

-  bot.start(async (ctx: MainContext) => {
+  bot.start(async (ctx: CommunityContext) => {

239-250: Context type inconsistency.

This handler still uses MainContext instead of CommunityContext. Consider updating it for consistency.

-  bot.on('text', userMiddleware, async (ctx: MainContext, next: () => void) => {
+  bot.on('text', userMiddleware, async (ctx: CommunityContext, next: () => void) => {

326-393: Inconsistent null check pattern.

This handler uses a different pattern for checking null values from validateParams compared to other handlers:

const validatedParams = (await validateParams(ctx, 2, '\\<_order id_\\>'))!;
if (validatedParams == null) return;
const [orderId] = validatedParams;

This is safer than just using non-null assertion, but inconsistent with other handlers. Consider standardizing the approach across all handlers.

-  bot.command('cancelorder', adminMiddleware, async (ctx: MainContext) => {
+  bot.command('cancelorder', adminMiddleware, async (ctx: CommunityContext) => {
     try {
-      const validatedParams = (await validateParams(ctx, 2, '\\<_order id_\\>'))!;
-      if (validatedParams == null) return;
-      const [orderId] = validatedParams;
+      const validatedParams = await validateParams(ctx, 2, '\\<_order id_\\>');
+      if (!validatedParams) return;
+      const [orderId] = validatedParams;
bot/messages.ts (6)

376-376: Added non-null assertion operator

The code now uses the non-null assertion operator ! with fiatAmount to address TypeScript's strict null checking. While this works, consider using optional chaining or a default value for a more defensive approach.

-        fiatAmount: numberFormat(fiatCode, fiatAmount!),
+        fiatAmount: numberFormat(fiatCode, fiatAmount ?? 0),

490-490: Similar non-null assertion usage

Again using the non-null assertion operator ! with order.fiat_amount. Consider using optional chaining or a default value for more robust code.

-        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount!),
+        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount ?? 0),

863-863: Non-null assertion for fiat_amount

Using the non-null assertion operator ! with order.fiat_amount. Consider using optional chaining or a default value for more defensive programming.

-        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount!),
+        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount ?? 0),

885-885: Another non-null assertion usage

Again using the non-null assertion operator ! with order.fiat_amount. Consider using optional chaining or a default value for a more defensive approach.

-        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount!),
+        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount ?? 0),

1267-1267: Non-null assertion usage

Using the non-null assertion operator ! with order.fiat_amount. Consider using optional chaining or a default value for more robust code.

-        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount!),
+        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount ?? 0),

1327-1327: Consistent non-null assertion

Using the non-null assertion operator ! with order.fiat_amount. Consider using optional chaining or a default value for a more defensive approach.

-        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount!),
+        fiatAmount: numberFormat(order.fiat_code, order.fiat_amount ?? 0),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22a0ec9 and 08b1333.

📒 Files selected for processing (30)
  • app.ts (2 hunks)
  • bot/commands.ts (10 hunks)
  • bot/index.ts (3 hunks)
  • bot/messages.ts (27 hunks)
  • bot/middleware/stage.ts (1 hunks)
  • bot/middleware/user.ts (3 hunks)
  • bot/modules/community/commands.ts (0 hunks)
  • bot/modules/community/index.ts (1 hunks)
  • bot/modules/community/scenes.communityAdmin.ts (1 hunks)
  • bot/modules/community/scenes.ts (1 hunks)
  • bot/modules/dispute/actions.ts (1 hunks)
  • bot/modules/dispute/commands.ts (1 hunks)
  • bot/modules/dispute/index.ts (1 hunks)
  • bot/modules/events/community.ts (0 hunks)
  • bot/modules/events/orders.ts (0 hunks)
  • bot/modules/language/index.ts (1 hunks)
  • bot/modules/nostr/index.ts (1 hunks)
  • bot/modules/orders/commands.ts (0 hunks)
  • bot/modules/orders/index.ts (1 hunks)
  • bot/modules/orders/takeOrder.ts (1 hunks)
  • bot/modules/user/index.ts (1 hunks)
  • bot/ordersActions.ts (1 hunks)
  • bot/scenes.ts (1 hunks)
  • bot/start.ts (22 hunks)
  • bot/validations.ts (1 hunks)
  • jobs/cancel_orders.ts (2 hunks)
  • jobs/communities.ts (1 hunks)
  • jobs/delete_published_orders.ts (2 hunks)
  • jobs/node_info.ts (1 hunks)
  • jobs/pending_payments.ts (2 hunks)
💤 Files with no reviewable changes (4)
  • bot/modules/events/community.ts
  • bot/modules/events/orders.ts
  • bot/modules/orders/commands.ts
  • bot/modules/community/commands.ts
✅ Files skipped from review due to trivial changes (12)
  • bot/modules/community/scenes.communityAdmin.ts
  • bot/modules/orders/takeOrder.ts
  • bot/validations.ts
  • bot/modules/community/index.ts
  • bot/modules/dispute/actions.ts
  • bot/scenes.ts
  • bot/modules/user/index.ts
  • bot/ordersActions.ts
  • bot/modules/orders/index.ts
  • bot/modules/community/scenes.ts
  • bot/modules/dispute/commands.ts
  • bot/middleware/stage.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (70)
bot/index.ts (3)

1-3: Syntax modernization successfully implemented.

The conversion from CommonJS requires to ES6 imports is correctly implemented here. This aligns with the PR objectives to modernize the code.


13-37: Imports properly structured with named exports.

The ES6 import syntax is correctly implemented with named exports, maintaining code structure while modernizing the syntax.


39-76:

❓ Verification inconclusive

Verify removal of beginDisputeMessage from exports.

The export statement now uses ES6 syntax which is correct, but I notice that beginDisputeMessage has been removed from the exports. Ensure this is intentional and won't break any code that might be depending on this export.


🏁 Script executed:

#!/bin/bash
# Search for any usages of beginDisputeMessage in the codebase
echo "Searching for usages of beginDisputeMessage in the codebase..."
rg "beginDisputeMessage" --type ts --type js

Length of output: 179


Action Required: Manually Verify Removal of beginDisputeMessage

I ran a search for any occurrences of beginDisputeMessage across TypeScript/JavaScript files, and no references were found. This confirms that its removal from the export in bot/index.ts appears to be intentional. However, please manually verify that no external dependencies (including tests or client integrations) rely on this export before finalizing the merge.

  • File: bot/index.ts
    • Export removal appears intentional, but double-check external dependencies.
jobs/communities.ts (2)

5-5: Import for CommunityContext properly added.

The import for CommunityContext is correctly added to support the type change in the function parameter.


7-7: Type signature updated to use CommunityContext.

The function signature update from MainContext to CommunityContext aligns with the broader transition in the codebase.

Consider running the following to verify that this change is consistent across the entire application:

#!/bin/bash
# Check for any remaining usages of MainContext that might need updating
echo "Checking for remaining MainContext usages that might need updating..."
rg "MainContext" --type ts
bot/modules/dispute/index.ts (2)

4-5: Middleware imports modernized correctly.

The conversion from CommonJS requires to ES6 imports for middleware is done correctly, and the CommunityContext import is properly added.


7-7: Bot parameter type updated to CommunityContext.

The function signature update is consistent with the pattern being applied across the codebase, changing from MainContext to CommunityContext.

bot/modules/language/index.ts (3)

1-1: Middleware import modernized correctly.

The conversion from CommonJS require to ES6 import for userMiddleware is correctly implemented.


6-6: CommunityContext import properly added.

The import for CommunityContext is correctly added to support the type change in the function signature.


8-8: Export syntax and type signature modernized.

The function signature has been updated from exports.configure to export const configure and the type has been changed from Telegraf to Telegraf<CommunityContext>, which aligns with the PR objectives.

jobs/node_info.ts (3)

4-4: Correctly migrated to use CommunityContext.

This import change is aligned with the PR objective to modernize the code, switching from MainContext to the new CommunityContext type.


6-6: Properly converted require to import statement.

Good job converting the CommonJS require syntax to ES module import syntax for the logger, which aligns with the PR objectives.


8-8: Updated function signature to use CommunityContext.

The type of the bot parameter has been correctly updated from MainContext to CommunityContext, maintaining type safety while modernizing the codebase.

app.ts (3)

3-3: Properly converted require to import statement.

Good job converting the CommonJS require syntax to ES module import syntax for the start function.


8-9: Properly converted require to import statements and added CommunityContext.

Good job converting the CommonJS require syntax to ES module import syntax for the delay utility and adding the necessary CommunityContext import.


28-28: Updated options type to use CommunityContext.

The type for the options variable has been correctly updated to use CommunityContext instead of MainContext, maintaining type consistency with the rest of the application.

bot/modules/nostr/index.ts (2)

8-9: Properly converted require to import statements and added CommunityContext.

Good job converting the CommonJS require syntax to ES module import syntax for CommunityEvents and adding the necessary CommunityContext import.


11-11: Updated function signature to use CommunityContext.

The type of the bot parameter has been correctly updated from MainContext to CommunityContext, maintaining type safety while modernizing the codebase.

jobs/delete_published_orders.ts (3)

4-7: Properly converted require to import statements and added necessary imports.

Good job converting the CommonJS require syntax to ES module import syntax for deleteOrderFromChannel and adding the necessary imports for CommunityContext and IOrder.


9-9: Updated function signature to use CommunityContext.

The type of the bot parameter has been correctly updated from MainContext to CommunityContext, maintaining type safety while modernizing the codebase.


25-25: Improved type safety with explicit type assertion.

Adding the explicit type assertion to orderCloned improves type safety and makes the code more robust.

bot/middleware/user.ts (1)

1-5: Appropriate context type transition.

The middleware functions have been correctly updated to use CommunityContext instead of MainContext, which is consistent with the PR objective to modernize the codebase.

jobs/pending_payments.ts (3)

7-10: Successful transition to ES6 import syntax.

The code has been successfully updated to use ES6 import syntax instead of CommonJS require statements, which is consistent with the PR objective.


12-12: Function signature updated correctly.

The function signature has been properly updated to use Telegraf<CommunityContext> instead of Telegraf<MainContext>.


122-122: Function signature updated correctly.

The function signature has been properly updated to use Telegraf<CommunityContext> instead of Telegraf<MainContext>.

bot/start.ts (7)

6-32: Successful transition to ES6 import syntax.

The code has been successfully updated to use ES6 import syntax instead of CommonJS require statements, which is consistent with the PR objective.


157-164: Bot initialization properly updated to use CommunityContext.

The initialize function has been correctly updated to use CommunityContext instead of MainContext, ensuring type consistency.


396-418: Approve context type update.

The handler has been correctly updated to use CommunityContext instead of MainContext.


421-465: Approve context type update and ES6 imports usage in ordersActions.

  1. The handler correctly uses CommunityContext.
  2. The code now uses ordersActions imported via ES6 import syntax.

467-529: Approve context type update.

The handler has been correctly updated to use CommunityContext instead of MainContext.


764-825: Approve consistent context type updates.

All action handlers have been correctly updated to use CommunityContext instead of MainContext.


938-950: Approve function signature update.

The start function signature has been correctly updated to use CommunityContext instead of MainContext.

bot/commands.ts (13)

10-12: Clean transition to ES6 module syntax

Good job converting the CommonJS require to ES6 import syntax for the OrderEvents module and updating the imports for utility functions. This aligns with modern JavaScript practices and enhances code maintainability.


19-20: Proper type imports for context objects

Good job importing both HasTelegram and MainContext from ./start along with CommunityContext from the community module. This facilitates the transition to the new context types being used throughout the codebase.


23-23: Function signature update for bot parameter

You've correctly updated the waitPayment function to use HasTelegram for the bot parameter instead of MainContext, which aligns with the PR objective of modernizing the codebase and improving type safety.


33-33: Good use of removeLightningPrefix utility

Correctly using the newly imported removeLightningPrefix utility function to sanitize the buyer invoice, which provides better encapsulation of this functionality.


90-90: Enhanced function signature with default parameter

The function signature has been updated to use CommunityContext instead of MainContext for the context parameter, and HasTelegram for the bot parameter. Additionally, a default value of null has been added for the order parameter, making the function more flexible.


179-179: Proper update of context types

The rateUser function now correctly uses CommunityContext for the context parameter and HasTelegram for the bot parameter, which is consistent with the other function signature updates in this file.


242-242: Updated function signature with optional parameter

The cancelAddInvoice function has been updated to use CommunityContext for the context parameter and to set a default value of null for the order parameter, which improves function flexibility.


359-359: Consistent parameter type updates

The showHoldInvoice function signature has been updated to use CommunityContext for the context parameter and HasTelegram for the bot parameter, which is consistent with the pattern applied to other functions in this file.


429-429: Updated function signature with optional parameter

The cancelShowHoldInvoice function has been updated to use CommunityContext for the context parameter and to set a default value of null for the order parameter, consistent with similar functions in the file.


552-552: Consistent context type updates

The addInvoicePHI function signature has been updated to use CommunityContext for the context parameter and HasTelegram for the bot parameter, maintaining consistency with other function signatures in this file.


576-576: Function signature enhancement with optional parameter

The cancelOrder function has been updated to use CommunityContext for the context parameter and to set a default value of null for the user parameter, which improves function flexibility and maintains consistency with similar function signatures.


702-702:

✅ Verification successful

Note: Different context type used

The fiatSent function uses MainContext instead of CommunityContext which is different from most other functions in this file. Verify if this is intentional or if it should be updated to use CommunityContext for consistency.


🏁 Script executed:

#!/bin/bash
# Check if other functions in the file are using MainContext or if this is an oversight

# Search for all function signature declarations in the file that include MainContext
rg -A 1 "async.*MainContext" bot/commands.ts

# Check if fiatSent is called from anywhere that passes a specific context type
rg "fiatSent\(" --include=*.ts

Length of output: 893


Final Decision: Consistent Context Type Usage in fiatSent

After verifying the context types used in bot/commands.ts, we found that other functions (e.g., waitPayment and release) also use MainContext. This confirms that the fiatSent function’s use of MainContext is intentional and consistent with the rest of the file. No changes are required.


738-738:

❓ Verification inconclusive

Same context type inconsistency

The release function also uses MainContext instead of CommunityContext. This appears to be consistent with the fiatSent function but inconsistent with most other functions in this file. Verify if this distinction is intentional based on how these functions are used.


🏁 Script executed:

#!/bin/bash
# Check if release is called from anywhere that specifically requires MainContext

# Search for all calls to the release function
rg "release\(" --include=*.ts

Length of output: 475


Verify the Context Consistency in Function Usage

The release function uses MainContext (consistent with fiatSent) while most other functions in this file use CommunityContext. Before approving this divergence, please manually verify that using MainContext here is intentional and fits the function’s invocation sites. Notably, our earlier search attempt encountered a flag error; re-running the search with the corrected command (above) should help confirm if any call sites explicitly require MainContext.


🏁 Script executed:

#!/bin/bash
# Re-run search for all invocations of the release function in .ts files using the correct flag
rg "release\(" -g "*.ts"

Length of output: 259


Review Action: Confirm Context Type Usage for release

The search shows that the release function is only called in bot/start.ts where the passed ctx is used consistently with MainContext. This mirrors the approach used in the fiatSent function. However, since most functions in the file use CommunityContext, please double-check whether using MainContext here is intentional and correctly reflects the intended usages.

bot/messages.ts (25)

3-15: Clean transition to ES6 module syntax

Good job using ES6 destructuring syntax for imports from '../util'. This makes the code more maintainable and aligns with modern JavaScript practices.


16-16: Improved import syntax for OrderEvents

Successfully converted the CommonJS require to ES6 import * as OrderEvents from './modules/events/orders';, which is consistent with the changes in other files.


28-28: Added CommunityContext import

Correctly added import for CommunityContext which is needed for the updated function signatures throughout the file.


70-74: Enhanced null checking logic

The code now properly handles the case where currencyObject or currencyObject.symbol_native might be undefined, using the logical AND operator with the nullish check. This improves code robustness.


206-206: Updated context type in function signature

The expiredInvoiceOnPendingMessage function now uses Telegraf<CommunityContext> instead of MainContext for the bot parameter, which aligns with the PR objective.


309-309: Consistent function signature update

The invalidDataMessage function now uses HasTelegram for the bot parameter instead of MainContext, which is consistent with other function signature updates.


317-317: Simplified function signature

The genericErrorMessage function signature has been updated to use HasTelegram for the bot parameter and no longer requires a context parameter, which simplifies the function usage.


368-371: Improved null checking logic

Similar to earlier changes, this code properly handles the case where currencyObject or currencyObject.symbol_native might be undefined, enhancing code robustness.


516-516: Updated bot parameter type

The takeSellWaitingSellerToPayMessage function now correctly uses HasTelegram for the bot parameter instead of MainContext, which is consistent with other function signature updates.


551-551: Updated context type in function signature

The rateUserMessage function now uses Telegraf<CommunityContext> for the bot parameter, aligned with the PR objectives and consistent with other function signature updates.


608-609: Improved error handling

Added explicit check for undefined channel and throw an appropriate error message, which provides better error diagnostics.


645-646: Consistent error handling

Similar to the previous check, added explicit check for undefined channel and throw an appropriate error message, maintaining consistency in error handling.


681-682: Enhanced error handling

Added explicit check for undefined message and throw an appropriate error message, providing better error diagnostics.


855-855: Updated bot parameter type

The addInvoiceMessage function now uses HasTelegram for the bot parameter instead of MainContext, consistent with other function signature updates.


877-877: Updated bot parameter type

The sendBuyerInfo2SellerMessage function now uses HasTelegram for the bot parameter instead of MainContext, aligned with the PR objectives.


1023-1023: Function signature update for context type

The successCancelOrderByAdminMessage function now uses Telegraf<CommunityContext> for the bot parameter, consistent with the pattern applied to other functions.


1042-1042: Updated bot parameter type

The successCompleteOrderByAdminMessage function now uses HasTelegram for the bot parameter instead of MainContext, aligned with the PR objectives.


1207-1207: Updated bot parameter type

The priceApiFailedMessage function now uses HasTelegram for the bot parameter instead of MainContext, consistent with other function signature updates.


1364-1364: Updated bot parameter type

The expiredOrderMessage function now uses HasTelegram for the bot parameter instead of MainContext, consistent with other function signature updates.


1381-1381: Updated bot parameter type

The toBuyerExpiredOrderMessage function now uses HasTelegram for the bot parameter instead of MainContext, aligned with the PR objectives.


1392-1392: Updated bot parameter type

The toSellerExpiredOrderMessage function now uses HasTelegram for the bot parameter instead of MainContext, maintaining consistency with other function signature updates.


1485-1487: Updated context type in function signature

The toAdminChannelPendingPaymentSuccessMessage function now uses Telegraf<CommunityContext> for the bot parameter, which is aligned with the pattern applied to other functions in this file.


1509-1511: Consistent context type updates

The toBuyerPendingPaymentSuccessMessage function now uses Telegraf<CommunityContext> for the bot parameter, maintaining consistency with similar functions.


1530-1530: Updated context type in function signature

The toBuyerPendingPaymentFailedMessage function now uses Telegraf<CommunityContext> for the bot parameter, which is consistent with the pattern applied to other functions.


1556-1558: Consistent context type updates

The toAdminChannelPendingPaymentFailedMessage function now uses Telegraf<CommunityContext> for the bot parameter, maintaining consistency with similar functions in this file.

Comment on lines 9 to 10
bot.command('setlang', userMiddleware, ctx => commands.setlang(ctx as unknown as MainContext));
bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, ctx => actions.setLanguage(ctx as unknown as MainContext));
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Risky type assertion may lead to compatibility issues.

The code is using a potentially dangerous type assertion (ctx as unknown as MainContext) while the bot is now configured with CommunityContext. This inconsistency could lead to runtime errors if these context types aren't compatible.

Consider refactoring these handlers to work with CommunityContext instead of using type assertions:

- bot.command('setlang', userMiddleware, ctx => commands.setlang(ctx as unknown as MainContext));
- bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, ctx => actions.setLanguage(ctx as unknown as MainContext));
+ bot.command('setlang', userMiddleware, commands.setlang);
+ bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, actions.setLanguage);

Then update the command and action implementations to accept CommunityContext instead of MainContext.

Alternatively, if you need to maintain backwards compatibility temporarily:

- bot.command('setlang', userMiddleware, ctx => commands.setlang(ctx as unknown as MainContext));
- bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, ctx => actions.setLanguage(ctx as unknown as MainContext));
+ // TODO: Update commands.setlang and actions.setLanguage to use CommunityContext
+ bot.command('setlang', userMiddleware, ctx => commands.setlang(ctx));
+ bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, ctx => actions.setLanguage(ctx));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bot.command('setlang', userMiddleware, ctx => commands.setlang(ctx as unknown as MainContext));
bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, ctx => actions.setLanguage(ctx as unknown as MainContext));
bot.command('setlang', userMiddleware, commands.setlang);
bot.action(/^setLanguage_([a-z]{2})$/, userMiddleware, actions.setLanguage);

@grunch grunch requested a review from Catrya March 4, 2025 14:53
@webwarrior-ws webwarrior-ws changed the title bot, jobs: convert requires to imports bot, jobs,tests/bot: convert requires to imports Mar 5, 2025
Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

tACK

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.

2 participants