search

lnp2pBot / bot

Peer-to-peer lightning network telegram bot
MIT License
206 stars 105 forks source link

Convert JS to TS (commits 7-13 of PR435) #557

Closed webwarrior-ws closed 2 months ago

knocte commented 2 months ago

@webwarrior-ws please fix the conflicts

knocte commented 2 months ago

@grunch hey Francisco, this PR doesn't have conflicts anymore, can you review? Cheers

grunch commented 2 months ago

@grunch hey Francisco, this PR doesn't have conflicts anymore, can you review? Cheers

I can't merge, the button is disabled

knocte commented 2 months ago

I can't merge, the button is disabled

Argh, new conflicts again because other PR was merged before. @webwarrior-ws hey mind rebasing again? thanks

webwarrior-ws commented 2 months ago

Rebased.

Also re-reviewed changes and made 2 corrections:

This is comparison to the old branch: https://github.com/webwarrior-ws/lnp2pBot/compare/27e805ae26bba8305d25d8bbbafcf4e27953b422..d36a6b75b3436b4d599c83db983b0a4965968c67

grunch commented 2 months ago

Hey @webwarrior-ws and @knocte we are having trouble with this PR, disputes are not working well, I have to revert this PR, sorry, let's use this PR as base of a new one which we will test more.

BTW we need to start fixing unit test, while the bot is getting bigger testing new code is way more difficult

knocte commented 2 months ago

disputes are not working well

Hey Francisco, last time you reverted something you were not specific enough and because of that we couldn't get to the bottom of it. Let's not do the same here: what do you mean with not working well??

grunch commented 2 months ago

disputes are not working well

Hey Francisco, last time you reverted something you were not specific enough and because of that we couldn't get to the bottom of it. Let's not do the same here: what do you mean with not working well??

The problem I found was when a user started a dispute the bot stop working, I got this on the logs

[2024-08-29T10:45:04.731-03:00] error: ctx.update.message.text is not available. Error: ctx.update.message.text is not available.
    at /home/grunch/dev/lnp2pbot/bot/validations.js:184:27
    at step (/home/grunch/dev/lnp2pbot/bot/validations.js:56:23)
    at Object.next (/home/grunch/dev/lnp2pbot/bot/validations.js:37:53)
    at /home/grunch/dev/lnp2pbot/bot/validations.js:31:71
    at new Promise (<anonymous>)
    at __awaiter (/home/grunch/dev/lnp2pbot/bot/validations.js:27:12)
    at validateAdmin (/home/grunch/dev/lnp2pbot/bot/validations.js:177:49)
    at exports.takeDispute (/home/grunch/dev/lnp2pbot/bot/modules/dispute/actions.js:8:23)
    at execute (/home/grunch/dev/lnp2pbot/node_modules/telegraf/lib/composer.js:468:23)
    at /home/grunch/dev/lnp2pbot/node_modules/telegraf/lib/composer.js:469:27
knocte commented 2 months ago

@grunch beautiful thanks; @webwarrior-ws please take a look at this on Monday

webwarrior-ws commented 2 months ago

Looks like @Mersho copy-pasted the same guard code in several places without checking what properties should be present. I pushed a commit with fix to https://github.com/webwarrior-ws/lnp2pBot/commits/convert-to-ts-remaining/

knocte commented 2 months ago

@webwarrior-ws awesome, please create new PR tomorrow