Merge branch 'import-improve-project-update' into 'master'

Import improve project update

See merge request gitlab-org/gitlab!18007

app/models/project.rb

@@ -2258,6 +2258,16 @@ class Project < ApplicationRecord
     setting
   end
 
+  def drop_visibility_level!
+    if group && group.visibility_level < visibility_level
+      self.visibility_level = group.visibility_level
+    end
+
+    if Gitlab::CurrentSettings.restricted_visibility_levels.include?(visibility_level)
+      self.visibility_level = Gitlab::VisibilityLevel::PRIVATE
+    end
+  end
+
   private
 
   def closest_namespace_setting(name)

lib/gitlab/import_export/project_tree_restorer.rb

@@ -32,7 +32,7 @@ module Gitlab
 
         ActiveRecord::Base.uncached do
           ActiveRecord::Base.no_touching do
-            update_project_params
+            update_project_params!
             create_relations
           end
         end
@@ -70,7 +70,7 @@ module Gitlab
       # the configuration yaml file too.
       # Finally, it updates each attribute in the newly imported project.
       def create_relations
-        project_relations_without_project_members.each do |relation_key, relation_definition|
+        project_relations.each do |relation_key, relation_definition|
           relation_key_s = relation_key.to_s
 
           if relation_definition.present?
@@ -124,56 +124,40 @@ module Gitlab
         # no-op
       end
 
-      def project_relations_without_project_members
-        # We remove `project_members` as they are deserialized separately
-        project_relations.except(:project_members)
-      end
-
       def project_relations
-        reader.attributes_finder.find_relations_tree(:project)
+        @project_relations ||= reader.attributes_finder.find_relations_tree(:project)
       end
 
-      def update_project_params
+      def update_project_params!
         Gitlab::Timeless.timeless(@project) do
-          @project.update(project_params)
-        end
+          project_params = @tree_hash.reject do |key, value|
+            project_relations.include?(key.to_sym)
           end
 
-      def project_params
-        @project_params ||= begin
-          attrs = json_params.merge(override_params).merge(visibility_level, external_label)
+          project_params = project_params.merge(present_project_override_params)
 
           # Cleaning all imported and overridden params
-          Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: attrs,
+          project_params = Gitlab::ImportExport::AttributeCleaner.clean(
+            relation_hash: project_params,
             relation_class: Project,
             excluded_keys: excluded_keys_for_relation(:project))
-        end
-      end
 
-      def override_params
-        @override_params ||= @project.import_data&.data&.fetch('override_params', nil) || {}
-      end
-
-      def json_params
-        @json_params ||= @tree_hash.reject do |key, value|
-          # return params that are not 1 to many or 1 to 1 relations
-          value.respond_to?(:each) && !Project.column_names.include?(key)
+          @project.assign_attributes(project_params)
+          @project.drop_visibility_level!
+          @project.save!
         end
       end
 
-      def visibility_level
-        level = override_params['visibility_level'] || json_params['visibility_level'] || @project.visibility_level
-        level = @project.group.visibility_level if @project.group && level.to_i > @project.group.visibility_level
-        level = Gitlab::VisibilityLevel::PRIVATE if level == Gitlab::VisibilityLevel::INTERNAL && Gitlab::CurrentSettings.restricted_visibility_levels.include?(level)
-
-        { 'visibility_level' => level }
+      def present_project_override_params
+        # we filter out the empty strings from the overrides
+        # keeping the default values configured
+        project_override_params.transform_values do |value|
+          value.is_a?(String) ? value.presence : value
+        end.compact
       end
 
-      def external_label
-        label = override_params['external_authorization_classification_label'].presence ||
-          json_params['external_authorization_classification_label'].presence
-
-        { 'external_authorization_classification_label' => label }
+      def project_override_params
+        @project_override_params ||= @project.import_data&.data&.fetch('override_params', nil) || {}
       end
 
       # Given a relation hash containing one or more models and its relationships,

spec/models/project_spec.rb

@@ -5179,6 +5179,61 @@ describe Project do
     end
   end
 
+  describe '#drop_visibility_level!' do
+    context 'when has a group' do
+      let(:group) { create(:group, visibility_level: group_visibility_level) }
+      let(:project) { build(:project, namespace: group, visibility_level: project_visibility_level) }
+
+      context 'when the group `visibility_level` is more strict' do
+        let(:group_visibility_level) { Gitlab::VisibilityLevel::PRIVATE }
+        let(:project_visibility_level) { Gitlab::VisibilityLevel::INTERNAL }
+
+        it 'sets `visibility_level` value from the group' do
+          expect { project.drop_visibility_level! }
+            .to change { project.visibility_level }
+            .to(Gitlab::VisibilityLevel::PRIVATE)
+        end
+      end
+
+      context 'when the group `visibility_level` is less strict' do
+        let(:group_visibility_level) { Gitlab::VisibilityLevel::INTERNAL }
+        let(:project_visibility_level) { Gitlab::VisibilityLevel::PRIVATE }
+
+        it 'does not change the value of the `visibility_level` field' do
+          expect { project.drop_visibility_level! }
+            .not_to change { project.visibility_level }
+        end
+      end
+    end
+
+    context 'when `restricted_visibility_levels` of the GitLab instance exist' do
+      before do
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
+      end
+
+      let(:project) { build(:project, visibility_level: project_visibility_level) }
+
+      context 'when `visibility_level` is included into `restricted_visibility_levels`' do
+        let(:project_visibility_level) { Gitlab::VisibilityLevel::INTERNAL }
+
+        it 'sets `visibility_level` value to `PRIVATE`' do
+          expect { project.drop_visibility_level! }
+            .to change { project.visibility_level }
+            .to(Gitlab::VisibilityLevel::PRIVATE)
+        end
+      end
+
+      context 'when `restricted_visibility_levels` does not include `visibility_level`' do
+        let(:project_visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
+
+        it 'does not change the value of the `visibility_level` field' do
+          expect { project.drop_visibility_level! }
+            .to not_change { project.visibility_level }
+        end
+      end
+    end
+  end
+
   def rugged_config
     rugged_repo(project.repository).config
   end