Skip to content

Fixed cyclical dependency of `custom_project_templates` feature check

Taras Tadai requested to merge 423237-fix-cyclical-dependency into master

What does this MR do and why?

There is plan to move custom_project_templates feature to Registration Features but currently this feature is blocked by the cyclical dependency as License.feature_available? has a cyclical dependency with ApplicationSetting:

Creating default ApplicationSetting -> validates import_sources -> checks License.feature_available?(:custom_project_templates) -> verifies GitlabSubscriptions::Features.usage_ping_feature?(feature) -> verifies Gitlab::CurrentSettings.usage_ping_features_enabled? -> will try create default app settings again as it does not exist yet.

  1. To fix that we append on: :update to the validates_each :import_sources so that we don't validate that at creation as ApplicationSetting.create_from_defaults already provides valid initial values.

  2. Applied additional fix for custom_project_templates_group_id method to prevent further cyclical dependencies. The default value for custom_project_templates_group_id is nil so swapping the condition should skip custom_project_templates_enabled? check and avoid cyclical dependency.

  3. Removed empty FEATURES_NOT_SUPPORTING_USAGE_PING as not required anymore.

How to set up and validate locally

Existing spec that verifies that:

https://gitlab.com/gitlab-org/gitlab/-/blob/0abbd0bfc6ae42c631920ce42b1e0d42554caded/ee/spec/models/application_setting_spec.rb#L10-19

Next existing specs should not fail:

spec/lib/gitlab/current_settings_spec.rb
ee/spec/models/application_setting_spec.rb
bundle exec spring rspec spec/lib/gitlab/current_settings_spec.rb ee/spec/models/application_setting_spec.rb

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #423237 (closed)

Edited by Taras Tadai

Merge request reports