Ruby on Rails: check the amount of products a shop owns

Question

I'm messing around with a test/exercise project just to understand Rails better.

In my case I have three models: Shop, User and Product.

A Shop can be of three types: basic, medium, large. Basic can have 10 Products maximum, medium 50, large 100.

I'm trying to validate this kind of data, the type of Shop and check how many products it owns when creating a new product.

So far, I came up with this code (in shop.rb) but it doesn't work:

  def lol
      account = Shop.find_by_sql "SELECT account FROM shops WHERE user_id = 4 LIMIT 1"
    products = Product.count_by_sql "SELECT COUNT(*) FROM products WHERE shop_id = 13"
    if account = 1 && products >= 10
        raise "message"
    elsif   account = 2 && products >= 50
        raise "message"
    else account = 3 && products >= 100
        raise "message"
    end
end

I don't even know if the logic behind my solution is right or what. Maybe I should validate using

has_many

and its "size" method? I don't know. :)

Answer 1

This should help you.

Answer 2

At least change account = 1 to account == 1. Same goes for account = 2 and account = 3.

Other than that I would recommend you looking at Rails Guides to get a feel for using Rails.

That being said, I suggest something like this:

class Shop < ActiveRecord::Base
  has_many :products
  validates :products_within_limit

  # Instead of the 'account' column, you could make a 'max_size' column.
  # Then you can simply do:
  def products_within_limit
    if products.size > max_size
      errors.add_to_base("Shop cannot own more products than its limit")
    end
  end

  def is_basic?
    products.size >= 10 && products.size < 50 
  end

  def is_medium?
    products.size >= 50 && products.size < 100
  end

  def is_big?
    products.size >= 100
  end

  def shop_size
    if self.is_basic?
      'basic'
    elsif self.is_medium?
      'medium'
    elsif self.is_big?
      'big'
    end
  end
end

This allows you to do:

# Get shop with id = 1
shop = Shop.find(1)

# Suppose shop '1' has 18 products:
shop.is_big? # output false
shop.is_medium? # output false
shop.is_basic? # output true
shop.shop_size # output 'basic'

Answer 3

Here's one possible more Rails-y way of achieving this. This is my own flavor of making Ruby imitate the behavior of an abstract class (Shop). YMMV.

EDIT: Note that I'm replacing the 'account' variable from OP's example with inheritance using ActiveRecord's Single Table Inheritance, which uses a 'type' column to perform basically the same function, but using inheritance to express the different kinds of shops and their respective product limits. OP's original example likely violates the Liskov Substitution Principle, and STI is one way of fixing that.

EDIT: As if I wasn't being pedantic enough, technically it's not really a Liskov violation as much as an Open/Closed violation. They're all variations on the same theme. You get the idea.

class Product < ActiveRecord::Base
  belongs_to :shop
end

class Shop < ActiveRecord::Base
  has_many :products  
  belongs_to :user
  validates :products_within_limit

  def products_within_limit
    if products.count > limit
      errors.add_to_base("Shop cannot own more products than its limit")
    end
  end

  def limit
    raise "limit must be overridden by a subclass of Shop."
  end
end

class BasicShop < Shop
  def limit
    10
  end
end

class MediumShop < Shop
  def limit
    50
  end
end

class LargeShop < Shop
  def limit
    100
  end
end

shop = BasicShop.create
10.times {Product.create(:shop => shop)}
shop.reload
shop.valid? # is true
shop.products << Product.new
shop.valid? # is false

Answer 4

Hi there, it does not need to be so hard:

class Shop < ActiveRecord::Base
  has_many :products

  validate :check_nr_of_products

  def check_nr_of_products
    nr_of_products = products.size
    errors[:base] << "Basic shops can have max 10 products" if account == 1 && nr_of_products > 10
    errors[:base] << "Medium shops can have max 50 products" if account == 2 && nr_of_products > 50
    errors[:base] << "Big shops can have max 100 products" if account == 3 && nr_of_products > 100
  end        

this validation is checked every time you save. You do not need to retrieve the "account-type", assuming it is a field of the shop. Likewise, instead of writing the query to count the nr of products, use the size function that does just that.

This is a simple solution. The STI solution suggested by @Dave_Sims is valid and more object-oriented.

Answer 5

Well, first of all thanks to everyone for the big help and insightful discussion. :)

I took bits from your answers in order to assemble a solution that I can understand myself. It seems when it comes to programming I can only understand if else statements, nothing more complex. :(

What I did was:

class Shop < ActiveRecord::Base

belongs_to :user
has_many :products, :dependent => :destroy
validate :is_account                                                

def is_account
    if account == 1 && products.size < 11
    elsif account == 2 && products.size < 51
    else account == 3 && products.size < 101
    end
end

Then in products_controller.rb I put these lines:

def new
if current_user.shop.nil?
    flash[:notice] = I18n.t 'shops.create.must' #this should check is the user owns a shop, otherwise can't add a product. It seems to work, so far
    redirect_to :action => :index
elsif current_user.shop.valid?
    flash[:notice] = I18n.t 'shops.upgrade.must'
    redirect_to :action => :index
  else
    @product = Product.new
  end
end

The shop now is a type 1 and has only 9 products but whenever I click the "New Product" link I'm redirected to /products with the shops.upgrade.must message.

I don't know, it seems that

account 

in shop.rb doesn't return the correct value. That column is a int(11) type, so I guess it could only return a number, but still...

Answer 6

Again, thanks for the huge support. I ended up stealing bits from your solutions and implementing this code:

#in shop.rb
validate :is_account                                                

def is_account
    if account == 1
        limit = 10
    elsif account == 2
        limit = 50
    else account == 3
        limit = 100
    end
    errors.add(:base, "Reached maximum number of items for shop") if account == account && products.size >= limit
end

#in products_controller.rb

def new
if current_user.shop.nil?
    flash[:alert] = I18n.t 'shops.create.must'
    redirect_to :action => :index
elsif current_user.shop.invalid?
    flash[:alert] = I18n.t 'shops.upgrade.must'
    redirect_to :action => :index
else
    @product = Product.new
  end
end

It seems to work so far. Hope I didn't make any blatant mistake.

Thanks again! :)