RailsCasts Pro episodes are now free!

Learn more or hide this

Andrew Greenberg's Profile

GitHub User: wizardwerdna

Comments by Andrew Greenberg

Avatar

Excellent presentation, but its an excellent presentation of a kludge. After so many, "yes, but... no... do this.. but then that won't work the way it used to." twists, it is apparent that the problem wasn't with the pedagogy, but rather the feature itself.

Rails isn't ready for this solution, and I share with you the hope that new versions down the pike are more promising.

Avatar

And if you decide to use Twitter Benchmark, it includes a number of mixing similar to those found in Bourbon.

Avatar

it may have to do with the version of bootstrap i am running, but the padding modification Ryan suggested, while looking beautiful at wide screens, leaves an icky space on top on narrower media sizes. I used the following instead, which worked with my version:

scss
  // Landscape phones and down
  @media (max-width: 980px) { padding-top: 0px;}

  // Portrait tablet to landscape and desktop
  @media (min-width: 980px) { padding-top: 60px;}

At the moment, by the way, I am preferring the following gem, which provides scss flavored bootstrap, but open to see where the community is coming down. I confess enjoying working in a single preprocessor for all my css.

gemfile
gem 'anjlab-bootstrap-rails', '>= 2.0', :require => 'bootstrap-rails'
Avatar

A straightforward way to avoid the code smells, particularly redundant logic scattered through the code manipulating sessions hash (and perhaps later on the cookies hash) is to use a non-database-backed model (tying back to recent ActiveAttr podcast) that handles Sessions, which persists to whichever hash you find suitable to send it. Thus, your controller can look like:

ruby
def create
  @session = Session.new(params[:session]
  if @session.on(session).save
    redirect_to root_url, notice: "Logged in!"
  else
    flash.now.alert = "Email or password is invalid"
    render "new"
  end
end

and then, of course, you can use the form generators on Session.new, instead of hand-coding the, albeit simple form. Note that using simple_form_for, particularly when using something like twitter bootstrap is MUCH easier on the eyes. Finally, your controller code to check for current user simply uses Session.new.on(session).current_user, and now everything is abstracted and DRY in the fat model with skinny controllers.

Even nicer, you can now test the heck out of the Session model in isolation from the controllers, making the acceptance and integration tests a piece of cake, without any need to mess around with hacks to access controller-level code.

Just a thought.

Avatar

@Andy Stewart, correction granted. I have been looking at the problem, and it seems like -- at least for purposes of an undelete function -- it ought to be straightforward-ish, at least for certain types of associations.

Clearly, if the programmer is setting up a complicated transaction in which he is "manually" manipulating associations, she should be preparing to undo the transaction herself, either by embedding the information in the paper-trail records or so forth.

But because Rails permits associations to be manipulated using the :dependent => :foo options, a bunch of records can be deleted without the programmer under the metal. But in this case, it seems to me that it ought to be straightforward, using the Activerecord reflection capabilities to identify the tables that will contain autodeleted records, and then to set up a query on the versions of that table "pointing" back to the id of the principal deleted record to find the contemporaneous delete versions. and then to undelete those records, and so on.

I think reflection can automate this to some extent, at least for has_many and belongs_to types of associations with :dependent options. I don't need to look at harder questions for the undelete function, thankfully.

Or am I missing stuff?

Avatar

This is a beautiful thing. As more and more experts in user design are bringing web interfaces into the 21st century, its a gem that our tools are starting to catch up.

Aza Raskin (Jef's son) recently wrote on this subject, explaining its virtues in http://www.alistapart.com/articles/neveruseawarning/

and Ryan comes up with this pretty ditty just days after I started thinking about it. I got excited and started messing around it at once.

A few areas for refactoring:

1) REST. I'm not sanguine about a versions controller delivering a reversion command in modern-day rails. Though its not a meaningful difference, it feels more right to implement this as a reversion subresource of a a version resource, so that versions#reversion becomes reversion#new. Ok, I'm anal after drinking the kool-aid, but it was cherry kool-aid! So, I prefer

resources versions do
   resource reversion
end

even though the urls are completely the same.

2. MVC. I really don't like making links in controllers, particularly with all that logic there. To me, this stuff belongs in the view and/or model layers. And rails provides for communication with flash, so why not use it? I would suggest:

redirect_to url, :flash => {:notice => msg, :reversion => x.versions.etc.id}

and then build your interface in the view layer, where you can custom-tailer the presentation as you always should.

3) There is a bug. Things blow up if you are updating a product that does not already have at least one version (at least the create version). This can (and is likely) to happen when you are building an undo structure over an existing database. Many ways to address this.

I, too, lament the limitations of paper_trail in terms of losing associations for a deleted resource. I wonder if model layer CRUD is too low-level for a higher-level concept such as an undo, which is generally more transactional in nature?

Great stuff, Ryan. You tweaked my mind with an elegant and straightforward spike at a non-trivial problem. Next step, how to integrate this undo functionality more generally, ultimately for inclusion in the framework itself... Aza thinks we should, and when has he ever been wrong?