scope show to context chain of course/accounts

resolves CNVS-19289

Testplan:
 - the api should handle the show action for all contexts
   (root accounts, sub accounts and courses)
 - a 404 Not Found should be thrown when attempting to access
   a grading period not 'owned' by the current context

Change-Id: Idb3e4c2d88b9e68e81f636d014bc027d0c30bde6
Reviewed-on: https://gerrit.instructure.com/50733
Tested-by: Jenkins
Reviewed-by: Cameron Matheson <cameron@instructure.com>
Product-Review: Derek Bender <dbender@instructure.com>
QA-Review: Derek Bender <dbender@instructure.com>

app/controllers/grading_periods_controller.rb

@@ -29,6 +29,11 @@
 #           "example": 1023,
 #           "type": "integer"
 #         },
+#         "title": {
+#           "description": "The title for the grading period.",
+#           "example": "First Block",
+#           "type": "string"
+#         },
 #         "start_date": {
 #           "description": "The start date of the grading period.",
 #           "example": "2014-01-07T15:04:00Z",
@@ -68,7 +73,6 @@ class GradingPeriodsController < ApplicationController
   #
   def index
     if authorized_action(@context, @current_user, :read)
-      # TODO: inheritance check instead of #get_context?
       grading_periods = GradingPeriod.for(@context).sort_by(&:start_date)
       paginated_grading_periods, meta = paginate_for(grading_periods)
 
@@ -87,11 +91,12 @@ class GradingPeriodsController < ApplicationController
   #   }
   #
   def show
-    @grading_period = @context.grading_periods.active.find(params[:id])
-    # TODO: if there is no grading period found then this action will return
-    #       an empty body which is probably not what we want
-    if @grading_period && authorized_action(@grading_period, @current_user, :read)
-      render json: serialize_json_api(@grading_period)
+    params_id = params[:id].to_i
+    grading_period = GradingPeriod.context_find(context: @context, id: params_id)
+    fail ActionController::RoutingError.new('Not Found') if grading_period.blank?
+
+    if authorized_action(grading_period, @current_user, :read)
+      render json: serialize_json_api(grading_period)
     end
   end
 

app/models/grading_period.rb

@@ -27,10 +27,21 @@ class GradingPeriod < ActiveRecord::Base
     joins(:grading_period_group).where(grading_period_groups: context_with_ids)
   }
 
+  # Takes a context and returns an Array (not an ActiveRecord::Relation)
   def self.for(context)
     "GradingPeriod::#{context.class}GradingPeriodFinder".constantize.new(context).grading_periods
   end
 
+  # the keyword arguemnts version of this method is as follow:
+  # def self.context_find(context: context, id: id)
+  def self.context_find(options = {}) # in preperation for keyword arguments
+    fail ArgumentCountError unless options.count == 2
+    fail ArgumentError unless context = options.fetch(:context)
+    fail ArgumentError unless id = options.fetch(:id)
+
+    self.for(context).detect { |grading_period| grading_period.id == id }
+  end
+
   # save the previous definition of `destroy` and alias it to `destroy!`
   # Note: `destroy!` now does NOT throw errors while the newly defined
   # `destroy` DOES throw errors due to `save!`

app/models/grading_standard.rb

@@ -18,15 +18,18 @@
 
 class GradingStandard < ActiveRecord::Base
   include Workflow
+
   attr_accessible :title, :standard_data
+
   belongs_to :context, :polymorphic => true
-  validates_inclusion_of :context_type, :allow_nil => true, :in => ['Account', 'Course']
   belongs_to :user
   has_many :assignments
 
+
   EXPORTABLE_ATTRIBUTES = [:id, :title, :data, :context_id, :context_type, :created_at, :updated_at, :user_id, :usage_count, :context_code, :workflow_state, :version]
   EXPORTABLE_ASSOCIATIONS = [:context, :user, :assignments]
 
+  validates_inclusion_of :context_type, :allow_nil => true, :in => ['Account', 'Course']
   validates_presence_of :context_id, :context_type, :workflow_state, :data
   validate :valid_grading_scheme_data
 

spec/apis/v1/grading_periods_api_spec.rb

@@ -29,37 +29,6 @@ describe GradingPeriodsController, type: :request do
     end
 
     context "multiple grading periods feature flag turned on" do
-      describe 'GET show' do
-        before :once do
-          @grading_period = @account.grading_periods.first
-        end
-
-        def get_show(raw = false, data = {})
-          helper = method(raw ? :raw_api_call : :api_call)
-          helper.call(:get,
-                      "/api/v1/accounts/#{@account.id}/grading_periods/#{@grading_period.id}",
-          { controller: 'grading_periods', action: 'show', format: 'json',
-            account_id: @account.id,
-            id: @grading_period.id,
-          }, data)
-        end
-
-        it "retrieves the grading period specified" do
-          json = get_show
-          period = json['grading_periods'].first
-          expect(period['id']).to eq(@grading_period.id.to_s)
-          expect(period['weight']).to eq(@grading_period.weight)
-          expect(period['title']).to eq(@grading_period.title)
-          expect(period['permissions']).to eq("read" => true, "manage" => true)
-        end
-
-        it "doesn't return deleted grading periods" do
-          @grading_period.destroy
-          get_show(true)
-          expect(response.status).to eq 404
-        end
-      end
-
       describe 'POST create' do
         def post_create(params, raw=false)
           helper = method(raw ? :raw_api_call : :api_call)

spec/controllers/grading_periods_controller_spec.rb

@@ -18,37 +18,37 @@
 require_relative '../spec_helper'
 
 describe GradingPeriodsController do
+  let(:root_account) { Account.default }
+  let(:sub_account) { root_account.sub_accounts.create! }
+
+  let(:create_grading_periods) { -> (context, start) {
+      context.grading_period_groups.create!
+             .grading_periods.create!(weight: 1,
+                                      start_date: start,
+                                      end_date: (Time.zone.now + 10.days))
+    }
+  }
+
+  let(:remove_while) { -> (string) {
+      string.sub(%r{^while\(1\);}, '')
+    }
+  }
+
+  before do
+    account_admin_user(account: sub_account)
+    user_session(@admin)
+
+    root_account.allow_feature!(:multiple_grading_periods)
+    root_account.enable_feature!(:multiple_grading_periods)
+
+    create_grading_periods.call(root_account, 3.days.ago)
+    create_grading_periods.call(sub_account, Time.zone.now)
+  end
+
   describe "GET index" do
-    let(:root_account) { Account.default }
-    let(:sub_account) { root_account.sub_accounts.create! }
-
-    let(:create_grading_periods) { ->(context, start) {
-        context.grading_period_groups.create!
-               .grading_periods.create!(weight: 1,
-                                        start_date: start,
-                                        end_date: (Time.zone.now + 10.days))
-      }
-    }
-
-    let(:remove_while) { ->(string) {
-        string.sub(%r{^while\(1\);}, '')
-      }
-    }
-
-    before do
-      account_admin_user(account: sub_account)
-      user_session(@admin)
-
-      root_account.allow_feature!(:multiple_grading_periods)
-      root_account.enable_feature!(:multiple_grading_periods)
-
-      create_grading_periods.call(root_account, 3.days.ago)
-      create_grading_periods.call(sub_account, Time.zone.now)
-    end
-
     context "when context is a sub account" do
       before do
-        get :index, {account_id: sub_account.id}
+        get :index, { account_id: sub_account.id }
         @json = JSON.parse(remove_while.call(response.body))
       end
 
@@ -73,7 +73,7 @@ describe GradingPeriodsController do
 
         create_grading_periods.call(course, 5.days.ago)
 
-        get :index, {course_id: course.id}
+        get :index, { course_id: course.id }
         @json = JSON.parse(remove_while.call(response.body))
       end
 
@@ -82,5 +82,29 @@ describe GradingPeriodsController do
       end
     end
   end
+
+  describe "GET show" do
+
+    context "when context is a sub account" do
+      it "contains one grading periods" do
+        get :show, { account_id: sub_account.id, id: sub_account.grading_periods.first.id }
+        @json = JSON.parse(remove_while.call(response.body))
+        expect(@json['grading_periods'].count).to eql 1
+      end
+    end
+
+    context "when context is a course" do
+      let(:course) { Course.create!(account: sub_account) }
+      let(:grading_period) { create_grading_periods.call(course, 5.days.ago) }
+      let(:root_grading_period) { root_account.grading_periods.first }
+
+      it "matches ids" do
+        get :show, { course_id: course.id, id: root_grading_period.id }
+        json = JSON.parse(remove_while.call(response.body))
+        period = json['grading_periods'].first
+        expect(period['id']).to eq root_grading_period.id.to_s
+      end
+    end
+  end
 end
 

spec/models/grading_period_spec.rb

@@ -61,7 +61,7 @@ describe GradingPeriod do
     it { is_expected.to be_destroyed }
   end
 
-  describe "#for" do
+  describe ".for" do
 
     context "when context is an account" do
       let(:account) { Account.new }
@@ -86,6 +86,20 @@ describe GradingPeriod do
     end
   end
 
+  describe ".context_find" do
+    let(:account) { mock }
+    let(:finder) { mock }
+    let(:grading_period) { mock }
+    let(:id) { 1 }
+
+    it "delegates" do
+      grading_period.expects(:id).returns(1)
+      GradingPeriod.expects(:for).with(account).returns([grading_period])
+
+      expect(GradingPeriod.context_find(context: account, id: id)).to eq grading_period
+    end
+  end
+
   describe "#assignments" do
     it "filters assignments for grading period" do
       course_with_teacher active_all: true

spec/spec_helper.rb

@@ -60,6 +60,7 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
 
 ActionView::TestCase::TestController.view_paths = ApplicationController.view_paths
 
+
 module RSpec::Rails
   module ViewExampleGroup
     module ExampleMethods