def self.get_order_price(id, type)
    order_id = Order.where(user_id: id).where(order_status: type).pluck(:id)[0]
    all.find(order_id).order_items.get_total_price
  end

This is better written as:

 def self.get_order_price(id, user_id, type)
    order = Order.where(user_id: user_id, order_status: type, id: id)
    order.order_items.get_total_price
  end

But it seems like this method is not being used anywhere at all. Then it should be removed. Same for the next method.

  def self.get_total_items_price(id)
    all.find(id).order_items.get_total_price
  end

This should instead be an instance method:

  def get_total_items_price
    order_items.get_total_price
  end

And where it is being invoked, it can be:

  def self.get_profit
    orders = Order.where(order_status: "completed")
    total_profit = orders.inject(0) do |sum, order|
      sum += order.get_total_items_price
      sum
    end
    total_profit
  end

See official Enumerable Ruby docs for inject - it is a very useful method, often also called reduce or fold.

The runtime behaviour of this line is ambiguous to me, and I strongly suspect it doesn't work the way we want it to work. So instead of assigning to a variable in class initialization, create a

   def current_user
     @current_user
   end

in ApplicationController. This method will then be available across all controllers and you can use it without relying on the instance variable.

This is something I picked up from Denislav Zhelyazkov (product designer).

Don't use Sign up and Sign in at the same time.

Just by reading it gets confusing, now imagine your customer in a crowded subway trying to log in to your product. Most likely they will hit the wrong one, simply because it will require them extra cognitive effort to tell the buttons apart because the labels are extremely similar.

He recommends the following alternative text pairings to reduce the cognitive load in users.

• Sign up and Log In
• Sign In and Create Account
• Log In and Get Started
• Register and Log In

It would really convenient to navigate back to home by clicking on the icon. You can do that by wrapping the navigation logo in an anchor tag.

Deepankar Code Review Feedback

• MenuItemsController allows all users to edit menu info. Ideally you should create an ensure_admin method in ApplicationController and use it as a before_action in all the places where you want to limit to only admins/ownders

• Same with MenuController#create

• There is a lot of application logic in OrderItemsController#create. See how you can move that logic into the model instead? Maybe create a method like OrderItem.create_from_cart (a self method) and pass the details into it and let it do all the work. The controller should be thin and act as just a middleman between the URL route and the Model actions. There are very important reason why this should be, we’ll explain it later in the course.

• Same with logic in OrdersController#create

• In views/user_completed_orders/index.html.erb, Order.get_users_completed_order_ids(@current_user.id) should be made into an instance variable in the controller and that should be used in the view. Otherwise there is too much logic in the view..

• in order.rb: the get_profit method should be using an ActiveRecord query, and not do it in Ruby land.

If we go thru ActiveRecord, then it will generate an SQL query which is faster to execute. If we do it in Ruby, then imagine we have 1 million entries in our order_items table. In the current approach we have to iterate over each of them - and for that we have to transport that data from the Postgres server into our Ruby process (this could be two different computers sitting in a network) - and instantiate an ActiveRecord object for each row - and then add them up. But instead if we call the SQL sum directly, then all the computation is done by the Postgres server which is very efficient at these things, and the Ruby process only gets a single summed value.

Now doing this thru ActiveRecord (and SQL underneath it) would require a “join” operation so we can get only the OrderItems that are “completed”. For this you can do something like:

orders = Order.
  where(order_status: "completed").
  joins(:order_items)
orders.sum("order_items.count * order_items.menu_item_price")

I didn’t test this btw, and you need to know SQL properly to figure it out (Google for “SQL summing values from join table”) . Let me know if this is proving tough.

• I’m not able to understand Order#check_if_in_active_menu.. (it can be simplified in code, not explained with comments)

This is a follow up to issue #9.

Write a tutorial for testing a rails application. Add it as a markdown file to this repo. It will be a useful reference for others following the same path. We will also consider it for integration back into the SaaS 201 course.

Ref removed

We're right now leaking the internal representation of roles to this controller. As a codebase grows larger this becomes problematic because things are spread out across the codebase. Instead, we should create a method in the User model that looks like:

def is_owner?
  role == "owner"
end

and so on for each role, and use those methods from across the application.

This will let you centralize the details of the internal representation of our role in the model.

For a deployed application every modification introduces an element of risk. It could introduce unexpected behaviour. An existing feature could stop working. This is difficult to notice without tests. You may only find out when a user reports the bug in a live application.

You now have a working & live implementation of cafeman. This is a great point to start adding automated tests.

Rails has an excellent guide to get you started on testing here: https://guides.rubyonrails.org/testing.html You should add the following tests:

1. model tests,
2. integration tests and,
3. functional tests for your controller.

Note: Please ignore system testing in the documentation. You do not have to add these.

Since this is a new topic, feel free to ask lots of questions and discuss it 😃

One suggestion for the variable: is_order_still_valid