Skip to content

WIP: Refactor user namespace association

What does this MR do?

Refactors the User model's has_one :namespace association to be managed with before_validation and autosave instead of after_save.

It seems better to build/edit/validate namespace at the same time that the user is validated. If namespace validation fails, user.valid? should return false, rather than raising an exception in after_save and rolling back the transaction.

Are there points in the code the reviewer needs to double check?

User and Namespace both perform dynamic_path validation. I've suppressed dynamic_path validation on Namespace if it is associated with a User so it doesn't create duplicate looking errors.

But now I am a little worried that someone might later modify Namespace directly instead of User (this actually happened once recently), so I started adding custom Namespace validation that checks that path and name match the owner.username if kind == 'user'. The problem is a lot of existing code fundamentally assumes Namespace can be used without Group or a User, so this validation causes a lot of non-trivial to fix errors. Should I forge ahead and require Namespace to either be a Group or have a User?

Why was this MR needed?

We were prevented from using a convenient delegate :full_path, to: :namespace, allow_nil: true

because because we create the namespace after_save, which makes namespace nil (which makes full_path nil) during validation on create, causing DynamicPathValidator to fail on user create.

See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10370#note_28936356

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #31896 (moved)

Edited by username-removed-1144264

Merge request reports