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

Added support for LNBits "Invoices" extension #82

Merged
merged 11 commits into from
Nov 1, 2023

Conversation

fenixlibertas
Copy link
Contributor

No description provided.

@bitkarrot
Copy link
Member

What's a DTO?

@fenixlibertas
Copy link
Contributor Author

fenixlibertas commented Oct 21, 2023

DTO is Data Transfer Object used to transfer data in remote API calls, not sure if 'pattern' is used in Python..

https://twitter.com/unclebobmartin/status/1132613913726423043
https://medium.com/@gushakov/clean-architecture-domain-entities-and-interface-adapters-4152b9ee22d2

Found some usages in python:
https://hackernoon.com/dto-in-python-an-explanation

@bitkarrot
Copy link
Member

Ok - understood on the DTO.

There is a deprecation warning that came up

/workspaces/pylnbits/tests/test_invoices.py:77: DeprecationWarning: There is no current event loop
  loop = asyncio.get_event_loop()```

Running the tests appear to work fine with 0.10.9 LNBits testnet instance. However I had to manually edit the invoice IDs, moving forward it might be better to write the unit tests where we don't hard code the invoice IDs but dynamically get them in the test script (I am aware the other tests also need to be updated and aren't doing this). It would make it easier for other users and/or reviewers to test the code. 

here are the results I got:

Invoices:
[{'id': 'B9DwJiZXscZQWLLNjovW5W', 'wallet': '9af197b1e3964216881f689106f34f37', 'status': 'draft', 'currency': 'EUR', 'company_name': 'my_company_name10', 'first_name': 'my_first_name', 'last_name': 'my_last_name', 'email': 'my_email@email.com', 'phone': 'my_phone_number', 'address': 'my_address', 'time': 1698116650}]

Invoice:
{'id': 'B9DwJiZXscZQWLLNjovW5W', 'wallet': '9af197b1e3964216881f689106f34f37', 'status': 'draft', 'currency': 'EUR', 'company_name': 'my_company_name10', 'first_name': 'my_first_name', 'last_name': 'my_last_name', 'email': 'my_email@email.com', 'phone': 'my_phone_number', 'address': 'my_address', 'time': 1698116650, 'items': [{'id': 'Hy8cKgVuYMpW8xeQnKLAsT', 'invoice_id': 'B9DwJiZXscZQWLLNjovW5W', 'description': 'item1', 'amount': 500}, {'id': '8kxnaaPvE8TeJZtGayYiDC', 'invoice_id': 'B9DwJiZXscZQWLLNjovW5W', 'description': 'item2', 'amount': 300}], 'payments': 0}

Created invoice
{'id': 'nrJSrRmf7HfyjNP9Liidnw', 'wallet': '9af197b1e3964216881f689106f34f37', 'status': 'draft', 'currency': 'EUR', 'company_name': 'my_company_name10', 'first_name': 'my_first_name', 'last_name': 'my_last_name', 'email': 'my_email@email.com', 'phone': 'my_phone_number', 'address': 'my_address', 'time': 1698116835, 'items': [{'id': 'bDMnUs8iWhxJcdGSQLtNxn', 'invoice_id': 'nrJSrRmf7HfyjNP9Liidnw', 'description': 'item1', 'amount': 500}, {'id': 'YMAG7MMLFyAavyW8Q7SLQo', 'invoice_id': 'nrJSrRmf7HfyjNP9Liidnw', 'description': 'item2', 'amount': 300}]}

Updated invoice:
{'id': 'B9DwJiZXscZQWLLNjovW5W', 'wallet': '9af197b1e3964216881f689106f34f37', 'status': 'draft', 'currency': 'EUR', 'company_name': 'my_company_name10_1', 'first_name': 'my_first_name1', 'last_name': 'my_last_name1', 'email': 'my_email@email.com1', 'phone': 'my_phone_number1', 'address': 'my_address1', 'time': 1698116650, 'items': []}

Invoice payment::
{'detail': 'Amount exceeds invoice due.'}

Invoice payment status:
{'paid': False, 'preimage': '0000000000000000000000000000000000000000000000000000000000000000'}

Delete Invoice result::
{"status":true}


Only issue appears to be the "Invoice payment" with "amount exceeds invoice due", if it was 8 euros worth of sats, approximately 24,912 sats.  However, there was more than that available balance to pay from the wallet. 

One item to check is if you can pay the invoice as a draft, it might need to be set to open status. 

@bitkarrot
Copy link
Member

There are a few pyproject.toml and requirements.txt updates, if you could kindly resolve, would appreciate it. @fenixlibertas

@fenixlibertas
Copy link
Contributor Author

Thanks bitkarrot, I'll take a look

@fenixlibertas
Copy link
Contributor Author

ct.toml and requirements.txt updates, if you could kindly resolve, would appreciate it. @fenixlibertas

Done

@fenixlibertas
Copy link
Contributor Author

  1. Invoice payment done.
  2. InvoiceId is now more dynamic
  3. Bug reported for Invoice item update action: update_invoice_items does not work lnbits/invoices#8

@bitkarrot
Copy link
Member

OK Will take a look, thanks @fenixlibertas

@bitkarrot
Copy link
Member

@fenixlibertas ok i think this looks fine, only comment would be to add a note in the InvoiceDTO.py file and state what the DTO abbreviation is, in case someone doesn't know the reference.

I will run this code in the AM, and then merge it in

@fenixlibertas
Copy link
Contributor Author

DTO is Data Transfer Object

@bitkarrot
Copy link
Member

@fenixlibertas There were a bunch of formatting issues, I just pushed straight to your branch and fixed them, You might want to look up PEP8 and get a good formatter for VSCode for future python.

Screenshot 2023-10-31 at 6 31 46 PM

@bitkarrot bitkarrot merged commit f55b0d8 into lightningames:main Nov 1, 2023
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