## Summary The Organizations user transfer service (`Organizations::Transfer::UsersService`) auto-discovers models to transfer via a dynamic mechanism (`migratable_models`). The discovery heuristic is too permissive: it matches **any model with exactly one `User` association and one `Organization` association**, then moves rows by whatever foreign key that User association happens to use (`created_by_id`, `updated_by_id`, `author_id`, `creator_id`, `user_id`, ...). This conflates **metadata** user associations (who created/updated a record) with **ownership**. The result: records that are actually owned by a namespace/project/parent (and therefore should move with the group during a *group* transfer) are instead dragged along by a *user* transfer — or worse, moved twice / inconsistently. This was first observed with `Ai::Catalog::ItemVersion`: items transferred with the group, but their versions did not, because versions were being transferred by the user service via `created_by_id`. ## Root cause `app/services/organizations/transfer/users_service.rb`: ```ruby def migratable_models ApplicationRecord.descendants.select do |model| !model.to_s.in?(skipped_transfer_models) && model.reflect_on_all_associations.one? { |assoc| assoc.class_name == 'User' } && model.reflect_on_all_associations.one? { |assoc| assoc.class_name == 'Organizations::Organization' } end end def update_associated_organization_ids(user_ids) self.class.migratable_models.each do |model_class| user_assoc = model_class.reflect_on_all_associations.find { |assoc| assoc.class_name == "User" } user_key = user_assoc.foreign_key update_organization_id_for(model_class) do |relation| relation.where(user_key => user_ids) end end end ``` The foreign key is taken from whatever the single User association is, regardless of whether it represents ownership. ## Models currently auto-discovered (24 total) Running `Organizations::Transfer::UsersService.migratable_models` yields 24 models. Grouped by the FK they are moved on: ### Confirmed incorrect — metadata FK or parent-owned record | Model | Table | FK used | Real owner | Problem | |---|---|---|---|---| | `Ai::Catalog::ItemVersion` | `ai_catalog_item_versions` | `created_by_id` | `item` → project/namespace | Original bug. Moves by creator, diverges from its item. | | `Ai::Catalog::ItemStar` | `ai_catalog_item_stars` | `user_id` | `item` → project/namespace | Join record owned by the item. A star should follow its item, not the starring user. (FK *is* `user_id`, so a pure "require user_id" heuristic would NOT catch this.) | | `Ai::Catalog::McpServerBlock` | `ai_catalog_mcp_server_blocks` | `created_by_id` | `mcp_server` + `namespace` | Owned by its server/namespace; moves by creator. | | `Analytics::CustomDashboards::DashboardVersion` | `custom_dashboard_versions` | `updated_by_id` | `dashboard` → namespace | **Double-transferred**: the group service already moves it by dashboard (`ee/app/services/ee/organizations/transfer/groups_service.rb`), and the user service *also* moves it by `updated_by_id`. Conflicting ownership. | | `AwardEmoji` | `award_emoji` | `user_id` | `awardable` (polymorphic) → project/group | A reaction follows its awardable, not the reactor. | | `ResourceEvents::AbuseReportEvent` | `abuse_report_events` | `user_id` | `abuse_report` | Event belongs to the report; should follow it — compare `AntiAbuse::Event`, which is correctly in the skip list and moved by `abuse_report_id`. | ### Ambiguous — needs a product/ownership decision rather than silent discovery | Model | FK | Notes | |---|---|---| | `Ai::Catalog::McpServer` | `created_by_id` | Purely org-level (org + creator, no project/namespace). Not handled by the group service. Should a user transfer move an org resource by its creator? | | `RemoteDevelopment::OrganizationClusterAgentMapping` | `creator_id` | `user` association backed by `creator_id` — pure metadata. Owned by org + agent. | | `Import::PlaceholderUserDetail` | `placeholder_user_id` | Not a real "owning" user. | ### Appear correct (genuinely user-owned, `user_id`/`resource_owner_id`) `Ai::Conversation::Thread`, `Ai::UsageEvent`, `Ai::Catalog::McpServersUser`, `AuthenticationEvent`, `Authz::OauthConsentGrant`, `BulkImport`, `DeployKey`/`Key`/`LDAPKey`, `Import::Offline::Export`, `OauthAccessGrant`, `OauthAccessToken`, `PersonalAccessToken`, `SpamLog`, `PersonalSnippet` (explicitly skip-listed and handled separately). ## Why the "require `user_id`" heuristic alone is not enough Your initial instinct — require a literal `user_id` FK rather than any User association — eliminates the `created_by`/`updated_by`/`creator` cases. But join/child records such as `Ai::Catalog::ItemStar` and `AwardEmoji` legitimately have a `user_id` yet are still owned by a parent (item/awardable) that follows the group. So FK name is necessary but not sufficient. A model should be eligible for **user** transfer only if it is genuinely user-owned, i.e.: 1. The User FK is an ownership FK (`user_id`, plus an explicit allowlist such as `resource_owner_id` for OAuth), **and** 2. It has **no** `belongs_to` to a group/project/namespace-anchored parent — no `project_id`/`namespace_id`/`group_id`, and no FK to a model that is itself group-scoped. ## Immediate correctness bug `Analytics::CustomDashboards::DashboardVersion` is actively double-handled. Both the group service (by dashboard) and the user service (by `updated_by_id`) write `organization_id`. The user-service pass can also move versions whose dashboard did **not** move (authored/updated by a different user), splitting a dashboard's versions across organizations. ## Open question Is a dynamic, reflection-based discovery mechanism the right approach here at all, or will it keep producing these false positives as new models are added? An explicit allowlist (like the group service uses) is more verbose but makes ownership intent reviewable per-model. Worth discussing as part of the fix. ## Proposed next steps - [ ] Decide: keep dynamic discovery with a stricter predicate, or move to an explicit allowlist. - [ ] If keeping dynamic discovery: add the ownership-FK + no-group-parent checks described above. - [ ] Add the confirmed problem models to `skipped_transfer_models` with explanatory comments, and ensure the group service owns the ones that should follow the group (`ItemVersion` versions, `ItemStar`, `McpServerBlock`, `DashboardVersion`, `AwardEmoji`, `AbuseReportEvent`). - [ ] Fix the `DashboardVersion` double-transfer. - [ ] Add a guard spec that fails when a newly added model is auto-discovered via a metadata FK, so this cannot regress silently.