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

add option to query account details including personal data #159

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

marcusfey
Copy link
Contributor

The API so far does not read the account data from Traderepublic. A lot of this data is not that interesting and pretty static (name, address, etc). But there is also the main account including IBAN.

I need this for my own asset management application where I aggregate all the current/saving accounts from all my banks. To achieve this I added modules for each bank, they send the account information to the main app. Thus only the balance is not enough, at least IBAN is needed to sort things out for the main app.

I'm aware that returning the dict and writing out the JSON is not that much of a processing. Please consider adding this "anyway", change the code or tell me what I should add.

Addmittedly I'm pretty new to Python.

This is roughly what you will get:

{
	"phoneNumber":"+491xxxx",
	"jurisdiction":"DE",
	"name":{"first":"xxxx", "last":"xxx"},
	"email":{"address":"[email protected]", "verified":true},
	"duplicateTradingEmail":null,
	"postalAddress":{"street":"SomeStreet", "houseNo":"4", "zip":"123456", "city":"Somecity", "country":"DE"},
	"birthdate":"1902-01-01",
	"birthplace":{"birthplace":"Somecity", "birthcountry":"DE"},
	"mainNationality":"DE",
	"additionalNationalities":[],
	"mainTaxResidency":{"tin":"12345987", "countryCode":"DE"},
	"usTaxResidency":false,
	"additionalTaxResidencies":[],
	"taxInformationSyncTimestamp":172177300000,
	"taxExemptionOrder":{"minimum":0, "maximum":9900, "current":111, "applied":2131, "syncStatus":12414, "validFrom":1242, "validUntil":2312}, 
	"registrationAccount":false,
	"cashAccount":{"iban":"DE00000000000000000000", "bic":"TRBKDEBBXXX", "bankName":"J.P. Morgan SE", "logoUrl":null},
	"referenceAccount":{"iban":"-", "bic":null},
	"referenceAccountV2":null,
	"referenceAccountList":[{"iban":"DE00012134874564878747", "bic":null, "bankName":"Bank", "logoUrl":"logos/bank_bank/v2"}],
	"securitiesAccountNumber":"321354657",
	"experience":{"stock":{"tradeCount":0, "level":"LOSER", "showsRiskWarning":true},
	"fund":{"tradeCount":0, "level":"LOSER", "showsRiskWarning":true},
	"derivative":{"tradeCount":0, "level":"LOSER", "showsRiskWarning":true},
	"crypto":{"tradeCount":100, "level":"GODMODE", "showsRiskWarning":true},
	"bond":{"level":"xxxx", "showsRiskWarning":true}},
	"referralDetails":null,
	"supportDocuments":{
	   "accountClosing":"https://assets.traderepublic.com/assets/files/foilename.pdf",
	   "imprint":"https://traderepublic.com/de-de/imprint",
	   "addressConfirmation":"https://support.traderepublic.com/de-de/102"},
	   "tinFormat":{"placeholder":"99999999999",
	   "keyboardLayout":"numerical"
	},
	"personId":"babdbddb-5441-23-124-423423542532112"
}

Copy link
Collaborator

@NiklasRosenstein NiklasRosenstein left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @marcusfey

@@ -0,0 +1,16 @@
import json

class Accountdetails:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
class Accountdetails:
class AccountDetails:

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this class has any right to be here, it's such a thin wrapper. :)

What would be neat is this was a TypedDict instead that describes the fields returned by TradeRepublicApi.get_account_details().

pytr/main.py Outdated
Comment on lines 303 to 309
elif args.command == "accountdetails":
ad = Accountdetails(
login(phone_no=args.phone_no, pin=args.pin, web=not args.applogin),
)
ad.get()
if args.jsonoutput is not None:
ad.data_to_file(args.jsonoutput)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
elif args.command == "accountdetails":
ad = Accountdetails(
login(phone_no=args.phone_no, pin=args.pin, web=not args.applogin),
)
ad.get()
if args.jsonoutput is not None:
ad.data_to_file(args.jsonoutput)
elif args.command == "accountdetails":
tr = login(phone_no=args.phone_no, pin=args.pin, web=not args.applogin)
account_details = tr.get_account_details()
args.jsonoutput.write(json.dumps(account_details))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A question here: this would write the json args.jsonoutput without checking if this parameter is actually set.
Are the any implicit checks I'm not aware of? Or should I change this to

    elif args.command == "accountdetails":
        tr = login(phone_no=args.phone_no, pin=args.pin, web=not args.applogin)
        account_details = tr.get_account_details()
        if args.jsonoutput is not None:
          args.jsonoutput.write(json.dumps(account_details))

Copy link
Collaborator

Choose a reason for hiding this comment

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

The other change above I suggested makes it so args.jsonpath is always set, to a file-like object in write mode. If no argument is specified, it defaults to - which argparse.FileType interprets as "stdout".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right! I wasn't aware initially that those -o parameter are per command. It's a nice solution.

@marcusfey
Copy link
Contributor Author

Thanks for your review! I'm completely fine with what you requested.

Actually for me TradeRepublicApi.get_account_details() is all I need. You are right of course, Accountdetails is not needed as it doesn't do much as is. I wasn't aware writing or didn't think hard enough regarding writing the json to a file without a class.

Thus: Should I remove Accountdetails altogether because it's useless?

@NiklasRosenstein
Copy link
Collaborator

Thus: Should I remove Accountdetails altogether because it's useless?

Yes, I think that's okay! If you care to invest the time, it would be nice to have a TypedDict description of the account details as well. (Can probably pipe the output you passed through an LLM and double check the result if you want).

@marcusfey
Copy link
Contributor Author

I added the TypedDict with 1ce3145. But I really not sure if that's what you had in mind 🫣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants