Saving a recommended web article from form in Rails controller

Question

I need criticisms and refactoring advice for my controller code:

class ItemsController < ApplicationController
 def new
 end
 def create
 params = items_params
 if not titles_and_urls_are_same_length(params)
 render :new
 end
 collection = false
 if params[:titles].length > 1
 collection = true
 end
 element = nil
 if collection
 collection = RecommendedCollection.new
 params[:titles].each_with_index do |title, index|
 resource = RecommendedResource.new(title: title, url: params[:urls][index])
 collection.resources << resource
 end
 if not collection.save
 return render :new
 end
 else
 resource = RecommendedResource.new(title: params[:titles][0], url: params[:urls][0])
 if not resource.save
 return render :new
 end
 end
 if not RecommendedItem.create(element: element)
 return render :new
 end
 render :new
 end
 private
 def titles_and_urls_are_same_length(params)
 params[:titles].length == params[:urls].length
 end
 def items_params
 params.permit(:description, titles: [], urls: [])
 end
end

Parameter example:

Parameters: {"utf8"=>"✓", "authenticity_token"=>"......", "titles"=>["Learning to program in ruby", "Refactoring Code blog"], "urls"=>["http://www.hi.com", "http://www.ruby-master.com"], "description"=>"Improve your ruby skills", "commit"=>"Recommend"}

The idea behind the controller is that the user submits a link along with a URL and description to a site they like. If you submit a single link, then it is just a RecommendedResource, if you submit multiple links then it is to be turned into a "collection" or RecommendedCollection. That is why I check for the length of the params (which is probably not a good way to write the code).

They all go into a RecommendedItem to show in the feed, so I loop through one model only.

Here is an image that explains the visual results:

enter image description here

• Jamal: Your original title was actually more ideal. We prefer such titles, not ones that specify a review request.

Answer 1

Your controller is very creative but its got some pretty big problems. In Rails controllers are notoriously hard to test and can quickly become very buggy if they contain too much logic. Skinny controllers are a vital part of good MVC code.

Any time your controller action has more than two possible code branches than you should stop and rethink it.

The Rails way of approaching this problem is nested routes and nested parameters.

Lets start out with the nested routes:

resources :collections, shallow: true do
 resources :recommendations 
end

This would give us routes like:

 Prefix Verb URI Pattern Controller#Action
 collection_recommendations GET /collections/:collection_id/recommendations(.:format) recommendations#index
 POST /collections/:collection_id/recommendations(.:format) recommendations#create
new_collection_recommendation GET /collections/:collection_id/recommendations/new(.:format) recommendations#new
 edit_recommendation GET /recommendations/:id/edit(.:format) recommendations#edit
 recommendation GET /recommendations/:id(.:format) recommendations#show
 PATCH /recommendations/:id(.:format) recommendations#update
 PUT /recommendations/:id(.:format) recommendations#update
 DELETE /recommendations/:id(.:format) recommendations#destroy
 collections GET /collections(.:format) collections#index
 POST /collections(.:format) collections#create
 new_collection GET /collections/new(.:format) collections#new
 edit_collection GET /collections/:id/edit(.:format) collections#edit
 collection GET /collections/:id(.:format) collections#show
 PATCH /collections/:id(.:format) collections#update
 PUT /collections/:id(.:format) collections#update
 DELETE /collections/:id(.:format) collections#destroy

So the user would create a brand new collection from /collections/new - and POST to /collections.

but we want the user to be able to create a recommendation on the fly as well. So lets make so that we can pass recommendations into the collection.

So what we do is add accepts_nested_attributes_for to RecommendedCollection:

class RecommendedCollection < ActiveRecord::Base
 has_many :recommendations, class_name: 'RecommendedResource'
 accepts_nested_attributes_for :recommendations, reject_if: :all_blank?
end

So from our Collection controller we can create both the collection and the recommendation

class CollectionController < ApplicationController
 def new
 @collection = RecommendedCollection.new
 @collection.recommendations.build # create an empty recommendation for the form.
 end
 def create
 @collection = RecommendedCollection.create(collection_parameters)
 if @collection.persisted?
 redirect_to @collection
 else
 render :new, notice: 'Collection could not be created.'
 end
 end
 def collection_parameters
 params.require(:collection).allow(recommendation_attributes: [:title, :url])
 end
end 

So now if we do POST /collection collection: { recommendation_attributes: ['Test', 'http://www.example.com'] } we create a collection and a recommendation.

The form would look something like this:

<%= form_for(@ollection) do |f| %>
 <%= f.fields_for(:recommendations) do |nested_fields| %>
 <%= nested_fields.text_field :url %> 
 <%= nested_fields.text_field :title %> 
 <% end %>
<%= end %>

Later when other users or the same user wants to add more recommendations to a collection they would post to /collections/1/recommendations.

• Wenqin Ye: This is exactly what I was looking for thank you! But one question, is Collection a new class or is it a RecommendedCollection?
• papirtiger: Its a RecommendedCollection, just a misstake on my part.