Add post policy to update and create course API endpoints

Please note that this PS also updates the permissions
checked for setting a course's post policy through the
Graphql mutation.  This is to ensure REST and Graphql
permissions are consistent to eliminate confusion.

closes OUT-6401
flag=none

test plan:
- Test plan:
- Follow instructions in
https://instructure.atlassian.net/wiki/spaces/ENG/pages/40206418/Canvas+API
to set up Postman OR log in to your local canvas as an admin
- Testing Creating a Course (without post_manually)
- Past the below after your canvas domain which will call the create a
  course API endpoint
  /api/v1/accounts/{account id}/courses?course[name]=PostPolicyCourseFalse
- Take note of the course id returned in the json response
- Navigate the course -> grades -> click on the gear icon -> choose
  "Grade posting policy" and assert the default setting of "Automatically"
  is set for the course
- Create another course using the same API endpoint but add post_manually
  course parameter with a value of true
  /api/v1/accounts/{account id}/courses?course[name]=PostPolicyCourseTrue
    &course[post_manually]=true
- Take note of the course id returned in the json response
- Navigate the course -> grades -> click on the gear icon -> choose
  "Grade posting policy" and assert the default setting of "Manually"
  is set for the course
- Testing Updating a Course
- Use the course id from the first course created and update the course
  name to "PostPolicyCourseFalseUpdated"
  /api/v1/courses/{course id}?course[post_manually]=true
- Navigate the course -> grades -> click on the gear icon -> choose
  "Grade posting policy" and assert the default setting of "Manually"
  is set for the course
- Use the course id from the second course created and update the course
  post policy to false
  /api/v1/courses/{course id}?course[post_manually]=false
- Navigate the course -> grades -> click on the gear icon -> choose
  "Grade posting policy" and assert the default setting of "Automatically"
  is set for the course
- Use the course id from the second course created and update the name
  only to ensure update performs as expected without post_manually
  parameter
  /api/v1/courses/{course id}?course[name]=SomethingElse
- Navigate to the course and notice the name has been updated
- Testing permissions up on graphql endpoint
- navigate to Graphql interface on your local instance
  http://{canvas domain}/graphiql
- Run the below mutation on the first course created
 mutation MyMutation {
  __typename
  setCoursePostPolicy(input: {courseId: "{first course created id}",
                              postManually: true})
  {
    postPolicy {
      _id
      postManually
      }
    }
  }
- Assert that the course post policy has been updated to manually
- Log in as a student and perform the same mutation
- Assert in the response you receive an error message saying "not found"
- Log out as student and log in as an instructor that is not
  enrolled in the first created course
- Perform the same mutation and assert in the response you receive an
  error message saying "not found"
- Log in as the instructor of the course, perform the same mutation
  and assert the mutation runs successfully

Change-Id: I46d35138aacc8d324c6c642550f5dac5f5f0a9b9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/353158
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Dave Wenzlick <david.wenzlick@instructure.com>
QA-Review: Dave Wenzlick <david.wenzlick@instructure.com>
Product-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
This commit is contained in:
Chrystal Langston 2024-06-26 14:28:49 -04:00
parent 21d3e2eaee
commit 4622427706
5 changed files with 118 additions and 13 deletions

View File

@ -865,6 +865,11 @@ class CoursesController < ApplicationController
# @argument course[course_format] [String] # @argument course[course_format] [String]
# Optional. Specifies the format of the course. (Should be 'on_campus', 'online', or 'blended') # Optional. Specifies the format of the course. (Should be 'on_campus', 'online', or 'blended')
# #
# @argument course[post_manually] [Boolean]
# Default is false.
# When true, all grades in the course must be posted manually, and will not be automatically posted.
# When false, all grades in the course will be automatically posted.
#
# @argument enable_sis_reactivation [Boolean] # @argument enable_sis_reactivation [Boolean]
# When true, will first try to re-activate a deleted course with matching sis_course_id if possible. # When true, will first try to re-activate a deleted course with matching sis_course_id if possible.
# #
@ -894,6 +899,12 @@ class CoursesController < ApplicationController
sis_course_id = params[:course].delete(:sis_course_id) sis_course_id = params[:course].delete(:sis_course_id)
apply_assignment_group_weights = params[:course].delete(:apply_assignment_group_weights) apply_assignment_group_weights = params[:course].delete(:apply_assignment_group_weights)
# params_for_create is used to build the course object
# since post_manually is not an actual attribute of the course object,
# we need to remove it from the params. We will use it later to
# apply the post policy to the course after the course is saved.
post_manually = params_for_create.delete(:post_manually) if params_for_create.key?(:post_manually)
# accept end_at as an alias for conclude_at. continue to accept # accept end_at as an alias for conclude_at. continue to accept
# conclude_at for legacy support, and return conclude_at only if # conclude_at for legacy support, and return conclude_at only if
# the user uses that name. # the user uses that name.
@ -931,6 +942,7 @@ class CoursesController < ApplicationController
@course.account = @sub_account if @sub_account @course.account = @sub_account if @sub_account
end end
end end
@course ||= (@sub_account || @account).courses.build(params_for_create) @course ||= (@sub_account || @account).courses.build(params_for_create)
if can_manage_sis if can_manage_sis
@ -971,6 +983,11 @@ class CoursesController < ApplicationController
# force the user to refresh the page after the job finishes to see the changes # force the user to refresh the page after the job finishes to see the changes
@course.sync_homeroom_participation @course.sync_homeroom_participation
end end
# Cannot set the course PostPolicy until the course has saved a PostPolicy
# is a polymorphic association
@course.apply_post_policy!(post_manually: value_to_boolean(post_manually)) unless post_manually.nil?
format.html { redirect_to @course } format.html { redirect_to @course }
format.json do format.json do
render json: course_json( render json: course_json(
@ -3026,6 +3043,11 @@ class CoursesController < ApplicationController
# @argument course[conditional_release] [Boolean] # @argument course[conditional_release] [Boolean]
# Enable or disable individual learning paths for students based on assessment # Enable or disable individual learning paths for students based on assessment
# #
# @argument course[post_manually] [Boolean]
# When true, all grades in the course will be posted manually.
# When false, all grades in the course will be automatically posted.
# Use with caution as this setting will override any assignment level post policy.
#
# @argument override_sis_stickiness [boolean] # @argument override_sis_stickiness [boolean]
# Default is true. If false, any fields containing “sticky” changes will not be updated. # Default is true. If false, any fields containing “sticky” changes will not be updated.
# See SIS CSV Format documentation for information on which fields can have SIS stickiness # See SIS CSV Format documentation for information on which fields can have SIS stickiness
@ -3150,6 +3172,14 @@ class CoursesController < ApplicationController
update_grade_passback_setting(grade_passback_setting) update_grade_passback_setting(grade_passback_setting)
end end
if params_for_update.key?(:post_manually)
@course.apply_post_policy!(post_manually: value_to_boolean(params_for_update[:post_manually]))
# attributes in params_for_update will be applied to the course
# since post_manually is not an attribute on the Course model, it needs to be removed
params_for_update.delete :post_manually
end
unless @course.account.grants_right? @current_user, session, :manage_storage_quotas unless @course.account.grants_right? @current_user, session, :manage_storage_quotas
params_for_update.delete :storage_quota params_for_update.delete :storage_quota
params_for_update.delete :storage_quota_mb params_for_update.delete :storage_quota_mb
@ -4237,7 +4267,8 @@ class CoursesController < ApplicationController
:friendly_name, :friendly_name,
:enable_course_paces, :enable_course_paces,
:default_due_time, :default_due_time,
:conditional_release :conditional_release,
:post_manually
) )
end end

View File

@ -55,6 +55,10 @@ class Mutations::BaseMutation < GraphQL::Schema::Mutation
raise GraphQL::ExecutionError, "not found" unless obj.grants_right?(current_user, session, perm) raise GraphQL::ExecutionError, "not found" unless obj.grants_right?(current_user, session, perm)
end end
def verify_any_authorized_actions!(obj, perms)
raise GraphQL::ExecutionError, "not found" unless obj.grants_any_right?(current_user, session, *Array(perms))
end
# TODO: replace this with model validation where applicable # TODO: replace this with model validation where applicable
def validation_error(message, attribute: "message") def validation_error(message, attribute: "message")
{ {

View File

@ -33,7 +33,10 @@ class Mutations::SetCoursePostPolicy < Mutations::BaseMutation
raise GraphQL::ExecutionError, "A course with that id does not exist" raise GraphQL::ExecutionError, "A course with that id does not exist"
end end
verify_authorized_action!(course, :manage_grades) # checking if the current user has manage_content or
# manage_course_content_edit permission in the current course
perms = %i[manage_content manage_course_content_edit]
verify_any_authorized_actions!(course, perms)
course.apply_post_policy!(post_manually: input[:post_manually]) course.apply_post_policy!(post_manually: input[:post_manually])
{ post_policy: course.default_post_policy } { post_policy: course.default_post_policy }

View File

@ -2519,6 +2519,38 @@ describe CoursesController do
expect(Course.find(json["id"]).grade_passback_setting).to eq "nightly_sync" expect(Course.find(json["id"]).grade_passback_setting).to eq "nightly_sync"
end end
describe "post policy" do
it "sets to true" do
post "create",
params: {
account_id: @account.id,
course: {
name: "new course with post policy set to true",
post_manually: true
}
},
format: :json
json = response.parsed_body
expect(Course.find(json["id"]).post_manually?).to be true
end
it "sets to false" do
post "create",
params: {
account_id: @account.id,
course: {
name: "new course with post policy set to false",
post_manually: false
}
},
format: :json
json = response.parsed_body
expect(Course.find(json["id"]).post_manually?).to be false
end
end
it "does not allow visibility to be set when we don't have permission" do it "does not allow visibility to be set when we don't have permission" do
@visperm.enabled = false @visperm.enabled = false
@visperm.save @visperm.save
@ -3017,6 +3049,24 @@ describe CoursesController do
expect(@course.account_id).to eq account2.id expect(@course.account_id).to eq account2.id
end end
describe "post policy" do
before do
user_session(@teacher)
end
it "updates to true" do
put "update", params: { id: @course.id, course: { post_manually: true } }
@course.reload
expect(@course.post_manually?).to be true
end
it "updates to false" do
put "update", params: { id: @course.id, course: { post_manually: false } }
@course.reload
expect(@course.post_manually?).to be false
end
end
describe "touching content when public visibility changes" do describe "touching content when public visibility changes" do
before do before do
user_session(@teacher) user_session(@teacher)

View File

@ -23,7 +23,7 @@ require_relative "../graphql_spec_helper"
describe Mutations::SetCoursePostPolicy do describe Mutations::SetCoursePostPolicy do
let(:assignment) { course.assignments.create! } let(:assignment) { course.assignments.create! }
let(:course) { Course.create!(workflow_state: "available") } let(:course) { Course.create!(workflow_state: "available", root_account: Account.default) }
let(:student) { course.enroll_user(User.create!, "StudentEnrollment", enrollment_state: "active").user } let(:student) { course.enroll_user(User.create!, "StudentEnrollment", enrollment_state: "active").user }
let(:teacher) { course.enroll_user(User.create!, "TeacherEnrollment", enrollment_state: "active").user } let(:teacher) { course.enroll_user(User.create!, "TeacherEnrollment", enrollment_state: "active").user }
@ -56,7 +56,7 @@ describe Mutations::SetCoursePostPolicy do
CanvasSchema.execute(mutation_str, context:) CanvasSchema.execute(mutation_str, context:)
end end
context "when user has manage_grades permission" do context "when user has permission (aka teacher role)" do
let(:context) { { current_user: teacher } } let(:context) { { current_user: teacher } }
it "requires that courseId be passed in the query" do it "requires that courseId be passed in the query" do
@ -79,7 +79,7 @@ describe Mutations::SetCoursePostPolicy do
end end
it "returns the related post policy" do it "returns the related post policy" do
result = execute_query(mutation_str(course_id: course.id, post_manually: true), context) result = execute_query(mutation_str(course_id: course.id, post_manually: true), { current_user: teacher })
policy = PostPolicy.find_by(course:, assignment: nil) policy = PostPolicy.find_by(course:, assignment: nil)
expect(result.dig("data", "setCoursePostPolicy", "postPolicy", "_id").to_i).to be policy.id expect(result.dig("data", "setCoursePostPolicy", "postPolicy", "_id").to_i).to be policy.id
end end
@ -139,17 +139,34 @@ describe Mutations::SetCoursePostPolicy do
end end
end end
context "when user does not have manage_grades permission" do context "when user does not have proper permissions" do
let(:context) { { current_user: student } } shared_examples "user cannot interact with endpoint" do
it "returns an error" do
result = execute_query(mutation_str(course_id: course.id, post_manually: true), context)
expect(result.dig("errors", 0, "message")).to eql "not found"
end
it "returns an error" do it "does not return data for the related post policy" do
result = execute_query(mutation_str(course_id: course.id, post_manually: true), context) result = execute_query(mutation_str(course_id: course.id, post_manually: true), context)
expect(result.dig("errors", 0, "message")).to eql "not found" expect(result.dig("data", "setCoursePostPolicy")).to be_nil
end
end end
it "does not return data for the related post policy" do describe "when user is a student" do
result = execute_query(mutation_str(course_id: course.id, post_manually: true), context) let(:context) { { current_user: student } }
expect(result.dig("data", "setCoursePostPolicy")).to be_nil
it_behaves_like "user cannot interact with endpoint"
end
describe "when user is a teacher" do
let(:context) { { current_user: teacher } }
before do
Account.default.role_overrides.create!(role: Role.find_by(name: "TeacherEnrollment"), permission: "manage_content", enabled: false)
Account.default.role_overrides.create!(role: Role.find_by(name: "TeacherEnrollment"), permission: "manage_course_content_edit", enabled: false)
end
it_behaves_like "user cannot interact with endpoint"
end end
end end
end end