Merge branch 'gh-importer-use-default-project-visibility-for-non-private-projects' into 'master'

GitHub importer use default project visibility for non-private projects

## What does this MR do?

GitHub importer use the `Default Project Visibility` setting for all non-private projects.

## Why was this MR needed?

This allow users to import projects when `Public` visibility is restricted.

## What are the relevant issue numbers?

Fixes #21437

See merge request !6023

CHANGELOG

@@ -9,6 +9,7 @@ v 8.11.4
   - Do not enforce using hash with hidden key in CI configuration. !6079
   - Fix hover leading space bug in pipeline graph !5980
   - Fix sorting issues by "last updated" doesn't work after import from GitHub
+  - GitHub importer use default project visibility for non-private projects
   - Creating an issue through our API now emails label subscribers !5720
   - Block concurrent updates for Pipeline
   - Fix resolving conflicts on forks

lib/gitlab/github_import/project_creator.rb

@@ -17,7 +17,7 @@ module Gitlab
           path: repo.name,
           description: repo.description,
           namespace_id: namespace.id,
-          visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC,
+          visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility,
           import_type: "github",
           import_source: repo.full_name,
           import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@"),

spec/lib/gitlab/github_import/project_creator_spec.rb

@@ -2,33 +2,59 @@ require 'spec_helper'
 
 describe Gitlab::GithubImport::ProjectCreator, lib: true do
   let(:user) { create(:user) }
+  let(:namespace) { create(:group, owner: user) }
+
   let(:repo) do
     OpenStruct.new(
       login: 'vim',
       name: 'vim',
-      private: true,
       full_name: 'asd/vim',
-      clone_url: "https://gitlab.com/asd/vim.git",
-      owner: OpenStruct.new(login: "john")
+      clone_url: 'https://gitlab.com/asd/vim.git'
     )
   end
-  let(:namespace) { create(:group, owner: user) }
-  let(:token) { "asdffg" }
-  let(:access_params) { { github_access_token: token } }
+
+  subject(:service) { described_class.new(repo, namespace, user, github_access_token: 'asdffg') }
 
   before do
     namespace.add_owner(user)
+    allow_any_instance_of(Project).to receive(:add_import_job)
   end
 
-  it 'creates project' do
-    allow_any_instance_of(Project).to receive(:add_import_job)
+  describe '#execute' do
+    it 'creates a project' do
+      expect { service.execute }.to change(Project, :count).by(1)
+    end
+
+    it 'handle GitHub credentials' do
+      project = service.execute
+
+      expect(project.import_url).to eq('https://asdffg@gitlab.com/asd/vim.git')
+      expect(project.safe_import_url).to eq('https://*****@gitlab.com/asd/vim.git')
+      expect(project.import_data.credentials).to eq(user: 'asdffg', password: nil)
+    end
+
+    context 'when Github project is private' do
+      it 'sets project visibility to private' do
+        repo.private = true
 
-    project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user, access_params)
-    project = project_creator.execute
+        project = service.execute
 
-    expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git")
-    expect(project.safe_import_url).to eq("https://*****@gitlab.com/asd/vim.git")
-    expect(project.import_data.credentials).to eq(user: "asdffg", password: nil)
         expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
       end
+    end
+
+    context 'when Github project is public' do
+      before do
+        allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL)
+      end
+
+      it 'sets project visibility to the default project visibility' do
+        repo.private = false
+
+        project = service.execute
+
+        expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
+      end
+    end
+  end
 end