stop RSS feeds when a course has concluded

fixes COMMS-751

Test Plan
* Make a course
* Go to the announcements page and be a teacher for that course
* Add real external RSS feeds
* Make sure that new feeds show up from those feeds after running
the delayed job script in ./script/delayed_job
* Conclude the course
* Run the script maybe after new posts have been posted and make
sure new feeds dont appear in the ExternalFeed model

Change-Id: I77d00741b61b15a3757b32a0a046e95fe0fbadc4
Reviewed-on: https://gerrit.instructure.com/140857
Reviewed-by: Venk Natarajan <vnatarajan@instructure.com>
Reviewed-by: Steven Burnett <sburnett@instructure.com>
Tested-by: Jenkins
QA-Review: Landon Gilbert-Bland <lbland@instructure.com>
Product-Review: Pert Eilers <peilers@instructure.com>
This commit is contained in:
Aaron Kc Hsu 2018-02-13 16:00:55 -07:00
parent 1d8549adf1
commit a645be00fc
7 changed files with 153 additions and 13 deletions

View File

@ -1473,6 +1473,12 @@ class Course < ActiveRecord::Base
conclude_at && conclude_at < Time.now.utc && conclude_at > 1.month.ago
end
# People may conclude courses and then unclude them. This is a good alias_method
# to check for in situations where we are dependent on those cases
def inactive?
self.deleted? || self.completed?
end
# Public: Return true if the end date for a course (or its term, if the course doesn't have one) has passed.
#
# Returns boolean or nil.

View File

@ -59,6 +59,10 @@ class ExternalFeed < ActiveRecord::Base
where("external_feeds.consecutive_failures<5 AND external_feeds.refresh_at<?", start).order(:refresh_at)
}
def inactive?
!self.context || self.context.root_account.deleted? || self.context.inactive?
end
def add_rss_entries(rss)
items = rss.items.map{|item| add_entry(item, rss, :rss) }.compact
self.context.add_aggregate_entries(items, self) if self.context && self.context.respond_to?(:add_aggregate_entries)

View File

@ -195,6 +195,10 @@ class Group < ActiveRecord::Base
raise "DONT USE THIS, use .short_name instead" unless Rails.env.production?
end
def inactive?
self.context.deleted? || (self.context.is_a?(Course) && self.context.inactive?)
end
def context_available?
return false unless self.context
case self.context

View File

@ -36,11 +36,10 @@ class ExternalFeedAggregator
feeds.each do |feed|
Shackles.activate(:master) do
if !feed.context || feed.context.root_account.deleted? || feed.context.deleted?
feed.update_attribute(:refresh_at, success_wait_seconds.seconds.from_now)
if feed.inactive?
feed.update_attribute(:refresh_at, inactive_wait_seconds.seconds.from_now)
next
end
process_feed(feed)
end
end
@ -111,6 +110,10 @@ class ExternalFeedAggregator
feed.update_attribute(:refresh_at, failure_wait_seconds.seconds.from_now)
end
def inactive_wait_seconds
Setting.get('external_feed_success_wait_seconds', 48.hours.to_s).to_f
end
def success_wait_seconds
Setting.get('external_feed_success_wait_seconds', 2.hours.to_s).to_f
end

View File

@ -65,6 +65,22 @@ describe Course do
@course.save!
end
it "should correctly identify course as active" do
@course.enrollment_term = EnrollmentTerm.create!(root_account: Account.default, workflow_state: :active)
expect(@course.inactive?).to eq false
end
it "should correctly identify destroyed course as not active" do
@course.enrollment_term = EnrollmentTerm.create!(root_account: Account.default, workflow_state: :active)
@course.destroy!
expect(@course.inactive?).to eq true
end
it "should correctly identify concluded course as not active" do
@course.complete!
expect(@course.inactive?).to eq true
end
describe "#recompute_student_scores" do
it "should use all student ids except concluded and deleted if none are passed" do
@course.save!

View File

@ -33,7 +33,74 @@ describe ExternalFeed do
expect(res[2].title).to eql("The Engine That Does More")
expect(res[3].title).to eql("Astronauts' Dirty Laundry")
end
it "rss feed should be active for active accounts" do
@feed = external_feed_model
course_with_student(course: @course, external_feeds: [@feed]).update!(workflow_state: :active)
@feed.context = @course
expect(@feed.inactive?).to be(false)
end
it "rss feed should be inactive for deleted accounts" do
@feed = external_feed_model
account1 = account_model
course_with_student(:account => account1, :course => @course).update!(workflow_state: :active)
Account.default.update!(workflow_state: :deleted)
expect(@feed.inactive?).to be(true)
end
it "rss feed should be active for concluded courses" do
account1 = account_model
course_with_student(:account => account1, :course => @course).update!(workflow_state: :active)
@feed = external_feed_model(context: @course)
expect(@feed.inactive?).to be(false)
end
it "rss feed should be inactive for concluded courses" do
account1 = account_model
course_with_student(:account => account1, :course => @course)
@feed = external_feed_model(context: @course)
@course.complete!
expect(@feed.inactive?).to be(true)
end
it "rss feed should be inactive for deleted courses" do
account1 = account_model
course_with_student(:account => account1, :course => @course)
@feed = external_feed_model(context: @course)
@course.destroy!
expect(@feed.inactive?).to be(true)
end
it "rss feed should be active for groups with active courses" do
account1 = account_model
course_with_student(:account => account1, :course => @course).update!(workflow_state: :active)
@feed = external_feed_model
@group = group_model(:is_public => true, :context => @course, :external_feeds => [@feed])
@feed.update!(context: @group)
expect(@feed.inactive?).to be(false)
end
it "rss feed should be inactive for groups with active courses" do
account1 = account_model
course_with_student(:account => account1, course: @course)
@feed = external_feed_model
@group = group_model(:is_public => true, :context => @course, :external_feeds => [@feed])
@feed.update!(context: @group)
@course.complete!
expect(@feed.inactive?).to be(true)
end
it "rss feed should be inactive for groups with deleted courses" do
account1 = account_model
course_with_student(:account => account1, course: @course)
@feed = external_feed_model
group_model(:is_public => true, :context => @course, :external_feeds => [@feed])
@feed.update!(context: @group)
@course.destroy!
expect(@feed.inactive?).to be(true)
end
it "should add rss entries as course announcements" do
@course = course_model
@feed = external_feed_model(:context => @course)
@ -50,7 +117,7 @@ describe ExternalFeed do
@feed.add_rss_entries(rss)
expect(@course.announcements.count).to eql(4)
end
it "should add atom entries" do
@feed = external_feed_model
require 'atom'
@ -61,7 +128,7 @@ describe ExternalFeed do
expect(res[0].valid?).to be_truthy
expect(res[0].title).to eql("Atom-Powered Robots Run Amok")
end
it "should add atom entries as course announcements" do
@course = course_model
@feed = external_feed_model(:context => @course)
@ -94,7 +161,7 @@ end
def atom_example
%{<?xml version="1.0" encoding="utf-8"?>
<feed xmlns="http://www.w3.org/2005/Atom">
<title>Example Feed</title>
<subtitle>A subtitle.</subtitle>
<link href="http://example.org/feed/" rel="self"/>
@ -105,7 +172,7 @@ def atom_example
<email>johndoe@example.com</email>
</author>
<id>urn:uuid:60a76c80-d399-11d9-b91C-0003939e0af6</id>
<entry>
<title>Atom-Powered Robots Run Amok</title>
<link href="http://example.org/2003/12/13/atom03"/>
@ -113,7 +180,7 @@ def atom_example
<updated>2003-12-13T18:30:02Z</updated>
<summary>Some text.</summary>
</entry>
</feed>}
end
@ -132,7 +199,7 @@ def rss_example
<managingEditor>editor@example.com</managingEditor>
<webMaster>webmaster@example.com</webMaster>
<ttl>5</ttl>
<item>
<title>Star City</title>
<link>http://liftoff.msfc.nasa.gov/news/2003/news-starcity.asp</link>
@ -142,7 +209,7 @@ def rss_example
<pubDate>Tue, 03 Jun 2003 09:39:21 GMT</pubDate>
<guid>http://liftoff.msfc.nasa.gov/2003/06/03.html#item573</guid>
</item>
<item>
<title>Space Exploration</title>
<link>http://liftoff.msfc.nasa.gov/</link>
@ -151,7 +218,7 @@ def rss_example
<pubDate>Fri, 30 May 2003 11:06:42 GMT</pubDate>
<guid>http://liftoff.msfc.nasa.gov/2003/05/30.html#item572</guid>
</item>
<item>
<title>The Engine That Does More</title>
<link>http://liftoff.msfc.nasa.gov/news/2003/news-VASIMR.asp</link>
@ -161,7 +228,7 @@ def rss_example
<pubDate>Tue, 27 May 2003 08:37:32 GMT</pubDate>
<guid>http://liftoff.msfc.nasa.gov/2003/05/27.html#item571</guid>
</item>
<item>
<title>Astronauts' Dirty Laundry</title>
<link>http://liftoff.msfc.nasa.gov/news/2003/news-laundry.asp</link>

View File

@ -61,6 +61,46 @@ describe Group do
expect(@group.time_zone.to_s).to match /Mountain Time/
end
it "should correctly identify group as active" do
course_with_student(:active_all => true)
group_model(:group_category => @communities, :is_public => true)
group.add_user(@student)
expect(@group.inactive?).to eq false
end
it "should correctly identify destroyed course as not active" do
course_with_student(:active_all => true)
group_model(:group_category => @communities, :is_public => true)
group.add_user(@student)
@group.context = @course
@course.destroy!
expect(@group.inactive?).to eq true
end
it "should correctly identify concluded course as not active" do
course_with_student(:active_all => true)
group_model(:group_category => @communities, :is_public => true)
group.add_user(@student)
@group.context = @course
@course.complete!
expect(@group.inactive?).to eq true
end
it "should correctly identify account group as not active" do
@account = account_model
group_model(:group_category => @communities, :is_public => true, :context => @account)
group.add_user(@student)
@group.context.destroy
expect(@group.inactive?).to eq true
end
it "should correctly identify account group as active" do
@account = account_model
group_model(:group_category => @communities, :is_public => true, :context => @account)
group.add_user(@student)
expect(@group.inactive?).to eq false
end
context "#peer_groups" do
it "should find all peer groups" do
context = course_model