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

List account transactions API end-point #109

Closed
dmugtasimov opened this issue Apr 20, 2021 · 10 comments · Fixed by #341 or #371
Closed

List account transactions API end-point #109

dmugtasimov opened this issue Apr 20, 2021 · 10 comments · Fixed by #341 or #371

Comments

@dmugtasimov
Copy link
Contributor

dmugtasimov commented Apr 20, 2021

TODO:

  • GET /api/v1/accounts/{id}/transactions/
  • Pagination
  • Ordering asc/desc chronologically
  • If node does not have complete blockchain (all blocks till number 0) and transaction beyond known blocks are requested raise NotImplemented error (later we will have to implement sync initialization or otherwise redirection of client to get the entire list of transaction from somewhere else, for example from current PV)
  • Use serializer based on transaction dataclass
  • Add a TODO about a need of caching implementation
@dmugtasimov dmugtasimov added this to Backlog in Blockchain Tasks via automation Apr 20, 2021
@dmugtasimov dmugtasimov added this to the Support beta architecture on API level milestone May 16, 2021
@dmugtasimov dmugtasimov modified the milestones: Support beta architecture on API level, Client (Account Manager) facing API Aug 13, 2021
@dmugtasimov dmugtasimov changed the title List account transactions List account transactions API end-point Aug 13, 2021
@fonar fonar moved this from Backlog to In progress in Blockchain Tasks Aug 16, 2021
@fonar
Copy link
Contributor

fonar commented Aug 18, 2021

@dmugtasimov i want to propose of using the "drf-nested-routers" library that i sucessfully used for several projects.
It will allow to have the drf transactions viewset that will be nested under acounts viewset, keeping them separate.
that is very usefull and keeping clean architecture for api's in drf-style.
What do you think?
https://github.com/alanjds/drf-nested-routers

@dmugtasimov
Copy link
Contributor Author

@fonar sure, go ahead

@fonar
Copy link
Contributor

fonar commented Aug 21, 2021

@dmugtasimov questions :

  1. should this API be
    GET /api/v1/account_states/{id}/transactions/
    instead of
    GET /api/v1/accounts/{id}/transactions/
    as mentioned in issue description?

  2. By transaction dataclass, do you mean CoinTransferTransaction dataclass?

fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 22, 2021
@fonar fonar linked a pull request Aug 22, 2021 that will close this issue
fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 22, 2021
@fonar
Copy link
Contributor

fonar commented Aug 23, 2021

@dmugtasimov in PR logic and tests is not complete and i will improve, just 3 quick qustions: (1) if endpoint URL is ok (2) what blockchain iterface (mixin) to use (3) how to check for imcomplete blockchain. I see this in code but don't have full understanding how to use it.

@dmugtasimov
Copy link
Contributor Author

@fonar replied in discord

fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 29, 2021
fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 29, 2021
fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 29, 2021
@fonar fonar moved this from In progress to Code review in Blockchain Tasks Aug 29, 2021
@fonar
Copy link
Contributor

fonar commented Aug 29, 2021

@dmugtasimov ready for review

@dmugtasimov
Copy link
Contributor Author

@fonar thank you, merged. Please, address this comment https://github.com/thenewboston-developers/thenewboston-node/pull/341/files#r698550152 in another PR

@dmugtasimov
Copy link
Contributor Author

@fonar I isolated and corrected incomplete blockchain related logic: #370

fonar added a commit to fonar/thenewboston-node that referenced this issue Aug 31, 2021
@fonar fonar linked a pull request Aug 31, 2021 that will close this issue
@fonar
Copy link
Contributor

fonar commented Aug 31, 2021

@dmugtasimov added PR #371

@fonar fonar moved this from In progress to Code review in Blockchain Tasks Aug 31, 2021
@dmugtasimov
Copy link
Contributor Author

@fonar thank you, merged

@dmugtasimov dmugtasimov moved this from Code review to Done in Blockchain Tasks Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
2 participants