Commit f67ed510 authored by Josianne Hyson's avatar Josianne Hyson

Fix issue with importable repo list not refreshing

Prior to this change, the filter function on the GitHub repo import page
would set the filter on the state and clear the repos, which would
result in no repos being shown in the filtering. After setting the
filter, we also need to re-fetch the repos from the backend that match
this filter.

Add a call to fetch repos after setting the filter and make the filter
case insensitive.

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45783
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/36556
parent 591b1dc3
...@@ -37,8 +37,6 @@ const restartJobsPolling = () => { ...@@ -37,8 +37,6 @@ const restartJobsPolling = () => {
if (eTagPoll) eTagPoll.restart(); if (eTagPoll) eTagPoll.restart();
}; };
const setFilter = ({ commit }, filter) => commit(types.SET_FILTER, filter);
const setImportTarget = ({ commit }, { repoId, importTarget }) => const setImportTarget = ({ commit }, { repoId, importTarget }) =>
commit(types.SET_IMPORT_TARGET, { repoId, importTarget }); commit(types.SET_IMPORT_TARGET, { repoId, importTarget });
...@@ -172,12 +170,9 @@ const fetchNamespacesFactory = (namespacesPath = isRequired()) => ({ commit }) = ...@@ -172,12 +170,9 @@ const fetchNamespacesFactory = (namespacesPath = isRequired()) => ({ commit }) =
}); });
}; };
const setPage = ({ state, commit, dispatch }, page) => { const setFilter = ({ commit, dispatch }, filter) => {
if (page === state.pageInfo.page) { commit(types.SET_FILTER, filter);
return null;
}
commit(types.SET_PAGE, page);
return dispatch('fetchRepos'); return dispatch('fetchRepos');
}; };
...@@ -188,7 +183,6 @@ export default ({ endpoints = isRequired() }) => ({ ...@@ -188,7 +183,6 @@ export default ({ endpoints = isRequired() }) => ({
setFilter, setFilter,
setImportTarget, setImportTarget,
importAll, importAll,
setPage,
fetchRepos: fetchReposFactory({ reposPath: endpoints.reposPath }), fetchRepos: fetchReposFactory({ reposPath: endpoints.reposPath }),
fetchImport: fetchImportFactory(endpoints.importPath), fetchImport: fetchImportFactory(endpoints.importPath),
fetchJobs: fetchJobsFactory(endpoints.jobsPath), fetchJobs: fetchJobsFactory(endpoints.jobsPath),
......
...@@ -48,18 +48,14 @@ class Import::BaseController < ApplicationController ...@@ -48,18 +48,14 @@ class Import::BaseController < ApplicationController
private private
def filter_attribute
:name
end
def sanitized_filter_param def sanitized_filter_param
@filter ||= sanitize(params[:filter]) @filter ||= sanitize(params[:filter])&.downcase
end end
def filtered(collection) def filtered(collection)
return collection unless sanitized_filter_param return collection unless sanitized_filter_param
collection.select { |item| item[filter_attribute].include?(sanitized_filter_param) } collection.select { |item| item[:name].to_s.downcase.include?(sanitized_filter_param) }
end end
def serialized_provider_repos def serialized_provider_repos
......
...@@ -132,8 +132,4 @@ class Import::BitbucketController < Import::BaseController ...@@ -132,8 +132,4 @@ class Import::BitbucketController < Import::BaseController
refresh_token: session[:bitbucket_refresh_token] refresh_token: session[:bitbucket_refresh_token]
} }
end end
def sanitized_filter_param
@filter ||= sanitize(params[:filter])
end
end end
...@@ -170,10 +170,6 @@ class Import::BitbucketServerController < Import::BaseController ...@@ -170,10 +170,6 @@ class Import::BitbucketServerController < Import::BaseController
BitbucketServer::Paginator::PAGE_LENGTH BitbucketServer::Paginator::PAGE_LENGTH
end end
def sanitized_filter_param
sanitize(params[:filter])
end
def bitbucket_connection_error(error) def bitbucket_connection_error(error)
flash[:alert] = _("Unable to connect to server: %{error}") % { error: error } flash[:alert] = _("Unable to connect to server: %{error}") % { error: error }
clear_session_data clear_session_data
......
...@@ -245,14 +245,6 @@ class Import::GithubController < Import::BaseController ...@@ -245,14 +245,6 @@ class Import::GithubController < Import::BaseController
def extra_import_params def extra_import_params
{} {}
end end
def sanitized_filter_param
@filter ||= sanitize(params[:filter])
end
def filter_attribute
:name
end
end end
Import::GithubController.prepend_if_ee('EE::Import::GithubController') Import::GithubController.prepend_if_ee('EE::Import::GithubController')
---
title: Fix project import search box and make it case insensitive
merge_request: 45783
author:
type: fixed
...@@ -33,7 +33,6 @@ describe('ImportProjectsTable', () => { ...@@ -33,7 +33,6 @@ describe('ImportProjectsTable', () => {
const importAllFn = jest.fn(); const importAllFn = jest.fn();
const importAllModalShowFn = jest.fn(); const importAllModalShowFn = jest.fn();
const setPageFn = jest.fn();
const fetchReposFn = jest.fn(); const fetchReposFn = jest.fn();
function createComponent({ function createComponent({
...@@ -60,7 +59,6 @@ describe('ImportProjectsTable', () => { ...@@ -60,7 +59,6 @@ describe('ImportProjectsTable', () => {
stopJobsPolling: jest.fn(), stopJobsPolling: jest.fn(),
clearJobsEtagPoll: jest.fn(), clearJobsEtagPoll: jest.fn(),
setFilter: jest.fn(), setFilter: jest.fn(),
setPage: setPageFn,
}, },
}); });
......
...@@ -16,6 +16,7 @@ import { ...@@ -16,6 +16,7 @@ import {
RECEIVE_NAMESPACES_SUCCESS, RECEIVE_NAMESPACES_SUCCESS,
RECEIVE_NAMESPACES_ERROR, RECEIVE_NAMESPACES_ERROR,
SET_PAGE, SET_PAGE,
SET_FILTER,
} from '~/import_projects/store/mutation_types'; } from '~/import_projects/store/mutation_types';
import actionsFactory from '~/import_projects/store/actions'; import actionsFactory from '~/import_projects/store/actions';
import { getImportTarget } from '~/import_projects/store/getters'; import { getImportTarget } from '~/import_projects/store/getters';
...@@ -40,7 +41,7 @@ const { ...@@ -40,7 +41,7 @@ const {
fetchImport, fetchImport,
fetchJobs, fetchJobs,
fetchNamespaces, fetchNamespaces,
setPage, setFilter,
} = actionsFactory({ } = actionsFactory({
endpoints, endpoints,
}); });
...@@ -359,21 +360,17 @@ describe('import_projects store actions', () => { ...@@ -359,21 +360,17 @@ describe('import_projects store actions', () => {
], ],
); );
}); });
});
describe('setPage', () => { describe('setFilter', () => {
it('dispatches fetchRepos and commits setPage when page number differs from current one', async () => { it('dispatches sets the filter value and dispatches fetchRepos', async () => {
await testAction( await testAction(
setPage, setFilter,
2, 'filteredRepo',
{ ...localState, pageInfo: { page: 1 } }, localState,
[{ type: SET_PAGE, payload: 2 }], [{ type: SET_FILTER, payload: 'filteredRepo' }],
[{ type: 'fetchRepos' }], [{ type: 'fetchRepos' }],
); );
});
it('does not perform any action if page equals to current one', async () => {
await testAction(setPage, 2, { ...localState, pageInfo: { page: 2 } }, [], []);
});
}); });
}); });
}); });
...@@ -157,6 +157,16 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do ...@@ -157,6 +157,16 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do
expect(json_response.dig("namespaces", 0, "id")).to eq(group.id) expect(json_response.dig("namespaces", 0, "id")).to eq(group.id)
end end
it 'filters the list, ignoring the case of the name' do
get :status, params: { filter: 'EMACS' }, format: :json
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.dig("imported_projects").count).to eq(0)
expect(json_response.dig("provider_repos").count).to eq(1)
expect(json_response.dig("provider_repos", 0, "id")).to eq(repo_2.id)
expect(json_response.dig("namespaces", 0, "id")).to eq(group.id)
end
context 'when user input contains html' do context 'when user input contains html' do
let(:expected_filter) { 'test' } let(:expected_filter) { 'test' }
let(:filter) { "<html>#{expected_filter}</html>" } let(:filter) { "<html>#{expected_filter}</html>" }
...@@ -167,6 +177,23 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do ...@@ -167,6 +177,23 @@ RSpec.shared_examples 'a GitHub-ish import controller: GET status' do
expect(assigns(:filter)).to eq(expected_filter) expect(assigns(:filter)).to eq(expected_filter)
end end
end end
context 'when the client returns a non-string name' do
before do
repos = [build(:project, name: 2, path: 'test')]
client = stub_client(repos: repos)
allow(client).to receive(:each_page).and_return([OpenStruct.new(objects: repos)].to_enum)
end
it 'does not raise an error' do
get :status, params: { filter: '2' }, format: :json
expect(response).to have_gitlab_http_status :ok
expect(json_response.dig("provider_repos").count).to eq(1)
end
end
end end
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