-
Notifications
You must be signed in to change notification settings - Fork 7
[explorer/rest] feat: Added account endpoint #1907
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
base: explorer/puller-process-accounts
Are you sure you want to change the base?
[explorer/rest] feat: Added account endpoint #1907
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## explorer/puller-process-accounts #1907 +/- ##
====================================================================
- Coverage 96.92% 96.90% -0.03%
====================================================================
Files 270 267 -3
Lines 19473 19315 -158
Branches 215 215
====================================================================
- Hits 18875 18717 -158
Misses 592 592
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| ) = result | ||
|
|
||
| return AccountView( | ||
| address=str(Address(address)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What format is the incoming address? I guess maybe bytes from the database?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, bytes from DB.
Or do you think the formatting better happen under AccoutView's to_dict method?
AccountView(Address(address))
to_dict -> str(self.address )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, here is fine.
At first look I thought this str(Address(address)) was converting str_address -> Address->back to string but then I notice its coming from the DB😅
|
|
||
| return [self._create_block_view(result) for result in results] | ||
|
|
||
| def get_account(self, address=None, public_key=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be better to split this into two functions. One gets an account by address, and another by public_key. This will require a value to be passed into the function.
With the current function, calling - get_account(), without passing a value for either address or public_key. This will cause a runtime error.
| return [block.to_dict() for block in blocks] | ||
|
|
||
| def get_account(self, address=None, public_key=None): | ||
| """Gets account by address or public key.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. The function can be called with no argument or with both address and public_key.
| except ValueError: | ||
| abort(400) | ||
|
|
||
| result = nem_api_facade.get_account(address=address, public_key=public_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the public_key and address are validated here. 👍🏾 I am leaving the comments above since get_account is part of the API and can be called elsewhere.
| nem_db = NemDatabase(self.db_config, self.network) | ||
|
|
||
| # Act: | ||
| account_view = nem_db.get_account(public_key=ACCOUNTS[0].public_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also have test for nem_db.get_account() since this is valid.
Problem: Missing account endpoint in the rest service.
Solution: Added a new account endpoint that can query account information from the database by address or public key.