Skip to content

G
gitlab-ce
Commit 0f385b5c authored by Illya Klymov's avatar Illya Klymov
Browse files
Options

Fix project importers pagination issues 

* fix project importers pagination issue if all repos on page were
already imported

Changelog: fixed
parent 9a5456ab
Hide whitespace changes
Inline Side-by-side
Showing with 26 additions and 97 deletions
app/assets/javascripts/import_entities/import_projects/components/import_projects_table.vue
View file @ 0f385b5c
......@@ -32,7 +32,7 @@ export default {
},
computed: {
...mapState(['filter', 'repositories', 'namespaces', 'defaultTargetNamespace']),
...mapState(['filter', 'repositories', 'namespaces', 'defaultTargetNamespace', 'pageInfo']),
...mapGetters([
'isLoading',
'isImportingAnyRepo',
......@@ -43,7 +43,7 @@ export default {
]),
pagePaginationStateKey() {
return `${this.filter}-${this.repositories.length}`;
return `${this.filter}-${this.repositories.length}-${this.pageInfo.page}`;
},
availableNamespaces() {
......
app/assets/javascripts/import_entities/import_projects/store/actions.js
View file @ 0f385b5c
......@@ -53,7 +53,6 @@ const importAll = ({ state, dispatch }) => {
const fetchReposFactory = ({ reposPath = isRequired() }) => ({ state, commit }) => {
const nextPage = state.pageInfo.page + 1;
commit(types.SET_PAGE, nextPage);
commit(types.REQUEST_REPOS);
const { provider, filter } = state;
......@@ -67,11 +66,10 @@ const fetchReposFactory = ({ reposPath = isRequired() }) => ({ state, commit })
}),
)
.then(({ data }) => {
commit(types.SET_PAGE, nextPage);
commit(types.RECEIVE_REPOS_SUCCESS, convertObjectPropsToCamelCase(data, { deep: true }));
})
.catch((e) => {
commit(types.SET_PAGE, nextPage - 1);
if (hasRedirectInError(e)) {
redirectToUrlInError(e);
} else if (tooManyRequests(e)) {
......
app/assets/javascripts/import_entities/import_projects/store/mutations.js
View file @ 0f385b5c
......@@ -9,7 +9,7 @@ const makeNewImportedProject = (importedProject) => ({
sanitizedName: importedProject.name,
providerLink: importedProject.providerLink,
},
importedProject,
importedProject: { ...importedProject },
});
const makeNewIncompatibleProject = (project) => ({
......@@ -63,15 +63,17 @@ export default {
factory: makeNewIncompatibleProject,
});
state.repositories = [
...newImportedProjects,
...state.repositories,
...repositories.providerRepos.map((project) => ({
const existingProjects = [...newImportedProjects, ...state.repositories];
const newProjects = repositories.providerRepos
.filter(
(project) => !existingProjects.find((p) => p.importSource.fullName === project.fullName),
)
.map((project) => ({
importSource: project,
importedProject: null,
})),
...newIncompatibleProjects,
];
}));
state.repositories = [...existingProjects, ...newProjects, ...newIncompatibleProjects];
if (incompatibleRepos.length === 0 && repositories.providerRepos.length === 0) {
state.pageInfo.page -= 1;
......
app/controllers/import/bitbucket_controller.rb
View file @ 0f385b5c
......@@ -62,14 +62,10 @@ class Import::BitbucketController < Import::BaseController
protected
# rubocop: disable CodeReuse/ActiveRecord
override :importable_repos
def importable_repos
already_added_projects_names = already_added_projects.map(&:import_source)
bitbucket_repos.reject { |repo| already_added_projects_names.include?(repo.full_name) || !repo.valid? }
bitbucket_repos.filter { |repo| repo.valid? }
end
# rubocop: enable CodeReuse/ActiveRecord
override :incompatible_repos
def incompatible_repos
......
app/controllers/import/bitbucket_server_controller.rb
View file @ 0f385b5c
......@@ -62,16 +62,10 @@ class Import::BitbucketServerController < Import::BaseController
protected
# rubocop: disable CodeReuse/ActiveRecord
override :importable_repos
def importable_repos
# Use the import URL to filter beyond what BaseService#find_already_added_projects
already_added_projects = filter_added_projects('bitbucket_server', bitbucket_repos.map(&:browse_url))
already_added_projects_names = already_added_projects.map(&:import_source)
bitbucket_repos.reject { |repo| already_added_projects_names.include?(repo.browse_url) || !repo.valid? }
bitbucket_repos.filter { |repo| repo.valid? }
end
# rubocop: enable CodeReuse/ActiveRecord
override :incompatible_repos
def incompatible_repos
......@@ -90,12 +84,6 @@ class Import::BitbucketServerController < Import::BaseController
private
# rubocop: disable CodeReuse/ActiveRecord
def filter_added_projects(import_type, import_sources)
current_user.created_projects.where(import_type: import_type, import_source: import_sources).with_import_state
end
# rubocop: enable CodeReuse/ActiveRecord
def client
@client ||= BitbucketServer::Client.new(credentials)
end
......
app/controllers/import/fogbugz_controller.rb
View file @ 0f385b5c
......@@ -77,11 +77,7 @@ class Import::FogbugzController < Import::BaseController
# rubocop: disable CodeReuse/ActiveRecord
override :importable_repos
def importable_repos
repos = client.repos
already_added_projects_names = already_added_projects.map(&:import_source)
repos.reject { |repo| already_added_projects_names.include? repo.name }
client.repos
end
# rubocop: enable CodeReuse/ActiveRecord
......
app/controllers/import/github_controller.rb
View file @ 0f385b5c
......@@ -64,9 +64,7 @@ class Import::GithubController < Import::BaseController
# rubocop: disable CodeReuse/ActiveRecord
override :importable_repos
def importable_repos
already_added_projects_names = already_added_projects.pluck(:import_source)
client_repos.reject { |repo| already_added_projects_names.include?(repo.full_name) }
client_repos.to_a
end
# rubocop: enable CodeReuse/ActiveRecord
......
app/controllers/import/gitlab_controller.rb
View file @ 0f385b5c
......@@ -39,16 +39,10 @@ class Import::GitlabController < Import::BaseController
protected
# rubocop: disable CodeReuse/ActiveRecord
override :importable_repos
def importable_repos
repos = client.projects(starting_page: 1, page_limit: MAX_PROJECT_PAGES, per_page: PER_PAGE_PROJECTS)
already_added_projects_names = already_added_projects.map(&:import_source)
repos.reject { |repo| already_added_projects_names.include? repo["path_with_namespace"] }
client.projects(starting_page: 1, page_limit: MAX_PROJECT_PAGES, per_page: PER_PAGE_PROJECTS)
end
# rubocop: enable CodeReuse/ActiveRecord
override :incompatible_repos
def incompatible_repos
......
spec/controllers/import/manifest_controller_spec.rb
View file @ 0f385b5c
......@@ -75,16 +75,6 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do
expect(json_response.dig("provider_repos", 0, "id")).to eq(repo1[:id])
expect(json_response.dig("provider_repos", 1, "id")).to eq(repo2[:id])
end
it "does not show already added project" do
project = create(:project, import_type: 'manifest', namespace: user.namespace, import_status: :finished, import_url: repo1[:url])
get :status, format: :json
expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id)
expect(json_response.dig("provider_repos").length).to eq(1)
expect(json_response.dig("provider_repos", 0, "id")).not_to eq(repo1[:id])
end
end
context 'when the data is stored via Gitlab::ManifestImport::Metadata' do
......
spec/frontend/import_entities/import_projects/store/actions_spec.js
View file @ 0f385b5c
......@@ -85,7 +85,7 @@ describe('import_projects store actions', () => {
afterEach(() => mock.restore());
it('commits SET_PAGE, REQUEST_REPOS, RECEIVE_REPOS_SUCCESS mutations on a successful request', () => {
it('commits REQUEST_REPOS, SET_PAGE, RECEIVE_REPOS_SUCCESS mutations on a successful request', () => {
mock.onGet(MOCK_ENDPOINT).reply(200, payload);
return testAction(
......@@ -93,8 +93,8 @@ describe('import_projects store actions', () => {
null,
localState,
[
{ type: SET_PAGE, payload: 1 },
{ type: REQUEST_REPOS },
{ type: SET_PAGE, payload: 1 },
{
type: RECEIVE_REPOS_SUCCESS,
payload: convertObjectPropsToCamelCase(payload, { deep: true }),
......@@ -104,19 +104,14 @@ describe('import_projects store actions', () => {
);
});
it('commits SET_PAGE, REQUEST_REPOS, RECEIVE_REPOS_ERROR and SET_PAGE again mutations on an unsuccessful request', () => {
it('commits REQUEST_REPOS, RECEIVE_REPOS_ERROR mutations on an unsuccessful request', () => {
mock.onGet(MOCK_ENDPOINT).reply(500);
return testAction(
fetchRepos,
null,
localState,
[
{ type: SET_PAGE, payload: 1 },
{ type: REQUEST_REPOS },
{ type: SET_PAGE, payload: 0 },
{ type: RECEIVE_REPOS_ERROR },
],
[{ type: REQUEST_REPOS }, { type: RECEIVE_REPOS_ERROR }],
[],
);
});
......@@ -135,7 +130,7 @@ describe('import_projects store actions', () => {
expect(requestedUrl).toBe(`${MOCK_ENDPOINT}?page=${localStateWithPage.pageInfo.page + 1}`);
});
it('correctly updates current page on an unsuccessful request', () => {
it('correctly keeps current page on an unsuccessful request', () => {
mock.onGet(MOCK_ENDPOINT).reply(500);
const CURRENT_PAGE = 5;
......@@ -143,10 +138,7 @@ describe('import_projects store actions', () => {
fetchRepos,
null,
{ ...localState, pageInfo: { page: CURRENT_PAGE } },
expect.arrayContaining([
{ type: SET_PAGE, payload: CURRENT_PAGE + 1 },
{ type: SET_PAGE, payload: CURRENT_PAGE },
]),
expect.arrayContaining([]),
[],
);
});
......@@ -159,12 +151,7 @@ describe('import_projects store actions', () => {
fetchRepos,
null,
{ ...localState, filter: 'filter' },
[
{ type: SET_PAGE, payload: 1 },
{ type: REQUEST_REPOS },
{ type: SET_PAGE, payload: 0 },
{ type: RECEIVE_REPOS_ERROR },
],
[{ type: REQUEST_REPOS }, { type: RECEIVE_REPOS_ERROR }],
[],
);
......@@ -183,8 +170,8 @@ describe('import_projects store actions', () => {
null,
{ ...localState, filter: 'filter' },
[
{ type: SET_PAGE, payload: 1 },
{ type: REQUEST_REPOS },
{ type: SET_PAGE, payload: 1 },
{
type: RECEIVE_REPOS_SUCCESS,
payload: convertObjectPropsToCamelCase(payload, { deep: true }),
......
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb
View file @ 0f385b5c
......@@ -82,16 +82,6 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do
expect(json_response.dig("provider_repos", 1, "id")).to eq(org_repo.id)
end
it "does not show already added project" do
project = create(:project, import_type: provider, namespace: user.namespace, import_status: :finished, import_source: 'asd/vim')
stub_client(repos: [repo], orgs: [], each_page: [OpenStruct.new(objects: [repo])].to_enum)
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
stub_client(repos: [], orgs: [], each_page: [])
......
spec/support/shared_examples/controllers/import_controller_status_shared_examples.rb
View file @ 0f385b5c
......@@ -19,14 +19,4 @@ RSpec.shared_examples 'import controller status' do
expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id)
expect(json_response.dig("provider_repos", 0, "id")).to eq(repo_id)
end
it "does not show already added project" do
project = create(:project, import_type: provider_name, namespace: user.namespace, import_status: :finished, import_source: import_source)
stub_client(client_repos_field => [repo])
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
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7