Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • This project
    • Loading...
  • Sign in / Register
V
VietTH-VenShop
  • Overview
    • Overview
    • Details
    • Activity
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Issues 0
    • Issues 0
    • List
    • Board
    • Labels
    • Milestones
  • Merge Requests 4
    • Merge Requests 4
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • Tran Hoang Viet
  • VietTH-VenShop
  • Merge Requests
  • !3

Merged
Opened Jul 03, 2015 by Tran Hoang Viet@vietth 
  • Report abuse
Report abuse

Vietth feature product management

  • Discussion 22
  • Commits 5
  • Changes
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Nguyen Ngoc Lan
    @lannn started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/products_controller.rb 0 → 100644
    6
    7 def new
    8 end
    9
    10 def create
    11 @product = Product.create(params_permitted)
    12 if @product.valid?
    13 redirect_to root_path
    14 else
    15 render :new
    16 end
    17 end
    18
    19 private
    20 def set_product
    21 @product = Product.find_by(id: params[:id]) || Product.new
    • Nguyen Ngoc Lan @lannn commented Jul 06, 2015
      Developer

      @vietth set_product method should be for actions which take a product as input like: edit, update, destroy

      With new, create do not have any existed product. So, to decouple logic, you can separate logic for new and existed product

      Edited Jul 06, 2015
      @vietth set_product method should be for actions which take a product as input like: edit, update, destroy With new, create do not have any existed product. So, to decouple logic, you can separate logic for new and existed product
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/application_controller.rb
    4 4 # Prevent CSRF attacks by raising an exception.
    5 5 # For APIs, you may want to use :null_session instead.
    6 6 protect_from_forgery with: :exception
    7 end
    7
    8 before_action :get_sidebar_categories
    9
    10 protected
    11 def params_permitted data = nil
    • Nguyen Ngoc Lan @lannn commented Jul 06, 2015
      Developer

      @vietth You have put strong params from other controllers to application controller, right?

      But problem is coupling, each controller defines its own strong params. So i think it is better to manage params by moving them to specific controller

      Edited Jul 06, 2015
      @vietth You have put strong params from other controllers to application controller, right? But problem is coupling, each controller defines its own strong params. So i think it is better to manage params by moving them to specific controller
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/application_controller.rb
    4 4 # Prevent CSRF attacks by raising an exception.
    5 5 # For APIs, you may want to use :null_session instead.
    6 6 protect_from_forgery with: :exception
    7 end
    7
    8 before_action :get_sidebar_categories
    9
    10 protected
    11 def params_permitted data = nil
    12 data ||= self.class::PERMIT
    13 params.require(data.keys.first).permit(data.values.first)
    14 end
    15
    16 def get_sidebar_categories
    • Nguyen Ngoc Lan @lannn commented Jul 06, 2015
      Developer

      @vietth Do you want to set categories as component in layout like header, footer? If right, it is better to move this logic to view

      @vietth Do you want to set categories as component in layout like header, footer? If right, it is better to move this logic to view
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/products_controller.rb 0 → 100644
    1 class ProductsController < ApplicationController
    2 before_action :set_product, only: :new
    3 before_action :set_categories, only: [:new, :create]
    4
    5 PERMIT = { product: %i(title price category_id image) }
    6
    7 def new
    8 end
    9
    10 def create
    11 @product = Product.create(params_permitted)
    • Nguyen Ngoc Lan @lannn commented Jul 06, 2015
      Developer

      @vietth Your flow is: Create a product -> Check valid? -> Redirect Or Render :new

      Please compare with: Initialize a new product -> Try to save it -> Redirect Or Render :new

      Edited Jul 06, 2015
      @vietth Your flow is: Create a product -> Check valid? -> Redirect Or Render :new Please compare with: Initialize a new product -> Try to save it -> Redirect Or Render :new
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/products_controller.rb 0 → 100644
    1 class ProductsController < ApplicationController
    2 before_action :set_product, only: :new
    3 before_action :set_categories, only: [:new, :create]
    4
    5 PERMIT = { product: %i(title price category_id image) }
    6
    7 def new
    8 end
    9
    10 def create
    11 @product = Product.create(params_permitted)
    • Tran Hoang Viet @vietth commented Jul 06, 2015
      Master

      I think the logic is follows: My logic: Product.create (include initialize and check valid?) -> check valid? -> Redirect or render .Your logic: Product.new (only initialize) -> save (check valid?) -> redirect or render .It seems i has excess of 1 times check valid?. Ok fixed

      Edited Jul 06, 2015
      I think the logic is follows: My logic: Product.create (include initialize and check valid?) -> check valid? -> Redirect or render .Your logic: Product.new (only initialize) -> save (check valid?) -> redirect or render .It seems i has excess of 1 times check valid?. Ok fixed
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/application_controller.rb
    4 4 # Prevent CSRF attacks by raising an exception.
    5 5 # For APIs, you may want to use :null_session instead.
    6 6 protect_from_forgery with: :exception
    7 end
    7
    8 before_action :get_sidebar_categories
    9
    10 protected
    11 def params_permitted data = nil
    12 data ||= self.class::PERMIT
    13 params.require(data.keys.first).permit(data.values.first)
    14 end
    15
    16 def get_sidebar_categories
    • Tran Hoang Viet @vietth commented Jul 06, 2015
      Master

      I think logic shouldn't placed in view. So to better, I can move before_action: get_sidebar_categories to each controller that I want show categories. But in this case, if allowed put logic into the view is better.

      I think logic shouldn't placed in view. So to better, I can move *before_action: get_sidebar_categories* to each controller that I want show categories. But in this case, if allowed put logic into the view is better.
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/application_controller.rb
    4 4 # Prevent CSRF attacks by raising an exception.
    5 5 # For APIs, you may want to use :null_session instead.
    6 6 protect_from_forgery with: :exception
    7 end
    7
    8 before_action :get_sidebar_categories
    9
    10 protected
    11 def params_permitted data = nil
    • Tran Hoang Viet @vietth commented Jul 06, 2015
      Master

      In each controller will define a constant PERMIT and params_permitted will use it to permit params. In basic cases, i can use PERMIT or params_permitted(OTHER_PERMIT). What do you think ?

      In each controller will define a constant PERMIT and params_permitted will use it to permit params. In basic cases, i can use PERMIT or params_permitted(OTHER_PERMIT). What do you think ?
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on an old version of the diff Jul 06, 2015
    app/controllers/products_controller.rb 0 → 100644
    6
    7 def new
    8 end
    9
    10 def create
    11 @product = Product.create(params_permitted)
    12 if @product.valid?
    13 redirect_to root_path
    14 else
    15 render :new
    16 end
    17 end
    18
    19 private
    20 def set_product
    21 @product = Product.find_by(id: params[:id]) || Product.new
    • Tran Hoang Viet @vietth commented Jul 06, 2015
      Master

      Okie :)

      Okie :)
    Please register or sign in to reply
  • Tran Hoang Viet @vietth

    Added 2 commits:

    • 048a291e - VietTH: Implement breadcrumb, add to cart, checkout
    • b24e4e5f - VietTH: Implement send email after checkout
    Jul 06, 2015

    Added 2 commits:

    • 048a291e - VietTH: Implement breadcrumb, add to cart, checkout
    • b24e4e5f - VietTH: Implement send email after checkout
    Added 2 commits: * 048a291e - VietTH: Implement breadcrumb, add to cart, checkout * b24e4e5f - VietTH: Implement send email after checkout
    Toggle commit list
  • Tran Hoang Viet @vietth

    Added 2 commits:

    • 8366883a - VietTH: Update template email checkout
    • d570482e - VietTH: Implement show category detail
    Jul 07, 2015

    Added 2 commits:

    • 8366883a - VietTH: Update template email checkout
    • d570482e - VietTH: Implement show category detail
    Added 2 commits: * 8366883a - VietTH: Update template email checkout * d570482e - VietTH: Implement show category detail
    Toggle commit list
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/categories_controller.rb 0 → 100644
    1 class CategoriesController < ApplicationController
    2 before_action :set_category, only: [:show]
    3
    4 def show
    5 @products = @category.products.page(params[:page])
    6 end
    7
    8 private
    9 def set_category
    10 @category = Category.find_by(id: params[:id])
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth what happen when category not found?

      @vietth what happen when category not found?
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/helpers/application_helper.rb
    1 1 module ApplicationHelper
    2 def session_cart
    3 (session[:cart_info] ||= {items: [], total: 0}).with_indifferent_access
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth You have stored all cart information into session. This makes session bloated. I think you only need store cart_id in session.

      Then each time, user go to cart, you will find cart by id for user and show cart's information

      @vietth You have stored all cart information into session. This makes session bloated. I think you only need store cart_id in session. Then each time, user go to cart, you will find cart by id for user and show cart's information
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/products_controller.rb 0 → 100644
    13
    14 def create
    15 @product = Product.new(product_params)
    16 if @product.save
    17 redirect_to root_path
    18 else
    19 render :new
    20 end
    21 end
    22
    23 def show
    24 add_breadcrumb(@product.category.decorate.title, category_path(@product.category))
    25 add_breadcrumb(@product.title)
    26 end
    27
    28 def add_cart
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth Does this action create cart_item records?

      If right, you can move it into "create" action in CartItemsController

      @vietth Does this action create cart_item records? If right, you can move it into "create" action in CartItemsController
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/carts_controller.rb 0 → 100644
    1 class CartsController < ApplicationController
    2 include CartsManagement
    3
    4 before_action :authenticate_user!
    5
    6 def index
    7 @cart = Cart.new
    8 end
    9
    10 def create
    11 cart = current_user.carts.build(cart_params)
    12
    13 product_ids = []
    14 quantites = {}
    15 cart_params['cart_items_attributes'].each do |item|
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth the controller should be skinny - not much logic. Where should this logic be in?

      @vietth the controller should be skinny - not much logic. Where should this logic be in?
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/carts_controller.rb 0 → 100644
    7 @cart = Cart.new
    8 end
    9
    10 def create
    11 cart = current_user.carts.build(cart_params)
    12
    13 product_ids = []
    14 quantites = {}
    15 cart_params['cart_items_attributes'].each do |item|
    16 id = item.last['product_id']
    17 product_ids << id
    18 quantites[id] = item.last['quantity']
    19 end
    20
    21 products = Product.where(id: [product_ids]).select(:id, :price)
    22 total = products.inject(0) do |sum, product|
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth this logic also should not in controller

      @vietth this logic also should not in controller
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/carts_controller.rb 0 → 100644
    21 products = Product.where(id: [product_ids]).select(:id, :price)
    22 total = products.inject(0) do |sum, product|
    23 sum + (product.price.to_i * quantites[product.id.to_s].to_i)
    24 end
    25 cart.total = total
    26
    27 if cart.save
    28 clear_cart
    29 self.send_checkout_email(cart)
    30 redirect_to root_path
    31 else
    32 render :index
    33 end
    34 end
    35
    36 def send_checkout_email(cart)
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth if this method is not called outside CartsController. Please make it private (encapsulation)

      Edited Jul 07, 2015
      @vietth if this method is not called outside CartsController. Please make it private (encapsulation)
    Please register or sign in to reply
  • Nguyen Ngoc Lan
    @lannn started a discussion on the diff Jul 07, 2015
    app/controllers/products_controller.rb 0 → 100644
    20 end
    21 end
    22
    23 def show
    24 add_breadcrumb(@product.category.decorate.title, category_path(@product.category))
    25 add_breadcrumb(@product.title)
    26 end
    27
    28 def add_cart
    29 add_to_cart @product
    30 redirect_to @product
    31 end
    32
    33 private
    34 def set_product
    35 @product = Product.find_by(id: params[:id])
    • Nguyen Ngoc Lan @lannn commented Jul 07, 2015
      Developer

      @vietth What happen if product not found?

      @vietth What happen if product not found?
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/categories_controller.rb 0 → 100644
    1 class CategoriesController < ApplicationController
    2 before_action :set_category, only: [:show]
    3
    4 def show
    5 @products = @category.products.page(params[:page])
    6 end
    7
    8 private
    9 def set_category
    10 @category = Category.find_by(id: params[:id])
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      Fixed

      Fixed
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/helpers/application_helper.rb
    1 1 module ApplicationHelper
    2 def session_cart
    3 (session[:cart_info] ||= {items: [], total: 0}).with_indifferent_access
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      All info cart is stored in session.

      All info cart is stored in session.
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/products_controller.rb 0 → 100644
    13
    14 def create
    15 @product = Product.new(product_params)
    16 if @product.save
    17 redirect_to root_path
    18 else
    19 render :new
    20 end
    21 end
    22
    23 def show
    24 add_breadcrumb(@product.category.decorate.title, category_path(@product.category))
    25 add_breadcrumb(@product.title)
    26 end
    27
    28 def add_cart
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      It's create product, not cart_item. Cart_item use nested_attributes to create

      It's create product, not cart_item. Cart_item use nested_attributes to create
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/carts_controller.rb 0 → 100644
    1 class CartsController < ApplicationController
    2 include CartsManagement
    3
    4 before_action :authenticate_user!
    5
    6 def index
    7 @cart = Cart.new
    8 end
    9
    10 def create
    11 cart = current_user.carts.build(cart_params)
    12
    13 product_ids = []
    14 quantites = {}
    15 cart_params['cart_items_attributes'].each do |item|
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      Logic is removed to service.

      Logic is removed to service.
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/carts_controller.rb 0 → 100644
    7 @cart = Cart.new
    8 end
    9
    10 def create
    11 cart = current_user.carts.build(cart_params)
    12
    13 product_ids = []
    14 quantites = {}
    15 cart_params['cart_items_attributes'].each do |item|
    16 id = item.last['product_id']
    17 product_ids << id
    18 quantites[id] = item.last['quantity']
    19 end
    20
    21 products = Product.where(id: [product_ids]).select(:id, :price)
    22 total = products.inject(0) do |sum, product|
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      Logic is removed to service.

      Logic is removed to service.
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/carts_controller.rb 0 → 100644
    21 products = Product.where(id: [product_ids]).select(:id, :price)
    22 total = products.inject(0) do |sum, product|
    23 sum + (product.price.to_i * quantites[product.id.to_s].to_i)
    24 end
    25 cart.total = total
    26
    27 if cart.save
    28 clear_cart
    29 self.send_checkout_email(cart)
    30 redirect_to root_path
    31 else
    32 render :index
    33 end
    34 end
    35
    36 def send_checkout_email(cart)
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      Fixed

      Fixed
    Please register or sign in to reply
  • Tran Hoang Viet
    @vietth started a discussion on the diff Jul 08, 2015
    app/controllers/products_controller.rb 0 → 100644
    20 end
    21 end
    22
    23 def show
    24 add_breadcrumb(@product.category.decorate.title, category_path(@product.category))
    25 add_breadcrumb(@product.title)
    26 end
    27
    28 def add_cart
    29 add_to_cart @product
    30 redirect_to @product
    31 end
    32
    33 private
    34 def set_product
    35 @product = Product.find_by(id: params[:id])
    • Tran Hoang Viet @vietth commented Jul 08, 2015
      Master

      Fixed

      Fixed
    Please register or sign in to reply
  • Tran Hoang Viet @vietth

    Status changed to merged

    Jul 13, 2015

    Status changed to merged

    Status changed to merged
    Toggle commit list
  • Tran Hoang Viet @vietth

    mentioned in commit b07e1f6f

    Jul 13, 2015

    mentioned in commit b07e1f6f

    mentioned in commit b07e1f6ff558220dde493048a1a3368144b252c9
    Toggle commit list
  • Write
  • Preview
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
Nguyen Ngoc Lan
Assignee
Nguyen Ngoc Lan @lannn
Assign to
None
Milestone
None
Assign milestone
Time tracking
2
2 participants
Reference: vietth/VietTH-VenShop!3