From 2658d2c05c619d77bd122d241d09599f0f54ea3c Mon Sep 17 00:00:00 2001 From: Spencer Olson Date: Wed, 6 Jan 2016 10:41:51 -0600 Subject: [PATCH] mobile: add grading period scores to endpoints adds support for a 'current_grading_period_scores' argument that can be passed to api/v1/courses, api/v1/courses/:id, api/v1/users/:user_id/courses, and api/v1/users/self/favorites/courses. if this argument is passed to these endpoints, scores for the current grading period will be returned. closes CNVS-26203 test plan: - run $ bundle exec rake doc:api to generate API documentation. - view the documentation at public/doc/api/index.html and navigate to the documentation for the Courses controller. - verify there is documentation explaining how to pass the 'current_grading_period_scores' argument, and explaining what will be returned. - hit the api/v1/courses, api/v1/courses/:id, api/v1/users/:user_id/courses, and api/v1/users/self/favorites/courses endpoints and pass in 'current_grading_period_scores' as an include[] argument. verify you get the expected responses back, as outlined in the API documentation for 'current_grading_period _scores'. - ping me with any questions :) Change-Id: Ic8985c6a85af120b9d07ef2cd4e8e049e0c327fc Reviewed-on: https://gerrit.instructure.com/69836 Tested-by: Jenkins Reviewed-by: Derek Bender QA-Review: Amber Taniuchi Product-Review: Keith T. Garner --- app/controllers/courses_controller.rb | 68 +++-- app/controllers/enrollments_api_controller.rb | 59 +++- app/controllers/gradebooks_controller.rb | 2 +- .../filter_with_overrides_by_due_at.rb | 7 +- ...lter_with_overrides_by_due_at_for_class.rb | 2 +- ...er_with_overrides_by_due_at_for_student.rb | 3 +- app/models/grading_period.rb | 4 + lib/api/v1/course_json.rb | 127 +++++++-- spec/apis/v1/course_json_spec.rb | 4 +- spec/apis/v1/courses_api_spec.rb | 264 +++++++++++++----- spec/models/grading_period_spec.rb | 25 +- 11 files changed, 440 insertions(+), 125 deletions(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index ef051dbbe93..7425dc1b500 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -318,7 +318,7 @@ class CoursesController < ApplicationController # 'StudentEnrollment', 'TeacherEnrollment', 'TaEnrollment', 'ObserverEnrollment', # or 'DesignerEnrollment'. # - # @argument include[] [String, "needs_grading_count"|"syllabus_body"|"total_scores"|"term"|"course_progress"|"sections"|"storage_quota_used_mb"|"total_students"|"favorites"|"teachers"|"observed_users"] + # @argument include[] [String, "needs_grading_count"|"syllabus_body"|"total_scores"|"term"|"course_progress"|"sections"|"storage_quota_used_mb"|"total_students"|"favorites"|"teachers"|"observed_users"|"current_grading_period_scores"] # - "needs_grading_count": Optional information to include with each Course. # When needs_grading_count is given, and the current user has grading # rights, the total number of submissions needing grading for all @@ -327,16 +327,23 @@ class CoursesController < ApplicationController # When syllabus_body is given the user-generated html for the course # syllabus is returned. # - "total_scores": Optional information to include with each Course. - # When total_scores is given, any enrollments with type 'student' will also - # include the fields 'calculated_current_score', 'calculated_final_score', - # 'calculated_current_grade', and 'calculated_final_grade'. - # calculated_current_score is the student's score in the course, ignoring - # ungraded assignments. calculated_final_score is the student's score in - # the course including ungraded assignments with a score of 0. - # calculated_current_grade is the letter grade equivalent of - # calculated_current_score (if available). calculated_final_grade is the - # letter grade equivalent of calculated_final_score (if available). This - # argument is ignored if the course is configured to hide final grades. + # When total_scores is given, any student enrollments will also + # include the fields 'computed_current_score', 'computed_final_score', + # 'computed_current_grade', and 'computed_final_grade' (see Enrollment + # documentation for more information on these fields). This argument + # is ignored if the course is configured to hide final grades. + # - "current_grading_period_scores": Optional information to include with + # each Course. When current_grading_period_scores is given and total_scores + # is given, any student enrollments will also include the fields + # 'multiple_grading_periods_enabled', + # 'totals_for_all_grading_periods_option', 'current_grading_period_title', + # 'current_period_computed_current_score', + # 'current_period_computed_final_score', + # 'current_period_computed_current_grade', and + # 'current_period_computed_final_grade' (see Enrollment documentation for + # more information on these fields). This argument is ignored if the course + # is configured to hide final grades or if the total_scores argument is not + # included. # - "term": Optional information to include with each Course. When # term is given, the information for the enrollment term for each course # is returned. @@ -417,7 +424,7 @@ class CoursesController < ApplicationController # @API List courses for a user # Returns a list of active courses for this user. To view the course list for a user other than yourself, you must be either an observer of that user or an administrator. # - # @argument include[] [String, "needs_grading_count"|"syllabus_body"|"total_scores"|"term"|"course_progress"|"sections"|"storage_quota_used_mb"|"total_students"|"favorites"] + # @argument include[] [String, "needs_grading_count"|"syllabus_body"|"total_scores"|"term"|"course_progress"|"sections"|"storage_quota_used_mb"|"total_students"|"favorites"|"current_grading_period_scores"] # - "needs_grading_count": Optional information to include with each Course. # When needs_grading_count is given, and the current user has grading # rights, the total number of submissions needing grading for all @@ -426,16 +433,23 @@ class CoursesController < ApplicationController # When syllabus_body is given the user-generated html for the course # syllabus is returned. # - "total_scores": Optional information to include with each Course. - # When total_scores is given, any enrollments with type 'student' will also - # include the fields 'calculated_current_score', 'calculated_final_score', - # 'calculated_current_grade', and 'calculated_final_grade'. - # calculated_current_score is the student's score in the course, ignoring - # ungraded assignments. calculated_final_score is the student's score in - # the course including ungraded assignments with a score of 0. - # calculated_current_grade is the letter grade equivalent of - # calculated_current_score (if available). calculated_final_grade is the - # letter grade equivalent of calculated_final_score (if available). This - # argument is ignored if the course is configured to hide final grades. + # When total_scores is given, any student enrollments will also + # include the fields 'computed_current_score', 'computed_final_score', + # 'computed_current_grade', and 'computed_final_grade' (see Enrollment + # documentation for more information on these fields). This argument + # is ignored if the course is configured to hide final grades. + # - "current_grading_period_scores": Optional information to include with + # each Course. When current_grading_period_scores is given and total_scores + # is given, any student enrollments will also include the fields + # 'multiple_grading_periods_enabled', + # 'totals_for_all_grading_periods_option', 'current_grading_period_title', + # 'current_period_computed_current_score', + # 'current_period_computed_final_score', + # 'current_period_computed_current_grade', and + # 'current_period_computed_final_grade' (see Enrollment documentation for + # more information on these fields). This argument is ignored if the course + # is configured to hide final grades or if the total_scores argument is not + # included. # - "term": Optional information to include with each Course. When # term is given, the information for the enrollment term for each course # is returned. @@ -2418,10 +2432,12 @@ class CoursesController < ApplicationController Canvas::Builders::EnrollmentDateBuilder.preload(enrollments) enrollments_by_course = enrollments.group_by(&:course_id).values enrollments_by_course = Api.paginate(enrollments_by_course, self, api_v1_courses_url) if api_request? - if includes.include?("teachers") - courses = enrollments_by_course.map(&:first).map(&:course) - ActiveRecord::Associations::Preloader.new.preload(courses, :teachers) - end + courses = enrollments_by_course.map(&:first).map(&:course) + preloads = [:account] + preloads << :teachers if includes.include?('teachers') + preloads << :grading_standard if includes.include?('total_scores') + ActiveRecord::Associations::Preloader.new.preload(courses, preloads) + enrollments_by_course.each do |course_enrollments| course = course_enrollments.first.course hash << course_json(course, user, session, includes, course_enrollments) diff --git a/app/controllers/enrollments_api_controller.rb b/app/controllers/enrollments_api_controller.rb index 4fc29667766..94e203c1bb5 100644 --- a/app/controllers/enrollments_api_controller.rb +++ b/app/controllers/enrollments_api_controller.rb @@ -164,12 +164,63 @@ # "type": "string" # }, # "grades": { -# "description": "The URL to the Canvas web UI page the grades associated with this enrollment.", +# "description": "The URL to the Canvas web UI page containing the grades associated with this enrollment.", # "$ref": "Grade" # }, -# "user": { -# "description": "A description of the user.", -# "type": "User" +# "computed_current_score": { +# "description": "optional: The student's score in the course, ignoring ungraded assignments. (applies only to student enrollments, and only available in course endpoints)", +# "example": 90.25, +# "type": "float" +# }, +# "computed_final_score": { +# "description": "optional: The student's score in the course including ungraded assignments with a score of 0. (applies only to student enrollments, and only available in course endpoints)", +# "example": 80.67, +# "type": "float" +# }, +# "computed_current_grade": { +# "description": "optional: The letter grade equivalent of computed_current_score, if available. (applies only to student enrollments, and only available in course endpoints)", +# "example": "A-", +# "type": "string" +# }, +# "computed_final_grade": { +# "description": "optional: The letter grade equivalent of computed_final_score, if available. (applies only to student enrollments, and only available in course endpoints)", +# "example": "B-", +# "type": "string" +# }, +# "multiple_grading_periods_enabled": { +# "description": "optional: Indicates whether the course the enrollment belongs to has the Multiple Grading Periods feature enabled. (applies only to student enrollments, and only available in course endpoints)", +# "example": true, +# "type": "boolean" +# }, +# "totals_for_all_grading_periods_option": { +# "description": "optional: Indicates whether the course the enrollment belongs to has the Display Totals for 'All Grading Periods' feature enabled. (applies only to student enrollments, and only available in course endpoints)", +# "example": true, +# "type": "boolean" +# }, +# "current_grading_period_title": { +# "description": "optional: The name of the currently active grading period, if one exists. If the course the enrollment belongs to does not have Multiple Grading Periods enabled, or if no currently active grading period exists, the value will be null. (applies only to student enrollments, and only available in course endpoints)", +# "example": "Fall Grading Period", +# "type": "string" +# }, +# "current_period_computed_current_score": { +# "description": "optional: The student's score in the course for the current grading period, ignoring ungraded assignments. If the course the enrollment belongs to does not have Multiple Grading Periods enabled, or if no currently active grading period exists, the value will be null. (applies only to student enrollments, and only available in course endpoints)", +# "example": 95.80, +# "type": "float" +# }, +# "current_period_computed_final_score": { +# "description": "optional: The student's score in the course for the current grading period, including ungraded assignments with a score of 0. If the course the enrollment belongs to does not have Multiple Grading Periods enabled, or if no currently active grading period exists, the value will be null. (applies only to student enrollments, and only available in course endpoints)", +# "example": 85.25, +# "type": "float" +# }, +# "current_period_computed_current_grade": { +# "description": "optional: The letter grade equivalent of current_period_computed_current_score, if available. If the course the enrollment belongs to does not have Multiple Grading Periods enabled, or if no currently active grading period exists, the value will be null. (applies only to student enrollments, and only available in course endpoints)", +# "example": "A", +# "type": "string" +# }, +# "current_period_computed_final_grade": { +# "description": "optional: The letter grade equivalent of current_period_computed_final_score, if available. If the course the enrollment belongs to does not have Multiple Grading Periods enabled, or if no currently active grading period exists, the value will be null. (applies only to student enrollments, and only available in course endpoints)", +# "example": "B", +# "type": "string" # } # } # } diff --git a/app/controllers/gradebooks_controller.rb b/app/controllers/gradebooks_controller.rb index 1bdb9511511..cb94b32a58f 100644 --- a/app/controllers/gradebooks_controller.rb +++ b/app/controllers/gradebooks_controller.rb @@ -240,7 +240,7 @@ class GradebooksController < ApplicationController @current_grading_period_id = params[:grading_period_id].to_i else return if view_all_grading_periods? - current = GradingPeriod.for(@context).find(&:current?) + current = GradingPeriod.current_period_for(@context) @current_grading_period_id = current ? current.id : 0 end end diff --git a/app/models/assignment/filter_with_overrides_by_due_at.rb b/app/models/assignment/filter_with_overrides_by_due_at.rb index 45a795a1ecf..9ec7ad4d6ab 100644 --- a/app/models/assignment/filter_with_overrides_by_due_at.rb +++ b/app/models/assignment/filter_with_overrides_by_due_at.rb @@ -18,6 +18,7 @@ module Assignment::FilterWithOverridesByDueAt def filter_assignments + ActiveRecord::Associations::Preloader.new.preload(assignments, :assignment_overrides) assignments.select { |assignment| in_grading_period?(assignment) } end @@ -51,7 +52,11 @@ module Assignment::FilterWithOverridesByDueAt end end + def any_active_overrides?(assignment) + active_overrides(assignment).any? + end + def no_active_overrides?(assignment) - active_overrides(assignment).none? + !any_active_overrides?(assignment) end end diff --git a/app/models/assignment/filter_with_overrides_by_due_at_for_class.rb b/app/models/assignment/filter_with_overrides_by_due_at_for_class.rb index 9800de02f2d..72ecef74ac8 100644 --- a/app/models/assignment/filter_with_overrides_by_due_at_for_class.rb +++ b/app/models/assignment/filter_with_overrides_by_due_at_for_class.rb @@ -34,7 +34,7 @@ class Assignment::FilterWithOverridesByDueAtForClass end def find_due_at(assignment) - due_at = no_active_overrides?(assignment) ? assignment.due_at : filter_date_from_overrides(assignment) + due_at = any_active_overrides?(assignment) ? filter_date_from_overrides(assignment) : assignment.due_at milliseconds_to_zero(due_at) end diff --git a/app/models/assignment/filter_with_overrides_by_due_at_for_student.rb b/app/models/assignment/filter_with_overrides_by_due_at_for_student.rb index e193b1ad59f..cb879a7db02 100644 --- a/app/models/assignment/filter_with_overrides_by_due_at_for_student.rb +++ b/app/models/assignment/filter_with_overrides_by_due_at_for_student.rb @@ -50,7 +50,8 @@ class Assignment::FilterWithOverridesByDueAtForStudent def most_lenient_due_at(assignment) date_to_use = assignment.due_at if assigned_to_everyone_else?(assignment) - if assignment.has_active_overrides? + + if any_active_overrides?(assignment) override_dates = override_dates_for_student(assignment) return nil if override_dates.any?(&:nil?) diff --git a/app/models/grading_period.rb b/app/models/grading_period.rb index 6cbd3fdee3b..db8de83b513 100644 --- a/app/models/grading_period.rb +++ b/app/models/grading_period.rb @@ -53,6 +53,10 @@ class GradingPeriod < ActiveRecord::Base .grading_periods end + def self.current_period_for(context) + self.for(context).find(&:current?) + end + # Takes a context and a grading_period_id and returns a grading period # if it is in the for collection. Uses Enumberable#find to query # collection. diff --git a/lib/api/v1/course_json.rb b/lib/api/v1/course_json.rb index bfa27a16f97..9d25a78aa38 100644 --- a/lib/api/v1/course_json.rb +++ b/lib/api/v1/course_json.rb @@ -37,7 +37,7 @@ module Api::V1 def to_hash set_sis_course_id(@hash, @course, @user) set_integration_id(@hash, @course, @user) - @hash['enrollments'] = extract_enrollments( @enrollments ) + @hash['enrollments'] = extract_enrollments(@enrollments) @hash['needs_grading_count'] = needs_grading_count(@enrollments, @course) @hash['public_description'] = description(@course) @hash['hide_final_grades'] = @course.hide_final_grades? @@ -82,27 +82,10 @@ module Api::V1 [ :create_discussion_topic, :create_announcement ] if include_permissions end - def extract_enrollments( enrollments ) - if enrollments - enrollments.map do |e| - h = { - :type => e.sis_type, - :role => e.role.name, - :role_id => e.role.id, - :user_id => e.user_id, - :enrollment_state => e.workflow_state - } - h[:associated_user_id] = e.associated_user_id if e.assigned_observer? - if include_total_scores && e.student? - h.merge!( - :computed_current_score => e.computed_current_score, - :computed_final_score => e.computed_final_score, - :computed_current_grade => e.computed_current_grade, - :computed_final_grade => e.computed_final_grade) - end - h - end - end + def extract_enrollments(enrollments) + return unless enrollments + current_period_scores = grading_period_scores_hash(enrollments) + enrollments.map { |e| enrollment_hash(e, current_period_scores) } end INCLUDE_CHECKERS.each do |key, val| @@ -111,8 +94,106 @@ module Api::V1 end end - def include_total_scores + def include_total_scores? @includes.include?(:total_scores) && !@course.hide_final_grades? end + + private + + def enrollment_hash(enrollment, grading_period_scores) + enrollment_hash = default_enrollment_attributes(enrollment) + enrollment_hash[:associated_user_id] = enrollment.associated_user_id if enrollment.assigned_observer? + + if include_total_scores? && enrollment.student? + enrollment_hash.merge!(total_scores(enrollment)) + enrollment_hash.merge!(grading_period_scores[enrollment.id]) if include_current_grading_period_scores? + end + enrollment_hash + end + + def default_enrollment_attributes(enrollment) + { + :type => enrollment.sis_type, + :role => enrollment.role.name, + :role_id => enrollment.role.id, + :user_id => enrollment.user_id, + :enrollment_state => enrollment.workflow_state + } + end + + def total_scores(student_enrollment) + { + :computed_current_score => student_enrollment.computed_current_score, + :computed_final_score => student_enrollment.computed_final_score, + :computed_current_grade => student_enrollment.computed_current_grade, + :computed_final_grade => student_enrollment.computed_final_grade + } + end + + def grading_period_scores(student_enrollments) + mgp_enabled = @course.feature_enabled?(:multiple_grading_periods) + totals_for_all_grading_periods_option = mgp_enabled && + @course.feature_enabled?(:all_grading_periods_totals) + current_period = mgp_enabled && GradingPeriod.current_period_for(@course) + if mgp_enabled && current_period + calculated_grading_period_scores(student_enrollments, current_period, totals_for_all_grading_periods_option) + else + nil_grading_period_scores(student_enrollments, mgp_enabled, totals_for_all_grading_periods_option) + end + end + + def grading_period_scores_hash(enrollments) + include_current_grading_period_scores? ? grading_period_scores(enrollments.select(&:student?)) : {} + end + + def calculated_grading_period_scores(student_enrollments, current_period, totals_for_all_grading_periods_option) + calculator = GradeCalculator.new( + student_enrollments.map(&:user_id), @course, grading_period: current_period + ) + current_period_scores = mgp_scores_from_calculator(calculator) + scores = {} + student_enrollments.each_with_index do |enrollment, index| + scores[enrollment.id] = current_period_scores[index].merge({ + multiple_grading_periods_enabled: true, + totals_for_all_grading_periods_option: totals_for_all_grading_periods_option, + current_grading_period_title: current_period.title + }) + end + scores + end + + + def nil_grading_period_scores(student_enrollments, mgp_enabled, totals_for_all_grading_periods_option) + scores = {} + student_enrollments.each do |enrollment| + scores[enrollment.id] = { + multiple_grading_periods_enabled: mgp_enabled, + totals_for_all_grading_periods_option: totals_for_all_grading_periods_option, + current_grading_period_title: nil, + current_period_computed_current_score: nil, + current_period_computed_final_score: nil, + current_period_computed_current_grade: nil, + current_period_computed_final_grade: nil + } + end + scores + end + + def mgp_scores_from_calculator(grade_calculator) + grade_calculator.compute_scores.map do |scores| + current_score = scores.first.first[:grade] + final_score = scores.second.first[:grade] + { + current_period_computed_current_score: current_score, + current_period_computed_final_score: final_score, + current_period_computed_current_grade: @course.score_to_grade(current_score), + current_period_computed_final_grade: @course.score_to_grade(final_score) + } + end + end + + def include_current_grading_period_scores? + include_total_scores? && @includes.include?(:current_grading_period_scores) + end end end diff --git a/spec/apis/v1/course_json_spec.rb b/spec/apis/v1/course_json_spec.rb index 0f8f618dc98..7486710fdc9 100644 --- a/spec/apis/v1/course_json_spec.rb +++ b/spec/apis/v1/course_json_spec.rb @@ -28,8 +28,8 @@ module Api end - describe '#include_total_scores' do - let(:predicate) { course_json.include_total_scores } + describe '#include_total_scores?' do + let(:predicate) { course_json.include_total_scores? } let(:course_settings) { Hash.new } let(:course) { stub( course_settings ) } diff --git a/spec/apis/v1/courses_api_spec.rb b/spec/apis/v1/courses_api_spec.rb index f0b76e7b0b9..d688dc6c29f 100644 --- a/spec/apis/v1/courses_api_spec.rb +++ b/spec/apis/v1/courses_api_spec.rb @@ -30,7 +30,6 @@ class TestCourseApi end describe Api::V1::Course do - describe '#course_json' do before :once do @test_api = TestCourseApi.new @@ -49,33 +48,33 @@ describe Api::V1::Course do expect(@test_api.course_json(@course1, @me, {}, ['html_url'], [])).to encompass({ "html_url" => "course_url(Course.find(#{@course1.id}), :host => #{HostUrl.context_host(@course1)})" }) - expect(@test_api.course_json(@course1, @me, {}, [], []).has_key?("html_url")).to be_falsey + expect(@test_api.course_json(@course1, @me, {}, [], [])).to_not include 'html_url' end it 'should only include needs_grading_count if requested' do - expect(@test_api.course_json(@course1, @me, {}, [], [teacher_enrollment]).has_key?("needs_grading_count")).to be_falsey + expect(@test_api.course_json(@course1, @me, {}, [], [teacher_enrollment])).to_not include 'needs_grading_count' end it 'should only include is_favorite if requested' do - expect(@test_api.course_json(@course1, @me, {}, ['favorites'], [teacher_enrollment]).key?('is_favorite')).to be_truthy + expect(@test_api.course_json(@course1, @me, {}, ['favorites'], [teacher_enrollment])).to include 'is_favorite' end it 'should honor needs_grading_count for teachers' do - expect(@test_api.course_json(@course1, @me, {}, ['needs_grading_count'], [teacher_enrollment]).has_key?("needs_grading_count")).to be_truthy + expect(@test_api.course_json(@course1, @me, {}, ['needs_grading_count'], [teacher_enrollment])).to include "needs_grading_count" end it 'should return storage_quota_used_mb if requested' do - expect(@test_api.course_json(@course1, @me, {}, ['storage_quota_used_mb'], [teacher_enrollment]).has_key?("storage_quota_used_mb")).to be_truthy + expect(@test_api.course_json(@course1, @me, {}, ['storage_quota_used_mb'], [teacher_enrollment])).to include "storage_quota_used_mb" end it 'should not honor needs_grading_count for designers' do @designer_enrollment = @course1.enroll_designer(@me) @designer_enrollment.accept! - expect(@test_api.course_json(@course1, @me, {}, ['needs_grading_count'], [@designer_enrollment]).has_key?("needs_grading_count")).to be_falsey + expect(@test_api.course_json(@course1, @me, {}, ['needs_grading_count'], [@designer_enrollment])).to_not include "needs_grading_count" end it 'should include apply_assignment_group_weights' do - expect(@test_api.course_json(@course1, @me, {}, [], []).has_key?("apply_assignment_group_weights")).to be_truthy + expect(@test_api.course_json(@course1, @me, {}, [], [])).to include "apply_assignment_group_weights" end it "should not show details if user is restricted from access by course dates" do @@ -401,7 +400,6 @@ describe CoursesController, type: :request do api_call_as_user(parent,:get,"/api/v1/users/#{@me.id}/courses", { :user_id => @me.id, :controller => 'courses', :action => 'user_index', :format => 'json' }, {}, {}, {:expected_status => 401}) - end it "should return courses from observed user's shard if different than observer" do @@ -437,10 +435,8 @@ describe CoursesController, type: :request do course_ids = json.select{ |c| c["id"]} expect(course_ids.length).to eq 2 end - end - it 'should paginate the course list' do json = api_call(:get, "/api/v1/courses.json?per_page=1", { :controller => 'courses', :action => 'index', :format => 'json', :per_page => '1' }) @@ -457,7 +453,7 @@ describe CoursesController, type: :request do expect(json.length).to eq 2 - courses = json.select { |c| c.has_key?("permissions") } + courses = json.select { |c| c.key?("permissions") } expect(courses.length).to eq 0 end @@ -1388,56 +1384,196 @@ describe CoursesController, type: :request do json.each { |course| expect(course['public_syllabus']).to be_truthy } end - it "should include scores in course list if requested" do - @course2.grading_standard_enabled = true - @course2.save - expected_current_score = 80 - expected_final_score = 70 - expected_final_grade = @course2.score_to_grade(expected_final_score) - @course2.all_student_enrollments.update_all( - :computed_current_score => expected_current_score, - :computed_final_score => expected_final_score) + describe "scores" do + before(:once) do + @course2.grading_standard_enabled = true + @course2.save + end - json = api_call(:get, "/api/v1/courses.json", - { :controller => 'courses', :action => 'index', :format => 'json' }, - { :include => ['total_scores'] }) + def courses_api_index_call(includes: ['total_scores']) + api_call( + :get, "/api/v1/courses.json", + { controller: 'courses', action: 'index', format: 'json' }, + { include: includes } + ) + end - # course2 (only care about student) - courses = json.select { |c| c['id'] == @course2.id } - expect(courses.length).to eq 1 - expect(courses[0]).to include('enrollments') - expect(courses[0]['enrollments'].length).to eq 1 - expect(courses[0]['enrollments'][0]).to include( - 'type' => 'student', - 'computed_current_score' => expected_current_score, - 'computed_final_score' => expected_final_score, - 'computed_final_grade' => expected_final_grade, - ) - end + def enrollment(json_response) + course2 = json_response.find { |course| course['id'] == @course2.id } + course2['enrollments'].first + end - it "should not include scores in course list, even if requested, if final grades are hidden" do - @course2.grading_standard_enabled = true - @course2.hide_final_grades = true - @course2.save - @course2.all_student_enrollments.update_all(:computed_current_score => 80, :computed_final_score => 70) + context "include total scores" do + before(:once) do + @course2.all_student_enrollments.update_all( + computed_current_score: 80, + computed_final_score: 70 + ) + end - json = api_call(:get, "/api/v1/courses.json", - { :controller => 'courses', :action => 'index', :format => 'json' }, - { :include => ['total_scores'] }) + it "includes scores in course list if requested" do + json_response = courses_api_index_call + expect(enrollment(json_response)).to include( + 'type' => 'student', + 'computed_current_score' => 80, + 'computed_final_score' => 70, + 'computed_final_grade' => @course2.score_to_grade(70) + ) + end - # course2 (only care about student) - courses = json.select { |c| c['id'] == @course2.id } - expect(courses.length).to eq 1 - expect(courses[0]).to include('enrollments') - expect(courses[0]['enrollments'].length).to eq 1 - expect(courses[0]['enrollments'][0]).to include( - 'type' => 'student', - ) - expect(courses[0]['enrollments'][0]).not_to include( - 'computed_current_score', - 'computed_final_score', - 'computed_final_grade', - ) + it "does not include scores in course list, even if requested, if final grades are hidden" do + @course2.hide_final_grades = true + @course2.save + json_response = courses_api_index_call + enrollment_json = enrollment(json_response) + expect(enrollment_json).to include 'type' => 'student' + expect(enrollment_json).not_to include( + 'computed_current_score', + 'computed_final_score', + 'computed_final_grade' + ) + end + end + + context "include current grading period scores" do + let(:grading_period_keys) do + [ 'multiple_grading_periods_enabled', + 'totals_for_all_grading_periods_option', + 'current_period_computed_current_score', + 'current_period_computed_final_score', + 'current_period_computed_current_grade', + 'current_period_computed_final_grade', + 'current_grading_period_title' ] + end + + before(:once) do + create_grading_periods_for( + @course2, grading_periods: [:old, :current, :future] + ) + end + + it "includes current grading period scores if 'total_scores' " \ + "and 'current_grading_period_scores' are requested" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json).to include *grading_period_keys + current_grading_period_title = 'Course Period 2: current period' + expect(enrollment_json['current_grading_period_title']).to eq(current_grading_period_title) + end + + it "does not include current grading period scores if 'total_scores' are " \ + "not requested, even if 'current_grading_period_scores' are requested" do + json_response = courses_api_index_call(includes: ['current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json).to_not include *grading_period_keys + end + + it "does not include current grading period scores if final grades are hidden, " \ + " even if 'total_scores' and 'current_grading_period_scores' are requested" do + @course2.hide_final_grades = true + @course2.save + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json).to_not include *grading_period_keys + end + + it "returns true for 'multiple_grading_periods_enabled' if the course has Multiple Grading Periods enabled" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json['multiple_grading_periods_enabled']).to eq(true) + end + + it "returns false for 'multiple_grading_periods_enabled' if the course has Multiple Grading Periods disabled" do + @course2.root_account.disable_feature!(:multiple_grading_periods) + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json['multiple_grading_periods_enabled']).to eq(false) + end + + context "computed scores" do + before(:once) do + assignment_in_current_period = @course2.assignments.create!( + title: "In current grading period - graded", + due_at: 2.days.from_now, + points_possible: 10 + ) + assignment_in_current_period.grade_student(@student, grader: @teacher, score: 9) + @course2.assignments.create!( + title: "In current grading period - not graded", + due_at: 2.days.from_now, + points_possible: 10 + ) + end + + context "all assignments for the course fall within the current grading period" do + it "current grading period scores match computed scores" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + + current_period_current_score = enrollment_json['current_period_computed_current_score'] + current_score = enrollment_json['computed_current_score'] + expect(current_period_current_score).to eq(current_score) + + current_period_final_score = enrollment_json['current_period_computed_final_score'] + final_score = enrollment_json['computed_final_score'] + expect(current_period_final_score).to eq(final_score) + end + + it "current grading period grades match computed grades" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + + current_period_current_grade = enrollment_json['current_period_computed_current_grade'] + current_grade = enrollment_json['computed_current_grade'] + expect(current_period_current_grade).to eq(current_grade) + + current_period_final_grade = enrollment_json['current_period_computed_final_grade'] + final_grade = enrollment_json['computed_final_grade'] + expect(current_period_final_grade).to eq(final_grade) + end + end + + context "assignments span across many grading periods" do + before(:once) do + assignment_in_future_grading_period = @course2.assignments.create!( + title: "In future grading period", + due_at: 3.months.from_now, + points_possible: 10 + ) + assignment_in_future_grading_period.grade_student(@student, grader: @teacher, score: 10) + end + + it "current grading period scores and grades do not match computed scores and grades" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + expect(enrollment_json['current_period_computed_current_score']) + .to_not eq(enrollment_json['computed_current_score']) + expect(enrollment_json['current_period_computed_final_score']) + .to_not eq(enrollment_json['computed_final_score']) + expect(enrollment_json['current_period_computed_current_grade']) + .to_not eq(enrollment_json['computed_current_grade']) + expect(enrollment_json['current_period_computed_final_grade']) + .to_not eq(enrollment_json['computed_final_grade']) + end + + it "current grading period scores are correct" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + + expect(enrollment_json['current_period_computed_current_score']).to eq(90) + expect(enrollment_json['current_period_computed_final_score']).to eq(45) + end + + it "current grading period grades are correct" do + json_response = courses_api_index_call(includes: ['total_scores', 'current_grading_period_scores']) + enrollment_json = enrollment(json_response) + + expect(enrollment_json['current_period_computed_current_grade']).to eq('A-') + expect(enrollment_json['current_period_computed_final_grade']).to eq('F') + end + end + end + end end it "should only return teacher enrolled courses on ?enrollment_type=teacher" do @@ -2424,23 +2560,23 @@ describe CoursesController, type: :request do it 'should include permissions' do # Make sure it only returns permissions when asked json = api_call(:get, "/api/v1/courses/#{@course1.id}.json", { :controller => 'courses', :action => 'show', :id => @course1.to_param, :format => 'json' }) - expect(json.has_key?("permissions")).to be_falsey + expect(json).to_not include "permissions" # When its asked to return permissions make sure they are there json = api_call(:get, "/api/v1/courses/#{@course1.id}.json?include[]=permissions", { :controller => 'courses', :action => 'show', :id => @course1.to_param, :format => 'json', :include => [ "permissions" ] }) - expect(json.has_key?("permissions")).to be_truthy + expect(json).to include "permissions" end it 'should include permission create_discussion_topic' do json = api_call(:get, "/api/v1/courses/#{@course1.id}.json?include[]=permissions", { :controller => 'courses', :action => 'show', :id => @course1.to_param, :format => 'json', :include => [ "permissions" ] }) - expect(json.has_key?("permissions")).to be_truthy - expect(json["permissions"].has_key?("create_discussion_topic")).to be_truthy + expect(json).to include "permissions" + expect(json["permissions"]).to include "create_discussion_topic" end it 'should include permission create_announcement' do json = api_call(:get, "/api/v1/courses/#{@course1.id}.json?include[]=permissions", { :controller => 'courses', :action => 'show', :id => @course1.to_param, :format => 'json', :include => [ "permissions" ] }) - expect(json.has_key?("permissions")).to be_truthy - expect(json["permissions"].has_key?("create_announcement")).to be_truthy + expect(json).to include "permissions" + expect(json["permissions"]).to include "create_announcement" expect(json["permissions"]["create_announcement"]).to be_truthy # The setup makes this user a teacher of the course too end @@ -2529,7 +2665,6 @@ describe CoursesController, type: :request do end end - context "course files" do include_examples "file uploads api with folders" include_examples "file uploads api with quotas" @@ -2940,5 +3075,4 @@ describe ContentImportsController, type: :request do expect(json['state']).to eq('completed') expect(json['issues']).to eq(['mock_issue']) end - end diff --git a/spec/models/grading_period_spec.rb b/spec/models/grading_period_spec.rb index 4f5596a2b3a..188e60fbf4e 100644 --- a/spec/models/grading_period_spec.rb +++ b/spec/models/grading_period_spec.rb @@ -74,7 +74,6 @@ describe GradingPeriod do end describe ".for" do - context "when context is an account" do let(:account) { Account.new } let(:finder) { mock } @@ -98,6 +97,30 @@ describe GradingPeriod do end end + describe ".current_period_for" do + let(:account) { Account.new } + let(:not_current_grading_period) { mock } + let(:current_grading_period) { mock } + + it "returns the current grading period given a context" do + GradingPeriod.expects(:for).with(account).returns([not_current_grading_period, current_grading_period]) + not_current_grading_period.expects(:current?).returns(false) + current_grading_period.expects(:current?).returns(true) + expect(GradingPeriod.current_period_for(account)).to eq(current_grading_period) + end + + it "returns nil if grading periods exist for the given context, but none are current" do + GradingPeriod.expects(:for).with(account).returns([not_current_grading_period]) + not_current_grading_period.expects(:current?).returns(false) + expect(GradingPeriod.current_period_for(account)).to be_nil + end + + it "returns nil if no grading periods exist for the given context" do + GradingPeriod.expects(:for).with(account).returns([]) + expect(GradingPeriod.current_period_for(account)).to be_nil + end + end + describe ".context_find" do let(:account) { mock } let(:finder) { mock }