Skip to content

Conversation

@deepalipawade
Copy link

@deepalipawade deepalipawade commented Sep 14, 2020

Tasks to which I contributed and file names

  1. . Create Schema for Cart -
    migrations/1599535672_create_cart.up.sql
    migrations/1599535672_create_cart.down.sql

  2. Create Hanlder for Cart -
    service/cart_http.go
    service/cart_http_test.go
    service/common_http_test.go

  3. Database Interaction for Cart -
    db/cart.go
    db/cart_test.go
    db/common_test.go

  4. Added GetCart as Storer Interface method -
    db/db.go

  5. Added route for cart -
    service/router.go

@deepalipawade deepalipawade changed the base branch from project_setup to development September 14, 2020 11:35
Copy link

@wsushmita wsushmita left a comment

Choose a reason for hiding this comment

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

@deepalipawade I have added some comments , pls do requested changes

config/config.go Outdated

// JWTExpiryDurationHours - returns duration for jwt expiry in int
func JWTExpiryDurationHours() int {
return int(ReadEnvInt("JWT_EXPIRY_DURATION_HOURS"))

Choose a reason for hiding this comment

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

ReadEnvInt returns int , here you don't need to use int()

db/cart.go Outdated
var pids, quantities []int
var products []Product

err = s.db.Select(&pids, getCartQuery, user_id)

Choose a reason for hiding this comment

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

  1. handle err != nil , log error & return
  2. Why are you using two query to get data from two tables? add a single query to fetch data from cart

db/cart.go Outdated
}

query, args, err := sqlx.In(getProductsQuery, pids)
query = s.db.Rebind(query)

Choose a reason for hiding this comment

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

here don't use sqlx.In with rebind .You can use exec() to fire any query

db/cart.go Outdated
for index, product := range products {
var category, image_urls []string
err = s.db.Select(&category, getCategoryQuery, product.CategoryId)
err = s.db.Select(&image_urls, getProductImageQuery, product.Id)

Choose a reason for hiding this comment

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

use join to get details from other table

db/cart.go Outdated
logger.WithField("err", err.Error()).Error("Error listing cart")
return
}
fmt.Println(category, image_urls, index)

Choose a reason for hiding this comment

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

always remove fmt

// @Failure 400 {object}
func getCartHandler(deps Dependencies) http.HandlerFunc {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
// request_params := mux.Vars(req)

Choose a reason for hiding this comment

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

remove commented code

func listUsersHandler(deps Dependencies) http.HandlerFunc {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
users, err := deps.Store.ListUsers(req.Context())
fmt.Println(users)

Choose a reason for hiding this comment

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

remove fmt

cart_products, err := deps.Store.GetCart(req.Context(), int(userID))
if err != nil {
logger.WithField("err", err.Error()).Error("Error fetching data")
rw.WriteHeader(http.StatusInternalServerError)

Choose a reason for hiding this comment

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

return error in json response

db/cart.go Outdated
return
}

for _, row := range joinCartProduct {

Choose a reason for hiding this comment

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

you are fetching category id for every record, you can join multiple tables using diff where clauses

db/cart.go Outdated
return
}

err = s.db.Select(&image_urls, getProductImageQuery, row.ProductId)

Choose a reason for hiding this comment

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

sane here , you can use join

@deepalipawade deepalipawade self-assigned this Sep 30, 2020
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