Add groups context route for membership service

fixes PLAT-1447

Test plan:
* Create a course with some students
* Regression test
  /api/lti/courses/:course_id/membership_service
* Create a group with some students
* Ensure that
  /api/lti/groups/:group_id/membership_service
  returns a properly formatted list of group members

Change-Id: Idc03c11728847258d621d365d87238465f739672
Reviewed-on: https://gerrit.instructure.com/77595
Tested-by: Jenkins
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Andrew Butterfield <abutterfield@instructure.com>

app/controllers/lti/membership_service_controller.rb

@@ -19,16 +19,25 @@ module Lti
     before_filter :require_context
     before_filter :require_user
 
-    def index
+    def course_index
+      render_page_presenter
+    end
+
+    def group_index
+      render_page_presenter
+    end
+
+    private
+
+    def render_page_presenter
       @page = MembershipService::PagePresenter.new(@context,
                                                    @current_user,
                                                    request.base_url,
                                                    membership_service_params)
+
       render json: @page
     end
 
-    private
-
     def membership_service_params
       keys = %w(role page per_page)
       params.select { |k,_| keys.include?(k) }

config/routes.rb

@@ -1859,7 +1859,8 @@ CanvasRails::Application.routes.draw do
     get  "tool_proxy/:tool_proxy_guid", controller: 'lti/ims/tool_proxy', action: :show, as: "show_lti_tool_proxy"
 
     # Membership Service
-    get "courses/:course_id/membership_service", controller: "lti/membership_service", action: :index, as: :course_membership_service
+    get "courses/:course_id/membership_service", controller: "lti/membership_service", action: :course_index, as: :course_membership_service
+    get "groups/:group_id/membership_service", controller: "lti/membership_service", action: :group_index, as: :group_membership_service
   end
 
   ApiRouteSet.draw(self, '/api/sis') do

db/migrate/20160419155551_add_lti_context_id_to_groups.rb

@@ -0,0 +1,7 @@
+class AddLtiContextIdToGroups < ActiveRecord::Migration
+  tag :predeploy
+
+  def change
+    add_column :groups, :lti_context_id, :string
+  end
+end

lib/lti/membership_service/page_presenter.rb

@@ -37,7 +37,8 @@ module Lti
 
       def next_page
         if @membership_collator.next_page?
-          course_membership_service_url(@membership_collator.context, next_page_query_params.merge(host: @base_url))
+          method = "#{@membership_collator.context.class.to_s.downcase}_membership_service_url".to_sym
+          send method, @membership_collator.context, next_page_query_params.merge(host: @base_url)
         end
       end
 

lib/user_search.rb

@@ -81,7 +81,7 @@ module UserSearch
       if enrollment_types.any?{ |et| !Enrollment.readable_types.keys.include?(et) }
         raise ArgumentError, 'Invalid Enrollment Type'
       end
-      users = users.where(:enrollments => { :type => enrollment_types })
+      users = users.joins(:enrollments).where(:enrollments => { :type => enrollment_types })
     end
 
     if exclude_groups

spec/controllers/lti/membership_service_controller_spec.rb

@@ -25,10 +25,10 @@ module Lti
         course_with_teacher
       end
 
-      describe "#index" do
+      describe "#course_index" do
         context 'without access token' do
           it 'requires a user' do
-            get 'index', course_id: @course.id
+            get 'course_index', course_id: @course.id
             assert_unauthorized
           end
         end
@@ -42,7 +42,7 @@ module Lti
           end
 
           it 'outputs the expected data in the expected format at the top level' do
-            get 'index', course_id: @course.id
+            get 'course_index', course_id: @course.id
             hash = json_parse.with_indifferent_access
             expect(hash.keys.size).to eq(6)
 
@@ -55,7 +55,7 @@ module Lti
           end
 
           it 'outputs the expected data in the expected format at the container level' do
-            get 'index', course_id: @course.id
+            get 'course_index', course_id: @course.id
             hash = json_parse.with_indifferent_access
             container = hash[:pageOf]
 
@@ -68,7 +68,7 @@ module Lti
           end
 
           it 'outputs the expected data in the expected format at the context level' do
-            get 'index', course_id: @course.id
+            get 'course_index', course_id: @course.id
             hash = json_parse.with_indifferent_access
             @course.reload
             context = hash[:pageOf][:membershipSubject]
@@ -82,7 +82,7 @@ module Lti
           end
 
           it 'outputs the expected data in the expected format at the membership level' do
-            get 'index', course_id: @course.id
+            get 'course_index', course_id: @course.id
             hash = json_parse.with_indifferent_access
             @teacher.reload
             memberships = hash[:pageOf][:membershipSubject][:membership]
@@ -129,7 +129,7 @@ module Lti
       describe '#as_json' do
         it 'provides the right next_page url when no page/per_page/role params are given' do
           Api.stubs(:per_page).returns(1)
-          get 'index', course_id: @course.id
+          get 'course_index', course_id: @course.id
           hash = json_parse.with_indifferent_access
 
           uri = URI(hash.fetch(:nextPage))
@@ -141,7 +141,7 @@ module Lti
 
         it 'provides the right next_page url when page/per_page/role params are given' do
           Api.stubs(:per_page).returns(1)
-          get 'index', course_id: @course.id, page: 2, per_page: 1, role: 'Instructor'
+          get 'course_index', course_id: @course.id, page: 2, per_page: 1, role: 'Instructor'
           hash = json_parse.with_indifferent_access
 
           uri = URI(hash.fetch(:nextPage))
@@ -153,7 +153,162 @@ module Lti
 
         it 'returns nil for the next page url when the last page in the collection was requested' do
           Api.stubs(:per_page).returns(1)
-          get 'index', course_id: @course.id, page: 3, per_page: 1, role: 'Instructor'
+          get 'course_index', course_id: @course.id, page: 3, per_page: 1, role: 'Instructor'
+          hash = json_parse.with_indifferent_access
+
+          expect(hash.fetch(:nextPage)).to be_nil
+        end
+      end
+    end
+
+    context 'group with single student' do
+      before(:each) do
+        course_with_teacher
+        @course.offer!
+        @student = user_model
+        @course.enroll_user(@student, 'StudentEnrollment', enrollment_state: 'active')
+        @group_category = @course.group_categories.create!(name: 'Membership')
+        @group = @course.groups.create!(name: 'Group 1', group_category: @group_category)
+        @group.add_user(@student)
+      end
+
+      describe "#group_index" do
+        context 'without access token' do
+          it 'requires a user' do
+            get 'group_index', group_id: @group.id
+            assert_unauthorized
+          end
+        end
+
+        context 'with access token' do
+          before(:each) do
+            pseudonym(@student)
+            @student.save!
+            token = @student.access_tokens.create!(purpose: 'test').full_token
+            @request.headers['Authorization'] = "Bearer #{token}"
+          end
+
+          it 'outputs the expected data in the expected format at the top level' do
+            get 'group_index', group_id: @group.id
+            hash = json_parse.with_indifferent_access
+            expect(hash.keys.size).to eq(6)
+
+            expect(hash.fetch(:@id)).to be_nil
+            expect(hash.fetch(:@type)).to eq 'Page'
+            expect(hash.fetch(:@context)).to eq 'http://purl.imsglobal.org/ctx/lis/v2/MembershipContainer'
+            expect(hash.fetch(:differences)).to be_nil
+            expect(hash.fetch(:nextPage)).to be_nil
+            expect(hash.fetch(:pageOf)).not_to be_nil
+          end
+
+          it 'outputs the expected data in the expected format at the container level' do
+            get 'group_index', group_id: @group.id
+            hash = json_parse.with_indifferent_access
+            container = hash[:pageOf]
+
+            expect(container.size).to eq 5
+            expect(container.fetch(:@id)).to be_nil
+            expect(container.fetch(:@type)).to eq 'LISMembershipContainer'
+            expect(container.fetch(:@context)).to eq 'http://purl.imsglobal.org/ctx/lis/v2/MembershipContainer'
+            expect(container.fetch(:membershipPredicate)).to eq 'http://www.w3.org/ns/org#membership'
+            expect(container.fetch(:membershipSubject)).not_to be_nil
+          end
+
+          it 'outputs the expected data in the expected format at the context level' do
+            get 'group_index', group_id: @group.id
+            hash = json_parse.with_indifferent_access
+            @group.reload
+            context = hash[:pageOf][:membershipSubject]
+
+            expect(context.size).to eq 5
+            expect(context.fetch(:@id)).to be_nil
+            expect(context.fetch(:@type)).to eq 'Context'
+            expect(context.fetch(:name)).to eq @group.name
+            expect(context.fetch(:contextId)).to eq @group.lti_context_id
+            expect(context.fetch(:membership)).not_to be_nil
+          end
+
+          it 'outputs the expected data in the expected format at the membership level' do
+            get 'group_index', group_id: @group.id
+            hash = json_parse.with_indifferent_access
+            @student.reload
+            memberships = hash[:pageOf][:membershipSubject][:membership]
+
+            expect(memberships.size).to eq 1
+
+            membership = memberships[0]
+
+            expect(membership.size).to eq 4
+            expect(membership.fetch(:@id)).to be_nil
+            expect(membership.fetch(:status)).to eq IMS::LIS::Statuses::SimpleNames::Active
+            expect(membership.fetch(:role)).to match_array([IMS::LIS::Roles::Context::URNs::Learner])
+
+            member = membership.fetch(:member)
+            expect(member.fetch(:@id)).to be_nil
+            expect(member.fetch(:name)).to eq @student.name
+            expect(member.fetch(:img)).to eq @student.avatar_image_url
+            expect(member.fetch(:email)).to eq @student.email
+            expect(member.fetch(:familyName)).to eq @student.last_name
+            expect(member.fetch(:givenName)).to eq @student.first_name
+            expect(member.fetch(:resultSourcedId)).to be_nil
+            expect(member.fetch(:sourcedId)).to be_nil
+            expect(member.fetch(:userId)).to eq(@student.lti_context_id)
+          end
+        end
+      end
+    end
+
+    context 'group with multiple students' do
+      before(:each) do
+        course_with_teacher
+        @course.offer!
+        @student1 = user_model
+        @course.enroll_user(@student1, 'StudentEnrollment', enrollment_state: 'active')
+        @student2 = user_model
+        @course.enroll_user(@student2, 'StudentEnrollment', enrollment_state: 'active')
+        @student3 = user_model
+        @course.enroll_user(@student3, 'StudentEnrollment', enrollment_state: 'active')
+
+        @group_category = @course.group_categories.create!(name: 'Membership')
+        @group = @course.groups.create!(name: 'Group 1', group_category: @group_category)
+        @group.add_user(@student1)
+        @group.add_user(@student2)
+        @group.add_user(@student3)
+
+        pseudonym(@student1)
+        @student1.save!
+        token = @student1.access_tokens.create!(purpose: 'test').full_token
+        @request.headers['Authorization'] = "Bearer #{token}"
+      end
+
+      describe '#as_json' do
+        it 'provides the right next_page url when no page/per_page/role params are given' do
+          Api.stubs(:per_page).returns(1)
+          get 'group_index', group_id: @group.id
+          hash = json_parse.with_indifferent_access
+
+          uri = URI(hash.fetch(:nextPage))
+          expect(uri.scheme).to eq 'http'
+          expect(uri.host).to eq 'test.host'
+          expect(uri.path).to eq "/api/lti/groups/#{@group.id}/membership_service"
+          expect(uri.query).to eq 'page=2&per_page=1'
+        end
+
+        it 'provides the right next_page url when page/per_page/role params are given' do
+          Api.stubs(:per_page).returns(1)
+          get 'group_index', group_id: @group.id, page: 2, per_page: 1, role: 'Instructor'
+          hash = json_parse.with_indifferent_access
+
+          uri = URI(hash.fetch(:nextPage))
+          expect(uri.scheme).to eq 'http'
+          expect(uri.host).to eq 'test.host'
+          expect(uri.path).to eq "/api/lti/groups/#{@group.id}/membership_service"
+          expect(uri.query).to eq 'page=3&per_page=1&role=Instructor'
+        end
+
+        it 'returns nil for the next page url when the last page in the collection was requested' do
+          Api.stubs(:per_page).returns(1)
+          get 'group_index', group_id: @group.id, page: 3, per_page: 1, role: 'Instructor'
           hash = json_parse.with_indifferent_access
 
           expect(hash.fetch(:nextPage)).to be_nil

spec/lib/lti/membership_service/lis_person_collator_spec.rb

@@ -237,5 +237,58 @@ module Lti::MembershipService
         expect(collator.next_page?).to eq(false)
       end
     end
+
+    context 'group with multiple students' do
+      before(:each) do
+        course_with_teacher
+        @course.offer!
+        @student1 = user_model
+        @course.enroll_user(@student1, 'StudentEnrollment', enrollment_state: 'active')
+        @student2 = user_model
+        @course.enroll_user(@student2, 'StudentEnrollment', enrollment_state: 'active')
+        @student3 = user_model
+        @course.enroll_user(@student3, 'StudentEnrollment', enrollment_state: 'active')
+
+        @group_category = @course.group_categories.create!(name: 'Membership')
+        @group = @course.groups.create!(name: 'Group 1', group_category: @group_category)
+        @group.add_user(@student1)
+        @group.add_user(@student2)
+        @group.add_user(@student3)
+      end
+
+      describe '#context' do
+        it 'returns the correct context' do
+          collator = LisPersonCollator.new(@group, @student1)
+
+          expect(collator.context).to eq(@group)
+        end
+      end
+
+      describe '#membership' do
+        it 'outputs the membership in a group' do
+          collator = LisPersonCollator.new(@group, @student1)
+
+          @student1.reload
+          @student2.reload
+          @student3.reload
+          memberships = collator.memberships
+          expect(memberships.size).to eq(3)
+
+          [@student1, @student2, @student3].each do |student|
+            membership = memberships.find { |m| m.member.user_id == student.lti_context_id }
+
+            expect(membership.status).to eq(IMS::LIS::Statuses::SimpleNames::Active)
+            expect(membership.role).to match_array([IMS::LIS::Roles::Context::URNs::Learner])
+            expect(membership.member.name).to eq(student.name)
+            expect(membership.member.given_name).to eq(student.first_name)
+            expect(membership.member.family_name).to eq(student.last_name)
+            expect(membership.member.img).to eq(student.avatar_image_url)
+            expect(membership.member.email).to eq(student.email)
+            expect(membership.member.result_sourced_id).to be_nil
+            expect(membership.member.sourced_id).to be_nil
+          end
+        end
+      end
+    end
   end
 end

spec/lib/lti/membership_service/page_presenter_spec.rb

@@ -24,6 +24,8 @@ module Lti::MembershipService
     let(:base_url) { 'https://localhost:3000' }
     let(:presenter) { PagePresenter.new(@course, @teacher, base_url) }
     let(:hash) { presenter.as_json }
+    let(:group_presenter) { PagePresenter.new(@group, @student, base_url) }
+    let(:group_hash) { group_presenter.as_json }
 
     context 'course with single enrollment' do
       before(:each) do
@@ -91,6 +93,79 @@ module Lti::MembershipService
       end
     end
 
+    context 'group with single student' do
+      before(:each) do
+        course_with_teacher
+        @course.offer!
+        @student = user_model
+        @course.enroll_user(@student, 'StudentEnrollment', enrollment_state: 'active')
+
+        @group_category = @course.group_categories.create!(name: 'Membership')
+        @group = @course.groups.create!(name: 'Group 1', group_category: @group_category)
+        @group.add_user(@student)
+      end
+
+      describe '#as_json' do
+        it 'outputs the expected data in the expected format at the top level' do
+          expect(group_hash.keys.size).to eq(6)
+
+          expect(group_hash.fetch(:@id)).to be_nil
+          expect(group_hash.fetch(:@type)).to eq 'Page'
+          expect(group_hash.fetch(:@context)).to eq 'http://purl.imsglobal.org/ctx/lis/v2/MembershipContainer'
+          expect(group_hash.fetch(:differences)).to be_nil
+          expect(group_hash.fetch(:nextPage)).to be_nil
+          expect(group_hash.fetch(:pageOf)).not_to be_nil
+        end
+
+        it 'outputs the expected data in the expected format at the container level' do
+          container = group_hash[:pageOf]
+
+          expect(container.size).to eq 5
+          expect(container.fetch(:@id)).to be_nil
+          expect(container.fetch(:@type)).to eq 'LISMembershipContainer'
+          expect(container.fetch(:@context)).to eq 'http://purl.imsglobal.org/ctx/lis/v2/MembershipContainer'
+          expect(container.fetch(:membershipPredicate)).to eq 'http://www.w3.org/ns/org#membership'
+          expect(container.fetch(:membershipSubject)).not_to be_nil
+        end
+
+        it 'outputs the expected data in the expected format at the context level' do
+          context = group_hash[:pageOf][:membershipSubject]
+
+          expect(context.size).to eq 5
+          expect(context.fetch(:@id)).to be_nil
+          expect(context.fetch(:@type)).to eq 'Context'
+          expect(context.fetch(:name)).to eq @group.name
+          expect(context.fetch(:contextId)).to eq @group.lti_context_id
+          expect(context.fetch(:membership)).not_to be_nil
+        end
+
+        it 'outputs the expected data in the expected format at the membership level' do
+          memberships = group_hash[:pageOf][:membershipSubject][:membership]
+          @student.reload
+
+          expect(memberships.size).to eq 1
+
+          membership = memberships[0]
+
+          expect(membership.size).to eq 4
+          expect(membership.fetch(:@id)).to be_nil
+          expect(membership.fetch(:status)).to eq IMS::LIS::Statuses::SimpleNames::Active
+          expect(membership.fetch(:role)).to match_array([IMS::LIS::Roles::Context::URNs::Learner])
+
+          member = membership.fetch(:member)
+          expect(member.fetch(:@id)).to be_nil
+          expect(member.fetch(:name)).to eq @student.name
+          expect(member.fetch(:img)).to eq @student.avatar_image_url
+          expect(member.fetch(:email)).to eq @student.email
+          expect(member.fetch(:familyName)).to eq @student.last_name
+          expect(member.fetch(:givenName)).to eq @student.first_name
+          expect(member.fetch(:resultSourcedId)).to be_nil
+          expect(member.fetch(:sourcedId)).to be_nil
+          expect(member.fetch(:userId)).to eq(@student.lti_context_id)
+        end
+      end
+    end
+
     context 'course with multiple enrollments' do
       before(:each) do
         course_with_teacher(:active_course => true)