Qluxzz / avanza

A Python library for the unofficial Avanza API
https://qluxzz.github.io/avanza/
MIT License
85 stars 40 forks source link

Fetch transactions details and fetch note (swedish: avräkningsnota) as a blob #33

Closed warna720 closed 3 years ago

warna720 commented 3 years ago

get_transactions() does not return data with urlParameterId which is the accountId encrypted by Avanza and also used as one part to fetch the note. Therefore get_transactions2() which returns data with urlParameterId.

and so we have the get_note_as_binary_blob(url_parameter_id, note_id) which returns the pdf as a binary blob.

Would be nice to get feedback fast so we can merge this and hopefully get this released ASAP.

warna720 commented 3 years ago

Would be nice to get your feedback also @tux2000

tux2000 commented 3 years ago

Sorry, I don't have a deep enough understanding of the codebase yet to give a qualified review. In general I would expect using the "mobile" API over the "website" API should shield us a little against changes on the other end. And when adding new functions and files it might be informative to have a clue on their come about in the functionname/filename instead of adding a counter. Alternatively get_transactions could be just replaced with the new code if the arguments and returns are compatible and the new code is considered superior...

warna720 commented 3 years ago

get_transactions2 is not backward compatible since we dont have the same filters available. get_transactions does not return result with urlParameterId, neither does any other endpoint from what I could find. And I cant find anywhere to get the note (avräkningsnota) in the app (Android) which could have pointed us in the direction where to get urlParameterId from the mobile API.

So neither is superior, they just have different filters/data, hence get_transactions2. Im open to more descriptive naming, alternatively merging them somehow if possible.

Maybe rename get_transactions2 to get_transactions_as_(desktop/web)? And then having get_transactions_as_mobile return the result from get_transactions?

Qluxzz commented 3 years ago

I think it would be better to name it after what it does, rather than how it does it. So if it returns more info, maybe get_transaction_with_extended_info or something similar would be a better name?

warna720 commented 3 years ago

Its not extended info, its a whole other endpoint with other filters. Some data is extended some data does not overlap e.g. totalNumberOfTransactions in get_transactions not included in get_transactions2

Maybe get_transactions_details?

warna720 commented 3 years ago

Changed it to get_transactions_details Maybe let the naming marinate in our brains for the night atleast before we decide

warna720 commented 3 years ago

Would be nice to get this merged & released today if possible, and we dont have any more change-requests :)