Skip to content
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

FEATURE: Make General the default category #18383

Merged
merged 8 commits into from
Sep 30, 2022
4 changes: 3 additions & 1 deletion app/assets/javascripts/discourse/app/models/composer.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ const Composer = RestModel.extend({
const oldCategoryId = this._categoryId;

if (isEmpty(categoryId)) {
categoryId = null;
// Set General as the default category
const generalCategoryId = this.siteSettings.general_category_id;
categoryId = generalCategoryId ? generalCategoryId : null;
}
this._categoryId = categoryId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,27 @@ acceptance("Composer", function (needs) {
});
needs.settings({
enable_whispers: true,
general_category_id: 1,
});
needs.site({
can_tag_topics: true,
categories: [
{
id: 1,
name: "General",
slug: "general",
permission: 1,
topic_template: null,
},
{
id: 2,
name: "test too",
slug: "test-too",
permission: 1,
topic_template: "",
},
],
});
needs.site({ can_tag_topics: true });
needs.pretender((server, helper) => {
server.post("/uploads/lookup-urls", () => {
return helper.response([]);
Expand Down Expand Up @@ -62,6 +81,8 @@ acceptance("Composer", function (needs) {
test("Composer is opened", async function (assert) {
await visit("/");
await click("#create-topic");
// Check that General category is selected
assert.strictEqual(selectKit(".category-chooser").header().value(), "1");

assert.strictEqual(
document.documentElement.style.getPropertyValue("--composer-height"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,23 @@ module(
assert.strictEqual(this.subject.header().label(), "category…");
});

test("with allowUncategorized=null and generalCategoryId present", async function (assert) {
this.siteSettings.allow_uncategorized_topics = false;
this.siteSettings.general_category_id = 4;

await render(hbs`
<CategoryChooser
@value={{this.value}}
@options={{hash
allowUncategorized=null
}}
/>
`);

assert.strictEqual(this.subject.header().value(), null);
assert.strictEqual(this.subject.header().label(), "");
});

test("with allowUncategorized=null none=true", async function (assert) {
this.siteSettings.allow_uncategorized_topics = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ export default ComboBoxComponent.extend({
) {
return Category.findUncategorized();
} else {
return this.defaultItem(null, htmlSafe(I18n.t("category.choose")));
const generalCategoryId = this.siteSettings.general_category_id;
if (!generalCategoryId) {
return this.defaultItem(null, htmlSafe(I18n.t("category.choose")));
}
}
},

Expand Down
5 changes: 5 additions & 0 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
s.set_regardless_of_locale(:download_remote_images_to_local, false)
s.set_regardless_of_locale(:unique_posts_mins, 0)
s.set_regardless_of_locale(:max_consecutive_replies, 0)

# Most existing tests were written assuming allow_uncategorized_topics
# was enabled, so we should set it to true.
s.set_regardless_of_locale(:allow_uncategorized_topics, true)

# disable plugins
if ENV['LOAD_PLUGINS'] == '1'
s.set_regardless_of_locale(:discourse_narrative_bot_enabled, false)
Expand Down
3 changes: 2 additions & 1 deletion config/site_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ posting:
max_emojis_in_title: 1
allow_uncategorized_topics:
client: true
default: true
default: false
refresh: true
allow_duplicate_topic_titles: false
allow_duplicate_topic_titles_category: false
Expand Down Expand Up @@ -2291,6 +2291,7 @@ uncategorized:
general_category_id:
default: -1
hidden: true
client: true
meta_category_id:
default: -1
hidden: true
Expand Down
25 changes: 25 additions & 0 deletions db/migrate/20220927171707_disable_allow_uncategorized_new_sites.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

class DisableAllowUncategorizedNewSites < ActiveRecord::Migration[7.0]
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 run this as a post-migration?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure? I used this as an example migration to follow #13175 where they enabled tags only for new sites.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is only for new sites, right? It should be safe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep only for new sites.

def up
result = execute <<~SQL
SELECT created_at
FROM schema_migration_details
ORDER BY created_at
LIMIT 1
SQL

# keep allow uncategorized for existing sites
if result.first['created_at'].to_datetime < 1.hour.ago
execute <<~SQL
INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
VALUES('allow_uncategorized_topics', 5, 't', NOW(), NOW())
ON CONFLICT (name) DO NOTHING
SQL
end
end

def down
raise ActiveRecord::IrreversibleMigration
end
end
2 changes: 1 addition & 1 deletion spec/models/topic_converter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
fab!(:author) { Fabricate(:user) }
fab!(:category) { Fabricate(:category, topic_count: 1) }
fab!(:private_message) { Fabricate(:private_message_topic, user: author) } # creates a topic without a first post
let(:first_post) { create_post(user: author, topic: private_message) }
let(:first_post) { create_post(user: author, topic: private_message, allow_uncategorized_topics: false) }
let(:other_user) { private_message.topic_allowed_users.find { |u| u.user != author }.user }

let(:uncategorized_category) do
Expand Down
6 changes: 6 additions & 0 deletions spec/support/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ def create_topic(args = {})
end

def create_post(args = {})
# Pretty much all the tests with `create_post` will fail without this
# since allow_uncategorized_topics is now false by default
unless args[:allow_uncategorized_topics] == false
SiteSetting.allow_uncategorized_topics = true
end

args[:title] ||= "This is my title #{Helpers.next_seq}"
args[:raw] ||= "This is the raw body of my post, it is cool #{Helpers.next_seq}"
args[:topic_id] = args[:topic].id if args[:topic]
Expand Down