From 0cea33a7dbc0cb3129400bec58cd73ea60637d6c Mon Sep 17 00:00:00 2001
From: Luke Bennett <lbennett@gitlab.com>
Date: Wed, 7 Nov 2018 16:44:21 +0000
Subject: [PATCH] Improve the GitHub and Gitea import feature table interface
These are backend changes.
Use Vue for the import feature UI for "githubish"
providers (GitHub and Gitea).
Add "Go to project" button after a successful import.
Use CI-style status icons and improve spacing of the
table and its component.
Adds ETag polling to the github and gitea import
jobs endpoint.
---
app/controllers/import/gitea_controller.rb | 20 +++-
app/controllers/import/github_controller.rb | 83 +++++++++++---
app/helpers/import_helper.rb | 24 +---
app/helpers/namespaces_helper.rb | 6 +-
app/models/user.rb | 4 +
app/serializers/namespace_basic_entity.rb | 6 +
app/serializers/namespace_serializer.rb | 5 +
app/serializers/project_import_entity.rb | 13 +++
app/serializers/project_serializer.rb | 12 +-
app/serializers/provider_repo_entity.rb | 25 +++++
app/serializers/provider_repo_serializer.rb | 5 +
app/views/import/_githubish_status.html.haml | 53 +--------
.../unreleased/import-go-to-project-cta.yml | 5 +
config/routes/import.rb | 4 +-
ee/spec/features/projects/new_project_spec.rb | 40 -------
lib/gitlab/etag_caching/router.rb | 8 ++
locale/gitlab.pot | 6 -
.../import/gitea_controller_spec.rb | 8 ++
.../import/github_controller_spec.rb | 4 +
spec/helpers/import_helper_spec.rb | 57 +---------
spec/models/user_spec.rb | 15 +++
spec/routing/import_routing_spec.rb | 15 ++-
.../namespace_basic_entity_spec.rb | 18 +++
spec/serializers/namespace_serializer_spec.rb | 9 ++
.../serializers/project_import_entity_spec.rb | 22 ++++
spec/serializers/project_serializer_spec.rb | 44 ++++++++
spec/serializers/provider_repo_entity_spec.rb | 24 ++++
.../provider_repo_serializer_spec.rb | 9 ++
...ubish_import_controller_shared_examples.rb | 103 ++++++++++++++----
29 files changed, 428 insertions(+), 219 deletions(-)
create mode 100644 app/serializers/namespace_basic_entity.rb
create mode 100644 app/serializers/namespace_serializer.rb
create mode 100644 app/serializers/project_import_entity.rb
create mode 100644 app/serializers/provider_repo_entity.rb
create mode 100644 app/serializers/provider_repo_serializer.rb
create mode 100644 changelogs/unreleased/import-go-to-project-cta.yml
create mode 100644 spec/serializers/namespace_basic_entity_spec.rb
create mode 100644 spec/serializers/namespace_serializer_spec.rb
create mode 100644 spec/serializers/project_import_entity_spec.rb
create mode 100644 spec/serializers/project_serializer_spec.rb
create mode 100644 spec/serializers/provider_repo_entity_spec.rb
create mode 100644 spec/serializers/provider_repo_serializer_spec.rb
diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb
index f067ef625aa0..68ad8650dbac 100644
--- a/app/controllers/import/gitea_controller.rb
+++ b/app/controllers/import/gitea_controller.rb
@@ -1,8 +1,10 @@
# frozen_string_literal: true
class Import::GiteaController < Import::GithubController
+ extend ::Gitlab::Utils::Override
+
def new
- if session[access_token_key].present? && session[host_key].present?
+ if session[access_token_key].present? && provider_url.present?
redirect_to status_import_url
end
end
@@ -12,8 +14,8 @@ def personal_access_token
super
end
+ # Must be defined or it will 404
def status
- @gitea_host_url = session[host_key]
super
end
@@ -23,25 +25,33 @@ def host_key
:"#{provider}_host_url"
end
- # Overridden methods
+ override :provider
def provider
:gitea
end
+ override :provider_url
+ def provider_url
+ session[host_key]
+ end
+
# Gitea is not yet an OAuth provider
# See https://github.com/go-gitea/gitea/issues/27
+ override :logged_in_with_provider?
def logged_in_with_provider?
false
end
+ override :provider_auth
def provider_auth
- if session[access_token_key].blank? || session[host_key].blank?
+ if session[access_token_key].blank? || provider_url.blank?
redirect_to new_import_gitea_url,
alert: 'You need to specify both an Access Token and a Host URL.'
end
end
+ override :client_options
def client_options
- { host: session[host_key], api_version: 'v1' }
+ { host: provider_url, api_version: 'v1' }
end
end
diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb
index e81ef6a33628..f52b41d76b1c 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -1,8 +1,11 @@
# frozen_string_literal: true
class Import::GithubController < Import::BaseController
+ include ImportHelper
+
before_action :verify_import_enabled
- before_action :provider_auth, only: [:status, :jobs, :create]
+ before_action :provider_auth, only: [:status, :realtime_changes, :create]
+ before_action :expire_etag_cache, only: [:status, :create]
rescue_from Octokit::Unauthorized, with: :provider_unauthorized
@@ -24,30 +27,37 @@ def personal_access_token
redirect_to status_import_url
end
- # rubocop: disable CodeReuse/ActiveRecord
def status
- @repos = client.repos
- @already_added_projects = find_already_added_projects(provider)
- already_added_projects_names = @already_added_projects.pluck(:import_source)
-
- @repos.reject! { |repo| already_added_projects_names.include? repo.full_name }
- end
- # rubocop: enable CodeReuse/ActiveRecord
-
- def jobs
- render json: find_jobs(provider)
+ # Request repos to display error page if provider token is invalid
+ # Improving in https://gitlab.com/gitlab-org/gitlab-ce/issues/55585
+ client_repos
+
+ respond_to do |format|
+ format.json do
+ render json: { imported_projects: serialized_imported_projects,
+ provider_repos: serialized_provider_repos,
+ namespaces: serialized_namespaces }
+ end
+ format.html
+ end
end
def create
result = Import::GithubService.new(client, current_user, import_params).execute(access_params, provider)
if result[:status] == :success
- render json: ProjectSerializer.new.represent(result[:project])
+ render json: serialized_imported_projects(result[:project])
else
render json: { errors: result[:message] }, status: result[:http_status]
end
end
+ def realtime_changes
+ Gitlab::PollingInterval.set_header(response, interval: 3_000)
+
+ render json: find_jobs(provider)
+ end
+
private
def import_params
@@ -58,10 +68,45 @@ def permitted_import_params
[:repo_id, :new_name, :target_namespace]
end
+ def serialized_imported_projects(projects = already_added_projects)
+ ProjectSerializer.new.represent(projects, serializer: :import, provider_url: provider_url)
+ end
+
+ def serialized_provider_repos
+ repos = client_repos.reject { |repo| already_added_project_names.include? repo.full_name }
+ ProviderRepoSerializer.new(current_user: current_user).represent(repos, provider: provider, provider_url: provider_url)
+ end
+
+ def serialized_namespaces
+ NamespaceSerializer.new.represent(namespaces)
+ end
+
+ def already_added_projects
+ @already_added_projects ||= find_already_added_projects(provider)
+ end
+
+ def already_added_project_names
+ @already_added_projects_names ||= already_added_projects.pluck(:import_source) # rubocop:disable CodeReuse/ActiveRecord
+ end
+
+ def namespaces
+ current_user.manageable_groups_with_routes
+ end
+
+ def expire_etag_cache
+ Gitlab::EtagCaching::Store.new.tap do |store|
+ store.touch(realtime_changes_path)
+ end
+ end
+
def client
@client ||= Gitlab::LegacyGithubImport::Client.new(session[access_token_key], client_options)
end
+ def client_repos
+ @client_repos ||= client.repos
+ end
+
def verify_import_enabled
render_404 unless import_enabled?
end
@@ -74,6 +119,10 @@ def import_enabled?
__send__("#{provider}_import_enabled?") # rubocop:disable GitlabSecurity/PublicSend
end
+ def realtime_changes_path
+ public_send("realtime_changes_import_#{provider}_path", format: :json) # rubocop:disable GitlabSecurity/PublicSend
+ end
+
def new_import_url
public_send("new_import_#{provider}_url", extra_import_params) # rubocop:disable GitlabSecurity/PublicSend
end
@@ -105,6 +154,14 @@ def provider
:github
end
+ def provider_url
+ strong_memoize(:provider_url) do
+ provider = Gitlab::Auth::OAuth::Provider.config_for('github')
+
+ provider&.dig('url').presence || 'https://github.com'
+ end
+ end
+
# rubocop: disable CodeReuse/ActiveRecord
def logged_in_with_provider?
current_user.identities.exists?(provider: provider)
diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb
index d3befd87cccd..2306963347e5 100644
--- a/app/helpers/import_helper.rb
+++ b/app/helpers/import_helper.rb
@@ -18,10 +18,8 @@ def import_project_target(owner, name)
"#{namespace}/#{name}"
end
- def provider_project_link(provider, full_path)
- url = __send__("#{provider}_project_url", full_path) # rubocop:disable GitlabSecurity/PublicSend
-
- link_to full_path, url, target: '_blank', rel: 'noopener noreferrer'
+ def provider_project_link_url(provider_url, full_path)
+ Gitlab::Utils.append_path(provider_url, full_path)
end
def import_will_timeout_message(_ci_cd_only)
@@ -81,22 +79,4 @@ def import_githubish_choose_repository_message
def import_all_githubish_repositories_button_label
_('Import all repositories')
end
-
- private
-
- def github_project_url(full_path)
- Gitlab::Utils.append_path(github_root_url, full_path)
- end
-
- def github_root_url
- strong_memoize(:github_url) do
- provider = Gitlab::Auth::OAuth::Provider.config_for('github')
-
- provider&.dig('url').presence || 'https://github.com'
- end
- end
-
- def gitea_project_url(full_path)
- Gitlab::Utils.append_path(@gitea_host_url, full_path)
- end
end
diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index 6dfb417198d7..477d6ed45e2e 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -5,11 +5,8 @@ def namespace_id_from(params)
params.dig(:project, :namespace_id) || params[:namespace_id]
end
- # rubocop: disable CodeReuse/ActiveRecord
def namespaces_options(selected = :current_user, display_path: false, groups: nil, extra_group: nil, groups_only: false)
- groups ||= current_user.manageable_groups
- .eager_load(:route)
- .order('routes.path')
+ groups ||= current_user.manageable_groups_with_routes
users = [current_user.namespace]
selected_id = selected
@@ -43,7 +40,6 @@ def namespaces_options(selected = :current_user, display_path: false, groups: ni
grouped_options_for_select(options, selected_id)
end
- # rubocop: enable CodeReuse/ActiveRecord
def namespace_icon(namespace, size = 40)
if namespace.is_a?(Group)
diff --git a/app/models/user.rb b/app/models/user.rb
index 486ba2dfae5a..29a5c2051d6e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1167,6 +1167,10 @@ def manageable_groups
Gitlab::ObjectHierarchy.new(owned_or_maintainers_groups).base_and_descendants
end
+ def manageable_groups_with_routes
+ manageable_groups.eager_load(:route).order('routes.path')
+ end
+
def namespaces
namespace_ids = groups.pluck(:id)
namespace_ids.push(namespace.id)
diff --git a/app/serializers/namespace_basic_entity.rb b/app/serializers/namespace_basic_entity.rb
new file mode 100644
index 000000000000..8bcbb2bca603
--- /dev/null
+++ b/app/serializers/namespace_basic_entity.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class NamespaceBasicEntity < Grape::Entity
+ expose :id
+ expose :full_path
+end
diff --git a/app/serializers/namespace_serializer.rb b/app/serializers/namespace_serializer.rb
new file mode 100644
index 000000000000..bf3f154b558f
--- /dev/null
+++ b/app/serializers/namespace_serializer.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class NamespaceSerializer < BaseSerializer
+ entity NamespaceBasicEntity
+end
diff --git a/app/serializers/project_import_entity.rb b/app/serializers/project_import_entity.rb
new file mode 100644
index 000000000000..9b51af685e76
--- /dev/null
+++ b/app/serializers/project_import_entity.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class ProjectImportEntity < ProjectEntity
+ include ImportHelper
+
+ expose :import_source
+ expose :import_status
+ expose :human_import_status_name
+
+ expose :provider_link do |project, options|
+ provider_project_link_url(options[:provider_url], project[:import_source])
+ end
+end
diff --git a/app/serializers/project_serializer.rb b/app/serializers/project_serializer.rb
index 23b96c2fc9e2..52ac2fa0e098 100644
--- a/app/serializers/project_serializer.rb
+++ b/app/serializers/project_serializer.rb
@@ -1,5 +1,15 @@
# frozen_string_literal: true
class ProjectSerializer < BaseSerializer
- entity ProjectEntity
+ def represent(project, opts = {})
+ entity =
+ case opts[:serializer]
+ when :import
+ ProjectImportEntity
+ else
+ ProjectEntity
+ end
+
+ super(project, opts, entity)
+ end
end
diff --git a/app/serializers/provider_repo_entity.rb b/app/serializers/provider_repo_entity.rb
new file mode 100644
index 000000000000..d70aaa913241
--- /dev/null
+++ b/app/serializers/provider_repo_entity.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ProviderRepoEntity < Grape::Entity
+ include ImportHelper
+
+ expose :id
+ expose :full_name
+ expose :owner_name do |provider_repo, options|
+ owner_name(provider_repo, options[:provider])
+ end
+
+ expose :sanitized_name do |provider_repo|
+ sanitize_project_name(provider_repo[:name])
+ end
+
+ expose :provider_link do |provider_repo, options|
+ provider_project_link_url(options[:provider_url], provider_repo[:full_name])
+ end
+
+ private
+
+ def owner_name(provider_repo, provider)
+ provider_repo.dig(:owner, :login) if provider == :github
+ end
+end
diff --git a/app/serializers/provider_repo_serializer.rb b/app/serializers/provider_repo_serializer.rb
new file mode 100644
index 000000000000..8a73f6fe6df8
--- /dev/null
+++ b/app/serializers/provider_repo_serializer.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ProviderRepoSerializer < BaseSerializer
+ entity ProviderRepoEntity
+end
diff --git a/app/views/import/_githubish_status.html.haml b/app/views/import/_githubish_status.html.haml
index f4a29ed18dcf..08349ce8a68c 100644
--- a/app/views/import/_githubish_status.html.haml
+++ b/app/views/import/_githubish_status.html.haml
@@ -1,56 +1,5 @@
- provider = local_assigns.fetch(:provider)
- provider_title = Gitlab::ImportSources.title(provider)
-%p.light
- = import_githubish_choose_repository_message
-%hr
-%p
- = button_tag class: "btn btn-import btn-success js-import-all" do
- = import_all_githubish_repositories_button_label
- = icon("spinner spin", class: "loading-icon")
+%h1= provider_title
-.table-responsive
- %table.table.import-jobs
- %colgroup.import-jobs-from-col
- %colgroup.import-jobs-to-col
- %colgroup.import-jobs-status-col
- %thead
- %tr
- %th= _('From %{provider_title}') % { provider_title: provider_title }
- %th= _('To GitLab')
- %th= _('Status')
- %tbody
- - @already_added_projects.each do |project|
- %tr{ id: "project_#{project.id}", class: project_status_css_class(project.import_status) }
- %td
- = provider_project_link(provider, project.import_source)
- %td
- = link_to project.full_path, [project.namespace.becomes(Namespace), project]
- %td.job-status
- = render 'import/project_status', project: project
-
- - @repos.each do |repo|
- %tr{ id: "repo_#{repo.id}", data: { qa: { repo_path: repo.full_name } } }
- %td
- = provider_project_link(provider, repo.full_name)
- %td.import-target
- %fieldset.row
- .input-group
- .project-path.input-group-prepend
- - if current_user.can_select_namespace?
- - selected = params[:namespace_id] || :current_user
- - opts = current_user.can_create_group? ? { extra_group: Group.new(name: repo.owner.login, path: repo.owner.login) } : {}
- = select_tag :namespace_id, namespaces_options(selected, opts.merge({ display_path: true })), { class: 'input-group-text select2 js-select-namespace qa-project-namespace-select', tabindex: 1 }
- - else
- = text_field_tag :path, current_user.namespace_path, class: "input-group-text input-large form-control", tabindex: 1, disabled: true
- %span.input-group-prepend
- .input-group-text /
- = text_field_tag :path, sanitize_project_name(repo.name), class: "input-mini form-control", tabindex: 2, autofocus: true, required: true
- %td.import-actions.job-status
- = button_tag class: "btn btn-import js-add-to-import" do
- = has_ci_cd_only_params? ? _('Connect') : _('Import')
- = icon("spinner spin", class: "loading-icon")
-
-.js-importer-status{ data: { jobs_import_path: url_for([:jobs, :import, provider]),
- import_path: url_for([:import, provider]),
- ci_cd_only: has_ci_cd_only_params?.to_s } }
diff --git a/changelogs/unreleased/import-go-to-project-cta.yml b/changelogs/unreleased/import-go-to-project-cta.yml
new file mode 100644
index 000000000000..ae719f087901
--- /dev/null
+++ b/changelogs/unreleased/import-go-to-project-cta.yml
@@ -0,0 +1,5 @@
+---
+title: Improve GitHub and Gitea project import table UI
+merge_request: 24606
+author:
+type: other
diff --git a/config/routes/import.rb b/config/routes/import.rb
index da5c31d00624..24013eb2c886 100644
--- a/config/routes/import.rb
+++ b/config/routes/import.rb
@@ -12,13 +12,13 @@
post :personal_access_token
get :status
get :callback
- get :jobs
+ get :realtime_changes
end
resource :gitea, only: [:create, :new], controller: :gitea do
post :personal_access_token
get :status
- get :jobs
+ get :realtime_changes
end
resource :gitlab, only: [:create], controller: :gitlab do
diff --git a/ee/spec/features/projects/new_project_spec.rb b/ee/spec/features/projects/new_project_spec.rb
index 81bc64a843fc..aa783a2aae39 100644
--- a/ee/spec/features/projects/new_project_spec.rb
+++ b/ee/spec/features/projects/new_project_spec.rb
@@ -122,46 +122,6 @@
end
end
- it 'creates CI/CD project from GitHub' do
- visit new_project_path
- find('#ci-cd-project-tab').click
-
- page.within '#ci-cd-project-pane' do
- find('.js-import-github').click
- end
-
- expect(page).to have_text('Connect repositories from GitHub')
-
- allow_any_instance_of(Gitlab::LegacyGithubImport::Client).to receive(:repos).and_return([repo])
-
- fill_in 'personal_access_token', with: 'fake-token'
- click_button 'List your GitHub repositories'
- wait_for_requests
-
- # Mock the POST `/import/github`
- allow_any_instance_of(Gitlab::LegacyGithubImport::Client).to receive(:repo).and_return(repo)
- project = create(:project, name: 'some-github-repo', creator: user, import_type: 'github')
- create(:import_state, :finished, import_url: repo.clone_url, project: project)
- allow_any_instance_of(CiCd::SetupProject).to receive(:setup_external_service)
- CiCd::SetupProject.new(project, user).execute
- allow_any_instance_of(Gitlab::LegacyGithubImport::ProjectCreator)
- .to receive(:execute).with(hash_including(ci_cd_only: true))
- .and_return(project)
-
- click_button 'Connect'
- wait_for_requests
-
- expect(page).to have_text('Started')
- wait_for_requests
-
- expect(page).to have_text('Done')
-
- created_project = Project.last
- expect(created_project.name).to eq('some-github-repo')
- expect(created_project.mirror).to eq(true)
- expect(created_project.project_feature).not_to be_issues_enabled
- end
-
it 'new GitHub CI/CD project page has link to status page with ?ci_cd_only=true param' do
visit new_import_github_path(ci_cd_only: true)
diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb
index aced02eed387..1d2820fcab15 100644
--- a/lib/gitlab/etag_caching/router.rb
+++ b/lib/gitlab/etag_caching/router.rb
@@ -54,6 +54,14 @@ class Router
Gitlab::EtagCaching::Router::Route.new(
%r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/environments\.json\z),
'environments'
+ ),
+ Gitlab::EtagCaching::Router::Route.new(
+ %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/import/github/realtime_changes\.json\z),
+ 'realtime_changes_import_github'
+ ),
+ Gitlab::EtagCaching::Router::Route.new(
+ %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/import/gitea/realtime_changes\.json\z),
+ 'realtime_changes_import_gitea'
)
].freeze
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 3c2bdacfc76e..cbf6caafa40d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2589,9 +2589,6 @@ msgstr ""
msgid "Configure the way a user creates a new account."
msgstr ""
-msgid "Connect"
-msgstr ""
-
msgid "Connect all repositories"
msgstr ""
@@ -4231,9 +4228,6 @@ msgstr ""
msgid "Free Trial of GitLab.com Gold"
msgstr ""
-msgid "From %{provider_title}"
-msgstr ""
-
msgid "From Bitbucket"
msgstr ""
diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb
index 5ba64ab3eed7..8cbec79095f3 100644
--- a/spec/controllers/import/gitea_controller_spec.rb
+++ b/spec/controllers/import/gitea_controller_spec.rb
@@ -40,4 +40,12 @@ def assign_host_url
end
end
end
+
+ describe "GET realtime_changes" do
+ it_behaves_like 'a GitHub-ish import controller: GET realtime_changes' do
+ before do
+ assign_host_url
+ end
+ end
+ end
end
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb
index bca5f3f65891..162dff98ec5c 100644
--- a/spec/controllers/import/github_controller_spec.rb
+++ b/spec/controllers/import/github_controller_spec.rb
@@ -60,4 +60,8 @@
describe "POST create" do
it_behaves_like 'a GitHub-ish import controller: POST create'
end
+
+ describe "GET realtime_changes" do
+ it_behaves_like 'a GitHub-ish import controller: GET realtime_changes'
+ end
end
diff --git a/spec/helpers/import_helper_spec.rb b/spec/helpers/import_helper_spec.rb
index af4931e3370d..6e8c13db9fe1 100644
--- a/spec/helpers/import_helper_spec.rb
+++ b/spec/helpers/import_helper_spec.rb
@@ -39,59 +39,12 @@
end
end
- describe '#provider_project_link' do
- context 'when provider is "github"' do
- let(:github_server_url) { nil }
- let(:provider) { OpenStruct.new(name: 'github', url: github_server_url) }
+ describe '#provider_project_link_url' do
+ let(:full_path) { '/repo/path' }
+ let(:host_url) { 'http://provider.com/' }
- before do
- stub_omniauth_setting(providers: [provider])
- end
-
- context 'when provider does not specify a custom URL' do
- it 'uses default GitHub URL' do
- expect(helper.provider_project_link('github', 'octocat/Hello-World'))
- .to include('href="https://github.com/octocat/Hello-World"')
- end
- end
-
- context 'when provider specify a custom URL' do
- let(:github_server_url) { 'https://github.company.com' }
-
- it 'uses custom URL' do
- expect(helper.provider_project_link('github', 'octocat/Hello-World'))
- .to include('href="https://github.company.com/octocat/Hello-World"')
- end
- end
-
- context "when custom URL contains a '/' char at the end" do
- let(:github_server_url) { 'https://github.company.com/' }
-
- it "doesn't render double slash" do
- expect(helper.provider_project_link('github', 'octocat/Hello-World'))
- .to include('href="https://github.company.com/octocat/Hello-World"')
- end
- end
-
- context 'when provider is missing' do
- it 'uses the default URL' do
- allow(Gitlab.config.omniauth).to receive(:providers).and_return([])
-
- expect(helper.provider_project_link('github', 'octocat/Hello-World'))
- .to include('href="https://github.com/octocat/Hello-World"')
- end
- end
- end
-
- context 'when provider is "gitea"' do
- before do
- assign(:gitea_host_url, 'https://try.gitea.io/')
- end
-
- it 'uses given host' do
- expect(helper.provider_project_link('gitea', 'octocat/Hello-World'))
- .to include('href="https://try.gitea.io/octocat/Hello-World"')
- end
+ it 'appends repo full path to provider host url' do
+ expect(helper.provider_project_link_url(host_url, full_path)).to match('http://provider.com/repo/path')
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bd8c9459c51b..efae57cfd601 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -953,6 +953,21 @@
expect(user.manageable_groups).to contain_exactly(group, subgroup)
end
end
+
+ describe '#manageable_groups_with_routes' do
+ it 'eager loads routes from manageable groups' do
+ control_count =
+ ActiveRecord::QueryRecorder.new(skip_cached: false) do
+ user.manageable_groups_with_routes.map(&:route)
+ end.count
+
+ create(:group, parent: subgroup)
+
+ expect do
+ user.manageable_groups_with_routes.map(&:route)
+ end.not_to exceed_all_query_limit(control_count)
+ end
+ end
end
end
diff --git a/spec/routing/import_routing_spec.rb b/spec/routing/import_routing_spec.rb
index 78ff9c6e6fd0..106f92082e47 100644
--- a/spec/routing/import_routing_spec.rb
+++ b/spec/routing/import_routing_spec.rb
@@ -23,6 +23,11 @@
# end
shared_examples 'importer routing' do
let(:except_actions) { [] }
+ let(:is_realtime) { false }
+
+ before do
+ except_actions.push(is_realtime ? :jobs : :realtime_changes)
+ end
it 'to #create' do
expect(post("/import/#{provider}")).to route_to("import/#{provider}#create") unless except_actions.include?(:create)
@@ -43,17 +48,22 @@
it 'to #jobs' do
expect(get("/import/#{provider}/jobs")).to route_to("import/#{provider}#jobs") unless except_actions.include?(:jobs)
end
+
+ it 'to #realtime_changes' do
+ expect(get("/import/#{provider}/realtime_changes")).to route_to("import/#{provider}#realtime_changes") unless except_actions.include?(:realtime_changes)
+ end
end
# personal_access_token_import_github POST /import/github/personal_access_token(.:format) import/github#personal_access_token
# status_import_github GET /import/github/status(.:format) import/github#status
# callback_import_github GET /import/github/callback(.:format) import/github#callback
-# jobs_import_github GET /import/github/jobs(.:format) import/github#jobs
+# realtime_changes_import_github GET /import/github/realtime_changes(.:format) import/github#jobs
# import_github POST /import/github(.:format) import/github#create
# new_import_github GET /import/github/new(.:format) import/github#new
describe Import::GithubController, 'routing' do
it_behaves_like 'importer routing' do
let(:provider) { 'github' }
+ let(:is_realtime) { true }
end
it 'to #personal_access_token' do
@@ -63,13 +73,14 @@
# personal_access_token_import_gitea POST /import/gitea/personal_access_token(.:format) import/gitea#personal_access_token
# status_import_gitea GET /import/gitea/status(.:format) import/gitea#status
-# jobs_import_gitea GET /import/gitea/jobs(.:format) import/gitea#jobs
+# realtime_changes_import_gitea GET /import/gitea/realtime_changes(.:format) import/gitea#jobs
# import_gitea POST /import/gitea(.:format) import/gitea#create
# new_import_gitea GET /import/gitea/new(.:format) import/gitea#new
describe Import::GiteaController, 'routing' do
it_behaves_like 'importer routing' do
let(:except_actions) { [:callback] }
let(:provider) { 'gitea' }
+ let(:is_realtime) { true }
end
it 'to #personal_access_token' do
diff --git a/spec/serializers/namespace_basic_entity_spec.rb b/spec/serializers/namespace_basic_entity_spec.rb
new file mode 100644
index 000000000000..f8b71ceb9f31
--- /dev/null
+++ b/spec/serializers/namespace_basic_entity_spec.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe NamespaceBasicEntity do
+ set(:group) { create(:group) }
+ let(:entity) do
+ described_class.represent(group)
+ end
+
+ describe '#as_json' do
+ subject { entity.as_json }
+
+ it 'includes required fields' do
+ expect(subject).to include :id, :full_path
+ end
+ end
+end
diff --git a/spec/serializers/namespace_serializer_spec.rb b/spec/serializers/namespace_serializer_spec.rb
new file mode 100644
index 000000000000..6e5bdd8c52d1
--- /dev/null
+++ b/spec/serializers/namespace_serializer_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe NamespaceSerializer do
+ it 'represents NamespaceBasicEntity entities' do
+ expect(described_class.entity_class).to eq(NamespaceBasicEntity)
+ end
+end
diff --git a/spec/serializers/project_import_entity_spec.rb b/spec/serializers/project_import_entity_spec.rb
new file mode 100644
index 000000000000..e476da82729b
--- /dev/null
+++ b/spec/serializers/project_import_entity_spec.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectImportEntity do
+ include ImportHelper
+
+ set(:project) { create(:project, import_status: :started, import_source: 'namespace/project') }
+ let(:provider_url) { 'https://provider.com' }
+ let(:entity) { described_class.represent(project, provider_url: provider_url) }
+
+ describe '#as_json' do
+ subject { entity.as_json }
+
+ it 'includes required fields' do
+ expect(subject[:import_source]).to eq(project.import_source)
+ expect(subject[:import_status]).to eq(project.import_status)
+ expect(subject[:human_import_status_name]).to eq(project.human_import_status_name)
+ expect(subject[:provider_link]).to eq(provider_project_link_url(provider_url, project[:import_source]))
+ end
+ end
+end
diff --git a/spec/serializers/project_serializer_spec.rb b/spec/serializers/project_serializer_spec.rb
new file mode 100644
index 000000000000..22f958fc17f3
--- /dev/null
+++ b/spec/serializers/project_serializer_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectSerializer do
+ set(:project) { create(:project) }
+ let(:provider_url) { 'http://provider.com' }
+
+ context 'when serializer option is :import' do
+ subject do
+ described_class.new.represent(project, serializer: :import, provider_url: provider_url)
+ end
+
+ before do
+ allow(ProjectImportEntity).to receive(:represent)
+ end
+
+ it 'represents with ProjectImportEntity' do
+ subject
+
+ expect(ProjectImportEntity)
+ .to have_received(:represent)
+ .with(project, serializer: :import, provider_url: provider_url, request: an_instance_of(EntityRequest))
+ end
+ end
+
+ context 'when serializer option is omitted' do
+ subject do
+ described_class.new.represent(project)
+ end
+
+ before do
+ allow(ProjectEntity).to receive(:represent)
+ end
+
+ it 'represents with ProjectEntity' do
+ subject
+
+ expect(ProjectEntity)
+ .to have_received(:represent)
+ .with(project, request: an_instance_of(EntityRequest))
+ end
+ end
+end
diff --git a/spec/serializers/provider_repo_entity_spec.rb b/spec/serializers/provider_repo_entity_spec.rb
new file mode 100644
index 000000000000..b67115bab10d
--- /dev/null
+++ b/spec/serializers/provider_repo_entity_spec.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProviderRepoEntity do
+ include ImportHelper
+
+ let(:provider_repo) { { id: 1, full_name: 'full/name', name: 'name', owner: { login: 'owner' } } }
+ let(:provider) { :github }
+ let(:provider_url) { 'https://github.com' }
+ let(:entity) { described_class.represent(provider_repo, provider: provider, provider_url: provider_url) }
+
+ describe '#as_json' do
+ subject { entity.as_json }
+
+ it 'includes requried fields' do
+ expect(subject[:id]).to eq(provider_repo[:id])
+ expect(subject[:full_name]).to eq(provider_repo[:full_name])
+ expect(subject[:owner_name]).to eq(provider_repo[:owner][:login])
+ expect(subject[:sanitized_name]).to eq(sanitize_project_name(provider_repo[:name]))
+ expect(subject[:provider_link]).to eq(provider_project_link_url(provider_url, provider_repo[:full_name]))
+ end
+ end
+end
diff --git a/spec/serializers/provider_repo_serializer_spec.rb b/spec/serializers/provider_repo_serializer_spec.rb
new file mode 100644
index 000000000000..f2be30c36d9d
--- /dev/null
+++ b/spec/serializers/provider_repo_serializer_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProviderRepoSerializer do
+ it 'represents ProviderRepoEntity entities' do
+ expect(described_class.entity_class).to eq(ProviderRepoEntity)
+ end
+end
diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb
index 697f999e4c4b..5bb1269a19df 100644
--- a/spec/support/controllers/githubish_import_controller_shared_examples.rb
+++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb
@@ -58,36 +58,54 @@ def assign_session_token(provider)
shared_examples 'a GitHub-ish import controller: GET status' do
let(:new_import_url) { public_send("new_import_#{provider}_url") }
let(:user) { create(:user) }
- let(:repo) { OpenStruct.new(login: 'vim', full_name: 'asd/vim') }
+ let(:repo) { OpenStruct.new(login: 'vim', full_name: 'asd/vim', name: 'vim', owner: { login: 'owner' }) }
let(:org) { OpenStruct.new(login: 'company') }
- let(:org_repo) { OpenStruct.new(login: 'company', full_name: 'company/repo') }
- let(:extra_assign_expectations) { {} }
+ let(:org_repo) { OpenStruct.new(login: 'company', full_name: 'company/repo', name: 'repo', owner: { login: 'owner' }) }
before do
assign_session_token(provider)
end
- it "assigns variables" do
- project = create(:project, import_type: provider, namespace: user.namespace)
+ it "returns variables for json request" do
+ project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'example/repo')
+ group = create(:group)
+ group.add_owner(user)
stub_client(repos: [repo, org_repo], orgs: [org], org_repos: [org_repo])
- get :status
+ get :status, format: :json
- expect(assigns(:already_added_projects)).to eq([project])
- expect(assigns(:repos)).to eq([repo, org_repo])
- extra_assign_expectations.each do |key, value|
- expect(assigns(key)).to eq(value)
- end
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id)
+ expect(json_response.dig("provider_repos", 0, "id")).to eq(repo.id)
+ expect(json_response.dig("provider_repos", 1, "id")).to eq(org_repo.id)
+ expect(json_response.dig("namespaces", 0, "id")).to eq(group.id)
end
it "does not show already added project" do
- project = create(:project, import_type: provider, namespace: user.namespace, import_source: 'asd/vim')
+ project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'asd/vim')
stub_client(repos: [repo], orgs: [])
+ get :status, format: :json
+
+ expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id)
+ expect(json_response.dig("provider_repos")).to eq([])
+ end
+
+ it "touches the etag cache store" do
+ expect(stub_client(repos: [], orgs: [])).to receive(:repos)
+ expect_next_instance_of(Gitlab::EtagCaching::Store) do |store|
+ expect(store).to receive(:touch) { "realtime_changes_import_#{provider}_path" }
+ end
+
+ get :status, format: :json
+ end
+
+ it "requests provider repos list" do
+ expect(stub_client(repos: [], orgs: [])).to receive(:repos)
+
get :status
- expect(assigns(:already_added_projects)).to eq([project])
- expect(assigns(:repos)).to eq([])
+ expect(response).to have_gitlab_http_status(200)
end
it "handles an invalid access token" do
@@ -100,13 +118,32 @@ def assign_session_token(provider)
expect(controller).to redirect_to(new_import_url)
expect(flash[:alert]).to eq("Access denied to your #{Gitlab::ImportSources.title(provider.to_s)} account.")
end
+
+ it "does not produce N+1 database queries" do
+ stub_client(repos: [repo], orgs: [])
+ group_a = create(:group)
+ group_a.add_owner(user)
+ create(:project, :import_started, import_type: provider, namespace: user.namespace)
+
+ control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+ get :status, format: :json
+ end.count
+
+ stub_client(repos: [repo, org_repo], orgs: [])
+ group_b = create(:group)
+ group_b.add_owner(user)
+ create(:project, :import_started, import_type: provider, namespace: user.namespace)
+
+ expect { get :status, format: :json }
+ .not_to exceed_all_query_limit(control_count)
+ end
end
shared_examples 'a GitHub-ish import controller: POST create' do
let(:user) { create(:user) }
- let(:project) { create(:project) }
let(:provider_username) { user.username }
let(:provider_user) { OpenStruct.new(login: provider_username) }
+ let(:project) { create(:project, import_type: provider, import_status: :finished, import_source: "#{provider_username}/vim") }
let(:provider_repo) do
OpenStruct.new(
name: 'vim',
@@ -145,6 +182,17 @@ def assign_session_token(provider)
expect(json_response['errors']).to eq('Name is invalid, Path is old')
end
+ it "touches the etag cache store" do
+ allow(Gitlab::LegacyGithubImport::ProjectCreator)
+ .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider)
+ .and_return(double(execute: project))
+ expect_next_instance_of(Gitlab::EtagCaching::Store) do |store|
+ expect(store).to receive(:touch) { "realtime_changes_import_#{provider}_path" }
+ end
+
+ post :create, format: :json
+ end
+
context "when the repository owner is the provider user" do
context "when the provider user and GitLab user's usernames match" do
it "takes the current user's namespace" do
@@ -351,7 +399,7 @@ def assign_session_token(provider)
it 'does not create a new namespace under the user namespace' do
expect(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider)
- .and_return(double(execute: build_stubbed(:project)))
+ .and_return(double(execute: project))
expect { post :create, params: { target_namespace: "#{user.namespace_path}/test_group", new_name: test_name }, format: :js }
.not_to change { Namespace.count }
@@ -365,7 +413,7 @@ def assign_session_token(provider)
it 'does not take the selected namespace and name' do
expect(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider)
- .and_return(double(execute: build_stubbed(:project)))
+ .and_return(double(execute: project))
post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :js
end
@@ -373,7 +421,7 @@ def assign_session_token(provider)
it 'does not create the namespaces' do
allow(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider)
- .and_return(double(execute: build_stubbed(:project)))
+ .and_return(double(execute: project))
expect { post :create, params: { target_namespace: 'foo/foobar/bar', new_name: test_name }, format: :js }
.not_to change { Namespace.count }
@@ -390,7 +438,7 @@ def assign_session_token(provider)
expect(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, test_name, group, user, access_params, type: provider)
- .and_return(double(execute: build_stubbed(:project)))
+ .and_return(double(execute: project))
post :create, params: { target_namespace: 'foo', new_name: test_name }, format: :js
end
@@ -407,3 +455,20 @@ def assign_session_token(provider)
end
end
end
+
+shared_examples 'a GitHub-ish import controller: GET realtime_changes' do
+ let(:user) { create(:user) }
+
+ before do
+ assign_session_token(provider)
+ end
+
+ it 'sets a Poll-Interval header' do
+ project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'example/repo')
+
+ get :realtime_changes
+
+ expect(json_response).to eq([{ "id" => project.id, "import_status" => project.import_status }])
+ expect(Integer(response.headers['Poll-Interval'])).to be > -1
+ end
+end
--
GitLab