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

commands: inform buyer when order is canceled #519

Closed
wants to merge 2 commits into from

Conversation

knocte
Copy link
Contributor

@knocte knocte commented May 13, 2024

When the seller cancels an order, the buyer was getting a confusing message:

You have cancelled the order ID: orderID

Now the bot sends a message that tells the buyer:

Order orderId has been cancelled. There is no need to create an invoice.

Fixes: #482

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

this is a good add, thank you @knocte

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

There is something we need to do before start using this, we need this new message to be added on the other languages, if we don't it will fail to users with non english settings

@knocte
Copy link
Contributor Author

knocte commented May 13, 2024

But I only speak 2 languages :) why don't you use automatic tools for this such as Transifex?

@grunch
Copy link
Member

grunch commented May 16, 2024

But I only speak 2 languages :) why don't you use automatic tools for this such as Transifex?

yeah me too, currently I'm translating manually new locales, do you have a guide to implement transifex here, if you can give us a hand on this it would be great 😃

@knocte
Copy link
Contributor Author

knocte commented May 18, 2024

currently I'm translating manually new locales,

What do you exactly mean with "translating manually"? Don't tell me you're using ChatGPT 😛

do you have a guide to implement transifex here,

No TBH. I come from the GNOME ecosystem, where all kinds of gettext-style tools were already integrated in the workflow, and only native speakers did the translation work. Then I learned about Transifex from the fact that Smuxi (a GNOME IRC client I contributed to) also received translations this way.

if you can give us a hand on this it would be great 😃

I'll try gathering more info about how a new project can join any of these systems.

@knocte
Copy link
Contributor Author

knocte commented May 18, 2024

I'll try gathering more info about how a new project can join any of these systems.

BTW, I brought this up because when using gettext, for example, if the repo gains new strings from a PR, the default text is always in English, but gains some metadata that the translators can pull, to know that their translations got outdated, and when a translation hasn't been updated yet, at least it would show in English (instead of empty or throwing an error).

@grunch
Copy link
Member

grunch commented May 20, 2024

currently I'm translating manually new locales,

What do you exactly mean with "translating manually"? Don't tell me you're using ChatGPT 😛

haha no, I'm using this one

https://www.deepl.com/translator

@grunch
Copy link
Member

grunch commented May 20, 2024

I'll try gathering more info about how a new project can join any of these systems.

BTW, I brought this up because when using gettext, for example, if the repo gains new strings from a PR, the default text is always in English, but gains some metadata that the translators can pull, to know that their translations got outdated, and when a translation hasn't been updated yet, at least it would show in English (instead of empty or throwing an error).

makes sense, in our case we should code it, if the yaml key is not available then use english

When the seller cancels an order, the buyer was getting a
confusing message:

```
You have cancelled the order ID: orderID
```

Now the bot sends a message that tells the buyer:

```
Order orderId has been cancelled. There is no need to create an invoice.
```

Fixes: lnp2pBot#482
@knocte
Copy link
Contributor Author

knocte commented Nov 12, 2024

@grunch sorry for the long delay to act on this, I've rebased the PR (because there was a conflict) and added translations.

But it's not finished yet, one more translation missing: fa. I have a friend from Iran so I'll ask him, stay tuned.

In the meantime, do you mind making a new release? would be better to release before merging this IMO. What versoin is LNp2pBot production running on at the moment?

See previous commit.

Translated using https://www.deepl.com/en/translator ,
as recommended by @grunch.

Incomplete, one more translation missing: farsi.
@Catrya
Copy link
Member

Catrya commented Nov 12, 2024

Hi @knocte I think that with pr #601, this PR #519 is no longer necessary, because #601 it covers the case you want to solve here (notify the buyer when the order is cancelled), but also notify the seller when the order is cancelled, as that case is not covered in this PR.

I think that the solution is to change the logic of how things happen, and not to create a new successCancelOrderInvoiceMessage, because already exists anothe message counterPartyCancelOrderMessage but was never sent.
Do yo agree?

@grunch
Copy link
Member

grunch commented Nov 12, 2024

@grunch sorry for the long delay to act on this, I've rebased the PR (because there was a conflict) and added translations.

But it's not finished yet, one more translation missing: fa. I have a friend from Iran so I'll ask him, stay tuned.

In the meantime, do you mind making a new release? would be better to release before merging this IMO. What versoin is LNp2pBot production running on at the moment?

Hey @knocte thanks for your contribution, this issue was fixed on #601 without adding a new locales, I'm closing this one in favor of #601

@grunch grunch closed this Nov 12, 2024
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.

Message "You have cancelled the order..." should rather be "Order<orderId> has been cancelled..."
4 participants