10 | Loose Ends

With that, our application basically works as we intend it to. Well done!

Well, kind of mostly works...

Before we can honestly say we're really done with development, there are a few things we need to wrap up, things ranging from nice-to-have's to half implemented features to security problems.

10.1 | Authorize MarksController

First and foremost, let's address a blatant security concern.

Remembering back to last chapter, we successfully locked down ChaptersController from unwanted requests with a pair of before_filter's?

Well, what about MarksController?

Despite the fact that there is no way for an unactivated user to ever see a UI to create a mark in our system, anyone with a knowledge of how our system works (really, anyone with a knack exploiting web applications and a knowledge of Rails) can very easily make requests that create or fetch marks.

They could be able delete or update them too, but we don't have actions for those in this particular controller.

This may or may not be a terrible thing, largely based on who the unauthorized user is and their intent, but it is most unequivocally not wanted; only activated users should be allowed to create new marks.

Perhaps unsurprisingly, this security concern is rather easy to correct.

Then why did we wait until now to address it?

Partially because I didn't want to derail what we were doing at the time, but mostly because I wanted to show how easy it is to overlook things like this. And regardless of how dangerous to the system this oversight could be, we very easily could have come to this point and thought "we have our controllers sufficiently locked down" and that would be untrue.

More about this in the security section.

First, let's prove that any jackal off the street can create a mark using curl from the terminal.

More on curl later, but we'll be using it here to make an HTTP request to our server from the terminal. This ensures that we have no session cookie and thus cannot possibly be not logged in.

The URL we're passing in is the route to create an erratum for a chapter and then the various flags tell curl to use an HTTP POST, configure the headers to use the Content-Type for JSON, and send along some JSON POST data.

Notice the return of this curl command is some JSON that we'd expect back from render :json => @mark in our create action. ...because that's exactly where it came from.

In other words, we've successfully created a record without being logged in. Want proof?

We can even hit our index action to see that our earlier curl request worked and that the mark was, in fact, saved and associated with a chapter!

So, clearly, our MarksController is wide open to the world.

The filthy, filthy world.

To change this, let's first remove the reject_unactivated_users and reject_nonadmin_users methods from ChaptersController (app/controllers/chapters_controller.rb) and put them in ApplicationController instead.

We can see that it's using the redirect we have set up in reject_unactivated_users, just like we want it to.

Now, if we were building a public API for this application, what we'd really want to do is a head :unauthorized in this situation to return an HTTP 401 Unauthorized to the client that made the unauthorized request. The client application could legitimately have an expired session or token, or simply never had one in the first place. It would make the most sense to inform it with a 401 that it needs to authenticate first (or knock it off), which it would likely handle by prompting the user to log in and then retry the request.

Now that sounds much nicer and cleaner, but it does require some more time to implement. And ultimately, in this case, we'd be spending our own time to return a semantic HTTP code to a malicious programmer (or a very, very confused one), which just isn't worth it any which way you think about it.

So we won't bother.

Crisis averted, MarksController is now secure.

Or is it?

...I honestly think it is and am pushing it up, but be ever suspicious.

10.2 | Mark Functionality

A funny thing happened to me while I was writing that last section. As I typed out the curl command and boasted "watch how we can make a mark without being logged in", I thought through the logic I thought I had and "realized" that it wouldn't save a new mark to the database after all. The "reason" being that I thought the action would try to assign the new mark to current_user, which would be nil since there was no session, the server would puke on the exception that raised, and I'd get an HTTP 500 Internal Server Error.

I ran the curl command anyway and was surprised to find that I got JSON back. And there was a persisted erratum.

What in the world?

Well, the thing I failed to remember is that we have not yet associated the current_user with a mark that they submit. We've talked about it, sure, but we didn't even have a User model when we started bookmarks and we certainly didn't write any code to handle it.

I say all of this to show that it's easy to blur what you did with what you meant to do. This is clearly a problem here in terms of functionality, since users would be unable to do so much as make a bookmark associated with themselves, but the things we forget can also amount to security problems.

The best way to protect ourselves is to write tests or specs for things you intend to do, even if they're blank, so that they stick around as pending tests instead of fading happily into your memory as things-you-think-you've-already-done.

"Testing! Yes! You want us to write tests! We get it."

So let's finish up marks.

First we're missing something: a user_id column for our marks table.

I actually also forgot that we didn't have this column too and started coding as if it was there.

I'm really on today.

How can we be so sure it's not there?

Well an easy way to check is to look at db/schema.rb which will reflect the current state of our DB. The relevant portion reads thusly:

And to further illustrate how forgetful I am today, I'm going to show you my exact terminal output from when I actually went about doing this. I do this not just to show that screwed up yet again, but because I believe that knowing how to fix your mistakes is almost as important as knowing how to avoid making them.

So what I do here is generate a migration and migrate my database, only to realize that I kind of wanted an index on the new column, since current_user.bookmark will be using it to look up the proper mark in the DB.

With how I'm doing today, it'll be a miracle if this actually works at all when we're done, but let's press on.

We've indexed to a column before, but only implicitly by using chapter:references in an earlier generator call. Here we see that we can also indicate that we want a column to be indexed by using the form user_id:integer:index.

Now that we have our column, we're going to want to tell our model that this value can be set using a .create call, so we'll need to add user_id to attr_accessible in our model. It'll end up looking like this:

We actually could get away without this, but not with the implementation we'll see below.

And finally, we'll want to set the user_id of every new mark to current_user.id in our marks#create.

But looking at the action, it's already cleanly creating the new record at the very top with if @mark = klass.create(params[:mark]). So in order to leave this as is, let's just set the mark's user_id at the end of prepare_params (which we are already calling before each request routed to create per a before_filter).

And with that, our marks will be associated with the proper user upon creation.

10.2.1 | Errata Functionality / Mailers

If we look back at our original requirements, we'll remember that we wanted our application to send an email to the author when a new erratum was created. Let's implement this feature next.

This clearly may not be practical in production. ...but it might be.

Either way, it's definitely a good reason to look at mailers.

Though we'll be making a mailer to handle this, we won't actually be sending any email. The reason for this is that it takes more work and dependencies to get it all working properly, which is a derail and a boring one at that, when what we really want to be looking at is how to develop a mailer.

So that's what we'll do.

There will be some information on the resources page about how to do this though.

To get started creating our mailer, we'll first need to do some setup.

Given an erratum, we'll want the ability to fetch the user and chapter that it was created for. This is just a matter of adding a couple associations to Erratum:

Remember, this model already has a user_id and a chapter_id, so belongs_to uses the values from those to provide a user and a chapter method, respectively, for us to retrieve those associated records.

Next, we're going to steal a line from config/environments/test.rb that is typically used to aid in testing. Here in development we'll just be using it to view generated emails as Ruby objects (since we won't really be sending them).

Adjust your config/environments/development.rb to look like the following:

The lines at the top will already be in the file, but add the others underneath them.

We see that we get a mailer created in app/mailers/, as we'd expect, but we also get something curious: a view directory, app/views/erratum_mailer.

What is that all about?

Well, it turns out that mailers use views in much the same way that controllers do. Instance variables defined in a controller are available to their views and, similarly, instance variables defined in a mailer are available in their views.

Also, much like controller actions will render views in a corresponding directory path and filename, mailer methods do too. So here, we're going to define a send_erratum method for ErratumMailer and this method will try to render the view at app/views/erratum_mailer/send_erratum.html.erb to serve as the email body.

This method accepts one parameter erratum, because we'll soon be passing each newly created Erratum into this method.

Next, we create several instance methods for values that we'll want to use in our view.

And Lastly we call the mail method. Notice that ErratumMailer inherits from ActionMailer::Base at the top of the file, because this is where the mail method comes from. In this particular call to mail, we set the subject as well as the email sender and recipient.

We should probably store the author's email somewhere else (e.g. a global constant), but for the sake of example, we just hardcode it above.

Something that appears to be missing from the process above is the setting of the email body, but mailer's render their views in the same way that controllers do: the view name and path is inferred from the mailer and action name, then rendered automatically.

We do have to actually write some html.erb though, so let's do that now:

Be sure to use the .deliver at the end of ErratumMailer.send_erratum(Erratum.last).deliverbecause the Mail::Message would be created but not "sent" without it.

So now that we can "send" emails, let's send one each time an erratum is created.

As always, there are a number of ways we could do this, but the cleanest is, without a doubt, accomplished by using an ActiveRecord callback. Using these, we can register methods to be run during different parts of the lifecycle of an ActiveRecord model instance. Here, we'll want to register an after_create callback on our Erratum model.

notify_author is not a method that will ever need to be called publicly (i.e. outside of the class), so we scope it as private. Also, this method simply makes a call to our mailer mailer action with ErratumMailer.send_erratum(self).deliver. Notice that we are passing self into send_erratum because inside of an instance method of a model (or any Ruby class really) self is a reference to that very instance of class; in this case, the instance of the newly created record.

Then up above, we're calling after_create and passing it the symbol :notify_author to indicate that we want that method run after each record is created.

If you try creating an erratum now, you'll see the contents of the generated email printed to the rails server output.

We'll get to the unless $seeding_db shortly.

Admin Errata View

Now that we have the mailer working, let's give the author a simple means of viewing all of the errata for a chapter in context within a chapter show view.

The /chapters/:chapter_id route should work as it already does, but let's configure /chapters/:chapter_id/errata to rendered chapters/show with errata added to the page in red boxes underneath the element they refer to. Additionally, these boxes should contain the erratum's note, if any, and an email link to the user who submitted the erratum.

This could be used to thank readers for their help or to angrily reply "No! I refuse to surrender my Oxford Comma!"

...from my cold, dead hands.

To make this easier on ourselves, let's throw some errata in with our seed data. And to make that useful we're going to need some users too. Here's what we end up with:

READERS - Did you remember what seed data is? I feel like a reminder here might be helpful.

While seeding the errata, we are selecting a random user and index for each. And we're using two different methods to do this.

For the index, we're using the rand method. We give this method 51 as a ceiling and it will pseudo-randomly return a value between 0 and 50 (that is, from the range 0..50), but never 51.

The math to deal with this might seem somewhat strange, but after you do this for a while you'll get used to it.

We will have 51 elements in #chapter-body (the title + 50 <p> elements), so we want the index to randomly be for any one of them.

But since we want the index of the element, we will be working with zero-based counting (i.e. the first element has an index of zero).

And since the range of 0 to 50 is comprised of 51 numbers, this works for us. (Don't believe me? Try (0..50).count in irb.)

There's no way to change the floor of rand to anything but 0, but we can offset. For instance, if we wanted a random number between 1 and 50, we could do rand(50) + 1.

To set a random user, we use the sample method, which will return a psuedo-random member of an array.

What does "psuedo-random" mean?

It means that computers are incapable of doing anything truly random and so they use tricks instead to pretend to be random.

And finally, we see that global $seeding_db variable again. The reason we're using this is that we don't need to send erratum creation emails for the errata that we're seeding in our database here. To avoid the sending the emails, we set this global variable in db/seeds.rb and then check it in app/models/erratum.rb to decide if we should run our after_create :notify_author or not.

Be sure to note that using a global variable is almost always a kludge, and that this is no exception. This will, however, help us to avoid an error when we push our application to production and so it's good enough for now. ...as kludges always are.

Now re-run $ rake db:setup and you'll have a fresh DB with users, chapters, and errata.

Next, let's work on the admin errata view.

Well, this isn't so much of a view of its own, so much as we'll be adding on to chapters/show. Furthermore, we can really place these anywhere in the view, because we'll be moving them to where they belong with jQuery later.

So feel free to place your errata anywhere inside chapters/show, as long as they're not inside of div#chapter-body. I put mine at the very top underneath the content_for:

So all we're doing here is looping through @chapter.errata and creating a p.erratum element for each. We make use of data-index on each <p> to store the index of the element in #chapter-body to which the erratum pertains. If the erratum has a note, that is included as well, but a mailto_link_for_user is added regardless.

Except for one problem: there is no mailto_link_for_user method defined.

That's because we haven't written it yet!

...and because we didn't bother to clutter up the view with the logic for it.

So where should it go?

This is a perfect example of something that belongs in a helper, so let's define this method inside of ChaptersHelper now:

You can see that the logic to generate the contents of mailto is quite messy and is not something we'd really want to have in our view. That makes this exactly the kind of thing you'd want to put in a helper.

To understand what all is going on here, let's first look at what this method gets us. Head over to /chapters/1 in Chrome.

After you register. ...again. (The $ rake db:setup blasted all the User records.)

You'll now see some text at the top in the format ^ "Erratum note text." - Zombie #1, where Zombie #1 is a link. If you dig a little deeper with Dev Tools, you'll see that the HTML for the links looks something like this:

The mailto: in the href is used to tell the browser that when the link is clicked it should open the user's email client and create a new email, using the rest of the href in the To field for that email. In the email client, the To field for the link above should look like this:

"Zombie #1"

From looking at the <a> that this helper method creates for us, we can see that the href has been URL encoded. Without the URL encoding, we'd just end up with a "Zombie in the To field of our email, because of the following space.

Also, if we want double quotes inside the Ruby string, we're going to have to suck it up and escape them ("like \"this\""), because we also want to interpolate values (`this = 'this'; "like #{this}") and for that we need to use double quotes to define string.

To URL encode a string in a Rails view or helper, we simply use the u method. And that's exactly what we do with the string assigned to mailto:

mailto = u("mailto:\"#{ user.name }\"")

So now that we have the errata on the page, let's call them out a little bit with some red background:

First we get all the top-level elements inside of #chapter-body that will be referenced by the errata and assign them to a variable named elements.

Next we loop over all of the .erratum elements that we just dumped haphazardly into the beginning of our chapter show view.

For each of these we .remove() the element from the page and store it in a variable named erratum. We then use jQuery's .data() to get the index out of the element's data-index attribute and store it in index.

Finally, we use this index value and jQuery's .eq() to get the element the erratum is referencing and use jQuery's .after() to put the erratum back into the page immediately after it.

Pretty simple.

Simple, but there's actually a subtle part of this logic that makes things considerably simpler than they would otherwise be. We avoided a potential index pitfall, can you spot it?

By using .after() and inserting elements into #chapter-body, we're increasing the number of elements in that <div>.

"Duh", right?

Well, that's the obvious part; the subtle part is that by doing that we're screwing up the indices for all of the elements after the one we insert the erratum for, because they all now have an extra element in front of them (the erratum we just inserted).

For instance, if we have elements with indices 0 through 9 and we insert an element after 8, then 10 is now the index for the element that used to have an index of 9; it got bumped down.

The way we get away without having to worry about this is by storing $('#chapter-body').children() in elements before we start looping and inserting elements. elements is basically a snapshot of the child elements of #chapter-body before we added anything, so we can continue referring to it and doing elements.eq(index) with the certainty that the indices will remain valid.

This is a great example of something that we could have fallen into thinking that we had to "do the hard way", when the cleanest solution was easy to find with a short pause and an objective assessment of the problem.

Don't Repeat Yourself, but also Don't Work Too Hard.

Refresh the page and you'll find all of the errata where they belong.

It's kind of hard to tell if they're in the right place since we didn't, strictly speaking, create them ourselves (i.e. they're seed errata), but if you inspect the elements and check the data-index values, it's clear that they're all in the right place.

So it might seem like we're done, but remember that we wanted errata to only be visible to admin and only under the /chapters/:chapter_id/errata route.

Let's tackle these one at a time, starting with the route.

We'll need to change the block passed into resources :chapters to look like the following:

Be sure to take :index out of resources :errata and add the line with the get below it.

With the route declared with get, we're routing to chapters#show instead of to the MarksController and we're using errata: true to set params[:errata] = true (a value we'll be using in our controller later).

We're also using on: :member to specify that we want the route on a member of the resource that it's nested in, meaning that it should be a route on a particular chapter (as in /chapters/1/errata).

Let's see how this route is different than a route without on: :member as well as its opposite, on: :collection.

From this we can see that the on: :collection route is indeed "on the collection" (/chapters) and that while the other two are both "on the member", they store the ID for the chapter differently. Because we want this be routed to the chapters#show action, this ID will need to be in params[:id] and so we will use the on: :member route.

So now /chapters/123/errata will point to chapters#show with params[:errata] == true. Let's set up that controller action to do something with that boolean.

This way the errata will only be placed in the rendered view if @errata is truthy and @errata will only be truthy if it was set to be in the controller.

So now if we visit /chapters/1 we won't see any errata, but if we visit /chapter/1/errata, we will.

Now that the errata are properly placed and included, we just need to restrict access to them to admin.

Typically we'd want to use something like before_filter :reject_nonadmin_users for the show action, but in this case we can't, because regular users hit that same action to view the plain, old chapters.

This might seem like it complicates things a lot, but we can actually accomplish what we want simply by adding a conditional to the @errata line in chapters#show:

If @bookmark_index has been assigned in controller, it will be placed in the data-index attribute for div#chapter-body, otherwise the attribute will just be blank. We'll write some JavaScript in a minute to do something useful with this index, but first we need to assign @bookmark_index in chapters#show:

I used params[:bookmark] here for the bookmark's index parameter, but feel free to use :bookmark_index or whatever you like. Just be sure to use it for the rest of the code below too.

And finally, we'll need to route to chapters#show with params[:bookmark] == true somehow.

Ordinarily we'd probably just route requests for /bookmark to chapters#show for something like this, but that would leave the user with /bookmark in their URL bar and that isn't really where the user will have ever ended up. What we really want the user to land on is /chapter/:bookmarked_chapter_id with the bookmark displayed, rather than having them actually arrive at the path /bookmark.

So we don't need /bookmark to be a destination, but a redirect. We are going to need some additional logic though, so we'll just redirect from marks#show, a route that you'll recall is one that will only be used for Bookmark's.

READERS - Did I explain this well? I tried re-writing the explanation above a few times but still feel like I'm not getting my point across.

Here we just grab current_user.bookmark and then redirect to the proper chapter and pass the bookmark index along to chapters#show using bookmark: @index.

This is accomplished using in the ugliest means possible, query string parameters, because we can't rightly pass POST data without making an HTTP POST. We might be able to make the URL look better, but I think it's more worthwhile to keep moving.

With a quick $ rake routes, it's clear that we already have the route we need to get to marks#show from what we've done earlier: bookmark GET /bookmark(.:format) marks#show. So we're good to go from a routing, controller, and view perspective.

We still need some JavaScript to highlight the proper element in the page and jump to it, but more critically, our current_user.bookmark will blow up at the moment, because we haven't yet defined that association in our User model.

We've used has_many to define associations before, but never has_one. Don't worry though, because it works exactly as you would expect it to and gives us the .bookmark method we want for our User instances.

Next, we could add a belongs_to :user to Bookmark, but we don't really need that for anything. What we do need to do though is make sure that a User only ever has one and only one bookmark. Typically, has_one would handle some of this for us, because we would use current_user.bookmark = new_bookmark to associate a new bookmark and, in the process, unassociate the old bookmark. However, even this would not destroy the old bookmark, it would only set its user_id to nil and leave it to take up space in our database.

But instead of using something like current_user.bookmark = new_bookmark, we're making marks#create pull double duty using .create to make both errata and bookmarks for us. Because of this, we're not even unassigning old bookmarks from the user, we're just creating more and more of them.

So what we're going to need to do is destroy the old bookmarks ourselves. To do this we'll make use of another ActiveRecord callbacks: before_create.

We could really use Bookmark instead of self.class inside destroy_previous, since they are equivalent in this context, but self.class works just as well.

Something interesting about this code is that since we are in the model, we don't have access to current_user. This would be a problem except that we do have access to the new Bookmark being created as self, an object that will have a user_id attribute corresponding the User we want.

To avoid needlessly loading the User record, we just pass the user_id into a .where to find all bookmarks belonging to the user. And since .where returns an array-like object, we name the variable in the plural, old_bookmarks, and call .destroy_all on it to remove all of those records from the database.

Though there really should only ever be, at most, one.

Now all we need to do is highlight the bookmarked element and jump to it on the eventual chapter page load that results from hitting /bookmark.

We first get bookmark_index from #chapter-body's data-index with .data('index') and then check to make sure that it is of type 'number' using typeof. If it is, we know that we have a real bookmark index and that we should highlight it.

To do the actual highlighting, the logic is very simple; we use our elements variable from earlier (which contains $('#chapter-body').children()), calling .eq(bookmark_index) on it to get the element we want. We then call .addClass('bookmark') on the element to apply our highlighted .bookmark styles to it that we set in our CSS way, way back.

The "jump to" functionality involves a little bit more thought.

The process of setting the view to show the bookmark at the "very top" of the page can be broken down into three steps:

calculate the distance from the bookmarked element to the absolute top of the page

subtract the height of the top nav

scroll the page that distance

To get the distance we need, we'll use the .offset() method, which returns an object with two properties: left and top. Because we're concerned with the distance to the top, we use bookmark.offset().top to get this value.

We also don't want to scroll this full distance, because if we did, the bookmarked element would be placed at the absolute top of the page, which would place it behind the top nav. To avoid this, we subtract 50 from this value to keep from scrolling this entire distance.

All that's left is to do the actual scrolling. For this, we use .scrollTop and call it on the jQuery-ified version of window, $(window). When doing this, we pass in our topOffset to tell it how far to scroll.

And with that, we just need a Bookmark link in the top nav that points to /bookmark.

One interesting thing that I ran into while testing this was that an element with index of zero would not show up as bookmarked. After some fiddling, I realized that my logic was flawed, because of the way JavaScript elicits booleans out of non-boolean values.

The problem was that I had everything wrapped with if(bookmark_index){ ... }.

Doesn't seem like a problem, right?

Well, it isn't, until you expect 0 to be truthy, as I was when I had the first element bookmarked.

In JavaScript, !!0 returns false. (In Ruby, !!0 returns true.) This is just One Of Those Things you have to remember. ...or at least be aware of and test for.

To fix the problem here, I made the if check typeof instead, but could have done something else like bookmark_index >= 0 just as easily.