CODEWITHSUNDEEP
ruby-on-rails
user3224820
user3224820

Reputation: 211

Trouble updating nested attributes

I'm trying to build a small expense tracking app using Rails 4.1. When a user submits the expense request, it's state is marked as pending by default. The admin has to approve the request. I'm using state_machine gem to do this.

Comments are added from the expense show page using a nested_form_for like this:

<%= nested_form_for (@expense) do |f| %>
      <div class="form-group">
        <%= f.label :state %><br />
        <%= f.collection_select :state, @expense.state_transitions, :event, :human_to_name, :include_blank => @expense.human_state_name, class: "form-control" %>
       </div>
       <%= f.fields_for :comments, @expense.comments.build do |comment| %>
        <div class="form-group">
          <%= comment.label :comment%>
          <%= comment.text_area :comment, class: "form-control" %>
        </div>
      <% end %>
        <%= f.submit "Submit", class: "btn btn-primary" %>
    <% end %>

The controller looks like:

class ExpensesController < ApplicationController

    def new
        @expense = Expense.new
        @item = @expense.items.build
        @comment = @expense.comments.build
    end

    def show
        @expense = Expense.find(params[:id])
        @items = Item.where(:expense_id => @expense.id)
    end

    def update
        @expense = Expense.find(params[:id])
        if @expense.update(expense_params)
            if @expense.state == "approved"
                ExpenseMailer.expense_approved(@expense).deliver
                flash[:notice] = "Expense Report Updated"
                redirect_to expenses_path
            elsif  @expense.state = "rejected"
                ExpenseMailer.expense_declined(@expense).deliver
                flash[:notice] = "Expense Report Updated"
                redirect_to expenses_path
            end
        else
            render 'edit'
        end
    end

    private
    def expense_params
        params.require(:expense).permit(:claim, :department_id, :expense_type_id, :expense_attachment, :state, :notes, items_attributes: [:id, :description, :amount, :issue_date, :_destroy], comments_attributes:[:id, :comment, :expense_id])
    end

The problem is, if I add a comment without changing the state from the dropdown, I get the 'state is invalid' error and the edit page is shown. I can get past this by simply hitting the update button. But, the comments aren't created. On the other hand, if I change the state and add a comment, comments are shown without any issue.

The params value for that is:

Started PATCH "/expenses/14" for 127.0.0.1 at 2014-08-15 13:31:40 +0530
Processing by ExpensesController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"MAEL2UYzos76NV6/eumHkXcpR2ge09wm6eOGQ+eEGCA=", "expense"=>{"state"=>"", "comments_attributes"=>{"0"=>{"comment"=>"vv"}}}, "commit"=>"Submit", "id"=>"14"}

The expense model with state machine looks like:

state_machine initial: :pending do
        state :pending
        state :approved
        state :rejected

        event :approved do
            transition [:pending, :rejected] => :approved
        end
        event :rejected do
            transition [:pending, :approved] => :rejected
        end
   end

Guess I'm making some mistake when it comes to building the comment attributes. Can someone let me know where I have to make changes?

Logger info for rejection:

Started GET "/expenses/17" for 127.0.0.1 at 2014-08-15 16:22:43 +0530
Processing by ExpensesController#show as HTML
  Parameters: {"id"=>"17"}
  [1m[35mExpense Load (0.2ms)[0m  SELECT  "expenses".* FROM "expenses"  WHERE "expenses"."id" = ? LIMIT 1  [["id", 17]]
  [1m[36mItem Load (0.1ms)[0m  [1mSELECT "items".* FROM "items"  WHERE "items"."expense_id" = ?[0m  [["expense_id", 17]]
  [1m[35mComment Load (0.2ms)[0m  SELECT "comments".* FROM "comments"  WHERE "comments"."expense_id" = ?  [["expense_id", 17]]
  Rendered expenses/show.html.erb within layouts/application (16.2ms)
Completed 200 OK in 45ms (Views: 42.8ms | ActiveRecord: 0.5ms)


Started PATCH "/expenses/17" for 127.0.0.1 at 2014-08-15 16:22:53 +0530
Processing by ExpensesController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"MAEL2UYzos76NV6/eumHkXcpR2ge09wm6eOGQ+eEGCA=", "expense"=>{"state"=>"rejected", "comments_attributes"=>{"0"=>{"comment"=>"checking logger for rejected!"}}}, "commit"=>"Submit", "id"=>"17"}
  [1m[36mExpense Load (0.2ms)[0m  [1mSELECT  "expenses".* FROM "expenses"  WHERE "expenses"."id" = ? LIMIT 1[0m  [["id", 17]]
  [1m[35m (0.1ms)[0m  begin transaction
  [1m[36mSQL (8.1ms)[0m  [1mUPDATE "expenses" SET "state" = ?, "updated_at" = ? WHERE "expenses"."id" = 17[0m  [["state", "rejected"], ["updated_at", "2014-08-15 10:52:53.030676"]]
  [1m[35mSQL (0.2ms)[0m  INSERT INTO "comments" ("comment", "created_at", "expense_id", "updated_at") VALUES (?, ?, ?, ?)  [["comment", "checking logger for rejected!"], ["created_at", "2014-08-15 10:52:53.040889"], ["expense_id", 17], ["updated_at", "2014-08-15 10:52:53.040889"]]
  [1m[36m (4.2ms)[0m  [1mcommit transaction[0m
Redirected to http://localhost:3000/expenses
Completed 302 Found in 24ms (ActiveRecord: 12.8ms)

Logger info for approval:

Started GET "/expenses/16" for 127.0.0.1 at 2014-08-15 16:22:30 +0530
Processing by ExpensesController#show as HTML
  Parameters: {"id"=>"16"}
  [1m[35mExpense Load (0.3ms)[0m  SELECT  "expenses".* FROM "expenses"  WHERE "expenses"."id" = ? LIMIT 1  [["id", 16]]
  [1m[36mItem Load (0.2ms)[0m  [1mSELECT "items".* FROM "items"  WHERE "items"."expense_id" = ?[0m  [["expense_id", 16]]
  [1m[35mComment Load (0.3ms)[0m  SELECT "comments".* FROM "comments"  WHERE "comments"."expense_id" = ?  [["expense_id", 16]]
  Rendered expenses/show.html.erb within layouts/application (167.3ms)
Completed 200 OK in 244ms (Views: 213.7ms | ActiveRecord: 1.1ms)


Started PATCH "/expenses/16" for 127.0.0.1 at 2014-08-15 16:22:41 +0530
Processing by ExpensesController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"MAEL2UYzos76NV6/eumHkXcpR2ge09wm6eOGQ+eEGCA=", "expense"=>{"state"=>"approved", "comments_attributes"=>{"0"=>{"comment"=>"checking logger!"}}}, "commit"=>"Submit", "id"=>"16"}
  [1m[36mExpense Load (0.2ms)[0m  [1mSELECT  "expenses".* FROM "expenses"  WHERE "expenses"."id" = ? LIMIT 1[0m  [["id", 16]]
  [1m[35m (0.1ms)[0m  begin transaction
  [1m[36mSQL (0.5ms)[0m  [1mUPDATE "expenses" SET "state" = ?, "updated_at" = ? WHERE "expenses"."id" = 16[0m  [["state", "approved"], ["updated_at", "2014-08-15 10:52:41.604580"]]
  [1m[35mSQL (0.5ms)[0m  INSERT INTO "comments" ("comment", "created_at", "expense_id", "updated_at") VALUES (?, ?, ?, ?)  [["comment", "checking logger!"], ["created_at", "2014-08-15 10:52:41.607555"], ["expense_id", 16], ["updated_at", "2014-08-15 10:52:41.607555"]]
  [1m[36m (4.0ms)[0m  [1mcommit transaction[0m
Redirected to http://localhost:3000/expenses
Completed 302 Found in 17ms (ActiveRecord: 5.3ms)

Upvotes: 1

Views: 527

Answers (1)

Richard Peck
Richard Peck

Reputation: 76774

I don't know why you're getting an error, but I'll give you some ideas for the state_machine / aasm gems

--

State Machines

Since Rails is object-orientated, you have to appreciate how these state machine gems work - they are an extrapolation of the electronics method of setting up a "state machine" (to predicate finite "states" within a circuit):

enter image description here

What I'm trying to demonstrate with this is that by including a state machine in your application, you're actually indicating the state of an object (it's not just another attribute)

Currently, you're treating the state attribute of your Comment model as an attribute, when it can be treated as an object in itself

--

Object

Notice this functionality from the State Machine repo:

enter image description here

Notice how that has nothing to do with the state attribute?

I think you'd be better treating the state method as what it is - a way to influence the state_machine itself. I would do that in several ways:

  1. Set the state "default" state in the state_machine declaration
  2. Validate the state object using OOP principles
#app/models/expense.rb
Class Expense < ActiveRecord::Base
   state_machine :state, :initial => :pending do #-> sets the state to "pending" unless specified otherwise
   end
end

#app/controllers/expenses_controller.rb
Class ExpensesController < ApplicationController
   def update
      if @expense.approved?
          ...
      end
   end
end

--

Fix

In regards to your being unable to create a comment, I think the problem will be two-fold

Firstly, you're building your comments within the view. Apart from anything, this is bad practice (against MVC) - you'll be best building the associated objects inside your model:

#app/models/expense.rb
Class Expense < ActiveRecord::Base
   def self.build id
      expense = (id.present?) self.find id : self.new
      expense.comments.build
      return expense
   end
end

This allows you to perform the following:

#app/controllers/expenses_controller.rb
Class ExpensesController < ApplicationController
   def new
      @expense = Expense.build
   end

   def edit
      @expense = Expense.build params[:id]
   end
end

This will basically give your nested comments form the pre-built nested objects required to fire the form for the edit & new methods (so you don't need to call @expense.comments.build in your view)

In regards to the non-saving functionality - I would certainly look at how you're saving the state attribute. I suspect it will be down to you not passing the attribute correctly (IE that you're using an incorrect value for the state param upon default submission)

I would recommend using the following:

  1. Investigate your params from the "default" update
  2. Does the state attribute match your model definition of the attributes?
  3. If it does not, that will be your problem

--

UPDATE

Thanks for the update

Okay, so the problem seems to be that the state value is not being passed if it's default. I think the way to fix this will be to set a default value for the collection_select:

  • Remove :include_blank => @expense.human_state_name
  • Replace with <%= f.collection_select :state, @expense.state_transitions, :event, :human_to_name, { selected: @expense.human_state_name}, class: "form-control" %>

Update 2

Since state_machine gives you the ability to track & fire instance methods after a successful transition, you may wish to do the following:

#app/models/expense.rb
Class Expense < ActiveRecord::Base
    state_machine :state, :initial => :pending do
        state :pending
        state :approved
        state :rejected

        event :approve do
            transition [:pending, :rejected] => :approved
        end
        event :reject do
            transition [:pending, :approved] => :rejected
        end

        after_transition :on => :approved, :do => :send_approval_email
        after_transition :on => :rejected, :do => :send_rejection_email

        def send_approval_email 
           ExpenseMailer.expense_approved(self).deliver #-> might need to call outide of state_machine block
        end

        def send_rejection_email 
          ExpenseMailer.expense_declined(self).deliver
        end

    end
end

This will give you the ability to perform the following:

#app/controllers/expenses_controller.rb
Class ExpensesController < ApplicationController
   def update
        @expense = Expense.find params[:id]
        if @expense.update(expense_params)
            flash[:notice] = "Expense Report Updated"
            redirect_to expenses_path
        end
   end
end

By the way, you need to change your "events" to have different names to your "states". As per my object-oriented references above, you need to be able to call the likes of @object.approve etc

Upvotes: 1

Related Questions