ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

enable discussion attachments by default 

support has reported a lot of extra load surrounding the fact
that teachers expect students to be able to attach files to
dicussion posts by default and that the user flow to enable this
option is not immediately apparent.

we should update this to allow students to be able to attach
files to discussion posts by default.

Test Plan:
* as a teacher, create an assignment
* as a student in this assignment, navigate to the discussion page
* when creating a reply, in the bottom left of the reply panel
  should be an "Attach" button
* you should be able to attach and submit the reply

* as a teacher, navigate to the discussion page of the assignment
* in the top right, next to the new discussion button, click on
  the gear icon
* the "Attach files to discussion" option should be checked
* uncheck the "Attach files to discussion" option

* as a student, navigate to the discussions page
* when creating a reply, you should no longer have the "Attach"
  button in the bottom left and can no longer submit an attachment

fixes COMMS-2226

Change-Id: Id2df8d6cb9b9a5bc37316a7d26249dd0e663c8cc
Reviewed-on: https://gerrit.instructure.com/201823
Tested-by: Jenkins
Reviewed-by: Steven Burnett <sburnett@instructure.com>
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Ryan Norton <rnorton@instructure.com>
This commit is contained in:
Ryan Norton 2019-07-18 12:43:42 -06:00
parent b5f387843f
commit 45961e84b8
9 changed files with 60 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/models/course.rb
View File

@ -2949,6 +2949,7 @@ class Course < ActiveRecord::Base
add_setting :allow_final_grade_override, boolean: false, default: false
add_setting :allow_student_discussion_topics, :boolean => true, :default => true
add_setting :allow_student_discussion_editing, :boolean => true, :default => true
add_setting :allow_student_forum_attachments, :boolean => true, :default => true
add_setting :show_total_grade_as_points, :boolean => true, :default => false
add_setting :filter_speed_grader_by_student_group, boolean: true, default: false
add_setting :lock_all_announcements, :boolean => true, :default => false, :inherited => true

4
spec/apis/v1/courses_api_spec.rb
View File

@ -3588,7 +3588,7 @@ describe CoursesController, type: :request do
expect(json).to eq({
'allow_final_grade_override' => false,
'allow_student_discussion_topics' => true,
'allow_student_forum_attachments' => false,
'allow_student_forum_attachments' => true,
'allow_student_discussion_editing' => true,
'filter_speed_grader_by_student_group' => false,
'grading_standard_enabled' => false,
@ -3682,7 +3682,7 @@ describe CoursesController, type: :request do
expect(json).to eq({
'allow_final_grade_override' => false,
'allow_student_discussion_topics' => true,
'allow_student_forum_attachments' => false,
'allow_student_forum_attachments' => true,
'allow_student_discussion_editing' => true,
'filter_speed_grader_by_student_group' => false,
'grading_standard_enabled' => false,

2
spec/apis/v1/discussion_topics_api_spec.rb
View File

@ -119,7 +119,7 @@ describe Api::V1::DiscussionTopics do
expect(data[:permissions][:attach]).to eq true # teachers can always attach
data = @test_api.discussion_topic_api_json(@topic, @topic.context, @student, nil)
expect(data[:permissions][:attach]).to eq false # students can't attach by default
expect(data[:permissions][:attach]).to eq true # students can attach by default
@topic.context.update_attribute(:allow_student_forum_attachments, true)
AdheresToPolicy::Cache.clear

4
spec/controllers/discussion_entries_controller_spec.rb
View File

@ -85,6 +85,8 @@ describe DiscussionEntriesController do
end
it "should NOT attach a file if not authorized" do
@course.allow_student_forum_attachments = false
@course.save!
user_session(@student)
post 'create', params: {:course_id => @course.id, :discussion_entry => {:discussion_topic_id => @topic.id, :message => "yo"}, :attachment => {:uploaded_data => default_uploaded_data}}
expect(assigns[:topic]).to eql(@topic)
@ -159,6 +161,8 @@ describe DiscussionEntriesController do
end
it "should not replace the file to the entry if not authorized" do
@course.allow_student_forum_attachments = false
@course.save!
user_session(@student)
put 'update', params: {:course_id => @course.id, :id => @entry.id, :discussion_entry => {:message => "ahem"}, :attachment => {:uploaded_data => default_uploaded_data}}
expect(response).to be_redirect

2
spec/lib/api/v1/course_spec.rb
View File

@ -35,7 +35,7 @@ describe Api::V1::Course do
@course.save
course_settings = course_settings_json(@course)
expect(course_settings[:allow_student_discussion_topics]).to eq true
expect(course_settings[:allow_student_forum_attachments]).to eq false
expect(course_settings[:allow_student_forum_attachments]).to eq true
expect(course_settings[:allow_student_discussion_editing]).to eq true
expect(course_settings[:grading_standard_enabled]).to eq true
expect(course_settings[:grading_standard_id]).to eq grading_standard.id

12
spec/models/course_spec.rb
View File

@ -457,6 +457,18 @@ describe Course do
end
end
describe 'allow_student_forum_attachments' do
it 'should default to true' do
expect(@course.allow_student_forum_attachments).to eq true
end
it 'should allow setting and getting' do
@course.allow_student_forum_attachments = false
@course.save!
expect(@course.allow_student_forum_attachments).to eq false
end
end
describe "allow_student_discussion_topics" do
it "should default true" do

2
spec/models/discussion_topic_spec.rb
View File

@ -237,7 +237,7 @@ describe DiscussionTopic do
it "should not grant moderate permissions without read permissions" do
@course.account.role_overrides.create!(:role => teacher_role, :permission => 'read_forum', :enabled => false)
expect(@topic.reload.check_policy(@teacher2)).to eql [:create]
expect(@topic.reload.check_policy(@teacher2)).to eql [:create, :attach]
end
it "should grant permissions if it not locked" do

24
spec/selenium/discussions/discussions_new_page_spec.rb
View File

@ -195,12 +195,19 @@ describe "discussions" do
expect(f("#content")).not_to contain_css('#topic_publish_button')
end
it "should not show file attachment if allow_student_forum_attachments is not true", priority: "2", test_id: 223507 do
it 'should not show file attachment if allow_student_forum_attachments is not true', priority: '2', test_id: 223507 do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = false
course.save!
# expect
get url
expect(f("#content")).not_to contain_css('#disussion_attachment_uploaded_data')
# when
end
it 'should show file attachment if allow_student_forum_attachments is true', priority: '2' do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = true
course.save!
# expect
@ -208,15 +215,22 @@ describe "discussions" do
expect(f('#discussion_attachment_uploaded_data')).not_to be_nil
end
context "in a group" do
context 'in a group' do
let(:url) { "/groups/#{group.id}/discussion_topics/new" }
it "should not show file attachment if allow_student_forum_attachments is not true", priority: "2", test_id: 223508 do
it 'should not show file attachment if allow_student_forum_attachments is not true', priority: '2', test_id: 223508 do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = false
course.save!
# expect
get url
expect(f("#content")).not_to contain_css('label[for=discussion_attachment_uploaded_data]')
# when
end
it 'should show file attachment if allow_student_forum_attachments is true', priority: '2' do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = true
course.save!
# expect

24
spec/selenium/discussions/rcs/discussions_new_page_spec.rb
View File

@ -170,12 +170,19 @@ describe "discussions" do
expect(f("#content")).not_to contain_css('#topic_publish_button')
end
it "should not show file attachment if allow_student_forum_attachments is not true", priority: "2", test_id: 223507 do
it 'should not show file attachment if allow_student_forum_attachments is not true', priority: '2', test_id: 223507 do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = false
course.save!
# expect
get url
expect(f("#content")).not_to contain_css('#disussion_attachment_uploaded_data')
# when
end
it 'should show file attachment if allow_student_forum_attachments is true', priority: '2' do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = true
course.save!
# expect
@ -183,15 +190,22 @@ describe "discussions" do
expect(f('#discussion_attachment_uploaded_data')).not_to be_nil
end
context "in a group" do
context 'in a group' do
let(:url) { "/groups/#{group.id}/discussion_topics/new" }
it "should not show file attachment if allow_student_forum_attachments is not true", priority: "2", test_id: 223508 do
it 'should not show file attachment if allow_student_forum_attachments is not true', priority: '2', test_id: 223508 do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = false
course.save!
# expect
get url
expect(f("#content")).not_to contain_css('label[for=discussion_attachment_uploaded_data]')
# when
end
it 'should show file attachment if allow_student_forum_attachments is true', priority: '2' do
skip_if_safari(:alert)
# given
course.allow_student_forum_attachments = true
course.save!
# expect