Add category moderation groups and more attributes to generic importer #32561

RubenOussoren · 2025-05-02T15:25:58Z

This PR improves the generic bulk import and base scripts by adding support for several previously unimplemented features and attributes:

Category Moderation Groups: Implements the import of group-based moderation settings for categories.
Group Attributes: Adds import support for public_admission, public_exit, allow_membership_requests, and assignable_level.
User Attributes: Adds import support for trust_level and primary_group_id.
User Option: Adds import support for the hide_profile_and_presence user option.
Multiple Tag Groups per Tag: Updates tag import logic to handle tags belonging to multiple tag groups.

script/bulk_import/generic_bulk.rb

script/bulk_import/base.rb

script/bulk_import/generic_bulk.rb

s3lase · 2025-05-07T12:16:33Z

script/bulk_import/generic_bulk.rb

@@ -562,6 +631,7 @@ def import_user_options
        email_level: row["email_level"],
        email_messages_level: row["email_messages_level"],
        email_digests: row["email_digests"],
+        hide_profile_and_presence: row["hide_profile_and_presence"] || false,


There seems to be some work to make this setting more granular by splitting it into hide_profile and hide_presence, but hide_profile_and_presence still appears to be kept in sync as follows:

discourse/app/models/user_option.rb

Lines 228 to 235 in 4117b08

def update_hide_profile_and_presence

if hide_profile_changed? || hide_presence_changed?

self.hide_profile_and_presence = hide_profile || hide_presence

elsif hide_profile_and_presence_changed?

self.hide_profile = hide_profile_and_presence

self.hide_presence = hide_profile_and_presence

end

end

Simply setting hide_profile_and_presence might be problematic. You could:

Set both hide_profile and hide_presence to the value of hide_profile_and_presence, if it’s explicitly provided in process_user_options

Update the converter framework to support to also support hide_profile and hide_presence and replicate the logic above to keep them in sync

I've made a couple of updates here. Let me know if this would work? This is actually not used in the converter. It was due to an error that was caused during the import that requested this to be added if I remember correctly.

This was the error:

ERROR: null value in column "hide_profile_and_presence" of relation "user_options" violates not-null constraint

script/bulk_import/generic_bulk.rb

…user option, and multi-tag-group support

…ricted flags

…import script

…adge imports

script/bulk_import/base.rb

script/bulk_import/generic_bulk.rb

…ecks

gschlager · 2025-06-02T15:54:16Z

script/bulk_import/base.rb

@@ -76,6 +76,7 @@ def initialize
    @html_entities = HTMLEntities.new
    @encoding = CHARSET_MAP[charset]
    @bbcode_to_md = true if use_bbcode_to_md?
+    @assign_plugin_enabled = defined?(::DiscourseAssign)


Unused variable?

gschlager · 2025-06-03T16:02:20Z

script/bulk_import/generic_bulk.rb

+      group_data = {
        imported_id: row["id"],
        name: row["name"],
        full_name: row["full_name"],
+        public_admission: row["public_admission"] || false,
+        public_exit: row["public_exit"] || false,
+        allow_membership_requests: row["allow_membership_requests"] || false,
        visibility_level: row["visibility_level"],
        members_visibility_level: row["members_visibility_level"],
        mentionable_level: row["mentionable_level"],
        messageable_level: row["messageable_level"],
      }
+      group_data[:assignable_level] = row["assignable_level"]
+      group_data


Suggested change

group_data = {

imported_id: row["id"],

name: row["name"],

full_name: row["full_name"],

public_admission: row["public_admission"] || false,

public_exit: row["public_exit"] || false,

allow_membership_requests: row["allow_membership_requests"] || false,

visibility_level: row["visibility_level"],

members_visibility_level: row["members_visibility_level"],

mentionable_level: row["mentionable_level"],

messageable_level: row["messageable_level"],

}

group_data[:assignable_level] = row["assignable_level"]

group_data

{

imported_id: row["id"],

name: row["name"],

full_name: row["full_name"],

public_admission: row["public_admission"] || false,

public_exit: row["public_exit"] || false,

allow_membership_requests: row["allow_membership_requests"] || false,

visibility_level: row["visibility_level"],

members_visibility_level: row["members_visibility_level"],

mentionable_level: row["mentionable_level"],

messageable_level: row["messageable_level"],

assignable_level: row["assignable_level"]

}

gschlager · 2025-06-03T16:04:51Z

script/bulk_import/generic_bulk.rb

+        hide_profile_and_presence: false,
+        hide_profile: false,
+        hide_presence: false,


Why always false? If this is just about setting defaults, then the USER_OPTION_DEFAULTS hash in BulkImport is the right way to do it.

Please see discussion with @s3lase here - #32561 (comment)

gschlager · 2025-06-03T19:17:01Z

script/bulk_import/generic_bulk.rb

+      if row["tag_group_ids"] && !row["tag_group_ids"].empty?
+        intermediate_group_ids = JSON.parse(row["tag_group_ids"])
+      elsif row["tag_group_id"] && !row["tag_group_id"].empty?
+        # Support old single tag_group_id


I don't see a tag_group_id attribute in the current IntermediateDB anymore. We should only support the lasted version, so reduce complexity. So, unless I'm missing something, we should remove this and make stuff less complicated.

Hey @gschlager, I'm getting mixed messages between you and @s3lase. See his previous message - #32561 (comment)

Here's the tag_group_id and 021-tags.sql in intermediate.db.

gschlager · 2025-06-03T19:18:49Z

script/bulk_import/generic_bulk.rb

+
+      if intermediate_group_ids.any?
+        intermediate_group_ids.each do |intermediate_group_id|
+          intermediate_group_id = intermediate_group_id.to_i if intermediate_group_id.is_a?(String)


You can do this unconditionally:

Suggested change

intermediate_group_id = intermediate_group_id.to_i if intermediate_group_id.is_a?(String)

intermediate_group_id = intermediate_group_id.to_i

But is it really needed? Do we store numbers as JSON strings for some reason?

gschlager · 2025-06-03T19:25:27Z

script/bulk_import/generic_bulk.rb

+
+      UPDATE badges


Suggested change

UPDATE badges

UPDATE badges

RubenOussoren requested a review from s3lase May 2, 2025 15:25

RubenOussoren self-assigned this May 2, 2025

RubenOussoren requested a review from a team as a code owner May 2, 2025 15:25

github-actions bot added the migrations-tooling PR which includes changes to migrations tooling label May 2, 2025

s3lase reviewed May 5, 2025

View reviewed changes

script/bulk_import/generic_bulk.rb Outdated Show resolved Hide resolved

s3lase requested changes May 7, 2025

View reviewed changes

RubenOussoren requested review from s3lase and gschlager May 29, 2025 21:11

RubenOussoren added 6 commits May 29, 2025 17:34

feat(import): Add category moderation groups, user/group attributes, …

64cba80

…user option, and multi-tag-group support

feat(read_restricted): created a new method to properly set read_rest…

7195eb4

…ricted flags

fix added method call

3953988

fix removed old read_restricted references from import_categories

013a520

Refactor: Use core Category methods for read_restricted flag in bulk …

7b97076

…import script

Updated bulk import scripts based on Selase's feedback and extended b…

016ca7a

…adge imports

RubenOussoren force-pushed the dev/import-additional-permissions-with-fixes branch from 979c141 to 016ca7a Compare May 29, 2025 21:35

gschlager reviewed May 29, 2025

View reviewed changes

script/bulk_import/base.rb Outdated Show resolved Hide resolved

gschlager reviewed May 29, 2025

View reviewed changes

script/bulk_import/generic_bulk.rb Outdated Show resolved Hide resolved

Simplify group assignable_level import logic by removing redundant ch…

4640a22

…ecks

RubenOussoren requested a review from gschlager June 2, 2025 15:27

gschlager requested changes Jun 3, 2025

View reviewed changes

RubenOussoren requested a review from gschlager June 3, 2025 22:44

	def update_hide_profile_and_presence
	if hide_profile_changed? \|\| hide_presence_changed?
	self.hide_profile_and_presence = hide_profile \|\| hide_presence
	elsif hide_profile_and_presence_changed?
	self.hide_profile = hide_profile_and_presence
	self.hide_presence = hide_profile_and_presence
	end
	end

	intermediate_group_id = intermediate_group_id.to_i if intermediate_group_id.is_a?(String)
	intermediate_group_id = intermediate_group_id.to_i

Add category moderation groups and more attributes to generic importer #32561

Are you sure you want to change the base?

Add category moderation groups and more attributes to generic importer #32561

Uh oh!

Conversation

RubenOussoren commented May 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!