search

mendicant-original / puzzlenode

Quiz application inspired by Project Euler and the Internet Problem Solving Contest (IPSC)
puzzlenode.com
81 stars 43 forks source link

Puzzlenode: Published flag for puzzles #36

Closed gjp closed 13 years ago

gjp commented 13 years ago

Please review. I'm still kind of new to this workflow, so I figured I'd start small and ask for feedback, the pickier the better.

Tested against Ruby 1.9.3p0, Rails 3.1, Postgres 8.4.

One more thing - I found rake db:test:prepare necessary on a fresh clone. Should this be mentioned in the README, or did I miss a step?

Thanks!

jordanbyron commented 13 years ago

Just a few minor changes and I think we'll be ready to merge:

These lines in the migration can be simplified:

Puzzle.where("released_on <= ?", Date.today).update_all(:published => true)

Puzzle.published no longer needs to take into account the released_on date. So it can be re-written to:

def self.published(user=nil)
  if user && (user.draft_access || user.admin)
    self
  else
    where(:published => true)
  end
end

Hope that helps. Thanks for working on this!

gjp commented 13 years ago

Cool, I didn't know about those shortcuts. Need more experience with Rails 3.

Just to confirm: You wish to allow puzzles with a future release date to be visible if published = true? In that case I should also remove or reverse one of the integration tests.

jordanbyron commented 13 years ago

Just to confirm: You wish to allow puzzles with a future release date to be visible if published = true? In that case I should also remove or reverse one of the integration tests.

Ah yes I meant to mention that. Update the tests as well. Thanks!

gjp commented 13 years ago

Updated with suggested changes.

jordanbyron commented 13 years ago

Thanks for working on this Gregory!