tixpaul/loomio — pull request #1 by jonlemmon (2012-07-23 00:28:35+00:00)
Title: Feature/delete for admin

So, the main reason the specs were failing was because `post :archive, :id => @group.id` was failing because the logged in `@user` was not a member of `@group` and therefore didn't have permission to archive the group. Cancan in action!

But yeah, most of the specs in that controller are quite old/sloppy and aren't very good examples IMO. So I've re-written this archive spec as an example of how I tend to write tests these days. Note the following changes:

* I'm using "describe" instead of "context". Technically they do the same thing, but I like using "describe" to define features (e.g. - "archiving a group"), and "context" to define... well... context (e.g. "where group is viewable by public").
* I've created several different tests to test each behaviour of the method (testing separate functionality with separate tests is usually a good thing)
* I made a completely separate test for viewing an archived group. Note, that with the way the archived_at field is currently implemented, trying to view an archived group actually throws an error (and shows a 404 page I believe), so I've written the test to mimic that behavior. However the situation outlined in your tests (redirecting to the "private or not found" template instead of throwing an error) is definitely preferable. Implementing this functionality could probably be done a separate task/card though. (Note that, currently, if you try to view a group that doesn't exist, it doesn't actually render that "private or not found" template. It errors and gives a 404 page. So this undesirable functionality isn't specific to just archived groups).

Oh, and one very small pedantic change... I removed the trailing whitespace from `group.rb`. I'm not sure if this really matters, but I've been told it's good practice, so my text-editor is set up to automatically remove trailing whitespace.

Okay, I think that's all. =)

PS - You don't have to accept this pull request. I just made it because it's an easy way of doing code review. Let me know if you have any questions!