Skip to content

Commit 48ca088

Browse files
author
Eric Berry
committed
Locked graded discussions are now visible to students
fixes CNVS-5309 test steps: - as a teacher, create a few discussion topics with graded checked and due, begin and until dates are in the future, past and current. - as a student, view the discussions list and ensure that the present and past topics are visible but the future is not. - click the past discussion topic and make sure you are able to view the discussion but not reply to it. - click the present discussion and make sure that you can reply to it. - as the student and teacher, ensure that the visible discussion topics appear in the stream items (dashboard) and the future do not (for the student). Change-Id: Ibb0a3d05ae830e764c8068a6c9d4ead804e91139 Reviewed-on: https://gerrit.instructure.com/20082 Tested-by: Jenkins <jenkins@instructure.com> Product-Review: Marc LeGendre <marc@instructure.com> QA-Review: Marc LeGendre <marc@instructure.com> Reviewed-by: Zach Pendleton <zachp@instructure.com>
1 parent 4581193 commit 48ca088

11 files changed

Lines changed: 281 additions & 206 deletions

File tree

app/controllers/assignments_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ def show
7373
js_env :ROOT_OUTCOME_GROUP => outcome_group_json(@context.root_outcome_group, @current_user, session)
7474

7575
@locked = @assignment.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true)
76+
@locked.delete(:lock_at) if @locked.is_a?(Hash) && @locked.has_key?(:unlock_at) # removed to allow proper translation on show page
7677
@unlocked = !@locked || @assignment.grants_rights?(@current_user, session, :update)[:update]
7778
@assignment_module = ContextModuleItem.find_tag_with_preferred([@assignment], params[:module_item_id])
7879
@assignment.context_module_action(@current_user, :read) if @unlocked && !@assignment.new_record?

app/controllers/discussion_topics_controller.rb

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ def index
136136
@context.active_discussion_topics.only_discussion_topics)
137137
scope = scope.by_position
138138
@topics = Api.paginate(scope, self, topic_pagination_url(:only_announcements => params[:only_announcements]))
139-
@topics.reject! { |a| a.locked_for?(@current_user, :check_policies => true) }
140139
@topics.each { |t| t.current_user = @current_user }
141140
if api_request?
142141
render :json => discussion_topics_api_json(@topics, @context, @current_user, session)
@@ -208,12 +207,11 @@ def show
208207
redirect_to named_context_url(@context, :context_discussion_topics_url)
209208
return
210209
end
210+
211211
if authorized_action(@topic, @current_user, :read)
212212
@headers = !params[:headless]
213-
@locked = @topic.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true)
214-
unless @locked
215-
@topic.change_read_state('read', @current_user)
216-
end
213+
@locked = @topic.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true) || @topic.locked?
214+
@topic.change_read_state('read', @current_user)
217215
if @topic.for_group_assignment?
218216
@groups = @topic.assignment.group_category.groups.active.select{ |g| g.grants_right?(@current_user, session, :read) }
219217
topics = @topic.child_topics.to_a
@@ -243,10 +241,10 @@ def show
243241
:ID => @topic.id,
244242
},
245243
:PERMISSIONS => {
246-
:CAN_REPLY => !(@topic.for_group_assignment? || @topic.locked?),
247-
:CAN_ATTACH => @topic.grants_right?(@current_user, session, :attach),
248-
:CAN_MANAGE_OWN => @context.user_can_manage_own_discussion_posts?(@current_user),
249-
:MODERATE => @context.grants_right?(@current_user, session, :moderate_forum)
244+
:CAN_REPLY => @locked ? false : !(@topic.for_group_assignment? || @topic.locked?), # Can reply
245+
:CAN_ATTACH => @locked ? false : @topic.grants_right?(@current_user, session, :attach), # Can attach files on replies
246+
:CAN_MANAGE_OWN => @context.user_can_manage_own_discussion_posts?(@current_user), # Can moderate their own topics
247+
:MODERATE => @context.grants_right?(@current_user, session, :moderate_forum) # Can moderate any topic
250248
},
251249
:ROOT_URL => named_context_url(@context, :api_v1_context_discussion_topic_view_url, @topic),
252250
:ENTRY_ROOT_URL => named_context_url(@context, :api_v1_context_discussion_topic_entry_list_url, @topic),
@@ -419,14 +417,14 @@ def process_discussion_topic(is_new = false)
419417
discussion_topic_hash[:message] = process_incoming_html_content(discussion_topic_hash[:message])
420418
end
421419

422-
#handle locking/unlocking
423-
if params.has_key? :locked
424-
if value_to_boolean(params[:locked])
425-
@topic.lock
426-
else
427-
@topic.unlock
428-
end
429-
end
420+
#handle locking/unlocking
421+
if (params.has_key?(:locked) && !params[:locked].is_a?(Hash))
422+
if value_to_boolean(params[:locked])
423+
@topic.lock
424+
else
425+
@topic.unlock
426+
end
427+
end
430428

431429
if @topic.update_attributes(discussion_topic_hash)
432430
log_asset_access(@topic, 'topics', 'topics', 'participate')

app/helpers/stream_items_helper.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ def categorize_stream_items(stream_items, user = @current_user)
6060
next if item.context_type == "Assignment"
6161
end
6262

63+
if ["DiscussionTopic","Announcement"].include? category
64+
item.data.reload
65+
next if item.data.try(:visible_for?, user) == false
66+
end
67+
6368
categorized_items[category] << generate_presenter(category, item)
6469
end
6570
categorized_items

app/models/assignment.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -780,10 +780,11 @@ def locked_for?(user=nil, opts={})
780780
Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do
781781
locked = false
782782
assignment_for_user = self.overridden_for(user)
783-
if (assignment_for_user.unlock_at && assignment_for_user.unlock_at > Time.now)
784-
locked = {:asset_string => self.asset_string, :unlock_at => assignment_for_user.unlock_at}
785-
elsif (assignment_for_user.lock_at && assignment_for_user.lock_at <= Time.now)
786-
locked = {:asset_string => self.asset_string, :lock_at => assignment_for_user.lock_at}
783+
if ((assignment_for_user.unlock_at && assignment_for_user.unlock_at > Time.now) ||
784+
(assignment_for_user.lock_at && assignment_for_user.lock_at <= Time.now))
785+
locked = { :asset_string => self.asset_string,
786+
:unlock_at => assignment_for_user.unlock_at,
787+
:lock_at => assignment_for_user.lock_at }
787788
elsif self.could_be_locked && item = locked_by_module_item?(user, opts[:deep_check_if_needed])
788789
locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes}
789790
end

app/models/discussion_topic.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,20 @@ def user_name
631631
self.user ? self.user.name : nil
632632
end
633633

634+
def visible_for?(user=nil, opts={})
635+
return true if user == self.user
636+
if unlock_at = locked_for?(user, opts).try_rescue(:[], :unlock_at)
637+
unlock_at < Time.now
638+
else
639+
true
640+
end
641+
end
642+
643+
# Public: Determine if the discussion topic is locked for a specific user. The topic is locked when the
644+
# delayed_post_at is in the future or the group assignment is locked. This does not determine
645+
# the visibility of the topic to the user, only that they are unable to reply.
646+
#
647+
# Returns: boolean
634648
def locked_for?(user=nil, opts={})
635649
return false if opts[:check_policies] && self.grants_right?(user, nil, :update)
636650
Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do

0 commit comments

Comments
 (0)