Resources for the TIY-Durham Rails course.
Here’s a test that was blowing up inexplicably:
class GenresControllerTest < ActionController::TestCase test “should get show” do get :show assert response.body =~ /28/ refute response.body =~ /16/ end end
The problem was that show actions need a parameter. Specifically, an id, or at least something the code can use to look up the right genre (like a name). This is more likely to work:
class GenresControllerTest < ActionController::TestCase
test "should get show" do
get :show, id: 28
assert response.body =~ /28/
refute response.body =~ /16/
end
end
Check out this code:
def get_trails
(0..@data["places"].length-1).each do |x|
@trails << Trail.new(@data["places"][x])
end
@trails
end
That code digs into the indices and loops over places in the @data array. That’s log of juggling, and there’s no need! This is better:
def get_trails
@trails = @data["places"].map {|d| Trail.new(d)}
end
Take a look at these tests, and enjoy the associated comments:
def test_display_repository_name
#is this cheating???? or still a good test
repo = GithubApi.new("masonfmatthews")
assert_equal "rails_assignments", repo.get_repository[0]
end
def test_desplay_repo_last_update
#is this cheating???? or still a good test
update = GithubApi.new("masonfmatthews")
assert_equal "2015-05-11T16:10:43Z", update.last_update[0]
end
First off, this is not cheating. The tests are good. In terms of object design, though, it would be nicer if there was a Repository model as well. We’d set it up so that the GithubApi instances each had an array of repositories inside them. We could then loop over repositories, rather than looping over arrays of strings by their indices.
The only thing we really HAVE to change about this is the name of the get_repository and last_update methods. They’re returning many things rather than one thing, so the names of the methods should be plural.
Here’s a Rails form, but one that doesn’t trust Rails to do its job:
<%= form_tag("repositories/show", method: "post") do %>
<label for="name">username:</label>
<%= text_field_tag :name, nil, placeholder: "username" %>
<%= submit_tag "SEARCH" %>
<% end %>
This does work, but it has taken a lot more developer effort than it needs to. Since we have path helpers (and since forms are by default POSTs), the first line can get simpler. Also, the label can be generated by a helper like the text_field tag was. Observe:
<%= form_tag(repositories_show_path) do %>
<%= label_tag :name, "Username" %>
<%= text_field_tag :name, nil, placeholder: "Username" %>
<%= submit_tag "SEARCH" %>
<% end %>
Voila!
Here’s a test(?):
def test_repository_list
puts @github.repositories_list
end
Although the method name starts with _test, it’s not really a test. No asserts or refutes are present. I’m guessing this was a “whoops, I ran out of time” scenario, which is understandable. If you are in that situation, though, delete things like this and commit in a clean state!
Here is a fat action in a controller, and as we know fat here is something best avoided:
def show
user = UserApi.new(params[:username])
@username = user.username
@real_name = user.real_name
@user_id = user.user_id
@company = user.company
@location = user.location
@email = user.email
@join_date = user.join_date
@followers = user.number_of_followers
@following = user.number_following
@starred = user.number_starred
@repo_name = user.repo_name
@repo_description = user.repo_description
@repo_language = user.repo_language
@stargazers_count = user.repo_stargazers_count
@forks_count = user.repo_forks_count
end
Okay, gotta admit, this makes the view a little thinner. We can say <%= @email %>, which is about as succinct as it gets. However, it’s best to make the WHOLE object be your instance variable like this:
def show
@user = UserApi.new(params[:username])
end
The controller is WAY nicer, and in your view, you just have to say <%= @user.email %>. A little more, sure, but this is a perfectly acceptable thing to have in your view.
Take a look at this form in a Rails view:
<%= form_tag github_profiles_show_path do %>
<%= "Enter a GitHub username: " %>
<%= text_field_tag :name %>
<%= submit_tag :"View GitHub profile" %>
<% end %>
The first thing to note: no tabs!
<%= form_tag github_profiles_show_path do %>
<%= "Enter a GitHub username: " %>
<%= text_field_tag :name %>
<%= submit_tag :"View GitHub profile" %>
<% end %>
Okay. Now that we’re all breathing again, look at the second line. It’s a pure string in ERB, which is not necessary. We can do this instead:
<%= form_tag github_profiles_show_path do %>
Enter a GitHub username:
<%= text_field_tag :name %>
<%= submit_tag :"View GitHub profile" %>
<% end %>
Nice. Last thing. Look at the fourth line. I think there’s an accidental : there. It just so happens to work because it’s making a symbol out of a string, and submit_tag is okay taking a symbol as its first parameter. Here is the better way:
<%= form_tag github_profiles_show_path do %>
Enter a GitHub username:
<%= text_field_tag :name %>
<%= submit_tag "View GitHub profile" %>
<% end %>
In our code, it’s best to keep our data and logic in our models, and to keep that separate from the presentation of that data (which should happen in the views). Take a look at these two tests:
test "negative balance" do
Mony.create!(amount: -800.0, other_party: "Paycheck", date_of_transaction: 2015-05-30)
assert_equal "You have a negative account balance. Eat ramen for a while.",
Mony.negative_balance
end
test "spent" do
assert_equal "$50.0", Mony.spent
end
The first test has a class method on Mony (let’s pause and acknowledge how awesome this name is) which returns a string which is a message to the user. It would best for the model to have a Mony.negative? method which returns true or false, and which then gets called by the view. We want the message to be in the HTML view. After all, if we are creating an API, do we really want strings like that?
The same thing goes for the second test. Let Money.spent return a number, and then let the view give it a dollar sign and make it a string.
This routes.rb file adds a dashboard action to the monies controller. This is a fine way to go about it:
resources :monies
root 'monies#dashboard'
Having EXTRA actions does not prevent a controller from being RESTful. You might say that you’ve violated rest if you REMOVE actions that are dictated by REST.
An alternative (which we’ll talk about later) is this:
resources :monies do
collection do
get :dashboard
end
end
More later.
Almost everyone wrote a method like this to find all the transactions in the current month:
def self.current_month
self.all.select {|e| e.created_at.month == Time.now.month}
end
This is a REALLY good thing to talk about. The tests pass, but there’s a bug. And it’s a bug that you won’t detect until ONE WHOLE YEAR has passed. The problem is that this method returns all transactions from ANY month of June, not just June of this year. This would have been detected if test data (in fixtures) had spanned more than one year, but it’s easy to miss if you just make a bunch of transactions for this year. Of course, you only know to do that if you happen to think of it, which is a non-trivial task.
Here’s a better way to accomplish the same thing (and it uses some cool Rails methods):
def self.current_month
start = Time.now.beginning_of_month
stop = Time.now.end_of_month
self.all.select {|e| e.created_at >= start && e.created_at <= stop}
end
.sum method on ArrayCheck this out:
def self.current_debits
total = 0
self.current_month.each {|e| total += e.debits.to_f}
total
end
As you may know, we could convert this to a .reduce. Of course, .reduce is the hardest of all the Enumerable methods, as it passes two parameters to its block, yada yada yada. Rails, however, gives us the sum method!
def self.current_debits
self.current_month.sum {|e| e.debits}
end
W00t!
Here is a test with an awesome name:
test "get date to work please :-)" do
petsmart = Transaction.create(name: "PetSmart", negotiation: -200)
starbucks = Transaction.create(name: "Starbucks", negotiation: -100)
paycheck = Transaction.create(name: "Job", negotiation: 50)
assert_equal Time.now.month, petsmart.created_at.month
end
Name aside, if you look at it closely, you’ll realize a few things. First, only one of these Transactions is ever used. Let’s clear those out:
test "get date to work please :-)" do
petsmart = Transaction.create(name: "PetSmart", negotiation: -200)
assert_equal Time.now.month, petsmart.created_at.month
end
But, if you look at the create line, you’ll realize that Rails is setting the created_at value to be Time.now on its own. Which means that the assert_equal is just testing that the month hasn’t changed in the time between execution of the first line and execution of the second line. Not too helpful overall.
(Honestly, though, I have to admit that I’d be utterly confused if I happened to run this right at midnight on the last night of the month. “Wait… how is THIS test failing?!?!?!”)