Skip to content

Commit 40524e5

Browse files
committed
don't set last_reply_at for imported discussions
test plan: * create a discussion * note the "last post" date on the discussions index * copy the course * the new discussion should not have a "last post" closes #CNVS-14623 Change-Id: I1ac54a9023a5e9a574e886b2b37bc945c74fe30f Reviewed-on: https://gerrit.instructure.com/51212 Tested-by: Jenkins Reviewed-by: Jeremy Stanley <jeremy@instructure.com> QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com> Product-Review: James Williams <jamesw@instructure.com>
1 parent 0f54a4d commit 40524e5

7 files changed

Lines changed: 21 additions & 7 deletions

File tree

app/coffeescripts/views/DiscussionTopics/DiscussionView.coffee

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ define [
152152
if @model.get('locked') and !_.intersection(ENV.current_user_roles, ['teacher', 'ta', 'admin']).length
153153
base.permissions.delete = false
154154

155-
base.display_last_reply_at = I18n.l "#date.formats.medium", base.last_reply_at
155+
if base.last_reply_at
156+
base.display_last_reply_at = I18n.l "#date.formats.medium", base.last_reply_at
156157
base.ENV = ENV
157158
base.discussion_topic_menu_tools = ENV.discussion_topic_menu_tools
158159
_.each base.discussion_topic_menu_tools, (tool) =>

app/models/discussion_entry.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ def infer_root_entry_id
260260

261261
def update_topic
262262
if self.discussion_topic
263-
last_reply_at = [self.discussion_topic.last_reply_at, self.created_at].max
263+
last_reply_at = [self.discussion_topic.last_reply_at, self.created_at].compact.max
264264
DiscussionTopic.where(:id => self.discussion_topic_id).update_all(:last_reply_at => last_reply_at, :updated_at => Time.now.utc)
265265
end
266266
end

app/models/discussion_topic.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class DiscussionTopic < ActiveRecord::Base
3232
:plaintext_message, :podcast_enabled, :podcast_has_student_posts,
3333
:require_initial_post, :threaded, :discussion_type, :context, :pinned, :locked,
3434
:group_category
35-
attr_accessor :user_has_posted
35+
attr_accessor :user_has_posted, :saved_by
3636

3737
module DiscussionTypes
3838
SIDE_COMMENT = 'side_comment'
@@ -750,7 +750,7 @@ def self.per_page
750750

751751
def initialize_last_reply_at
752752
self.posted_at ||= Time.now.utc
753-
self.last_reply_at ||= Time.now.utc
753+
self.last_reply_at ||= Time.now.utc unless self.saved_by == :migration
754754
end
755755

756756
set_policy do

app/models/importers/discussion_topic_importer.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def find_or_create_topic(topic = nil)
6767
else
6868
context.discussion_topics.scoped.new
6969
end
70+
topic.saved_by = :migration
7071
topic
7172
end
7273

@@ -90,7 +91,7 @@ def run
9091
item.posted_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:posted_at])
9192
item.delayed_post_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options.delayed_post_at)
9293
item.lock_at = Canvas::Migration::MigratorHelper.get_utc_time_from_timestamp(options[:lock_at])
93-
item.last_reply_at = item.posted_at if item.new_record?
94+
item.last_reply_at = nil if item.new_record?
9495

9596
if options[:workflow_state].present?
9697
item.workflow_state = options[:workflow_state]
@@ -115,6 +116,7 @@ def run
115116
item.save_without_broadcasting!
116117
import_migration_item
117118
add_missing_content_links(missing_links)
119+
item.saved_by = nil
118120
item
119121
end
120122

app/views/jst/DiscussionTopics/discussion.handlebars

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
{{/if}}
2323
<a href="{{html_url}}" class="title">{{title}}</a>
2424
</h3>
25-
<small {{ contextSensitiveDatetimeTitle last_reply_at }}>{{#t "last_post_date"}}Last post {{display_last_reply_at}}{{/t}}</small>
25+
{{#if display_last_reply_at}}
26+
<small {{ contextSensitiveDatetimeTitle last_reply_at }}>{{#t "last_post_date"}}Last post {{display_last_reply_at}}{{/t}}</small>
27+
{{/if}}
2628
</div>
2729

2830
<div class="discussion-date-available">

spec/models/content_migration/course_copy_discussions_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def graded_discussion_topic
2929
attrs = ["title", "message", "discussion_type", "type", "pinned", "position", "require_initial_post"]
3030
expect(topic.attributes.slice(*attrs)).to eq new_topic.attributes.slice(*attrs)
3131

32-
expect(new_topic.last_reply_at.to_i).to eq new_topic.posted_at.to_i
32+
expect(new_topic.last_reply_at).to be_nil
3333
expect(topic.posted_at.to_i).to eq new_topic.posted_at.to_i
3434
end
3535

spec/models/discussion_entry_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,15 @@
241241
@topic.reload
242242
expect(@topic.last_reply_at).to eq @original_last_reply_at
243243
end
244+
245+
it "should still work with no last_reply_at" do
246+
@topic.saved_by = :migration
247+
@topic.last_reply_at = nil
248+
@topic.save!
249+
250+
@entry.reload
251+
@entry.update_topic
252+
end
244253
end
245254

246255
context "deleting entry" do

0 commit comments

Comments
 (0)