Skip to content

Commit c8b88c4

Browse files
committed
properly persist disable announcement comments setting.
fixes CNVS-3810 when re-enabling comments on course announcements, properly persist that change. test plan: * disable announcement comments on a course; * re-enable announcement comments on a course; * verify that the setting persists across page loads. Change-Id: I8c89ca793bc7baa97e0ce11056f739917de4300f Reviewed-on: https://gerrit.instructure.com/17632 Reviewed-by: Jon Jensen <jon@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: Marc LeGendre <marc@instructure.com>
1 parent b50ffa9 commit c8b88c4

4 files changed

Lines changed: 11 additions & 23 deletions

File tree

app/controllers/courses_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ def update
13111311
params[:course][:event] = :offer if params[:offer].present?
13121312

13131313
lock_announcements = params[:course].delete(:lock_all_announcements)
1314-
if lock_announcements
1314+
if value_to_boolean(lock_announcements)
13151315
@course.lock_all_announcements = true
13161316
Announcement.update_all(['workflow_state = ?', 'locked'],
13171317
:context_type => 'Course', :context_id => @course.id, :workflow_state => 'active')

app/models/announcement.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def infer_content
3939
def respect_context_lock_rules
4040
lock if active? &&
4141
context.is_a?(Course) &&
42-
context.settings[:lock_all_announcements]
42+
context.lock_all_announcements?
4343
end
4444
protected :respect_context_lock_rules
4545

app/views/courses/settings.html.erb

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -208,16 +208,6 @@ TEXT
208208
</div>
209209
</td>
210210
</tr>
211-
<tr>
212-
<th scope="row" style="width: 10%;">
213-
<%= before_label('announcements', 'Announcements') %>
214-
</th>
215-
<td>
216-
<div class="course_info announcement_lock_set">
217-
<%= @context.settings[:lock_all_announcements] ? t(:comments_disabled, 'Comments disabled') : t(:comments_enabled, 'Comments enabled') %>
218-
</div>
219-
</td>
220-
</tr>
221211
<tr>
222212
<th scope="row" style="vertical-align: top; width: 10%;"><%= f.blabel :license, :en => "License" %></th>
223213
<td colspan="3">
@@ -260,15 +250,13 @@ TEXT
260250
</span>
261251
<a href="#" class="course_form course_form_more_options_link" style="padding-left: 20px;"><%= t('links.more_options', %{more options}) %></a>
262252
<div class="course_form_more_options" style="display: none; padding-left: 20px;">
263-
<% f.fields_for :settings do |settings| %>
264-
<% if @context.self_enrollment_allowed? %>
265-
<%= f.check_box :self_enrollment, :class => 'self_enrollment_checkbox' %>
266-
<%= f.label :self_enrollment, :en => "Let students self-enroll by sharing with them a secret URL or code" %><br/>
267-
<div class="open_enrollment_holder" style="display: none;">
268-
<%= f.check_box :open_enrollment %>
269-
<%= f.label :open_enrollment, :en => "Add a \"Join this Course\" link to the course home page" %><br/>
270-
</div>
271-
<% end %>
253+
<% if @context.self_enrollment_allowed? %>
254+
<%= f.check_box :self_enrollment, :class => 'self_enrollment_checkbox' %>
255+
<%= f.label :self_enrollment, :en => "Let students self-enroll by sharing with them a secret URL or code" %><br/>
256+
<div class="open_enrollment_holder" style="display: none;">
257+
<%= f.check_box :open_enrollment %>
258+
<%= f.label :open_enrollment, :en => "Add a \"Join this Course\" link to the course home page" %><br/>
259+
</div>
272260
<% end %>
273261
<%= f.check_box :allow_student_forum_attachments %>
274262
<%= f.label :allow_student_forum_attachments, :en => "Let students attach files to discussions" %><br/>
@@ -282,7 +270,7 @@ TEXT
282270
<%= f.label :hide_final_grades, :en => "Hide totals in student grades summary" %><br/>
283271
<%= f.check_box :hide_distribution_graphs %>
284272
<%= f.label :hide_distribution_graphs, :en => "Hide grade distribution graphs from students" %><br/>
285-
<%= check_box_tag 'course[lock_all_announcements]', 1, @context.settings[:lock_all_announcements], :class => 'lock_all_announcements' %>
273+
<%= f.check_box :lock_all_announcements %>
286274
<%= label_tag :course_lock_all_announcements, :en => 'Disable comments on announcements' %><br />
287275
<label for="course_default_wiki_editing_roles">
288276
<%= f.select :default_wiki_editing_roles, [

spec/controllers/courses_controller_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@
689689
course_with_teacher_logged_in(:active_all => true)
690690
@course.lock_all_announcements = true
691691
@course.save!
692-
put 'update', :id => @course.id, :course => { }
692+
put 'update', :id => @course.id, :course => { :lock_all_announcements => 0 }
693693
assigns[:course].lock_all_announcements.should be_false
694694
end
695695

0 commit comments

Comments
 (0)