Skip to content

Feature: Added feed option to see recent transactions#56

Open
cmilhaupt wants to merge 2 commits intozackhsi:masterfrom
cmilhaupt:master
Open

Feature: Added feed option to see recent transactions#56
cmilhaupt wants to merge 2 commits intozackhsi:masterfrom
cmilhaupt:master

Conversation

@cmilhaupt
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Owner

@zackhsi zackhsi 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, this is a neat feature!

I left a few comments, let me know if I can clarify anything.

Cheers.

Comment thread venmo/user.py
response = venmo.singletons.session().get(venmo.settings.PROFILE_URL)
return response.json()['data']['user']['id']

def feed(limit=10):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you factor this out into two functions, one for data (feed) and one for presentation (print_feed)? The data function can be used when this library is imported, and the presentation function will be used for the CLI.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is the only one I'm not understanding. I can change the function argument to print_feed, but how do I call feed when the library is imported? Also how do you want to handle limit then? Cache n responses on load and query for more if limit > n?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

End the feed function on L54 with return response_dict['data']. This can be invoked when the library is imported.

Separately, create a print_feed function that calls feed then continues with the rest of the current feed function, the part that deals with presentation.

limit would be an argument on both feed and print_feed.

No need to deal with caching.

Comment thread venmo/user.py
def feed(limit=10):
venmo.auth.ensure_access_token()
user_id = get_profile_id()
response = venmo.singletons.session().get(venmo.settings.FEED_URL + user_id + "?limit=" + str(limit))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The limit=limit part of the URL can be created with the params argument.

Comment thread venmo/user.py
d['date_created'].split('T')[0])
else:
print(" " + d['payment']['actor']['display_name'] + " paid you $" +
str(d['payment']['amount']) + " on " +
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Format the string for two decimal places. The locale module may serve nicely.

Comment thread venmo/user.py
if d['type'] == 'transfer':
print(" Transfered $" + str(d['transfer']['amount']) + " to '" +
d['transfer']['destination']['name'] + "' on " +
d['transfer']['date_requested'].split('T')[0])
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This will be more resilient if we parse and format the dates. datetime.datetime.parse with %Y-%m-%dT%H:%M:%S should work.

>>> from datetime import datetime
>>> datetime.strptime('2015-05-07T19:30:56', '%Y-%m-%dT%H:%M:%S')
datetime.datetime(2015, 5, 7, 19, 30, 56)

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