From 534a61179e2d0d7f9f376af1d01ed536e27f5b6d Mon Sep 17 00:00:00 2001 From: Luke Bennett 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 | 81 ++++++++++++-- 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 + .../unreleased/import-go-to-project-cta.yml | 5 + config/routes/import.rb | 4 +- lib/gitlab/etag_caching/router.rb | 8 ++ .../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 ++++++++++++++---- 26 files changed, 426 insertions(+), 120 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 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) + # Request repos to display error page if provider token is invalid + # Improving in https://gitlab.com/gitlab-org/gitlab-ce/issues/55585 + client_repos - @repos.reject! { |repo| already_added_projects_names.include? repo.full_name } - end - # rubocop: enable CodeReuse/ActiveRecord - - def jobs - render json: find_jobs(provider) + 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