87f4477dced7fcff3ac2154e88d53210

I'm new to Ruby and Rails, I'm pretty sure there's a cleaner way of doing this... Creating an account, a personal website, a user and then the content folders. I just got done reading that controllers should be skinny, but I'm unsure of what to stick where (before_filter, after_save, etc...) Thanks in advance for help.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
 require 'FileUtils'

  def create
    @account = Account.new(params[:account])
    @account.short_name = make_short_name(@account.name)
    @website = @account.websites.build(params[:website])
    @website.update_attribute(:short_name, make_short_name(@website.name))
    @user = @account.users.build(params[:user])

    if @account.save
      make_base_folders(@account.short_name, @website.short_name)
      flash[:notice] = 'Your account was successfully created. Please login to begin :)'
      redirect_to login_path
    else
      render :layout => 'signup', :action => 'new'
    end
  end

private

  def make_short_name(name)
    name.gsub(/\W+|\s+/, '-').downcase
  end
  
  def make_base_folders(account_short_name, website_short_name)
    ['docs', 'pictures', 'users'].each do |f|
      FileUtils.mkdir_p("public/content/#{account_short_name}/#{website_short_name}/#{f}")
    end
  end

Refactorings

No refactoring yet !

1e8f141e7857d397d8020ed3b759e88a

Maciej Piechotka

August 3, 2008, August 03, 2008 09:02, permalink

1 rating. Login to rate!

May be something like that?

PS. Shouldn't short-name be called permalink?

app/controller/accounts_controller.rb

1
2
3
4
5
6
7
8
9
10
11
12
13
 require 'FileUtils'

  def create
    @account = Account.create!(params[:account])
    @website = @account.websites.create!(params[:website])
    @user = @account.users.create!(params[:user])

    flash[:notice] = 'Your account was successfully created. Please login to begin :)'
    redirect_to login_path
  rescue RecordNotSaved
    render :layout => 'signup', :action => 'new'
  end

In models (each)

1
2
3
4
5
6
  attr_protected :short_name
  before_create :set_short_name
  
  def set_short_name
    self.short_name ||= self.name.gsub(/\W+|\s+/, '-').downcase
  end

In account model

1
2
3
4
5
  after_create :create_dir
    
  def create_dir
    FileUtils.mkdir_p("public/content/#{self.short_name}")
  end

In website model

1
2
3
4
5
6
7
8
9
  belongs_to :account

  after_create :create_dir
    
  def create_dir
    %w{docs pictures users}.each do |d|
      FileUtils.mkdir_p("public/content/#{self.account.short_name}/#{self.short_name}/#{d}")
    end
  end
Be1e3ee645d23c95ba650c21bc885927

Fabien Jakimowicz

August 3, 2008, August 03, 2008 12:48, permalink

1 rating. Login to rate!

You should move a lot of logic down to models.

In this code, you should add some checks on folder creation and folder deletion on website/account destroy. You can also reduce controller code by nesting forms (user and website will be in account params hash) and using an after_create on account model to create user and website. But this could be overkill ;)

Account model

1
2
3
4
5
6
7
8
9
10
11
12
13
class Account < ActiveRecord::Base
  has_many :websites
  has_many :users

  validates_presence_of :name

  before_create :make_short_name

  private
  def make_short_name
    self.short_name = name.gsub(/\W+|\s+/, '-').downcase
  end
end

Website model

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
class Website < ActiveRecord::Base
  belongs_to :account

  validates_presence_of :name
  validates_associated :account # we rely on account short_name, we must check it
  validates_uniqueness_of :short_name, :scope => :account_id # You could have some surprises with your folders if you don't check it

  before_create :make_short_name
  after_create :make_base_folders

  Folders = %w{docs pictures users}

  private
  def make_short_name
    self.short_name = name.gsub(/\W+|\s+/, '-').downcase
  end

  def make_base_folders
    Folders.each {|folder_name| FileUtils.mkdir_p "#{RAILS_ROOT}/public/content/#{account.short_name}/#{short_name}/#{folder_name}"}
  end
end

User model

1
2
3
4
5
class User < ActiveRecord::Base
  belongs_to :account

  validates_associated :account
end

Controller

1
2
3
4
5
6
7
8
9
def create
  @account = Account.create!(params[:account])
  @user = @account.users.create!(params[:user])
  @website = @account.websites.create!(params[:website])
  flash[:notice] = 'Your account was successfully created. Please login to begin :)'
  redirect_to login_path
rescue
  render :layout => 'signup', :action => 'new'
end
87f4477dced7fcff3ac2154e88d53210

vineire

August 7, 2008, August 07, 2008 17:37, permalink

No rating. Login to rate!

Thank you for the time spent working on my fat controller. I've learned a lot by dissecting and looking at each of these examples...

Your refactoring





Format Copy from initial code

or Cancel