Before_save sprawdź stan magazynu

Zielona szkoła
1

Witam!
Chciałbym sprawdzić stan magazynu zanim zapiszę sprzedaż żeby zapobiec ujemnej wartości.
Mam trzy kontrolery:

Stocks - gdzie znajdują się nazwa produktu jego ilość.
Buys - zakupy produktu po zapisie zwiększa ilość w Stocks
Sells - sprzedaż produktu po zapisie zmniejsza ilość w Stocks

W jaki sposób zabezpieczyć przed sprzedażą większej ilości produktu niż aktualnie jest dostępny? A raczej w jaki sposób sprawdzić dostępność produktu zanim zapis się dokona?

class Sell < ActiveRecord::Base
belongs_to :stock
after_save :calculate_selled_stock
before_save :check_amount

 private
    def calculate_selled_stock
         Stock.find(stock_id)
         stock.decrement! :amount, amount
         stock.increment! :value, totalprice
    end
    def check_amount
    a = Stock.find(stock_id)
    if (a.amount < 1)
    self.errors.add(:flash, "It is not enough")
    end
    
    
    end
    
end
2

Wydaje mi się że fragement:

  def check_amount
    a = Stock.find(stock_id)
    if (a.amount < 1)
    self.errors.add(:flash, "It is not enough")
  end

Niekoniecznie będzie Ci działał. Przede wszystkim nikt nie będzie w stanie zakupić 1 sztuki, do tego jeśli ktoś będzie chciał zakupic 10 itemów a w magazynie będzie pokazanych 5 to i tak warunek będzie spełniony dla Twojej metody.

Dodaj metode do klasy Stock która sprawdzi czy rozmiar zamówienia nie przewyższa stanu magazynowego.

class Stock
   def amount_available?(amount)
     self.amount >= amount
   end
end

Dodaj sprawdzenie w kontrolerze dla sprzedazy przed zapisem. Moim zdaniem to najprostsze rozwiązanie i będzie bardziej czytelne niż before_save.

3

Dzięki za odpowiedź.
Tzn podczas akcji create trzeba dodać if i sprawdzić czy amount_available?

4

Np cos takiego:

if @stock.amount_available?(@sell.amount) && @sell.save
  redirect_to root_path
else
  render :new 
  flash[:error] = "Something went wrong"
end

Nie wiem jak masz zbudowane kontrolery wiec podaje jedynie najprostrzy przykład. Można pewnie to też zrobić jakimś before_save hookiem ale osobiście nie jestem fanem takich rozwiązań.

5

Dokumentacja mówi save i destroy zapewniają transakcje przez cały callback

zaproponowane wywołanie z kontrolera

if @stock.amount_available?(@sell.amount) ....

nie gwarantuje że inna transakcja nie zmniejszy stanu towaru pomiędzy sprawdzeniem jego stanu a save

przenieś ten kod do :before_save tam będzie bezpieczny

class Sell < ActiveRecord::Base
   .......
before_save :check_amount

private
   .......
  def check_amount
    item = Stock.find(stock_id)

    # bezpiecznik produkt może zostać usunięty inną transakcją
    return false unless item

    # kod z kontrolera
    item.amount_available?(self.amount)
  end

Jeżeli chcesz zostawić before/after w spokoju tworzysz własną metodę w Sell i na początku otwierasz transakcje jak w linku na początku

a najbezpieczniej by było zrobić migrację pilnującą w Sell amount

  • null: false
  • default: 0

i pilnować > 0 tu już trzeba się posiłkować constraintem z SQL Twojej bazy

tu pokazano jak dodać prosty constraint SQLowy pilnujący długości łańcucha

nie wiem jakiej bazy używasz więc nie przytocze gotowca

6

Witaj!
Dzięki za odpowiedź!
Wykorzystuje sqlite w tym momencie. Sprawdzę Twoje rozwiązanie.
Powiedz co myślisz o wykorzystaniu walidacji w tym przykładzie czy to ma sens? Podczas szukania rozwiązania przyszło mi to do głowy.

validate :validate_amount

  def validate_amount
a = Stock.find(stock_id)
errors.add(:amount, "Not enough in a stock") if( a.amount-self.amount) < 0

end

Constraint SQLowy pilnujący to niestety coś nowego dla mnie. Pozdrawiam!

7

http://thelazylog.com/understanding-locking-in-rails-activerecord/

Myślę, że link powyżej powinien Ci pomóc :wink:

1 Like
8

http://markdaggett.com/blog/2011/12/01/transactions-in-rails/

link stary ale jary

co robisz w validate_amount

  1. pobierash stock na podstawie ID
  2. sprawdzasz czy różnica stanu sell i stock da negatywa

jedyny błąd to ufanie parametrowi z kontrolera

  1. kontroler dostał request z parametrem stock_id
  2. kontroler znalazł rekord w Stock i wywołał Twój Sell_instance.save

pomiędzy znalezieniem Stock rekordu a wywołaniem Sell_instance.save inny proces mógł wejść w transakcję usuwania rekordu w Stock i go usunąć

Sell_instance.save zrobi

def validate_amount
  a = Stock.find(stock_id)
  #
  # stock usunięty a == nil
  #
  errors.add(:amount, "Not enough in a stock") if( a.amount-self.amount) < 0
  #
  # if z errors.add wyrzuci unexpected nil
  #
end

Kontroler zbiera dane z różnych miejsc dla response, nie ma pojęcia że:

  1. sprawdzenie stanu Stock
  2. utworzenie rekordu w Sell
  3. umniejszenie stanu stock

mają nastąpić “jednocześnie” i w razie błędu na którymkolwiek etapie bezpiecznie temat odkręcić

baza danych funkcjonuje nie tylko od COMMIT do COMMIT

transakcje dają możliwość stackować kilka operacji na różnych tabelach i bezpiecznie je cofać

kontroler wykonał jedną transakcję pytając Stock.find…

Sell_instance.save wykonał drugą transakcję…

Błąd bo pomiędzy nimi każdy inny proces może wcisnąć bazie swoją transakcję usuwającą rekord w stock

Inna sprawa to nie podoba mi się ręczne dodawanie błędów i brak walidatora w Stock

class Stock < ActiveRecord::Base
  # jeżeli ilość może być ułamkowa typu  2.5kg wyrzuć only_integer: true
  validates :amount, numericality: { only_integer: true, greater_than_or_equal: 0 }
end

z tym walidatorem możesz w Sell.before_save poprostu odjąć stan i spróbować Stock_instance.save… sam dorzuci błąd

podoba mi się rozwiązanie @Biscuit z amount_available?

Nawet jeśli to tylko demo “for science!” to właśnie teraz jest okazja na zapoznanie się z constraint / transaction / trigger. Przeczytaj cały artykuł o migracjach bo dorzucenie blokady null wartości domyślnej 0 dla amount w każdej tabeli to jest abecadło i constraint na amount > 0 odchudzi Ci walidacje w modelach

http://www.tutorialspoint.com/sqlite/sqlite_constraints.htm

constraint check z linka jest dla Ciebie

Magia railsów ActiveRecord pomaga, w zakresie w czym Cię wyręcza jest dobrze udokumentowana, niestety przysłania za dużo i SQL nie jest tak widoczny jak ruby :slight_smile:

9

Super rozpisane dziękuję. To tylko demko ale właśnie o to chodzi aby wyrobić sobie dobre nawyki i wiedzieć dokładnie jak co działa.
Pozdrawiam

10

dobrym nawykiem jest nie zaśmiecanie callbacków

Dla poszczególnych operacji utworzył bym pojedyncze metody w Sell/Buy i otwierał w nich transakcje

Jak dojdą Ci takie tematy jak autoryzacja (czy użytkownik ma uprawnienia do zapisu) albo maszyna stanowa - czy obecny “state” towaru pozwala na modyfikację stanu to nie będziesz chciał tego trzymać i analizować w 2 miejscach before_save i after_save

callbacki zostaw jako helpdesk na potrzeby debugowania / logowana operacji

masz dobry przykład bazy do ćwiczenia transakcji używasz pary Sell/Stock lub Buy/Stock i to będzie zawsze wymagało transakcji

Pozdrawiam

1 Like