## What and why `WorkItems::TypesFramework::HasType#validate_work_item_type_id` runs on every save of any model that includes `HasType` — `Issue`, `WorkItems::UserPreference`, `Visibility`, `VisibilityDefault`, `Mapping`, `TypeCustomLifecycle`, `TypeCustomField`. Its current implementation has two problems: 1. **It fires on every save, not just when the type is changing.** Once type visibility controls are live, a work item can legitimately have a disabled type (the type was disabled after the work item was created — we keep existing records as-is by design). If a user edits the title of such a work item, this validation fires, calls the Provider, gets back a `NamespacedType` where `can_user_create_items?` is `false`, and blocks the save. The work item becomes uneditable through no fault of the user. 2. **The check is too weak — and the error message is wrong.** `valid_work_item_type_id?` currently just checks `.present?` on the Provider result. The error `"must use a valid work item type ID"` talks about the ID, not the type, and gives the caller no useful signal. **Reference:** Original discussion in !221910 --- ## Implementation split (decided 2026-05-08) This issue is split into two pieces after investigating where the `can_user_create_items?` enforcement belongs. ### Part 1: HasType concern — `will_save_change` guard + error message (standalone) **MR:** !235290 - Add `will_save_change_to_work_item_type_id?` guard so the validation skips when the type isn't changing - Inline `valid_work_item_type_id?` (no external callers), improve error message - Existence check only at the concern level — appropriate for all 7 HasType models ### Part 2: BuildService — `can_user_create_items?` enforcement (depends on !234516) The `can_user_create_items?` check (blocking creation/type-change with disabled types) belongs in `Issues::BuildService`, not at the model level. The model-level approach fires on every `save!` including factory creation, seeds, imports, and system operations where visibility enforcement is inappropriate. After !234516 (Agnes's MR) merges, the extracted `resolve_work_item_type` method provides the natural insertion point. The check goes there, alongside the existing `create_issue_type_allowed?` logic. This part also includes the `importing?` skip for import paths (see Import/export interaction below). --- ## Server-side enforcement for type visibility This is the server-side enforcement layer for the type visibility controls work (epic [WS6](https://gitlab.com/groups/gitlab-org/-/work_items/20061), proposal #581946). Today, the entire creation flow — `Issues::BuildService`, `Issues::CreateService`, `WorkItems::CreateService`, and the GraphQL mutations — does not check whether a type is `enabled?` or whether `can_user_create_items?` returns `true`. The `NamespacedType` carries the correct visibility state from the Provider's SIWAR resolution, but nothing in the write path reads it. Visibility is currently advisory only — the frontend uses `enabled` and `canUserCreateItems` to hide UI elements, but the backend does not enforce it. The enforcement belongs at the service layer (`BuildService`), not the model layer. The service has proper context about whether this is a user-initiated creation. The model validation covers all 7 HasType models — most are configuration records (UserPreference, Visibility, Mapping, etc.) where creation semantics don't apply. --- ## Import/export interaction The `can_user_create_items?` enforcement in Part 2 must **not** block imports. The import contract agreed in [#586188](https://gitlab.com/gitlab-org/gitlab/-/work_items/586188#note_3328776746) is "import all + best-effort + audit trail." Imports should land records, not drop them. Three import paths create issues/work items: - **CSV import** — goes through `Issues::CreateService` -> `BuildService` - **Direct Transfer (BulkImports)** — bypasses services entirely, uses `RelationFactory` -> `save!` - **File-based export/import** — same as Direct Transfer ### Resolution For Part 2 (BuildService enforcement), the `importing?` skip pattern applies at the service level — `BuildService` can check whether the creation is an import context and skip the visibility check. This is on-pattern with how services already handle import-specific behaviour. **Full context and rationale:** [comment on #586188](https://gitlab.com/gitlab-org/gitlab/-/work_items/586188#note_3328776746) --- ## Interaction with !234391 !234391 (MR 3d, merged) enriches `NamespacedType#enabled?` to incorporate `!archived?` and `visible_in_context?`. `can_user_create_items?` automatically gains those checks — archived types and context-mismatched types (e.g. Epic in a project) return `false`. --- ## Related - Proposal: https://gitlab.com/gitlab-org/gitlab/-/work_items/581946 - Epic: https://gitlab.com/groups/gitlab-org/-/work_items/20061 - Import contract: #586188 ([comment](https://gitlab.com/gitlab-org/gitlab/-/work_items/586188#note_3328776746)) - Part 1 MR: !235290 - Part 2 depends on: !234516 (Agnes — raise error on unresolvable type)