Skip to content

Commit

Permalink
Merge branch 'master' of github.com:wicz/liskov
Browse files Browse the repository at this point in the history
* 'master' of github.com:wicz/liskov:
  naive passing student page test
  DRY'd up how PersonDecorator are created and added filter chain in students controller
  fixed test broken by new task factories
  • Loading branch information
wicz committed Mar 24, 2012
2 parents c4620b5 + fcdbeb8 commit 852bbdb
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 10 deletions.
4 changes: 2 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class ApplicationController < ActionController::Base

before_filter :person_required

helper_method :current_person, :signed_in?, :login_path
helper_method :current_person, :signed_in?, :login_path, :clubhouse_person

def current_person
begin
Expand Down Expand Up @@ -37,7 +37,7 @@ def login_path
private

def clubhouse_person(github_nickname)
PersonDecorator.new(Clubhouse::Client::Person.new(github_nickname))
PersonDecorator.from_github(github_nickname)
end

end
29 changes: 29 additions & 0 deletions app/controllers/students_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,33 @@
class StudentsController < ApplicationController

before_filter :find_course, :find_person, :find_membership

def show
@student = StudentDecorator.new(@membership)
end

private

#TODO: This is duped in TasksController. DRY it up!
def find_course
@course = Course.find(params[:course_id])
rescue ActiveRecord::RecordNotFound
redirect_to(root_url, alert: "Couldn't find course")
end

def find_person
@person = clubhouse_person(params[:id])

unless @person
redirect_to(@course, alert: "Student does not exist.")
end
end

def find_membership
@membership = @course.membership_for(@person)

unless @membership
redirect_to(@course, alert: "Student does not belong to course.")
end
end
end
8 changes: 7 additions & 1 deletion app/decorators/person_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ class PersonDecorator < ApplicationDecorator

allows :name, :email, :github_nickname, :permissions

def self.from_github(github_nickname)
PersonDecorator.new(Clubhouse::Client::Person.new(github_nickname))
rescue Clubhouse::Client::PersonNotFound
return nil
end

def membership_for(course)
course.course_memberships.for_person(person).first
course.membership_for(person)
end

def role_for(course)
Expand Down
8 changes: 8 additions & 0 deletions app/decorators/student_decorator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class StudentDecorator < ApplicationDecorator
decorates :course_membership

def tasks
course_membership.course.tasks.map(&:description)
end

end
4 changes: 4 additions & 0 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ class Course < ActiveRecord::Base
def participants
@participants ||= course_memberships.map { |cm| cm.person }
end

def membership_for(person)
course_memberships.for_person(person).first
end
end
4 changes: 1 addition & 3 deletions app/models/course_membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ class CourseMembership < ActiveRecord::Base
scope :for_person, lambda { |person| where(person_github_nickname: person.github_nickname) }

def person
@person ||= PersonDecorator.new(Clubhouse::Client::Person.new(person_github_nickname))
rescue Clubhouse::Client::PersonNotFound
return nil
@person ||= PersonDecorator.from_github(person_github_nickname)
end

def has_role?(has_role)
Expand Down
4 changes: 4 additions & 0 deletions app/views/students/show.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
%ul
- @student.tasks.each do |t|
%li= t

5 changes: 3 additions & 2 deletions test/integration/student_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

before do
@course = FactoryGirl.create(:webdev)
@instructor = build_person(:instructor, @course)
@student = build_person(:student, @course)
end

it "should display all of the course tasks on the student page" do
visit course_student_path @course, @student
sign_in @student
click_link "Web Development"
click_link "Student"

page.body.must_include "Community Service"
page.body.must_include "Personal Project"
Expand Down
4 changes: 2 additions & 2 deletions test/integration/tasks_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@

click_link "Web Development"
click_link "Add Task"
fill_in("Description", with: "Community Service")
fill_in("Description", with: "A new course task")
click_button "Create Task"
page.body.must_include("Community Service")
page.body.must_include("A new course task")
end

it "a student can only view course tasks" do
Expand Down

0 comments on commit 852bbdb

Please sign in to comment.