horizon/actions: use action handlers on transaction-related endpoints #1069
Conversation
This PR brings the changes in stellar#894 to two other transaction related endpoints.
| return nil, errors.Wrap(err, "getting horizon db session") | ||
| } | ||
|
|
||
| return actions.TransactionPage(ctx, &history.Q{horizonSession}, qp.accountID, qp.ledgerID, qp.includeFailedTxs, qp.pagingParams) |
There was a problem hiding this comment.
I'm thinking out loud (and also don't remember if we discussed this in previous PRs):
It looks like functions like getTransactionPage or streamTransactions are wrappers around actions.* methods that really just check if param is correct (interface{}...) and pass DB session. If these type checks were moved to actions we could remove all such functions. It requires some refactoring but maybe it's worth it.
There's an obvious problem of circular package dependency but maybe we can solve this by moving indexActionQueryParams and showActionQueryParams to actions package (?).
There was a problem hiding this comment.
I was thinking maybe we can have a general query structure for all endpoints, but it seems like we categorize them into show and index . And at this point, it doesn't make sense to combine them given that we validate them differently. Last but not least, I think it's better to keep the query processing in the web layer and let action package focus on processing the data from DB. It will become more clear what's better for us later when most of the endpoints are converted 😃
This PR brings the changes in #894 to two other transaction related endpoints.
This PR also generalizes our handlers and query params. Now we have two different query params for all the endpoints called
showActionQueryParamsandindexActionQueryParams. And we also have three different handlers calledshowActionHandler,streamShowActionHandler, andstreamIndexActionHander.