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:

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.