Search code examples
ruby-on-railsrubyruby-on-rails-7

Rails .build is not building has_many :options


I have a Poll app with 3 models.

Poll.rb

class poll < ApplicationRecord
  validates_presence_of :user, :title
  belongs_to :user
  has_many :questions, dependent: :destroy
  has_many :options, through: :questions

  accepts_nested_attributes_for :questions
end

Question.rb

class Question < ApplicationRecord
  validates_presence_of :poll_id, :question_id, :title
  belongs_to :poll
  has_many :options
  accepts_nested_attributes_for :options, reject_if: proc { |attributes| attributes['title'].blank? }
end

Option.rb

class Option < ApplicationRecord
  validates_presence_of :question_id, :title
  belongs_to :question
  belongs_to :poll
end

I want the question form to have a field for adding options so I've added this to the question _form.

  <%= form.fields_for :option do |o| %>
    <div>
      <%= o.label "Option", style: "display: block" %>
      <%= o.text_field :title, placeholder: "Enter Option here" %>
    </div>
  <% end %>

I can now see an option block which is good. Although I wish to have 3 possbile options so in the questions_controller.rb I've added the following:

 def new
    @question = @poll.questions.build
    3.times { @question.options.build } # 3 different options 
  end

Despite this I'm only seeing one option block instead of the 3. Why is this the case and how do i fix? Additionally I'm not seeing new entries into the options postgresql table.


Full questions_controller.rb

class QuestionsController < ApplicationController
  before_action :set_question, only: %i[ show edit update destroy ]
  before_action :set_poll

  # GET /questions or /questions.json
  def index
    @questions = Question.all
  end

  # GET /questions/1 or /questions/1.json
  def show
  end

  # GET /questions/new
  def new
    # @question = Question.new
    @question = @poll.questions.build
    3.times { @question.options.build } # 5 different options 
  end
  
  # GET /questions/1/edit
  def edit
  end

  # POST /questions or /questions.json
  def create
    @question = Question.new(question_params)
    

    respond_to do |format|
      if @question.save
        format.html { redirect_to polls_question_url(@question), notice: "Question was successfully created." }
        format.json { render :show, status: :created, location: @question }
      else
        format.html { render :new, status: :unprocessable_entity }
        format.json { render json: @question.errors, status: :unprocessable_entity }
      end
    end
  end

  # PATCH/PUT /questions/1 or /questions/1.json
  def update
    respond_to do |format|
      if @question.update(question_params)
        format.html { redirect_to polls_question_url(@question), notice: "Question was successfully updated." }
        format.json { render :show, status: :ok, location: @question }
      else
        format.html { render :edit, status: :unprocessable_entity }
        format.json { render json: @question.errors, status: :unprocessable_entity }
      end
    end
  end

  # DELETE /questions/1 or /questions/1.json
  def destroy
    poll_id = Question.find_by(params[:poll_id])
    session[:return_to] ||= request.referer
    @question.destroy

    respond_to do |format|
      format.html { redirect_to session.delete(:return_to), notice: "Question was successfully destroyed." }
      format.json { head :no_content }
    end
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_question
      @question = Question.find(params[:id])
    end

    # Only allow a list of trusted parameters through.
    def question_params
      params.require(:question).permit(:poll_id, :question_type, :title, :description, :randomize_selection, :voter_abstain, { option_attributes: [:question_id, :poll_id, :party_id, :title, :description] } )
    end

    def set_poll
      @poll = poll.find_by(params[:poll_id])
    end

end

routes.rb

  resources :users do
    resources :polls
  end
  resource :polls do
    resources :questions 
  end

  resource :questions do
    resources :options
  end

Edit:


Here is my questions form partial.

_form.html.erb

<%= form_with(model: [@Poll, question] ) do |form| %>
  <% if question.errors.any? %>
    <div style="color: red">
      <h2><%= pluralize(question.errors.count, "error") %> prohibited this question from being saved:</h2>
      <ul>
        <% question.errors.each do |error| %>
          <li><%= error.full_message %></li>
        <% end %>
      </ul>
    </div>
  <% end %>

  <div>
    <%= form.hidden_field :poll_id %>
  </div>

  <div>
    <%= form.label :question_type, style: "display: block" %>
    <%= form.text_field :question_type %>
  </div>

  <div>
    <%= form.label :title, style: "display: block" %>
    <%= form.text_field :title %>
  </div>

  <div>
    <%= form.label :description, style: "display: block" %>
    <%= form.text_area :description %>
  </div>

  <div>
    <%= form.label :randomize_selection, style: "display: block" %>
    <%= form.check_box :randomize_selection %>
  </div>

  <div>
    <%= form.label :voter_abstain, style: "display: block" %>
    <%= form.check_box :voter_abstain %>
  </div>

  <div>
  <%= form.fields_for :options do |o| %>
    <div>
      <%= o.label "Option", style: "display: block" %>
      <%= o.text_field :title, placeholder: "Enter Option here" %>
    </div>
  <% end %>
  </div>

  <div>
    <%= form.submit %>
  </div>
<% end %>

Here is the poll's show where I am rendering the forms.

show.html.erb

<p style="color: green"><%= notice %></p>

<p>
  <strong>Poll Title:</strong>
  <%= @poll.title %>
  <%= render @poll %>
</p>
<div>
  <%= link_to "Edit this poll", edit_user_poll_path(@poll) %> |
  <%= link_to "Back to polls", user_polls_path %> |
  <%= link_to "Destroy this poll", user_poll_path(@poll), method: :delete %>
</div>

<% if @poll.questions.any? %>
  <hr>
  <h2>Questions:</h2>
  <%= render @poll.questions %>
  
<% end %>
<hr>


<h2>Add a new Question:</h2>
<%= render "questions/form", question: @poll.questions.build %>

Solution

  • The argument you pass to fields_for has to match the name of the assocation on the model:

    <%= form.fields_for :options do |o| %>
      <div>
        <%= o.label "Option", style: "display: block" %>
        <%= o.text_field :title, placeholder: "Enter Option here" %>
      </div>
    <% end %>
    

    Pay very careful attention to plurization in Rails. Its a huge part of getting Convention over Configuration to work for you instead of against you.

    However there are a quite a few other problems with this code.

    • Constants should always be CamelCase or UPPERCASE in Ruby - you need to change class poll to class Poll and fix all the references to the class. This isn't just a matter of style since the interpreter treats identifiers that start with an uppercase letter completely differently.
    • You're not nesting it properly. You have a nested route but you're still treating it like a non-nested resource in your controller and docstrings.
    • You're passing the parent id in your params whitelist. :poll_id and :question_id should not be whitelisted. Do not pass the parent id with a hidden input. The question id is assigned by Rails - you should not trust the user to pass it.
    • The option should not need a poll_id. Use an indirect has_one assocation to go up the tree. This could cause a edge case where a question and its options belong to different polls.

    First lets fix the models:

    class Poll < ApplicationRecord
      # belongs_to assocations are required by default
      # adding validations will just cause duplicate error messages
      validates_presence_of :title
      belongs_to :user
      has_many :questions, dependent: :destroy
      has_many :options, through: :questions
      accepts_nested_attributes_for :questions
    end
    
    class Question < ApplicationRecord
      validates_presence_of :title
      belongs_to :poll
      has_many :options
      accepts_nested_attributes_for :options, reject_if: proc { |attributes| attributes['title'].blank? }
    end
    
    class Option < ApplicationRecord
      validates_presence_of :title
      belongs_to :question
      has_one :poll, through: :question
    end
    

    Then I would recommend that you use shallow nesting

    resource :polls do
      resources :questions, shallow: true
    end
    

    This creates the questions member routes (show, edit, delete) without the /polls/:poll_id prefix while the collection routes (index, create, new) are nested.

    And that you set controller up as:

    class QuestionsController < ApplicationController
      before_action :set_question, only: %i[ show edit update destroy ]
      before_action :set_poll, only: %i[ new create index ]
    
      # GET /polls/1/questions or /polls/1/questions.json
      def index
        @questions = @poll.questions.all
      end
    
      # GET /questions/1  or /polls/1/questions/1.json
      def show
      end
    
      # GET /polls/1/questions/new
      def new
        # build is just an alias of new for legacy compatibility with Rails 2...
        # its about time that we ditch it
        @question = @poll.questions.new 
        3.times { @question.options.new } # 5 different options 
      end
      
      # GET /questions/1/edit
      def edit
      end
    
      # POST /polls/1/questions or /polls/1/questions.json
      def create
        @question = @poll.questions.new(question_params)
        respond_to do |format|
          if @question.save
            format.html { redirect_to @question, notice: "Question was successfully created." }
            format.json { render :show, status: :created, location: @question }
          else
            format.html { render :new, status: :unprocessable_entity }
            format.json { render json: @question.errors, status: :unprocessable_entity }
          end
        end
      end
    
      # PATCH/PUT /questions/1 or /questions/1.json
      def update
        respond_to do |format|
          if @question.update(question_params)
            format.html { redirect_to @question, notice: "Question was successfully updated." }
            format.json { render :show, status: :ok, location: @question }
          else
            format.html { render :edit, status: :unprocessable_entity }
            format.json { render json: @question.errors, status: :unprocessable_entity }
          end
        end
      end
    
      # DELETE /questions/1 or /questions/2.json
      def destroy
        session[:return_to] ||= request.referer 
        @question.destroy
        respond_to do |format|
          format.html { redirect_to session.delete(:return_to), notice: "Question was successfully destroyed." }
          format.json { head :no_content }
        end
      end
    
      private
        # Use callbacks to share common setup or constraints between actions.
        def set_question
          @question = Questions.find(params[:id])
        end
    
        # Only allow a list of trusted parameters through.
        def question_params
          # do not write this in a single unreadable line
          params.require(:question).permit(
            :question_type, 
            :title, 
            :description,
            :randomize_selection, 
            :voter_abstain, 
            # do not wrap hash arguments in brackets 
            # as it will break if/when the `permit` method is changed to use real keyword arguments 
            # for has_many assocations the key naming convention is also plural_attributes
            options_attributes: [
              :party_id, 
              :title, 
              :description
            ]
          )
        end
    
        def set_poll
          @poll = Poll.find_by(params[:poll_id])
        end
    end
    

    The key difference here is that you should look up the poll by the parameter in the URL for the nested routes and create the question off the poll instance (which sets poll_id).

    Added:

    You're not actually using the model you initialized in your controller. If you want to render the form from a completely different action you need to initialize the instance variable there:

    class PollsController < ApplicationController
    
      def show
        @question = @poll.questions.new
        3.times { @question.options.new } # 5 different options ???
      end
      # ...
    end
    
    <%= render "questions/form", question: @question %>
    

    And in your partial you have a sneaky little bug. Ruby is case sensitive so @poll and @Poll are actually different variables.

    irb(main):049:0> @foo = "bar"                                                                                                                                                                                 => "bar"
    irb(main):050:0> @Foo
    => nil          
    

    Since instance variables are auto-vivified you're just get an unexpected nil instead of an error. What you actually want is:

    <%= form_with(model: [@poll, question] ) do |form| %>