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

controllers/main: Add APIAddress test #602

Closed
wants to merge 2 commits into from

Conversation

JoeGruffins
Copy link
Member

No description provided.

@JoeGruffins JoeGruffins force-pushed the testapiaddress branch 3 times, most recently from b4c99ff to 8f1b119 Compare March 6, 2020 01:53
@jholdstock
Copy link
Member

Will review this after #598 is merged. Feel free to give me a poke if 598 goes in and I forget to revisit this

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Mar 12, 2020

@jholdstock POKE

Feel free to give me a poke

Prepare yourself.

return nil, codes.Unauthenticated, "address error", errors.New("invalid api token")
uid, exists := c.Env["APIUserID"].(int64)
if !exists {
return nil, codes.Unauthenticated, "purchaseinfo error", errors.New("invalid api token")
Copy link
Member

Choose a reason for hiding this comment

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

afaict this should have stayed as "address error" rather than "purchaseinfo error"

user, err := models.GetUserByID(dbMap, uid)
if err != nil {
log.Warnf("failure to get GetUserByID for user %d: %v", uid, err)
return nil, codes.Internal, "purchaseinfo error", errors.New("internal error")
Copy link
Member

Choose a reason for hiding this comment

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

"address error" rather than "purchaseinfo error"

user, _ := models.GetUserByID(dbMap, c.Env["APIUserID"].(int64))
user, err := models.GetUserByID(dbMap, uid)
if err != nil {
log.Warnf("failure to get GetUserByID for user %d: %v", uid, err)
Copy link
Member

Choose a reason for hiding this comment

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

This should probably log at Error level because it could be something wrong with the data in the db, and also we are returning an error to the caller

c.Env["DbMap"] = dbMap
// Put userPubKeyAddr as form value.
r, _ := http.NewRequest("GET", "?UserPubKeyAddr="+test.userPubKeyAddr, nil)
_, errCode, _, _ := mc.APIAddress(c, r)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should also be checking the other values returned by APIAddress().

This actually makes me wonder, do we really need to return both a string response and an error? The error alone is probably sufficient, having both seems redundant.

@JoeGruffins JoeGruffins closed this Apr 9, 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.

2 participants