specify pagination order for outcome links

the outcome link endpoint was not specifying an order when paginating,
which could cause duplicate outcome links to be returned

fixes CNVS-16646

test plan:
- create a course with a lot of outcomes (~50)
- load the student outcomes gradebook
- it should show all of your outcomes

Change-Id: I68c9b6369c9ec25a8a1ad7d429ba12fa21799588
Reviewed-on: https://gerrit.instructure.com/44347
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Benjamin Porter <bporter@instructure.com>
QA-Review: Sean Lewis <slewis@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>

app/controllers/outcome_groups_api_controller.rb

@@ -132,6 +132,7 @@ class OutcomeGroupsApiController < ApplicationController
 
   before_filter :require_user
   before_filter :get_context
+  before_filter :require_context, :only => [:link_index]
 
   # @API Redirect to root outcome group for context
   #
@@ -174,10 +175,12 @@ class OutcomeGroupsApiController < ApplicationController
   def link_index
     return unless can_read_outcomes
 
-    url = polymorphic_url [:api_v1, @context || :global, :outcome_group_links]
-    links = Api.paginate(context_outcome_links, self, url)
+    url = polymorphic_url [:api_v1, @context, :outcome_group_links]
+    links = @context.learning_outcome_links.order(:id)
+    links = Api.paginate(links, self, url)
     render json: links.map { |link|
-      outcome_link_json(link, @current_user, session, params.slice(:outcome_style, :outcome_group_style))
+      outcome_params = params.slice(:outcome_style, :outcome_group_style)
+      outcome_link_json(link, @current_user, session, outcome_params)
     }
   end
 
@@ -653,13 +656,6 @@ class OutcomeGroupsApiController < ApplicationController
     LearningOutcomeGroup.for_context(@context).active
   end
 
-  def context_outcome_links
-    if @context
-      @context.learning_outcome_links
-    # else, there's no convenient way to find the global content tags; not supporting this for now
-    end
-  end
-
   # verify the outcome is eligible to be linked into the context,
   # returning the outcome if so
   def context_available_outcome(outcome_id)

spec/apis/v1/outcome_groups_api_spec.rb

@@ -174,26 +174,43 @@ describe "Outcome Groups API", type: :request do
   end
 
   describe "link_index" do
-    before :each do
+    before :once do
       @account = Account.default
       @account_user = @user.account_users.create(:account => @account)
       @group = @account.root_outcome_group
+      @links = (1..3).map{ create_outcome }
     end
 
     it "should return active links" do
-      @links = (1..3).map{ create_outcome }
       link = @links.pop
       link.workflow_state = 'deleted'
       link.save!
 
       json = api_call(:get, "/api/v1/accounts/#{@account.id}/outcome_group_links",
-        controller: 'outcome_groups_api', action: 'link_index', account_id: @account.id, format: 'json')
+                      controller: 'outcome_groups_api',
+                        action: 'link_index',
+                        account_id: @account.id,
+                        format: 'json')
       expected_outcome_ids = @links.map(&:content).map(&:id).sort
       expected_group_ids = @links.map(&:associated_asset).map(&:id).sort
       expect(json.map {|j| j['outcome']['id']}.sort).to eq expected_outcome_ids
       expect(json.map {|j| j['outcome_group']['id']}.sort).to eq expected_group_ids
     end
 
+    it "should return links ordered by id when paginated" do
+      json = api_call(:get, "/api/v1/accounts/#{@account.id}/outcome_group_links?per_page=2",
+                      controller: 'outcome_groups_api',
+                      action: 'link_index',
+                      account_id: @account.id,
+                      per_page: "2",
+                      format: 'json')
+
+      # intentionally not manually sorting either the expected or returned:
+      # - expected should be sorted by id because of creation time
+      # - returned should be sorted by id because of pagination ordering
+      expected_outcome_ids = @links.take(2).map(&:content).map(&:id)
+      expect(json.map {|j| j['outcome']['id']}).to eq expected_outcome_ids
+    end
   end
 
   describe "show" do