diff options
26 files changed, 427 insertions, 121 deletions
diff --git a/app/controllers/import/gitea_controller.rb b/app/controllers/import/gitea_controller.rb index f067ef625aa..68ad8650dba 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 @@ class Import::GiteaController < Import::GithubController super end + # Must be defined or it will 404 def status - @gitea_host_url = session[host_key] super end @@ -23,25 +25,33 @@ class Import::GiteaController < Import::GithubController :"#{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 3fbc0817e95..aa4aa0fbdac 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 @@ class Import::GithubController < Import::BaseController 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 @@ class Import::GithubController < Import::BaseController [: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 @@ class Import::GithubController < Import::BaseController __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 @@ class Import::GithubController < Import::BaseController :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 d3befd87ccc..2306963347e 100644 --- a/app/helpers/import_helper.rb +++ b/app/helpers/import_helper.rb @@ -18,10 +18,8 @@ module ImportHelper "#{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 @@ module ImportHelper 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 6c65e573307..ea3bcfc791a 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -5,11 +5,8 @@ module NamespacesHelper 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 @@ module NamespacesHelper 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 24101eda0b1..9553ad98dc2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1167,6 +1167,10 @@ class User < ApplicationRecord 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 00000000000..8bcbb2bca60 --- /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 00000000000..bf3f154b558 --- /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 00000000000..9b51af685e7 --- /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 23b96c2fc9e..52ac2fa0e09 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 00000000000..d70aaa91324 --- /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 00000000000..8a73f6fe6df --- /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/changelogs/unreleased/import-go-to-project-cta.yml b/changelogs/unreleased/import-go-to-project-cta.yml new file mode 100644 index 00000000000..ae719f08790 --- /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 da5c31d0062..24013eb2c88 100644 --- a/config/routes/import.rb +++ b/config/routes/import.rb @@ -12,13 +12,13 @@ namespace :import do 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/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb index 08e30214b46..0891f79198d 100644 --- a/lib/gitlab/etag_caching/router.rb +++ b/lib/gitlab/etag_caching/router.rb @@ -52,6 +52,14 @@ module Gitlab 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/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb index 5ba64ab3eed..8cbec79095f 100644 --- a/spec/controllers/import/gitea_controller_spec.rb +++ b/spec/controllers/import/gitea_controller_spec.rb @@ -40,4 +40,12 @@ describe Import::GiteaController do 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 bca5f3f6589..162dff98ec5 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -60,4 +60,8 @@ describe Import::GithubController do 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 af4931e3370..6e8c13db9fe 100644 --- a/spec/helpers/import_helper_spec.rb +++ b/spec/helpers/import_helper_spec.rb @@ -39,59 +39,12 @@ describe ImportHelper do 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 78477ab0a5a..1edd8e69b8f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -925,6 +925,21 @@ describe User do 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 78ff9c6e6fd..106f92082e4 100644 --- a/spec/routing/import_routing_spec.rb +++ b/spec/routing/import_routing_spec.rb @@ -23,6 +23,11 @@ require 'spec_helper' # 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 @@ shared_examples 'importer routing' do 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 @@ end # 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 00000000000..f8b71ceb9f3 --- /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 00000000000..6e5bdd8c52d --- /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 00000000000..e476da82729 --- /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 00000000000..22f958fc17f --- /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 00000000000..b67115bab10 --- /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 00000000000..f2be30c36d9 --- /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 697f999e4c4..5bb1269a19d 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 @@ end 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 @@ shared_examples 'a GitHub-ish import controller: GET status' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 @@ shared_examples 'a GitHub-ish import controller: POST create' do 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 |