Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

addressing code review items

  • Loading branch information...
commit 5c4043103521602a00a45498a20ab3b6777aef21 1 parent 22572aa
@seejee seejee authored
View
9 app/controllers/tasks_controller.rb
@@ -9,9 +9,10 @@ def create
task = @course.tasks.build(params[:task])
if task.save
- redirect_to course_path(@course)
+ flash[:success] = "Task created."
+ redirect_to @course
else
- flash[:error] = "Sorry, there was an error adding the task to the course"
+ render :new
end
end
@@ -19,10 +20,11 @@ def destroy
task = Task.find(params[:id])
if task.destroy
- redirect_to course_path(task.course)
+ 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
@@ -30,4 +32,5 @@ def destroy
def find_course
@course = Course.find(params[:course_id])
end
+
end
View
2  app/models/course.rb
@@ -7,7 +7,7 @@ def people
end
def instructor?(person)
- membership = course_memberships.find {|cm| cm.for_person?(person) }
+ membership = course_memberships.for_person(person).first
membership.instructor? if membership
end
View
6 app/models/course_membership.rb
@@ -7,16 +7,14 @@ 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 for_person?(person)
- person_github_nickname == person.github_nickname
- end
-
def instructor?
role == "Instructor"
end
View
18 test/integration/tasks_test.rb
@@ -18,23 +18,5 @@ class TasksTest < ActionDispatch::IntegrationTest
click_link "Web Development"
assert_not_includes(page.body, "Add Task")
-
- end
-
- def sign_in_as_instructor
- sign_in(Clubhouse::Client::Person.new('instructor'))
- end
-
- def sign_in_as_student
- sign_in(Clubhouse::Client::Person.new('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
27 test/test_helper.rb
@@ -4,18 +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
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
26 test/unit/course_membership_test.rb
@@ -0,0 +1,26 @@
+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.instructor?.must_equal false
+ end
+ end
+
+ describe "adding an instructor" do
+ before do
+ @membership = CourseMembership.new(role: 'Instructor')
+ end
+
+ it "should not identify the student as an instructor" do
+ @membership.instructor?.must_equal true
+ end
+ end
+end
+
View
40 test/unit/course_test.rb
@@ -1,40 +0,0 @@
-require "test_helper"
-require "minitest/spec"
-
-describe Course do
-
- before do
- @course = Course.new
- end
-
- describe "adding a student" do
-
- before do
- @person = clubhouse_person('student')
- @cm = @course.course_memberships.build(person_github_nickname: 'student', role: 'Student')
- end
-
- it "should not identify the student as an instructor" do
- @course.instructor?(@person).must_equal false
- end
-
- end
-
- describe "adding an instructor" do
-
- before do
- @person = clubhouse_person('instructor')
- @cm = @course.course_memberships.build(person_github_nickname: 'instructor', role: 'Instructor')
- end
-
- it "should identify the student as an instructor" do
- @course.instructor?(@person).must_equal true
- end
-
- end
-
-end
-
-def clubhouse_person(github_nickname)
- Clubhouse::Client::Person.new(github_nickname)
-end
Please sign in to comment.
Something went wrong with that request. Please try again.