Skip to content

Conversation

Signed-off-by: David Echelberger <david.echelberger@kaleido.io>
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

I think TokenTransfer should be returned rather than TokenAccount on the transfers routes.

FilterFactory: database.TokenTransferQueryFactory,
Description: i18n.MsgTBD,
JSONInputValue: nil,
JSONOutputValue: func() interface{} { return []*fftypes.TokenAccount{} },
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TokenAccount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, should be TokenTransfer. Looks like a pre-existing bug, but I'll address it here.

FilterFactory: database.TokenTransferQueryFactory,
Description: i18n.MsgTBD,
JSONInputValue: nil,
JSONOutputValue: func() interface{} { return []*fftypes.TokenAccount{} },
Copy link
Contributor

Choose a reason for hiding this comment

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

Again here on TokenAccount

if name == "" {
return i18n.NewError(ctx, i18n.MsgMissingTokensPluginConfig)
}
if err = fftypes.ValidateFFNameField(ctx, name, "name"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

great thought to add this here, and stop people getting themselves into a situation where the connector can't be returned 👍

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2021

Codecov Report

Merging #275 (b496cbd) into main (017a054) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #275   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          219       223    +4     
  Lines        12317     12356   +39     
=========================================
+ Hits         12317     12356   +39     
Impacted Files Coverage Δ
internal/apiserver/route_get_token_accounts.go 100.00% <100.00%> (ø)
...rnal/apiserver/route_get_token_accounts_by_pool.go 100.00% <100.00%> (ø)
internal/apiserver/route_get_token_connectors.go 100.00% <100.00%> (ø)
internal/apiserver/route_get_token_pools.go 100.00% <100.00%> (ø)
...nternal/apiserver/route_get_token_pools_by_type.go 100.00% <100.00%> (ø)
internal/apiserver/route_get_token_transfers.go 100.00% <100.00%> (ø)
...nal/apiserver/route_get_token_transfers_by_pool.go 100.00% <100.00%> (ø)
internal/assets/manager.go 100.00% <100.00%> (ø)
internal/assets/token_pool.go 100.00% <100.00%> (ø)
internal/assets/token_transfer.go 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 017a054...b496cbd. Read the comment docs.

@awrichar awrichar merged commit a3237de into hyperledger:main Oct 20, 2021
@awrichar awrichar deleted the routes branch October 20, 2021 17:45
@awrichar awrichar mentioned this pull request Oct 20, 2021
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.

3 participants