Re: Building a Glossary from a Legacy DB

It. WORKS!

I had to hack up my routes.rb in ways that you probably wouldnt approve of, but this looks like the perfect opportunity for me to start writing some tests for refactoring.

Sidenote: In order to solve the issue of clean URLs which match my existing links, I have utilized permalinks. In order to generate these I needed to make a column called "permalink" and then I redid my import migration with the following code added to my Terms Model (i thought terms was better read than glossaries)

def before_create
     @attributes['permalink'] = title.downcase.gsub(/\s+/, '-').gsub(/[^a-zA-Z0-9_]+/, '')
   end

def to_param
     permalink
   end


BOOM!

instant permalinks for all 4,000+ entries in my table. Of course I had more tweaking to do in my controller: Basically replacing find for find_by_permalink and :id for :permalink

@term = Term.find_by_permalink(params[:permalink])

My views also needed some tweakage

<%= link_to 'Show', :controller => 'terms', :action => 'show', :permalink => term.permalink %>

And there you have it!

Man it is so gratifying to know that I wont have to change any of the links in my existing articles. It's also a huge bonus being able to make my entries now without specifying the damn letter each time. Thanks for your coding and patience Ryan. smile

Re: Building a Glossary from a Legacy DB

This thread has really bumped up my programming skills.

Right now I am building the navigation and what was once a 66-line affair of spaghetti code in textpattern has been reduced to this:

# Controller before_filter method
  def nav
    @nav_items = ['1'] + ('A'..'Z').to_a
  end

# In my view

<% @nav_items.each do |nav| %>
<a href="/terms/<%= nav.downcase %>"><%= nav %></a>
<% end %>


That is freakin awesome!

But I am no Jedi yet. I sit here scratching my head wondering how to write a current_letter method so I can do this.

<a <%- if current_letter? %>class="active"<%- end %> href="/terms/<%= nav.downcase %>"><%= nav %></a>

The tricky part is not only figuring out what the current letter is (I imagine some kind of params hackery would do it) but also accounting for the exception of numbers, which my routes currently point to as "terms/1/"

Perhaps there is some way to read out the last bit of the URL so that whatever is in that slot after terms/ gets called. But I have no clue how.

ryanb Kenobi, you're my only hope!

Last edited by pimpmaster (2007-04-03 15:53:06)

Re: Building a Glossary from a Legacy DB

Fill the @nav_items with the URL letter instead of how you want it displayed. It will make things easier:

  def nav
    @nav_items = ['numbers'] + ('a'..'z').to_a
  end

Then you can create a helper method to handle the display of the letter:

def display_nav(letter)
  letter == "numbers" ? "0-9" : letter.upcase
end

This will return "0-9" for the number display and the uppercase letter of the letter display.

Now for generating the URL. I recommend creating a named route:

map.letter '/terms/:letter', :controller => 'glossaries', :action => 'letter'

Now you can call letter_path to generate the URL.

<% for letter in @nav_items %>
  <%= link_to display_nav(letter), letter_path(letter) %>
<% end %>

As for the "active" class, you might want to create another helper method for that:

# helper
def nav_class(letter)
  "active" if current_page? letter_path(letter)
end

# view
<%= link_to display_nav(letter), letter_path(letter), :class => nav_class(letter) %>


That current_page? method is something Rails provides.

There are still a few improvements you might want to make. I see now that making the "numbers" action in the glossary controller completely separate from the "letters" action might not have been the best way to go. This results in duplication in your routes file because you need to make a custom route for each. Instead I would merge it all under the "letter" action and have an if condition there checking for the "numbers" letter and handling that accordingly.

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

Yup, its all doing what its supposed to. And what's even better is that I understand all of the code, except for this line

letter == "numbers" ? "0-9" : letter.upcase

Whats the deal with that question mark? The colon looks weird to me as well. My guess is that its some kind of condensed conditional statement, but I could be wrong.

map.letter '/terms/:letter', :controller => 'glossaries', :action => 'letter'

The map.letter part was eye-opening. I didnt know it was that easy to make clean _paths like that. I will definitely be using this more often (in situations where REST is not already taking care of it)

As for my routes, as I mentioned in a previous post, they are doing some dirty WET things. I am in fact duping my letters and numbers routes, precisely because I wasn't sure how to combine

1. Both find methods
2. both controller actions

into one. Here is how they look now.

## CONTROLLER

def letter   
   @terms = Term.first_letter(params[:letter])
   if params[:letter] =~ /^[a-z]$/i
     render :action => "index"
   else
     redirect_to '/terms/a'
   end
end

def numbers
   @terms = Term.numbers
   render :action => "index"
end

## MODEL

def self.first_letter(letter)
  find(:all,
  :conditions => ['LEFT(title, 1) IN (?)', letter],
  :order => 'title ASC')
end

def self.numbers
  find(:all,
  :conditions => ['LEFT(title, 1) IN (?)', ['.','0','1','2','3','4','5','6','7','8','9']],
  :order => 'title ASC')
end


BTW I tried neatening that numbers find as you stated before with ['.'] + (0..9).map(:to_s), but get:

wrong number of arguments (1 for 0)

So I reverted to what was working

Re: Building a Glossary from a Legacy DB

pimpmaster wrote:

Whats the deal with that question mark? The colon looks weird to me as well. My guess is that its some kind of condensed conditional statement, but I could be wrong.

You're right. It's the same as this:

if letter == "numbers"
  "0-9"
else
  letter.upcase
end

I prefer the condensed version if the condition is extremely simple like this. But you can stick with the full version if you like that better.

pimpmaster wrote:

As for my routes, as I mentioned in a previous post, they are doing some dirty WET things. I am in fact duping my letters and numbers routes, precisely because I wasn't sure how to combine

1. Both find methods
2. both controller actions

into one.

No need for the "numbers" class method in the model. You can just pass the numbers array to the "first_letter" method:

 def numbers
   @terms = Term.first_letter(['.','0','1','2','3','4','5','6','7','8','9'])
   render :action => "index"
end

The reason you got the error before with the shorter version is because I forgot the "&" character:

Term.first_letter(['.'] + (0..9).map(&:to_s))

But you can probably just do this:

Term.first_letter(['.'] + ('0'..'9').to_a)

As for combining the two actions, this should work:

 def letter    
   if params[:letter] == "numbers"
     @terms = Term.first_letter(['.'] + ('0'..'9').to_a)
   elsif params[:letter] =~ /^[a-z]$/i
     @terms = Term.first_letter(params[:letter])
     render :action => "index"
   else
     redirect_to '/terms/a'
   end
end

You could probably use a case statement too which is a little cleaner:

case params[:letter]
when "numbers"
  @terms = Term.first_letter(['.'] + ('0'..'9').to_a)
  render :action => "index"
when /^[a-z]$/i
  @terms = Term.first_letter(params[:letter])
  render :action => "index"
else
  redirect_to '/terms/a'
end

Hmm, not sure if I like the duplication of the render statement. You can just rename the template to "letter.rhtml" now that you only have one action.

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

The Force is strong with you!

My code is not only working perfectly, but its leaner and much meaner than it was 20 minutes ago.

The reason you got the error before with the shorter version is because I forgot the "&" character:

Very interesting. You have used the infamous symbol to proc trick.. I didnt know that you could apply it to string methods like that. Will wonders never cease?

Re: Building a Glossary from a Legacy DB

There is some weird stuff happening with one of my redirects. When I edit an article, it redirects to the show action like it should. But when I try to redirect from the create action, it spits out this URL:

http://localhost:3000/terms/show?permalink=test

Granted, I am sort of breaking out of the box with permalinks here, but I dont understand why the same bit of code that works in my update action fails in my create action.   

#Controller

def update
  @term = Term.find_by_permalink(params[:permalink])

  respond_to do |format|
    if @term.update_attributes(params[:term])
      flash[:notice] = 'Term was successfully updated.'
      format.html { redirect_to :controller => 'terms', :action => 'show', :permalink => @term.permalink }
    else
      format.html { render :action => "edit" }
    end
  end
end

def create
  @term = Term.new(params[:term])

  respond_to do |format|
    if @term.save
      flash[:notice] = 'Term was successfully created.'
      format.html { redirect_to :controller => 'terms', :action => 'show', :permalink => @term.permalink }
    else
      format.html { render :action => 'new' }
    end
  end             
end

#Routes

map.connect 'terms/', :controller => 'terms', :action => 'start'
map.connect 'terms/new/', :controller => 'terms', :action => 'new'
map.connect 'terms/create/', :controller => 'terms', :action => 'create'
map.letter 'terms/:letter/', :controller => 'terms', :action => 'letter'
map.connect 'terms/:letter/:permalink/', :controller => 'terms', :action => 'show'
map.connect 'terms/edit/:letter/:permalink/', :controller => 'terms', :action => 'edit'
map.connect 'terms/update/:letter/:permalink/', :controller => 'terms', :action => 'update'
map.connect 'terms/destroy/:letter/:permalink', :controller => 'terms', :action => 'destroy'


My guess is that my routes are somehow not right. Any clue?

Re: Building a Glossary from a Legacy DB

It's because the :letter parameter isn't set. When redirecting from the update action, the :letter parameter is set in the URL so it will use that one. In the create action it doesn't know which one to use.

I recommend moving this into a method:

# in ApplicationController
def term_path(term)
  url_for(:controller => 'terms', :action => 'show', :letter => @term.title.chars.first, :permalink => @term.permalink)
end

You should do some comparison here for the "numbers" exception.

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

redirect_to term_path(@term)

Thats working great for my letters, numbers were bombing out though, so my next move was:

def term_path(term)
    if @term.title.chars.first =~ /^[a-z]$/i
      url_for(:controller => 'terms', :action => 'show', :letter => @term.title.chars.first, :permalink => @term.permalink)
    else
      url_for(:controller => 'terms', :action => 'show', :letter => 'numbers', :permalink => @term.permalink)
    end
  end

Which works perfectly... for now wink

I daresay I am getting the hang of this!

Re: Building a Glossary from a Legacy DB

For extra credit I removed the above duplication with a local variable

def term_path(term)
    if @term.title.chars.first =~ /^[a-z]$/i
      letter = @term.title.chars.first
    else
      letter = 'numbers'
    end
  url_for(:controller => 'terms', :action => 'show', :letter => letter, :permalink => @term.permalink)
  end

Last edited by pimpmaster (2007-04-04 14:35:58)

Re: Building a Glossary from a Legacy DB

Want double extra credit? Clean up the regexp comparison and move it into another method:

def term_path(term)
  url_for(:controller => 'terms', :action => 'show', :letter => letter_for_term(term), :permalink => term.permalink)
end

def letter_for_term(term)
  term.title =~ /^[a-z]/i ? term.title.chars.first : 'numbers'
end

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

Shoot, that one gets you on the Honor Roll!

Note to self: Get comfy with those one-line conditionals.

Re: Building a Glossary from a Legacy DB

I thought I was done, but after implementing my search engine, I am having some weird problems setting the :letter param

The term_path method no longer works when I am on my search results page (localhost/search/)

<% content_for :page_title do %><%= search_results %><% end %>
<% for term in @terms %>
<h3><%= link_to term.title, term_path(term) %></h3>
<p><%= term.body %></p>
<% end %>
<%= pagination_links(@pages, :params => {:query=>@query}) %>

At first I was getting metthod missing error on term_path, so i set a before_filter in application_controller and now I am getting "wrong number of arguments (0 for 1)"

Wacky!

Last edited by pimpmaster (2007-04-10 13:12:47)

Re: Building a Glossary from a Legacy DB

Update, I figured out the argument problem.. I cant just filter the method in without passing any arguments in the parenthesis.

So I am stuck with the method_missing error for term_path. I cant figure out why my controller can reroute to this method no problem, but my view code bombs out like this.

Last edited by pimpmaster (2007-04-10 13:18:23)

Re: Building a Glossary from a Legacy DB

pimpmaster wrote:

At first I was getting metthod missing error on term_path, so i set a before_filter in application_controller and now I am getting "wrong number of arguments (0 for 1)"

A before filter? What did you put in there?

To make the method available to the view, try adding this to the application controller:

helper_method :term_path, :letter_for_term

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

Of course your code worked perfectly... big surprise

ryanb wrote:

A before filter? What did you put in there?

Out of desperation I tried cramming my method(args) into the filter, which caused my app to explode into a flaming mess, of course.

I didnt know the proper way to make helpers available across the app.. can I assume that it's probably smarter for me to just put this inside Application Helper?

Last edited by pimpmaster (2007-04-10 15:05:19)

Re: Building a Glossary from a Legacy DB

Methods in ApplicationHelper aren't available to the controller, so the way I showed above is the best approach. If you want to keep the application controller clean then you can move this into another module and include that as a helper module in the ApplicationController.

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

I'm all set to deploy this puppy, but as always, the client wants "one more thing"

Pagination.

I thought it would be easy, but since I am using regex in my queries, I have no idea how to work this. The standard pagination helpers in Rails accepts find_all and find_by_sql, but due to the dynamic nature of my sections, I need to somehow work params[:letter} into the picture... otherwise the find method wont know which letter to pass to the query!

Which comes first, the chicken or the egg?

Last edited by pimpmaster (2007-05-04 09:30:08)

Re: Building a Glossary from a Legacy DB

This is one of my peeves with Rail's current pagination implementation. If you are doing the find in the model it makes it very difficult to add pagination.

But with_scope can help you.

I recommend keeping the find in the model (first_letter/numbers methods) but instead use with_scope and make them accept a block. For example:

def self.first_letter(letter, &block)
  with_scope(:find => { :conditions => ['LEFT(title, 1) IN (?)', letter], :order => 'title ASC' }, &block)
end

Basically you're passing the block off to with_scope. This will break your current implementation, what you'll have to do is pass a block to this method and call find in the block:

# in controller
Term.first_letter(params[:letter]) do
  # in here any finds will be automatically scoped to the first letter
  @terms = Term.find(:all) # fetch all finds with the first letter
end

Now what this allows you to do is add any kind of pagination here because it moves the actual find back into the controller:

Term.first_letter(params[:letter]) do
  # in here any finds will be automatically scoped to the first letter
  @term_pages, @terms = paginate :terms, ...
end

Does that make sense? I haven't tested this, but I think it will work.

Railscasts - Free Ruby on Rails Screencasts

Re: Building a Glossary from a Legacy DB

Cheers Ryan smile

I am about to give your code a whirl, but in the meantime I been researching pagination and apparently a LOT of people seem to think that Rails implementation could be a lot better.

As it stands, I am going to try and fuse your code examples with the paginating_find plugin, following along with the code provided here.

It seems to address the efficiency issues many people have with Rails default pagination.

Let me know what you think