Browse files

Merge branch 'master' of github.com:wicz/liskov

* 'master' of github.com:wicz/liskov:
  wicz is smart. better way to do role comparison
  removed dead code - CourseDecoratortest/unit/course_membership_test.rbhas_role?
  added person decorotor to improve readability
  using new has_role method
  using shorter form
  addressing code review items
  added integration test for a student not being able to add a task
  added lame error handling around task saving
  refactored spec
  added model tests for instructor evaluation
  revised permissions for instructors
  • Loading branch information...
2 parents 1be9073 + f10e47d commit 211f7c3076c7d793568a82fb39b73bb6c35378a1 @wicz wicz committed Mar 18, 2012
View
11 app/controllers/application_controller.rb
@@ -7,7 +7,7 @@ class ApplicationController < ActionController::Base
def current_person
begin
- @current_person ||= Clubhouse::Client::Person.new(session[:person_github_nickname])
+ @current_person ||= clubhouse_person(session[:person_github_nickname])
rescue Clubhouse::Client::PersonNotFound
end
@@ -33,4 +33,11 @@ def person_required
def login_path
Rails.env.production? ? '/auth/github' : '/auth/developer'
end
-end
+
+ private
+
+ def clubhouse_person(github_nickname)
+ PersonDecorator.new(Clubhouse::Client::Person.new(github_nickname))
+ end
+
+end
View
6 app/controllers/courses_controller.rb
@@ -1,7 +1,7 @@
class CoursesController < ApplicationController
def show
- @course = CourseDecorator.find(params[:id])
- @members = @course.course_memberships
- @tasks = @course.tasks
+ @course = CourseDecorator.find(params[:id])
+ @members = @course.course_memberships
+ @tasks = @course.tasks
end
end
View
2 app/controllers/sessions_controller.rb
@@ -3,7 +3,7 @@ class SessionsController < ApplicationController
def create
begin
- person = Clubhouse::Client::Person.new(auth_hash['info']['nickname'])
+ person = clubhouse_person(auth_hash['info']['nickname'])
rescue Clubhouse::Client::PersonNotFound
flash[:error] = "Sorry, but we couldn't find your record in Clubhouse"
end
View
20 app/controllers/tasks_controller.rb
@@ -7,19 +7,29 @@ def new
def create
task = @course.tasks.build(params[:task])
- task.save!
- redirect_to course_path(@course)
+
+ if task.save
+ redirect_to @course, flash: { success: "Task created" }
+ else
+ redirect_to new_course_task_path(@course)
+ end
end
def destroy
task = Task.find(params[:id])
- task.destroy
- redirect_to course_path(task.course)
+
+ if task.destroy
+ flash[:success] = "Task removed successfully"
+ else
+ flash[:error] = "Sorry, there was an error remove the task from the course"
+ end
+ redirect_to @course
end
private
def find_course
@course = Course.find(params[:course_id])
end
-end
+
+end
View
31 app/decorators/course_decorator.rb
@@ -1,37 +1,8 @@
class CourseDecorator < ApplicationDecorator
decorates :course
- # Accessing Helpers
- # You can access any helper via a proxy
- #
- # Normal Usage: helpers.number_to_currency(2)
- # Abbreviated : h.number_to_currency(2)
- #
- # Or, optionally enable "lazy helpers" by including this module:
- # include Draper::LazyHelpers
- # Then use the helpers with no proxy:
- # number_to_currency(2)
-
- # Defining an Interface
- # Control access to the wrapped subject's methods using one of the following:
- #
- # To allow only the listed methods (whitelist):
- # allows :method1, :method2
- #
- # To allow everything except the listed methods (blacklist):
- # denies :method1, :method2
-
- # Presentation Methods
- # Define your own instance methods, even overriding accessors
- # generated by ActiveRecord:
- #
- # def created_at
- # h.content_tag :span, time.strftime("%a %m/%d/%y"),
- # :class => 'timestamp'
- # end
-
def description
#TODO process with markdown
course.description
end
-end
+end
View
14 app/decorators/person_decorator.rb
@@ -0,0 +1,14 @@
+class PersonDecorator
+ extend Forwardable
+
+ def_delegators :@person, :name, :email, :github_nickname, :permissions
+
+ def initialize(person)
+ @person = person
+ end
+
+ def has_role?(role, course)
+ membership = course.course_memberships.for_person(@person).first
+ membership.has_role?(role) if membership
+ end
+end
View
6 app/models/course_membership.rb
@@ -7,12 +7,18 @@ class CourseMembership < ActiveRecord::Base
validates_uniqueness_of :person_github_nickname, :scope => :course_id
validate :person_permissions
+ scope :for_person, lambda { |person| where(person_github_nickname: person.github_nickname) }
+
def person
@person ||= Clubhouse::Client::Person.new(person_github_nickname)
rescue Clubhouse::Client::PersonNotFound
return nil
end
+ def has_role?(has_role)
+ has_role.to_s.capitalize == role.capitalize
+ end
+
private
def person_permissions
View
6 app/views/courses/_tasks.html.haml
@@ -1 +1,5 @@
-= render 'tasks/list'
+= render 'tasks/list'
+
+- if current_person.has_role?(:instructor, @course)
+ = link_to("Add Task", new_course_task_path(@course))
+
View
5 app/views/courses/show.html.haml
@@ -1,7 +1,4 @@
%h1= @course.name
-- if current_person.permissions["Liskov"] == "Instructor"
- = link_to("Add Task", new_course_task_path(@course))
-
= render 'participants'
-= render 'tasks'
+= render 'tasks'
View
10 test/fixtures/clubhouse/people/instructor.json
@@ -0,0 +1,10 @@
+{ "name":"Instructor",
+ "email":"instructor@gmail.com",
+ "email_hash":"6d2661c5a65f24c8f0bb579b6e0a0d81",
+ "group":"Visitor",
+ "website":null,
+ "membership_date":null,
+ "github_uid":"1234599",
+ "github_nickname":"instructor",
+ "permissions":{"Clubhouse":"Administrator","Community":"Administrator","Liskov":"Administrator"}
+}
View
10 test/fixtures/clubhouse/people/student.json
@@ -0,0 +1,10 @@
+{ "name":"Student",
+ "email":"student@gmail.com",
+ "email_hash":"6d2661c5a65f24c8f0bb579b6e0a0d81",
+ "group":"Visitor",
+ "website":null,
+ "membership_date":null,
+ "github_uid":"1234599",
+ "github_nickname":"student",
+ "permissions":{"Clubhouse":"Administrator","Community":"Administrator","Liskov":"Administrator"}
+}
View
5 test/fixtures/course_memberships.yml
@@ -0,0 +1,5 @@
+instructor:
+ course: webdev
+ person_github_nickname: instructor
+ role: Instructor
+
View
29 test/integration/tasks_test.rb
@@ -1,31 +1,22 @@
require "test_helper"
-# describe "Manage tasks integration" do
-# it "creates a new task" do
-# visit root_path
-# click_link "Wev Dev"
-# click_link "Add Task"
-# fill_in("Description", with: "Community Service")
-# click_button "Create Task"
-# page.body.must_include "Community Service"
-# end
-# end
-
class TasksTest < ActionDispatch::IntegrationTest
fixtures :all
- test "creates a new task" do
- visit root_url
- fill_in("Name", with: "instructor")
- fill_in("Email", with: "instructor@mendicantuniversity.org")
- fill_in("Nickname", with: "instructor")
- click_button "Sign In"
- assert_includes(page.body, "Welcome to Liskov")
+ test "an instructor can create a new course task" do
+ sign_in_as_instructor
click_link "Web Development"
click_link "Add Task"
fill_in("Description", with: "Community Service")
click_button "Create Task"
assert_includes(page.body, "Community Service")
end
-end
+
+ test "a student can only view course tasks" do
+ sign_in_as_student
+
+ click_link "Web Development"
+ assert_not_includes(page.body, "Add Task")
+ end
+end
View
31 test/test_helper.rb
@@ -4,16 +4,33 @@
require "capybara/rails"
+def clubhouse_person(github_nickname)
+ Clubhouse::Client::Person.new(github_nickname)
+end
+
class ActiveSupport::TestCase
- # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order.
- #
- # Note: You'll currently still have to declare fixtures explicitly in integration tests
- # -- they do not yet inherit this setting
fixtures :all
-
- # Add more helper methods to be used by all tests here...
end
class ActionDispatch::IntegrationTest
include Capybara::DSL
-end
+ teardown { Capybara.reset_sessions! }
+
+ def sign_in_as_instructor
+ sign_in clubhouse_person('instructor')
+ end
+
+ def sign_in_as_student
+ sign_in clubhouse_person('student')
+ end
+
+ def sign_in(person)
+ visit root_url
+ fill_in("Name", with: person.name)
+ fill_in("Email", with: person.email)
+ fill_in("Nickname", with: person.github_nickname)
+ click_button "Sign In"
+ assert_includes(page.body, "Welcome to Liskov")
+ end
+end
+
View
42 test/unit/course_membership_test.rb
@@ -0,0 +1,42 @@
+require "test_helper"
+require "minitest/spec"
+
+describe CourseMembership do
+
+ describe "adding a student" do
+ before do
+ @membership = CourseMembership.new(role: 'Student')
+ end
+
+ it "should not identify the student as an instructor" do
+ @membership.has_role?(:instructor).must_equal false
+ end
+
+ it "should not identify the student as an mentor" do
+ @membership.has_role?(:mentor).must_equal false
+ end
+
+ it "should identify the student as a student" do
+ @membership.has_role?(:student).must_equal true
+ end
+ end
+
+ describe "adding an instructor" do
+ before do
+ @membership = CourseMembership.new(role: 'Instructor')
+ end
+
+ it "should identify the instructor as an instructor" do
+ @membership.has_role?(:instructor).must_equal true
+ end
+
+ it "should not identify the instructor as an mentor" do
+ @membership.has_role?(:mentor).must_equal false
+ end
+
+ it "should not identify the instructor as a student" do
+ @membership.has_role?(:student).must_equal false
+ end
+ end
+end
+

0 comments on commit 211f7c3

Please sign in to comment.