diff --git a/app/controllers/discussion_topics_controller.rb b/app/controllers/discussion_topics_controller.rb index 6df13660657..4b6020141ec 100644 --- a/app/controllers/discussion_topics_controller.rb +++ b/app/controllers/discussion_topics_controller.rb @@ -262,7 +262,13 @@ class DiscussionTopicsController < ApplicationController @context.active_discussion_topics.only_discussion_topics end - scope = params[:order_by] == 'recent_activity' ? scope.by_last_reply_at : scope.by_position_legacy + scope = if params[:order_by] == 'recent_activity' + scope.by_last_reply_at + elsif params[:only_announcements] + scope.by_posted_at + else + scope.by_position_legacy + end scope = DiscussionTopic.search_by_attribute(scope, :title, params[:search_term]) diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index ee4f4da7cf3..ed3e92bec38 100644 --- a/app/models/discussion_topic.rb +++ b/app/models/discussion_topic.rb @@ -469,6 +469,8 @@ class DiscussionTopic < ActiveRecord::Base scope :by_position_legacy, -> { order("discussion_topics.position DESC, discussion_topics.created_at DESC, discussion_topics.id DESC") } scope :by_last_reply_at, -> { order("discussion_topics.last_reply_at DESC, discussion_topics.created_at DESC, discussion_topics.id DESC") } + scope :by_posted_at, -> { order("discussion_topics.posted_at DESC, discussion_topics.delayed_post_at DESC, discussion_topics.created_at DESC, discussion_topics.id DESC") } + scope :visible_to_students_in_course_with_da, lambda { |user_ids, course_ids| without_assignment_in_course(course_ids).union(joins_assignment_student_visibilities(user_ids, course_ids)) } diff --git a/app/models/importers/discussion_topic_importer.rb b/app/models/importers/discussion_topic_importer.rb index 0ffcdf78866..0d7be796edc 100644 --- a/app/models/importers/discussion_topic_importer.rb +++ b/app/models/importers/discussion_topic_importer.rb @@ -88,10 +88,10 @@ module Importers item.message = I18n.t('#discussion_topic.empty_message', 'No message') end - item.posted_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:posted_at]) - item.delayed_post_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options.delayed_post_at) - item.lock_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:lock_at]) - item.last_reply_at = nil if item.new_record? + item.posted_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:posted_at]) + item.delayed_post_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options.delayed_post_at) + item.lock_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:lock_at]) + item.last_reply_at = nil if item.new_record? if options[:workflow_state].present? item.workflow_state = options[:workflow_state] diff --git a/spec/apis/v1/discussion_topics_api_spec.rb b/spec/apis/v1/discussion_topics_api_spec.rb index 2908555cbc5..8fcaf651b9c 100644 --- a/spec/apis/v1/discussion_topics_api_spec.rb +++ b/spec/apis/v1/discussion_topics_api_spec.rb @@ -2413,6 +2413,37 @@ describe DiscussionTopicsController, type: :request do end end end + + it "should order Announcement items by posted_at rather than by position" do + course_with_teacher(:active_all => true) + account_admin_user(account: @course.account) # sets @admin + + ann_ids_ordered_by_posted_at = 10.times.map do |i| + ann = Announcement.create!({ + context: @course, + message: "Test Message", + }) + ann.posted_at = i.days.ago + ann.position = 1 + ann.save! + ann.id + end + + json = api_call( + :get, + "/api/v1/courses/#{@course.id}/discussion_topics?only_announcements=1", + { + controller: "discussion_topics", + action: "index", + format: "json", + course_id: @course.id.to_s, + only_announcements: 1, + }, + {} + ) + + expect(json.map{ |j| j["id"] }).to eq(ann_ids_ordered_by_posted_at) + end end def create_attachment(context, opts={}) diff --git a/spec/controllers/discussion_topics_api_controller.rb b/spec/controllers/discussion_topics_api_controller.rb index 889e71ff692..5a1fb842dcd 100644 --- a/spec/controllers/discussion_topics_api_controller.rb +++ b/spec/controllers/discussion_topics_api_controller.rb @@ -38,6 +38,5 @@ describe DiscussionTopicsApiController do expect(page_view.url).to match %r{^http://test\.host/api/v1/courses/\d+/discussion_topics} expect(page_view.participated).to be_truthy end - end end diff --git a/spec/controllers/discussion_topics_controller_spec.rb b/spec/controllers/discussion_topics_controller_spec.rb index 4ec3cca1b5b..a69d7e49792 100644 --- a/spec/controllers/discussion_topics_controller_spec.rb +++ b/spec/controllers/discussion_topics_controller_spec.rb @@ -42,6 +42,7 @@ describe DiscussionTopicsController do @topic.save @topic end + def topic_entry @entry = @topic.discussion_entries.create(:message => "some message", :user => @user) end @@ -89,6 +90,7 @@ describe DiscussionTopicsController do @assignment.only_visible_to_overrides = true @assignment.save! end + it "should return graded and visible group discussions properly" do cs = @student.enrollments.first.course_section create_section_override_for_assignment(@assignment, {course_section: cs}) diff --git a/spec/models/discussion_topic_spec.rb b/spec/models/discussion_topic_spec.rb index d7a38fe6346..bdc9c5efab6 100644 --- a/spec/models/discussion_topic_spec.rb +++ b/spec/models/discussion_topic_spec.rb @@ -1657,4 +1657,43 @@ describe DiscussionTopic do expect(rss.first.enclosure.url).to match(%r{download.mp4}) end end + + context "announcements" do + context "scopes" do + context "by_posted_at" do + let(:c) { Course.create! } + let(:new_ann) do + lambda do + Announcement.create!({ + context: c, + message: "Test Message", + }) + end + end + + it "sorts by the posted_at field descending" do + anns = 10.times.map do |i| + ann = new_ann.call + ann.posted_at = i.days.ago + ann.position = 1 + ann.save! + ann + end + expect(c.announcements.by_posted_at).to eq(anns) + end + + it "secondarily sorts by the delayed_post_at field descending" do + anns = 10.times.map do |i| + ann = new_ann.call + ann.delayed_post_at = i.days.ago + ann.posted_at = nil + ann.position = 1 + ann.save! + ann + end + expect(c.announcements.by_posted_at).to eq(anns) + end + end + end + end end