Skip to content

FEATURE: Allow QnA mode on any topic. #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<label class="topic-composer-enable-qa">
{{input type="checkbox" checked=model.isQuestion}}
{{i18n "topic.set_as_qa"}}
</label>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { CREATE_TOPIC } from "discourse/models/composer";

export default {
shouldRender(args, component) {
return (
component.siteSettings.qa_enabled_globally &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this dependent on siteSettings.qa_enabled too?

args.model.action === CREATE_TOPIC
);
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { withPluginApi } from "discourse/lib/plugin-api";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not running for me locally. I think it's due to the extension, i.e. it should be .js.es6?


export default {
name: "composer-topic-qa-mode",

initialize(container) {
const siteSettings = container.lookup("site-settings:main");
const currentUser = container.lookup("current-user:main");

if (
siteSettings.qa_enabled &&
siteSettings.qa_enabled_globally &&
currentUser
) {
withPluginApi("0.12.3", (api) => {
api.serializeOnCreate("is_question", "isQuestion");
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More a matter of opinion, but you could add api.serializeToDraft here to keep the state between drafts.

}
},
};
22 changes: 15 additions & 7 deletions assets/stylesheets/common/question-answer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@
margin: 0;
}

.time-gap + .topic-post.answer, .time-gap + .topic-post.comment {
.topic-avatar, .topic-body {
.time-gap + .topic-post.answer,
.time-gap + .topic-post.comment {
.topic-avatar,
.topic-body {
border-top: 1px solid $primary-low;
}
}

.topic-post.answer+.time-gap, .topic-post.comment+.time-gap {
.topic-post.answer + .time-gap,
.topic-post.comment + .time-gap {
display: none;
}

Expand All @@ -68,7 +71,8 @@ nav.post-controls button.comment {
}
}

&.comment, &.answer {
&.comment,
&.answer {
.post-notice.new-user {
display: none;
}
Expand All @@ -85,7 +89,6 @@ nav.post-controls button.comment {
}

.vote-container {

.vote-links {
text-align: right;

Expand Down Expand Up @@ -121,7 +124,7 @@ nav.post-controls button.comment {

.tip-details {
background-color: dark-light-diff($primary, $secondary, 95%, -85%);
box-shadow: shadow('dropdown');
box-shadow: shadow("dropdown");
padding: 10px 15px;
position: absolute;
top: 32px;
Expand Down Expand Up @@ -161,8 +164,13 @@ html.rtl {
}

.vote-container {
.voters, .vote-links {
.voters,
.vote-links {
text-align: left;
}
}
}

.topic-composer-enable-qa {
margin: 10px 0;
}
1 change: 1 addition & 0 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ en:
vote: "You upvoted this post."

topic:
set_as_qa: "Enable QnA for this topic"
one_to_many:
title: "New Entry"
help: "begin composing a new entry"
Expand Down
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ en:
qa_tl4_vote_limit: "The vote limit for the question and answer plugin for trust level 4 users"
qa_tl_allow_multiple_votes_per_post: "Allow users to vote multiple times for the same post."
qa_blacklist_tags: "Tags to disable QnA on topic"
qa_enabled_globally: "Users can enable QnA on any topic"

post_action_types:
vote:
Expand Down
3 changes: 3 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,6 @@ plugins:
qa_blacklist_tags:
type: list
default: ""
qa_enabled_globally:
default: false
client: true
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class SaveExistingPostVoteCountsToCustomFields < ::ActiveRecord::Migration[5.1]
def up
vote_totals = {}
Expand Down
4 changes: 2 additions & 2 deletions extensions/topic_tag_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ module TopicTagExtension
def self.included(base)
base.after_destroy :update_post_order, if: :qa_tag?
end

def qa_tag?
if tag = Tag.find_by(id: tag_id)
!([tag.name] & SiteSetting.qa_tags.split('|')).empty?
!([tag.name] & SiteSetting.qa_tags.split('|')).empty?
else
false
end
Expand Down
2 changes: 2 additions & 0 deletions lib/question_answer/engine.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module ::QuestionAnswer
class Engine < Rails::Engine
engine_name 'question_answer'
Expand Down
16 changes: 16 additions & 0 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,20 @@ class ::TopicTag
add_to_serializer(:user_card, :vote_count) do
object.vote_count
end

NewPostManager.add_handler do |manager|
is_regular = manager.args[:archetype].nil? || manager.args[:archetype] == Archetype.default
not_warning = !manager.args[:is_warning]
is_question = manager.args[:is_question] == 'true'

if is_regular && not_warning && is_question
manager.args[:subtype] = 'question'
end

false
end

add_permitted_post_create_param(:is_question)
NewPostManager.add_plugin_payload_attribute(:subtype)
TopicSubtype.register('question')
end
2 changes: 1 addition & 1 deletion spec/components/question_answer/topic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
it 'should return correct last_answered_at' do
expected = answers.last.created_at

expect(topic.last_answered_at).to eq(expected)
expect(topic.last_answered_at).to eq_time(expected)
end

it 'should return correct last_commented_on' do
Expand Down
2 changes: 1 addition & 1 deletion spec/jobs/qa_update_topics_post_order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
reply_to_post_number
end

describe Jobs::QAUpdateTopicsPostOrder do
describe Jobs::QaUpdateTopicsPostOrder do
let(:create_post) do
->(topic, post_number, sort_order, reply_to_post_number = nil) do
args = {
Expand Down
2 changes: 1 addition & 1 deletion spec/plugin_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
end
end

require 'rails_helper'
require 'rails_helper'
58 changes: 58 additions & 0 deletions spec/requests/question_answer/posts_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# frozen_string_literal: true

require 'rails_helper'

describe PostsController do
let(:user) { Fabricate(:user) }

describe '#create' do
before do
sign_in(user)
SiteSetting.qa_enabled = true
end

it 'sets the question subtype when the is_question param is set to true' do
post "/posts.json", params: {
raw: 'this is a test question',
title: 'this is the title for the question topic',
is_question: true,
}
end

it 'only sets the question subtype on regular topics' do
another_user = Fabricate(:user)
post "/posts.json", params: {
raw: 'this is a test question',
title: 'this is the title for the question topic',
is_question: true,
target_recipients: another_user.username,
archetype: Archetype.private_message
}

expect(response.status).to eq(200)

created_topic = Topic.last
expect(created_topic.subtype).not_to eq('question')
end

it 'preserves the subtype when a topic gets queued' do
SiteSetting.approve_post_count = 1

post "/posts.json", params: {
raw: 'this is the test content',
title: 'this is the test title for the topic',
is_question: true,
}

expect(response.status).to eq(200)
parsed = response.parsed_body
expect(parsed["action"]).to eq("enqueued")

rp = ReviewableQueuedPost.find_by(created_by: user)
expect(rp).to be_present

result = rp.perform(Discourse.system_user, :approve_post)
expect(result.created_post.topic.subtype).to eq('question')
end
end
end
14 changes: 9 additions & 5 deletions spec/serializers/question_answer/post_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
).as_json
end
end
let(:dependent_keys) { %i[last_answerer last_answered_at answer_count last_answer_post_number] }
let(:obj_keys) { %i[qa_vote_count qa_voted qa_enabled] }

context 'qa enabled' do
Expand Down Expand Up @@ -95,9 +94,12 @@
end

it 'should return correct value from topic' do
dependent_keys.each do |k|
expect(create_serializer.call[k]).to eq(post.topic.public_send(k))
end
serialized = create_serializer.call

expect(serialized.dig(:last_answerer, :id)).to eq(post.topic.last_answerer.id)
expect(serialized[:last_answered_at]).to eq(post.topic.last_answered_at)
expect(serialized[:answer_count]).to eq(post.topic.answer_count)
expect(serialized[:last_answer_post_number]).to eq(post.topic.last_answer_post_number)
end
end

Expand All @@ -109,7 +111,9 @@
end

it 'should not include dependent_keys' do
dependent_keys.each do |k|
keys = %i[last_answerer last_answered_at answer_count last_answer_post_number]

keys.each do |k|
expect(create_serializer.call.has_key?(k)).to eq(false)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
expect(serializer[attr]).to eq(topic.send(attr))
end

expect(serializer[:last_answerer].id).to eq(topic.last_answerer.id)
expect(serializer.dig(:last_answerer, :id)).to eq(topic.last_answerer.id)
end
end

Expand Down