Fix side_comment discussion type

refs VICE-4538
flag=discussion_create

test plan:
1.turn off the discussion_create feature flag
2.create a discussion with a comment
3.create threaded replies
4.turn on discussion_create feature flag
5.verify that the discussion's discussion type is threaded
6.verify that on the edit page the Disallow threaded discussions
checkbox is disabled

Change-Id: Ic03a753e32bc94defa512b1b554e55eb8f8b000f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/354113
Reviewed-by: Roland Beres <roland.beres@instructure.com>
QA-Review: Roland Beres <roland.beres@instructure.com>
Product-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This commit is contained in:
Dora, Csolakov 2024-08-01 14:07:59 +02:00 committed by Roland Beres
parent 1b70e5ec27
commit 1ced7965a9
5 changed files with 37 additions and 14 deletions

View File

@ -219,7 +219,7 @@ class DiscussionTopic < ActiveRecord::Base
end end
def threaded? def threaded?
discussion_type == DiscussionTypes::THREADED || (root_account&.feature_enabled?(:discussion_checkpoints) && checkpoints?) discussion_type == DiscussionTypes::THREADED || (root_account&.feature_enabled?(:discussion_checkpoints) && checkpoints?) || (DiscussionTypes::SIDE_COMMENT && discussion_entries.where.not(parent_id: nil).where.not(workflow_state: "deleted").exists?)
end end
alias_method :threaded, :threaded? alias_method :threaded, :threaded?

View File

@ -596,18 +596,7 @@ describe DiscussionEntry do
course_with_teacher course_with_teacher
end end
it "forces a root entry as parent if the discussion isn't threaded" do it "allows a sub-entry as parent" do
discussion_topic_model
root = @topic.reply_from(user: @teacher, text: "root entry")
sub1 = root.reply_from(user: @teacher, html: "sub entry")
expect(sub1.parent_entry).to eq root
expect(sub1.root_entry).to eq root
sub2 = sub1.reply_from(user: @teacher, html: "sub-sub entry")
expect(sub2.parent_entry).to eq root
expect(sub2.root_entry).to eq root
end
it "allows a sub-entry as parent if the discussion is threaded" do
discussion_topic_model(threaded: true) discussion_topic_model(threaded: true)
root = @topic.reply_from(user: @teacher, text: "root entry") root = @topic.reply_from(user: @teacher, text: "root entry")
sub1 = root.reply_from(user: @teacher, html: "sub entry") sub1 = root.reply_from(user: @teacher, html: "sub entry")

View File

@ -250,6 +250,21 @@ describe DiscussionTopic do
expect(@course.discussion_topics.first.message).to eql("<a href=\"#\">only this should stay</a>") expect(@course.discussion_topics.first.message).to eql("<a href=\"#\">only this should stay</a>")
end end
it "side-comment discussion type is threaded when it has threaded replies" do
topic = @course.discussion_topics.create!(message: "test")
topic.discussion_type = "side_comment"
entry = topic.discussion_entries.create!(message: "test")
entry.reply_from(user: @student, html: "reply 1")
expect(topic.threaded?).to be true
end
it "side-comment discussion type is not threaded when it does not have threaded replies" do
topic = @course.discussion_topics.create!(message: "test")
topic.discussion_type = "side_comment"
topic.discussion_entries.create!(message: "test")
expect(topic.threaded?).to be false
end
it "defaults to not_threaded type" do it "defaults to not_threaded type" do
d = DiscussionTopic.new d = DiscussionTopic.new
expect(d.discussion_type).to eq "not_threaded" expect(d.discussion_type).to eq "not_threaded"

View File

@ -190,7 +190,7 @@ function DiscussionTopicForm({
currentDiscussionTopic?.isAnonymousAuthor || true currentDiscussionTopic?.isAnonymousAuthor || true
) )
const [isThreaded, setIsThreaded] = useState( const [isThreaded, setIsThreaded] = useState(
currentDiscussionTopic?.discussionType === "threaded" || Object.keys(currentDiscussionTopic).length === 0 currentDiscussionTopic?.discussionType === "threaded" || (currentDiscussionTopic?.discussionType === "side_comment" && ENV?.DISCUSSION_TOPIC?.ATTRIBUTES?.has_threaded_replies) || Object.keys(currentDiscussionTopic).length === 0
) )
const [requireInitialPost, setRequireInitialPost] = useState( const [requireInitialPost, setRequireInitialPost] = useState(
currentDiscussionTopic?.requireInitialPost || false currentDiscussionTopic?.requireInitialPost || false

View File

@ -505,6 +505,25 @@ describe('DiscussionTopicForm', () => {
}) })
}) })
describe('Disallow threaded replies', () => {
it('disallow threaded replies checkbox is checked when discussion type is side comment and does not has threaded reply', () => {
window.ENV.DISCUSSION_TOPIC.ATTRIBUTES.has_threaded_replies = false
const {getByTestId} = setup({currentDiscussionTopic: {discussionType: "side_comment"}})
const checkbox = getByTestId('disallow_threaded_replies')
expect(checkbox.checked).toBe(true)
})
it('disallow threaded replies checkbox is disabled when discussion type is side comment and has threaded replies', () => {
window.ENV.DISCUSSION_TOPIC.ATTRIBUTES.has_threaded_replies = true
const {getByTestId} = setup({currentDiscussionTopic: {discussionType: "side_comment"}})
const checkbox = getByTestId('disallow_threaded_replies')
expect(checkbox.disabled).toBe(true)
expect(checkbox.checked).toBe(false)
})
})
describe('Graded', () => { describe('Graded', () => {
it('does not allow the automatic peer review per student input to go below 1', () => { it('does not allow the automatic peer review per student input to go below 1', () => {
const {getByTestId, getByLabelText} = setup() const {getByTestId, getByLabelText} = setup()