Search code examples
ruby-on-railsruby-on-rails-3refactoringcontrollers

Ruby on rails controller code, needs refactor best way to approach for more dry?


I have a welcome wizzard that builds a user profile when first login.Problem is it is quite messy implemented but I have tried to refactor it several times and rewrite it but cannot comeup with something better then below.

In ideal world it would be all inside welcome_controller.rb but this have caused much headache so Now i rewrote the update method for profile_controller instead.

Any thoughts on how to improve this make it more dry and clean? Would love to recieve some good input on this and thoughts perhaps to move all update stuff to welcome controller instead?

WelcomeController:

class WelcomeController < ApplicationController

  before_filter :authenticate_user!
  before_filter :load_step

  layout "welcome"

  def sub_layout
   "center"
  end

  def edit
    # form updates post to edit since
    # profile is non existant yet

    params[:step] = "photos" unless params[:step]

    @photos   = Photo.where(:attachable_id => current_user.id)
    @profile  = Profile.where(:user_id => current_user.id).first
    @photo    = Photo.new

    if ["photos", "basics", "details", "test"].member?(params[:step])

      # force rendering the correct step
      case current_user.profile.step
        when 1
          render :template => "/profiles/edit/edit_photos", :layout => "welcome"
        when 2
          render :template => "/profiles/edit/edit_basics", :layout => "welcome"
        when 3
          render :template => "/profiles/edit/edit_details", :layout => "welcome"
        when 4
          render :template => "/profiles/edit/edit_test", :layout => "welcome"
      end

    else
      render :action => "/profiles/edit/edit_photos"
    end
  end

  def load_step

    redirect_to root_path if current_user.profile.complete

    case current_user.profile.step
      when 1
        redirect_to "/welcome" unless params[:controller] == "welcome"
      when 2
        redirect_to "/welcome/basics" unless params[:controller] == "welcome" && params[:action] == "edit" && params[:step] == "basics"
      when 3
        redirect_to "/welcome/details" unless params[:controller] == "welcome" && params[:action] == "edit" && params[:step] == "details"
      when 4
        redirect_to "/welcome/test" unless params[:controller] == "welcome" && params[:action] == "edit" && params[:step] == "test"
    end
  end

end

ProfileController:

class ProfileController < ApplicationController

...
 def update
    @profile        = Profile.find(params[:id])
    @tags           = Session.tag_counts_on(:tags)
    @profile.form   = params[:form]
    @match          = Match.where(:user_id => current_user.id).first
    authorize! :update, @profile

    respond_to do |format|

      if @profile.update_attributes(params[:profile])

        if current_user.profile.complete
          format.html { redirect_to "/profiles/#{ current_user.username }/edit/#{ @profile.form }", notice: t('notice.saved') }
        else
          case current_user.profile.step
            when 1
              current_user.profile.update_attributes(:step => 2)
              format.html { redirect_to "/welcome/basics", notice: t('notice.saved') }
            when 2
              current_user.profile.update_attributes(:step => 3)
              format.html { redirect_to "/welcome/details", notice: t('notice.saved') }
            when 3
              current_user.profile.update_attributes(:step => 4)
              format.html { redirect_to "/welcome/test", notice: t('notice.saved') }
          end
        end

      else

        if current_user.profile.complete
          format.html { render action: "/edit/edit_" + params[:profile][:form], :what => @profile.form }
        else
          case current_user.profile.step
            when 1
              current_user.profile.update_attributes(:step => 2)
              format.html { redirect_to "/welcome/basics", notice: t('notice.saved') }
            when 2
              current_user.profile.update_attributes(:step => 3)
              format.html { redirect_to "/welcome/details", notice: t('notice.saved') }
            when 3
              current_user.profile.update_attributes(:complete => 1)
              format.html { redirect_to root_path }
          end
        end
      end

    end

  end
  ...
  end

Views are in /profiles/edit/*


Solution

  • Wizards are notoriously difficult to get right and I've never seen an implementation that fully satisfied me. I usually go with so called "form objects" and create a restful controller for each step.

    There is an excellent (but paid) Railscast on the subject.

    The gist is this: You make an object that quacks just like a regular ActiveRecord model, by using ActiveModel.

    For instance:

    class Welcome::BasicInformation
      include ActiveModel::Validations
      include ActiveModel::Conversion
      extend ActiveModel::Naming
    
      def persisted?
        false
      end
    
      def initialize(user)
        @user = user
      end
    
      attr_reader :user
    
      delegate :some_field, :some_other_field, to: :user
    
      validates_presence_of :some_field
    
      def save(params)
        user.some_field = params[:some_field]
        user.some_other_field = params[:some_other_field]
        if valid?
          user.step = 2
          user.save
        end
      end
    
      def photo
        @photo ||= Photo.new
      end
    
      def profile
        @profile ||= user.profiles.first
      end
    
    end
    

    You'd basically create a model like this for every step.

    Then you can create controllers for each step, with a specialized ApplicationController for all the steps:

    class Welcome::ApplicationController < ::ApplicationController
    
      layout "welcome"
      before_filter :authentice_user!
    
    end
    

    And for each step:

    class Welcome::BasicInformationsControlller < Welcome::ApplicationController
    
      def new
        @step = Welcome::BasicInformation.new(current_user)
      end
    
      def create
        @step = Welcome::BasicInformation.new(current_user)
        if @step.save(params[:welcome_basic_information])
          redirect_to welcome_some_other_step_path, notice: "Yay"
        else
          render :new
        end
      end
    
    end
    

    And create a route for each step:

    namespace :welcome do
      resource :basic_information, only: [:new, :create]
      resource :some_other_step,   only: [:new, :create]
    end
    

    This only leaves some automatic redirects to do, like prohibiting users from going to steps that they're not yet allowed to visit. This might not be as important now that you're using separate URLs for each step.

    You can store information about which step to visit in the form objects:

    class Welcome::BasicInformation
      # ...
      def allowed?
        user.profile.step == 1
      end
    end
    

    And then refactor the controllers a bit:

    class Welcome::BasicInformationsController < Welcome::ApplicationController
    
      before_filter :allowed?
    
      def new
      end
    
      def create
        if step.save(params[:welcome_basic_information])
          redirect_to welcome_some_other_step_path, notice: "Yay"
        else
          render :new
        end
      end
    
      private
    
      def step
        @step ||= Welcome::BasicInformation.new(current_user)
      end
      helper_method :step
    
      def allowed?
        redirect_to previous_step_path unless step.allowed?
      end
    
    end
    

    This might not be shorter, but I do like how flexible it is, how focussed each step is, how you can do different validations on each step and so on. Each controller/model combination is very easy to follow and will be understandable for others.