Ruby on Rails: check the amount of products a shop owns
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. :)
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'
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.
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
This should help you.
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...
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! :)
精彩评论