Skip to content

Commit dde9ee8

Browse files
committed
announcements rss feed shows correct list of visible announcements
fixes CNVS-19529 test plan: - in a course, create the following types of announcements: * a normal announcement that is visible * an announcement that is post_delayed for the future * an announcement that is closed for comments (create announcement, go to show page and close for comments in the cog menu) * an announcement that is deleted (create and the delete) - view the announcement rss feed as a teacher and a student - the teacher should see everything but the deleted discussion - the student should just see the normal one and the closed for comments one Change-Id: I46dd583c01cbd7817f3d599cb3816bf386cf3266 Reviewed-on: https://gerrit.instructure.com/51095 Tested-by: Jenkins Reviewed-by: Mike Nomitch <mnomitch@instructure.com> Reviewed-by: John Corrigan <jcorrigan@instructure.com> QA-Review: Adam Stone <astone@instructure.com> Product-Review: Simon Williams <simon@instructure.com>
1 parent 8376f3d commit dde9ee8

5 files changed

Lines changed: 41 additions & 13 deletions

File tree

app/controllers/announcements_controller.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ def show
4949

5050
def public_feed
5151
return unless get_feed_context
52-
announcements = @context.announcements.active.order('posted_at DESC').limit(15).reject{|a| a.locked_for?(@current_user, :check_policies => true) }
52+
announcements = @context.announcements.active.order('posted_at DESC').limit(15).
53+
select{|a| a.visible_for?(@current_user) }
5354

5455
respond_to do |format|
5556
format.atom {

app/controllers/discussion_topics_controller.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,8 @@ def public_feed
722722
f.id = polymorphic_url([@context, :discussion_topics])
723723
end
724724
@entries = []
725-
@entries.concat @context.discussion_topics.reject{|a| a.locked_for?(@current_user, :check_policies => true) }
725+
@entries.concat @context.discussion_topics.
726+
select{|dt| dt.visible_for?(@current_user) }
726727
@entries.concat @context.discussion_entries.active
727728
@entries = @entries.sort_by{|e| e.updated_at}
728729
@entries.each do |entry|

app/models/discussion_topic.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -968,10 +968,9 @@ def available_for?(user, opts = {})
968968
# Public: Determine if the given user can view this discussion topic.
969969
#
970970
# user - The user attempting to view the topic (default: nil).
971-
# options - Options passed to the locked_for? call (default: {}).
972971
#
973972
# Returns a boolean.
974-
def visible_for?(user = nil, options = {})
973+
def visible_for?(user = nil)
975974
# user is the topic's author
976975
return true if user == self.user
977976

lib/api/v1/discussion_topics.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ module Api::V1::DiscussionTopics
4141
# Returns an array of hashes.
4242
def discussion_topics_api_json(topics, context, user, session, opts={})
4343
topics.inject([]) do |result, topic|
44-
if topic.visible_for?(user, check_policies: true)
44+
if topic.visible_for?(user)
4545
result << discussion_topic_api_json(topic, context, user, session, opts)
4646
end
4747

spec/controllers/announcements_controller_spec.rb

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@
2323
course_with_student(:active_all => true)
2424
end
2525

26-
def course_announcement
27-
@announcement = @course.announcements.create!(
26+
def course_announcement(opts = {})
27+
@announcement = @course.announcements.create!({
2828
:title => "some announcement",
2929
:message => "some message"
30-
)
30+
}.merge(opts))
3131
end
3232

3333
describe "GET 'index'" do
@@ -73,13 +73,40 @@ def course_announcement
7373
end
7474

7575
it "shows the 15 most recent announcements" do
76-
announcements = []
77-
16.times { announcements << course_announcement.id }
78-
announcements.shift # Drop first announcement so we have the 15 most recent
76+
announcement_ids = []
77+
16.times { announcement_ids << course_announcement.id }
78+
announcement_ids.shift # Drop first announcement so we have the 15 most recent
79+
7980
get 'public_feed', :format => 'atom', :feed_code => @enrollment.feed_code
81+
82+
feed_entries = Atom::Feed.load_feed(response.body).entries
83+
feed_entry_ids = feed_entries.map{ |e| e.id.gsub(/.*topic_/, "").to_i }
84+
expect(feed_entry_ids).to match_array(announcement_ids)
85+
end
86+
87+
it "only shows announcements that are visible to the caller" do
88+
normal_ann = @a # from the announcement_model in the before block
89+
closed_for_comments_ann = course_announcement(locked: true)
90+
post_delayed_ann = @course.announcements.build({
91+
title: 'hi',
92+
message: 'blah',
93+
delayed_post_at: 1.day.from_now
94+
})
95+
post_delayed_ann.workflow_state = 'post_delayed'
96+
post_delayed_ann.save!
97+
deleted_ann = course_announcement
98+
deleted_ann.destroy
99+
100+
expect(closed_for_comments_ann).to be_locked
101+
expect(post_delayed_ann).to be_post_delayed
102+
expect(deleted_ann).to be_deleted
103+
visible_announcements = [normal_ann, closed_for_comments_ann]
104+
105+
get 'public_feed', :format => 'atom', :feed_code => @enrollment.feed_code
106+
80107
feed_entries = Atom::Feed.load_feed(response.body).entries
81-
feed_entries.map!{ |e| e.id.gsub(/.*topic_/, "").to_i }
82-
expect(feed_entries).to match_array(announcements)
108+
feed_entry_ids = feed_entries.map{ |e| e.id.gsub(/.*topic_/, "").to_i }
109+
expect(feed_entry_ids).to match_array(visible_announcements.map(&:id))
83110
end
84111
end
85112
end

0 commit comments

Comments
 (0)