Skip to content

Commit

Permalink
support group assignments for sg2
Browse files Browse the repository at this point in the history
refs EVAL-3842
flag=platform_service_speedgrader

Test Plan:
1. Enable Platform SpeedGrader
2. Create a group assignment (grading students as
   as group).
3. Go to SpeedGrader and verify the dropdown
   contains group names.
4. Change the student_id query param to be the ID
   of a student that is part of a group but is not
   the group represenative. Then navigate to
   SpeedGrader. Notice the student_id query param
   is changed to the group rep's ID.
5. Change the assignment to grade group students
   individually.
6. Go to SpeedGrader and verify the dropdown
   contains student names.
7. Verify the comments section includes the option
   to send the comment to the individual or the
   group. Try sending a comment to the individual
   and to the group and verify it works.

Change-Id: I3cbc9e57877b2b62ca3a018191483f3b75662451
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/344689
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
QA-Review: Drake Harper <drake.harper@instructure.com>
Product-Review: Cameron Ray <cameron.ray@instructure.com>
  • Loading branch information
spencerolson committed Apr 19, 2024
1 parent b2ff0fe commit 7144a54
Show file tree
Hide file tree
Showing 22 changed files with 481 additions and 29 deletions.
8 changes: 8 additions & 0 deletions app/graphql/interfaces/submission_interface.rb
Expand Up @@ -420,6 +420,14 @@ def assigned_assessments

field :assignment_id, ID, null: false

field :group_id, ID, null: true
def group_id
# Unfortunately, we can't use submissions.group_id, since that value is
# only set once the group has submitted, but not before then. So we have
# to jump through some hoops to load the correct group ID for a submission.
Loaders::SubmissionGroupIdLoader.load(object).then { |group_id| group_id }
end

field :preview_url, String, null: true
def preview_url
Loaders::SubmissionVersionNumberLoader.load(object).then do |version_number|
Expand Down
35 changes: 35 additions & 0 deletions app/graphql/loaders/submission_group_id_loader.rb
@@ -0,0 +1,35 @@
# frozen_string_literal: true

#
# Copyright (C) 2024 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#

class Loaders::SubmissionGroupIdLoader < GraphQL::Batch::Loader
def perform(submissions)
mapping = Group.ids_by_student_by_assignment(*student_and_assignment_ids(submissions))
submissions.each { |sub| fulfill(sub, mapping.dig(sub.assignment_id, sub.user_id)) }
end

private

def student_and_assignment_ids(submissions)
submissions.each_with_object([Set.new, Set.new]) do |sub, (student_ids, assignment_ids)|
student_ids << sub.user_id
assignment_ids << sub.assignment_id
end
end
end
18 changes: 16 additions & 2 deletions app/graphql/mutations/create_submission_comment.rb
Expand Up @@ -21,13 +21,18 @@
class Mutations::CreateSubmissionComment < Mutations::BaseMutation
graphql_name "CreateSubmissionComment"

argument :submission_id, ID, required: true, prepare: GraphQLHelpers.relay_or_legacy_id_prepare_func("Submission")
argument :attempt, Integer, required: false
argument :comment, String, required: true
argument :file_ids, [ID], required: false, prepare: GraphQLHelpers.relay_or_legacy_ids_prepare_func("Attachment")
argument :group_comment,
Boolean,
"Post comment to entire group (only relevant for group assignments grading students individually)",
required: false,
default_value: false
argument :media_object_id, ID, required: false
argument :media_object_type, String, required: false
argument :reviewer_submission_id, ID, required: false, prepare: GraphQLHelpers.relay_or_legacy_id_prepare_func("Submission")
argument :submission_id, ID, required: true, prepare: GraphQLHelpers.relay_or_legacy_id_prepare_func("Submission")

field :submission_comment, Types::SubmissionCommentType, null: true

Expand Down Expand Up @@ -77,7 +82,7 @@ def resolve(input:)
end

assignment = submission.assignment
opts[:group_comment] = assignment.grade_as_group?
opts[:group_comment] = group_comment?(assignment, input)

comment = assignment.add_submission_comment(submission.user, opts).first
comment.mark_read!(current_user)
Expand All @@ -87,4 +92,13 @@ def resolve(input:)
rescue ActiveRecord::RecordNotFound
raise GraphQL::ExecutionError, "not found"
end

private

def group_comment?(assignment, input)
return false unless assignment.has_group_category?
return true unless assignment.grade_group_students_individually?

input[:group_comment]
end
end
20 changes: 14 additions & 6 deletions app/graphql/types/assignment_type.rb
Expand Up @@ -454,6 +454,18 @@ def assignment_overrides
AssignmentOverrideApplicator.overrides_for_assignment_and_user(assignment, current_user)
end

field :has_group_category,
Boolean,
"specifies that this assignment is a group assignment",
method: :has_group_category?,
null: false

field :grade_as_group,
Boolean,
"specifies that students are being graded as a group (as opposed to being graded individually).",
method: :grade_as_group?,
null: false

field :group_set, GroupSetType, null: true
def group_set
load_association(:group_category)
Expand Down Expand Up @@ -495,13 +507,9 @@ def grading_standard
argument :order_by, [SubmissionSearchOrderInputType], required: false
end
def group_submissions_connection(filter: nil, order_by: nil)
return nil if current_user.nil? || assignment.group_category_id.nil?
return nil if assignment.group_category_id.nil?

filter = filter.to_h
order_by ||= []
filter[:states] ||= DEFAULT_SUBMISSION_STATES
filter[:order_by] = order_by.map(&:to_h)
scope = SubmissionSearch.new(assignment, current_user, session, filter).search
scope = submissions_connection(filter:, order_by:)
Promise.all([
Loaders::AssociationLoader.for(Assignment, :submissions).load(assignment),
Loaders::AssociationLoader.for(Assignment, :context).load(assignment)
Expand Down
6 changes: 6 additions & 0 deletions app/graphql/types/submission_search_filter_input_type.rb
Expand Up @@ -30,6 +30,12 @@ class SubmissionSearchFilterInputType < Types::BaseInputObject

argument :include_unsubmitted, Boolean, required: false

argument :representatives_only, Boolean, <<~MD, required: false
For group assignments, include submissions for group representatives only.
Has no effect on non-group assignments or group assignments where students
are being graded individually.
MD

argument :states, [SubmissionStateType], required: false, default_value: DEFAULT_SUBMISSION_STATES
argument :section_ids, [ID], required: false, prepare: GraphQLHelpers.relay_or_legacy_ids_prepare_func("Section")

Expand Down
12 changes: 9 additions & 3 deletions app/models/abstract_assignment.rb
Expand Up @@ -118,10 +118,11 @@ class AbstractAssignment < ActiveRecord::Base
belongs_to :context, polymorphic: [:course]
delegate :moderated_grading_max_grader_count, to: :course
belongs_to :grading_standard
belongs_to :group_category
belongs_to :group_category, inverse_of: :assignments
belongs_to :grader_section, class_name: "CourseSection", optional: true
belongs_to :final_grader, class_name: "User", optional: true
has_many :active_groups, -> { merge(GroupCategory.active).merge(Group.active) }, through: :group_category, source: :groups
has_many :group_memberships, through: :active_groups
has_many :assigned_students, through: :submissions, source: :user
has_many :enrollments_for_assigned_students, -> { active.not_fake.where("enrollments.course_id = submissions.course_id") }, through: :assigned_students, source: :enrollments
has_many :sections_for_assigned_students, -> { active.distinct }, through: :enrollments_for_assigned_students, source: :course_section
Expand Down Expand Up @@ -2629,7 +2630,7 @@ def grade_as_group?
# for group assignments, returns a single "student" for each
# group's submission. the students name will be changed to the group's
# name. for non-group assignments this just returns all visible users
def representatives(user:, includes: [:inactive], group_id: nil, section_id: nil, ignore_student_visibility: false, &block)
def representatives(user:, includes: [:inactive], group_id: nil, section_id: nil, ignore_student_visibility: false, include_others: false, &block)
return visible_students_for_speed_grader(user:, includes:, group_id:, section_id:, ignore_student_visibility:) unless grade_as_group?

submissions = self.submissions.to_a
Expand Down Expand Up @@ -2685,7 +2686,12 @@ def representatives(user:, includes: [:inactive], group_id: nil, section_id: nil
if block
sorted_reps_with_others.each(&block)
end
sorted_reps_with_others.map(&:first)

if include_others
sorted_reps_with_others
else
sorted_reps_with_others.map(&:first)
end
end

def groups_and_ungrouped(user, includes: [])
Expand Down
10 changes: 10 additions & 0 deletions app/models/group.rb
Expand Up @@ -273,6 +273,16 @@ def short_name
name
end

def self.ids_by_student_by_assignment(student_ids, assignment_ids)
GroupMembership.for_assignments(assignment_ids)
.for_students(student_ids)
.pluck("assignments.id", "group_memberships.group_id", "group_memberships.user_id")
.each_with_object({}) do |(assignment_id, group_id, user_id), acc|
acc[assignment_id] ||= {}
acc[assignment_id][user_id] = group_id
end
end

def self.find_all_by_context_code(codes)
ids = codes.filter_map { |c| c.match(/\Agroup_(\d+)\z/)[1] rescue nil }
Group.find(ids)
Expand Down
1 change: 1 addition & 0 deletions app/models/group_category.rb
Expand Up @@ -27,6 +27,7 @@ class GroupCategory < ActiveRecord::Base
belongs_to :context, polymorphic: [:course, :account]
belongs_to :sis_batch
belongs_to :root_account, class_name: "Account", inverse_of: :all_group_categories
has_many :assignments, inverse_of: :group_category
has_many :groups, dependent: :destroy
has_many :progresses, as: "context", dependent: :destroy
has_many :group_and_membership_importers, dependent: :destroy, inverse_of: :group_category
Expand Down
9 changes: 9 additions & 0 deletions app/models/group_membership.rb
Expand Up @@ -52,6 +52,15 @@ class GroupMembership < ActiveRecord::Base
joins(:group).active.where(user_id: users, groups: { context_id: context, workflow_state: "available" })
}

scope :for_assignments, lambda { |ids|
active.joins(group: { group_category: :assignments })
.merge(Group.active)
.merge(GroupCategory.active)
.merge(Assignment.active).where(assignments: { id: ids })
}

scope :for_students, ->(ids) { where(user_id: ids) }

resolves_root_account through: :group

alias_method :context, :group
Expand Down
1 change: 1 addition & 0 deletions app/models/submission.rb
Expand Up @@ -145,6 +145,7 @@ class Submission < ActiveRecord::Base

belongs_to :quiz_submission, class_name: "Quizzes::QuizSubmission"
has_many :all_submission_comments, -> { order(:created_at) }, class_name: "SubmissionComment", dependent: :destroy
has_many :group_memberships, through: :assignment
has_many :submission_comments, -> { order(:created_at).where(provisional_grade_id: nil) }
has_many :visible_submission_comments,
-> { published.visible.for_final_grade.order(:created_at, :id) },
Expand Down
36 changes: 29 additions & 7 deletions lib/submission_search.rb
Expand Up @@ -57,7 +57,7 @@ def add_filters(search_scope)

if @options[:user_id]
# Since user_id requires an ID and not just a partial name, the user_search_scope is not needed and the 3 characters limit is not applied
search_scope = search_scope.where(user_id: @options[:user_id])
search_scope = search_scope.where(user_id: representative_id(@options[:user_id]))
end

if @options[:enrollment_types].present?
Expand Down Expand Up @@ -128,13 +128,35 @@ def add_order_bys(search_scope)
private

def allowed_users
if @options[:apply_gradebook_enrollment_filters]
@course.users_visible_to(@searcher, true, exclude_enrollment_state: excluded_enrollment_states_from_gradebook_settings)
elsif @options[:include_concluded] || @options[:include_deactivated]
@course.users_visible_to(@searcher, true, exclude_enrollment_state: excluded_enrollment_states_from_filters)
else
@course.users_visible_to(@searcher)
users = if @options[:apply_gradebook_enrollment_filters]
@course.users_visible_to(@searcher, true, exclude_enrollment_state: excluded_enrollment_states_from_gradebook_settings)
elsif @options[:include_concluded] || @options[:include_deactivated]
@course.users_visible_to(@searcher, true, exclude_enrollment_state: excluded_enrollment_states_from_filters)
else
@course.users_visible_to(@searcher)
end

if @options[:representatives_only] && @assignment.grade_as_group?
rep_ids = representatives.map { |rep, _members| rep.id }
users = users.where(id: rep_ids)
end

users
end

def representative_id(user_id)
user_id_str = user_id.to_s
return user_id_str unless @assignment.grade_as_group?

rep, = representatives.find do |(rep, members)|
rep.id.to_s == user_id_str || members.any? { |member| member.id.to_s == user_id_str }
end

rep&.id&.to_s || user_id_str
end

def representatives
@representatives ||= @assignment.representatives(user: @searcher, ignore_student_visibility: true, include_others: true)
end

def excluded_enrollment_states_from_gradebook_settings
Expand Down
94 changes: 94 additions & 0 deletions spec/graphql/loaders/submission_group_id_loader_spec.rb
@@ -0,0 +1,94 @@
# frozen_string_literal: true

#
# Copyright (C) 2024 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#

describe Loaders::SubmissionGroupIdLoader do
before do
account = Account.create!
course = account.courses.create!
@first_student = course.enroll_student(User.create!, enrollment_state: "active").user
@second_student = course.enroll_student(User.create!, enrollment_state: "active").user
@third_student = course.enroll_student(User.create!, enrollment_state: "active").user
group_category = course.group_categories.create!(name: "My Category")
@group_a = course.groups.create!(name: "Group A", group_category:)
@group_b = course.groups.create!(name: "Group B", group_category:)
@group_a.add_user(@first_student)
@group_a.save!
@group_b.add_user(@second_student)
@group_b.save!
@assignment = course.assignments.create!(title: "Example Assignment", group_category:)
end

let(:group_a_submission) { @assignment.submissions.find_by(user: @first_student) }
let(:group_b_submission) { @assignment.submissions.find_by(user: @second_student) }
let(:ungrouped_submission) { @assignment.submissions.find_by(user: @third_student) }
let(:loader) { Loaders::SubmissionGroupIdLoader }

it "returns the group id associated with the user, before the group has submitted" do
GraphQL::Batch.batch do
loader.load(group_a_submission).then do |group_id|
expect(group_id).to eq @group_a.id
end

loader.load(group_b_submission).then do |group_id|
expect(group_id).to eq @group_b.id
end
end
end

it "returns the group id associated with the user, after the group has submitted" do
@assignment.submit_homework(@first_student, body: "help my legs are stuck under my desk!")
@assignment.submit_homework(@second_student, body: "hello world!")

GraphQL::Batch.batch do
loader.load(group_a_submission).then do |group_id|
expect(group_id).to eq @group_a.id
end

loader.load(group_b_submission).then do |group_id|
expect(group_id).to eq @group_b.id
end
end
end

it "returns nil for users not in groups" do
GraphQL::Batch.batch do
loader.load(ungrouped_submission).then do |group_id|
expect(group_id).to be_nil
end
end
end

it "returns nil for non-group assignments" do
@assignment.update!(group_category: nil)
GraphQL::Batch.batch do
loader.load(group_a_submission).then do |group_id|
expect(group_id).to be_nil
end

loader.load(group_b_submission).then do |group_id|
expect(group_id).to be_nil
end

loader.load(ungrouped_submission).then do |group_id|
expect(group_id).to be_nil
end
end
end
end

0 comments on commit 7144a54

Please sign in to comment.