One Article has_many Images. When creating a new Article, Users can add 2 images max.
In my controller I run "build" for images only twice, but when I submit the form that has 3 image fields, it succeeds. Is there any need to run "build" at all? It seems pointless in this scenario, is there another way to better ensure only 2 images are accepted?
articles_controller.rb
def new
@article = Article.new
2.times { @article.images.build }
end
Note the "2.times" here.
def create
@article = Article.new(place_params)
@article.user = current_user
respond_to do |format|
if @review.save
params[:images][:image_file].each do |image_params|
@image = @article.images.create(image_file: image_params, user: current_user)
end
end
end
end
_form.html.erb
<%= form_with(model: article, url: create_article_path(@article), local: true) do |form| %>
<div class="field">
<%= form.label :title %>
<%= form.text_area :title %>
</div>
<%= form.fields_for :images, @image do |image| %>
<div class="field">
<%= image.label :image_file_1, "Image 1" %>
<%= photo.file_field :image_file, name: "images[image_file][]", id: :article_images_image_file_1 %>
</div>
<div class="field">
<%= image.label :image_file_2, "Image 2" %>
<%= photo.file_field :image_file, name: "images[image_file][]", id: :article_images_image_file_2 %>
</div>
<div class="field">
<%= image.label :image_file_3, "Image 3" %>
<%= photo.file_field :image_file, name: "images[image_file][]", id: :article_images_image_file_3 %>
</div>
<% end %>
<div class="actions">
<%= form.submit %>
</div>
<% end %>
SUCCESS (But why?)
In short -- Your build statement is prepping the view to have 2 child objects. But you're manually creating them, so you're rendering the build statement as useless. You don't have to do it this way, you can declare nested attributes in the model, then whitelist in the controller, then auto-add them in the view. (see code example below)
Build itself does change how many objects are instantiated, but you're overriding that.
You are also manually saving the images, which you do not have to do. There's a bit of rails magic that saves all the children for you, if you've built them properly.
app/models/article.rb
class Article < ApplicationRecord
has_many :images
validates :images, length: {maximum: 2}
accepts_nested_attributes_for :images
end
2 bits of note here. Firstly, in your validation, only allow 2 object, if you try to save a third, it will fail. Secondly, accepting the attribute in the model allows you to create safe params in the controller, thus alleviating your need to manually create. (unless of course, you really want to)
<%= form_with(model: article, url: article_path(@article), local: true) do |form| %>
<div class="field">
<%= form.label :title %>
<%= form.text_area :title %>
</div>
<%= form.fields_for :images do |image_form| %>
<div class="field">
<%= image_form.label "image_file_#{image_form.index + 1}" %>
<%= image_form.file_field :image_file %>
<%= image_form.hidden_field :user_id, value: current_user.id %>
</div>
<% end %>
<div class="actions">
<%= form.submit %>
</div>
<% end %>
The change here is a) I added the user directly to the form b.) because you are accepting attribute in the model and we'll whitelist the attribute in the controller, you don't need to pass an object to the field_for -- :images
will do just fine. And because you will say to build it twice in your controller, you'll have 2 image objects in the form. Additionally, because you wanted a label of Image 1 and Image 2, with fields_for you automatically get access to the index of the object (just like you'd have with any array) by calling object.index
.
app/models/article.rb
Your action works perfectly well, keep it just as it is.
def new
@article = Article.new
2.times { @article.images.build }
end
def article_params
params.require(:article).permit(:title, :body, images_attributes: [:id, :article_id,:user_id, :image_file])
end
Whitelisting your params altogether will save time and it's easier to read than permitting them in each controller, though you CAN do that if you need to, for instance if params are allowed in certain actions but not in others.
def create
@article = Article.new(article_params)
respond_to do |format|
if @article.save
format.html { redirect_to @article, notice: 'Article was successfully created.' }
format.json { render :show, status: :created, location: @article }
else
format.html { render :new }
format.json { render json: @article.errors, status: :unprocessable_entity }
end
end
end
This will probably look similar if not identical to the default create action in a scaffold, and this is all you need. The child image objects will not be created unless the parent can be created, so you don't have to worry about adding them in an if save.