D41d8cd98f00b204e9800998ecf8427e

Proably it's possible to cleanup this controller by define method, but i don't know how i can do this.

class CampaignsController < ApplicationController
  before_filter :load_campaign_or_redirect, :except => [:index, :sent, :new, :queued, :create, :processing]

  def index
    @campaigns = @current_user.campaigns.drafts
  end

  def sent
    @campaigns = @current_user.campaigns.sent
  end

  def new
    @campaign = Campaign.new
  end

  def create
    @campaign = Campaign.new(params[:campaign])
    if @campaign.save
      redirect_to content_format_campaign_path(@campaign)
    else
      render :action => 'new'
    end
  end

  def edit; end

  def update
    respond_to do |format|
      if @campaign.update_attributes(params[:campaign])
        flash[:notice] = 'Campaign has been updated.'
        format.html { redirect_to(@campaign) }
        format.xml  { head :ok }
      else
        format.html { render :action => "edit" }
        format.xml  { render :xml => @campaign.errors, :status => :unprocessable_entity }
      end
    end
  end

  def content_format; end
  
  def content_format_save
    if @campaign.update_attributes(params[:campaign])
      if @campaign.format_of_campaign == 'html'
        redirect_to(html_source_campaign_path(@campaign))
      elsif @campaign.format_of_campaign == 'text'
        redirect_to(text_content_campaign_path(@campaign))
      end
    else
      render :action => 'content_format'
    end
  end

  def html_source; end
  
  def html_source_save
    if @campaign.update_attributes(params[:campaign])
      if @campaign.content_injection_method == 'internet'
        redirect_to(import_url_campaign_path(@campaign))
      elsif @campaign.content_injection_method == 'file'
        redirect_to(import_file_campaign_path(@campaign))
      end
    else
      render :action => 'html_source'
    end
  end

  def import_file; end
  
  def import_file_save
    if @campaign.update_attributes(params[:campaign])
      redirect_to(import_result_campaign_path(@campaign))
    else
      render :action => 'import_file'
    end
  end

  def import_url; end
  
  def import_url_save
    @campaign.step = :step_import_url
    if @campaign.update_attributes(params[:campaign])
      redirect_to(import_result_campaign_path(@campaign))
    else
      render :action => 'import_url'
    end
  end

  def import_result; end
  
  def import_result_save
    if @campaign.update_attributes(params[:campaign])
      render :action => 'import_result'
    else
      render :action => 'import_result'
    end
  end

  def text_content; end
  
  def text_content_save
    @campaign.step = :step_text_content
    if @campaign.update_attributes(params[:campaign])
      redirect_to accept_content_campaign_path(@campaign)
    else
      render :action => 'text_content'
    end
  end

  def accept_content; end

  def recipients_source; end

  def recipients_source_save
    if @campaign.update_attributes(params[:campaign])
      if @campaign.recipients_source == 'existing_list'
        redirect_to(select_lists_campaign_path(@campaign))
      elsif @campaign.recipients_source == 'create_new_list'
        redirect_to(type_recipients_campaign_path(@campaign))
      end
    else
      render :action => 'recipients_source'
    end
  end

  def type_recipients; end

  def type_recipients_save
    if @campaign.update_attributes(params[:campaign])
# some code for l8r
    else
      render :action => 'choose_recipients_source'
    end
  end

  def select_lists; end

  def select_lists_save
    if @campaign.update_attributes(params[:campaign])
      redirect_to(recipients_import_result_campaign_path(@campaign))
    else
      render :action => 'select_lists'
    end
  end

  def recipients_import_result; end
  
  def delivery_testing; end

  def send_test_email; end

  def schedule_delivery; end

  def schedule_delivery_save
    if @campaign.update_attributes(params[:campaign])
      redirect_to(summary_campaign_path(@campaign))
    else
      render :action => 'schedule_delivery'
    end
  end

  def summary; end

  def preview
    respond_to do |format|
      format.html { render :text => @campaign.body_html }
      format.text { render :text => @campaign.body_text }
    end
  end

  def show; end

  def processing
    if @current_user.campaigns.find(params[:id]).state != 'processing'
      redirect_to session[:return_path]
    end
  end
  
  private
  
  def load_campaign_or_redirect
    @campaign = @current_user.campaigns.find(params[:id])
    if @campaign.state == 'processing'
      session[:return_path] = request.request_uri
      redirect_to processing_campaign_path(@campaign)
    end
  end
end

Refactorings

No refactoring yet !

880cbab435f00197613c9cc2065b4f5a

danielharan

April 28, 2008, April 28, 2008 21:01, permalink

1 rating. Login to rate!

That's no longer really restful. Instead of what appears to be a wizard on the front-end, consider loading the form all at once - maybe with javascript that shows each successive part of the form, or with tabbed controls. For the RESTful actions, you could use http://jamesgolick.com/resource_controller

Cfaf074970ce958900608fceaac38543

schof

May 1, 2008, May 01, 2008 16:47, permalink

No rating. Login to rate!

I second the comments about resource_controller. It totally changed my opinion on the usefulness of REST in RoR.

Your refactoring





Format Copy from initial code

or Cancel