Unverified Commit adf52564 authored by Tamika Tannis's avatar Tamika Tannis Committed by GitHub

feat: Dashboard Filter Support (#482)

* Initial dashboard search support

* Fix some logic

* Fix aligment of FilterSection label; Unnest component tests

* Further modify search

* Fix merge issue

* WIP

* WIP

* Fix build

* Update ducks tests

* Fix test

* Cleanup; Add default helpText for consistency
parent e2c13990
......@@ -23,6 +23,7 @@ REQUEST_SESSION_TIMEOUT_SEC = 3
search_blueprint = Blueprint('search', __name__, url_prefix='/api/search/v0')
SEARCH_DASHBOARD_ENDPOINT = '/search_dashboard'
SEARCH_DASHBOARD_FILTER_ENDPOINT = '/search_dashboard_filter'
SEARCH_TABLE_ENDPOINT = '/search'
SEARCH_TABLE_FILTER_ENDPOINT = '/search_table'
SEARCH_USER_ENDPOINT = '/search_user'
......@@ -42,7 +43,7 @@ def search_table() -> Response:
search_type = request_json.get('searchType')
transformed_filters = transform_filters(filters=request_json.get('filters', {}))
transformed_filters = transform_filters(filters=request_json.get('filters', {}), resource='table')
results_dict = _search_table(filters=transformed_filters,
search_term=search_term,
......@@ -78,7 +79,7 @@ def _search_table(*, search_term: str, page_index: int, filters: Dict, search_ty
}
try:
if has_filters(filters=filters):
if has_filters(filters=filters, resource='table'):
query_json = generate_query_json(filters=filters, page_index=page_index, search_term=search_term)
url_base = app.config['SEARCHSERVICE_BASE'] + SEARCH_TABLE_FILTER_ENDPOINT
response = request_search(url=url_base,
......@@ -185,18 +186,24 @@ def _search_user(*, search_term: str, page_index: int, search_type: str) -> Dict
return results_dict
@search_blueprint.route('/dashboard', methods=['GET'])
@search_blueprint.route('/dashboard', methods=['POST'])
def search_dashboard() -> Response:
"""
Parse the request arguments and call the helper method to execute a dashboard search
:return: a Response created with the results from the helper method
"""
try:
search_term = get_query_param(request.args, 'query', 'Endpoint takes a "query" parameter')
page_index = get_query_param(request.args, 'page_index', 'Endpoint takes a "page_index" parameter')
search_type = request.args.get('search_type')
request_json = request.get_json()
results_dict = _search_dashboard(search_term=search_term, page_index=int(page_index), search_type=search_type)
search_term = get_query_param(request_json, 'term', '"term" parameter expected in request data')
page_index = get_query_param(request_json, 'pageIndex', '"pageIndex" parameter expected in request data')
search_type = request_json.get('searchType')
transformed_filters = transform_filters(filters=request_json.get('filters', {}), resource='dashboard')
results_dict = _search_dashboard(filters=transformed_filters,
search_term=search_term,
page_index=page_index,
search_type=search_type)
return make_response(jsonify(results_dict), results_dict.get('status_code', HTTPStatus.INTERNAL_SERVER_ERROR))
except Exception as e:
......@@ -206,7 +213,7 @@ def search_dashboard() -> Response:
@action_logging
def _search_dashboard(*, search_term: str, page_index: int, search_type: str) -> Dict[str, Any]:
def _search_dashboard(*, search_term: str, page_index: int, filters: Dict, search_type: str) -> Dict[str, Any]:
"""
Call the search service endpoint and return matching results
Search service logic defined here:
......@@ -228,9 +235,17 @@ def _search_dashboard(*, search_term: str, page_index: int, search_type: str) ->
}
try:
url_base = app.config['SEARCHSERVICE_BASE'] + SEARCH_DASHBOARD_ENDPOINT
url = f'{url_base}?query_term={search_term}&page_index={page_index}'
response = request_search(url=url)
if has_filters(filters=filters, resource='dashboard'):
query_json = generate_query_json(filters=filters, page_index=page_index, search_term=search_term)
url_base = app.config['SEARCHSERVICE_BASE'] + SEARCH_DASHBOARD_FILTER_ENDPOINT
response = request_search(url=url_base,
headers={'Content-Type': 'application/json'},
method='POST',
data=json.dumps(query_json))
else:
url_base = app.config['SEARCHSERVICE_BASE'] + SEARCH_DASHBOARD_ENDPOINT
url = f'{url_base}?query_term={search_term}&page_index={page_index}'
response = request_search(url=url)
status_code = response.status_code
if status_code == HTTPStatus.OK:
......
......@@ -3,12 +3,20 @@ from typing import Dict, List # noqa: F401
# These can move to a configuration when we have custom use cases outside of these default values
valid_search_fields = {
'badges',
'column',
'database',
'schema',
'table',
'tag'
'table': {
'badges',
'column',
'database',
'schema',
'table',
'tag'
},
'dashboard': {
'group_name',
'name',
'product',
'tag'
}
}
......@@ -27,14 +35,15 @@ def map_table_result(result: Dict) -> Dict:
}
def transform_filters(*, filters: Dict = {}) -> Dict:
def transform_filters(*, filters: Dict = {}, resource: str) -> Dict:
"""
Transforms the data shape of filters from the application to the data
shape required by the search service according to the api defined at:
https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/table/search_table_filter.yml
https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/dashboard/search_dashboard_filter.yml
"""
filter_payload = {}
for category in valid_search_fields:
for category in valid_search_fields.get(resource, {}):
values = filters.get(category)
value_list = [] # type: List
if values is not None:
......@@ -53,6 +62,7 @@ def generate_query_json(*, filters: Dict = {}, page_index: int, search_term: str
Transforms the given paramaters to the query json for the search service according to
the api defined at:
https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/table/search_table_filter.yml
https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/dashboard/search_dashboard_filter.yml
"""
return {
'page_index': int(page_index),
......@@ -64,12 +74,12 @@ def generate_query_json(*, filters: Dict = {}, page_index: int, search_term: str
}
def has_filters(*, filters: Dict = {}) -> bool:
def has_filters(*, filters: Dict = {}, resource: str = '') -> bool:
"""
Returns whether or not the filter dictionary passed to the search service
has at least one filter value for a valid filter category
"""
for category in valid_search_fields:
for category in valid_search_fields.get(resource, {}):
filter_list = filters.get(category, [])
if len(filter_list) > 0:
return True
......
......@@ -13,7 +13,7 @@ import {
CheckBoxFilterProps,
mapDispatchToProps,
mapStateToProps,
} from '..';
} from '.';
describe('CheckBoxFilter', () => {
const setup = (propOverrides?: Partial<CheckBoxFilterProps>) => {
......
......@@ -13,8 +13,8 @@ import {
FilterSectionProps,
mapDispatchToProps,
mapStateToProps,
} from '..';
import { CLEAR_BTN_TEXT } from '../../constants';
} from '.';
import { CLEAR_BTN_TEXT } from '../constants';
describe('FilterSection', () => {
const setup = (propOverrides?: Partial<FilterSectionProps>) => {
......
......@@ -6,13 +6,13 @@ import { GlobalState } from 'ducks/rootReducer';
import globalState from 'fixtures/globalState';
import { FilterType, ResourceType } from 'interfaces';
import { APPLY_BTN_TEXT } from '../../constants';
import { APPLY_BTN_TEXT } from '../constants';
import {
InputFilter,
InputFilterProps,
mapDispatchToProps,
mapStateToProps,
} from '..';
} from '.';
describe('InputFilter', () => {
const setStateSpy = jest.spyOn(InputFilter.prototype, 'setState');
......
......@@ -15,7 +15,7 @@ import {
SearchFilterProps,
FilterSection,
CheckboxFilterSection,
} from '..';
} from '.';
describe('SearchFilter', () => {
const setup = (propOverrides?: Partial<SearchFilterProps>) => {
......
......@@ -73,9 +73,11 @@ export const mapStateToProps = (state: GlobalState) => {
type: categoryConfig.type,
};
if (categoryConfig.type === FilterType.CHECKBOX_SELECT) {
section['options'] = categoryConfig.options.map((option) => {
return { value: option.value, label: option.displayName };
});
(section as CheckboxFilterSection).options = categoryConfig.options.map(
({ value, displayName }) => {
return { value, label: displayName };
}
);
}
filterSections.push(section);
});
......
......@@ -15,6 +15,10 @@
button {
margin: auto 4px;
}
label {
margin-bottom: 0;
}
}
}
......
......@@ -3,8 +3,8 @@ import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { ResourceType, Tag, SearchType } from 'interfaces';
import { submitSearchResource } from 'ducks/search/reducer';
import { SubmitSearchResourceRequest } from 'ducks/search/types';
import { updateSearchState } from 'ducks/search/reducer';
import { UpdateSearchStateRequest } from 'ducks/search/types';
import { logClick } from 'ducks/utilMethods';
import './styles.scss';
......@@ -15,7 +15,7 @@ interface OwnProps {
}
export interface DispatchFromProps {
searchTag: (tagName: string) => SubmitSearchResourceRequest;
searchTag: (tagName: string) => UpdateSearchStateRequest;
}
export type TagInfoProps = OwnProps & DispatchFromProps;
......@@ -57,13 +57,12 @@ export const mapDispatchToProps = (dispatch: any) => {
return bindActionCreators(
{
searchTag: (tagName: string) =>
submitSearchResource({
resourceFilters: { tag: tagName },
resource: ResourceType.table,
pageIndex: 0,
searchTerm: '',
searchType: SearchType.FILTER,
updateUrl: true,
updateSearchState({
filters: {
[ResourceType.dashboard]: { tag: tagName },
[ResourceType.table]: { tag: tagName },
},
submitSearch: true,
}),
},
dispatch
......
......@@ -60,6 +60,32 @@ const configDefault: AppConfig = {
iconClass: 'icon-mode',
},
},
filterCategories: [
{
categoryId: 'product',
displayName: 'Product',
helpText: 'Enter exact product name or a regex wildcard pattern',
type: FilterType.INPUT_SELECT,
},
{
categoryId: 'group_name',
displayName: 'Groups',
helpText: 'Enter exact group name or a regex wildcard pattern',
type: FilterType.INPUT_SELECT,
},
{
categoryId: 'name',
displayName: 'Name',
helpText: 'Enter exact dashboard name or a regex wildcard pattern',
type: FilterType.INPUT_SELECT,
},
{
categoryId: 'tag',
displayName: 'Tag',
helpText: 'Enter exact tag name or a regex wildcard pattern',
type: FilterType.INPUT_SELECT,
},
],
},
[ResourceType.table]: {
displayName: 'Datasets',
......
......@@ -18,6 +18,7 @@ jest.mock('axios');
describe('searchResource', () => {
let axiosMockGet;
let axiosMockPost;
let dashboardEnabledMock;
let userEnabledMock;
let mockSearchResponse: AxiosResponse<API.SearchAPI>;
beforeAll(() => {
......@@ -43,6 +44,7 @@ describe('searchResource', () => {
userEnabledMock = jest
.spyOn(ConfigUtils, 'indexUsersEnabled')
.mockImplementation(() => true);
dashboardEnabledMock = jest.spyOn(ConfigUtils, 'indexDashboardsEnabled');
});
describe('searchResource', () => {
......@@ -87,7 +89,7 @@ describe('searchResource', () => {
expect(axiosMockPost).not.toHaveBeenCalled();
});
describe('if not searching a table resource', () => {
describe('if searching a user resource', () => {
it('calls axios get with request for a resource', async () => {
axiosMockGet.mockClear();
axiosMockPost.mockClear();
......@@ -124,8 +126,8 @@ describe('searchResource', () => {
});
});
describe('if searching a table resource', () => {
it('calls axios post with request for a resource', async () => {
describe('if not searching a user resource', () => {
it('calls axios post with request for a table resource', async () => {
axiosMockGet.mockClear();
axiosMockPost.mockClear();
const pageIndex = 0;
......@@ -152,6 +154,34 @@ describe('searchResource', () => {
);
});
it('calls axios post with request for a dashboard resource', async () => {
axiosMockGet.mockClear();
axiosMockPost.mockClear();
dashboardEnabledMock.mockImplementationOnce(() => true);
const pageIndex = 0;
const resourceType = ResourceType.dashboard;
const term = 'test';
const filters = { name: 'test' };
const searchType = SearchType.SUBMIT_TERM;
await API.searchResource(
pageIndex,
resourceType,
term,
filters,
searchType
);
expect(axiosMockGet).not.toHaveBeenCalled();
expect(axiosMockPost).toHaveBeenCalledWith(
`${API.BASE_URL}/${resourceType}`,
{
filters,
pageIndex,
term,
searchType,
}
);
});
it('calls searchResourceHelper with api call response', async () => {
const searchResourceHelperSpy = jest.spyOn(API, 'searchResourceHelper');
await API.searchResource(
......
......@@ -42,8 +42,7 @@ export function searchResource(
) {
/* If resource support is not configured or if there is no search term for non-filter supported resources */
if (
(resource === ResourceType.dashboard &&
(!indexDashboardsEnabled() || term.length === 0)) ||
(resource === ResourceType.dashboard && !indexDashboardsEnabled()) ||
(resource === ResourceType.user &&
(!indexUsersEnabled() || term.length === 0))
) {
......@@ -51,7 +50,7 @@ export function searchResource(
}
/* Note: This logic must exist until query string endpoints are created for all resources */
if (resource === ResourceType.table) {
if (resource === ResourceType.table || resource === ResourceType.dashboard) {
return axios
.post(`${BASE_URL}/${resource}`, {
filters,
......
......@@ -35,6 +35,7 @@ export function updateFilterByCategory({
export type FilterOptions = { [id: string]: boolean };
export interface FilterReducerState {
[ResourceType.dashboard]?: ResourceFilterReducerState;
[ResourceType.table]: ResourceFilterReducerState;
}
......@@ -44,8 +45,10 @@ export interface ResourceFilterReducerState {
/* REDUCER */
export const initialTableFilterState = {};
export const initialDashboardFilterState = {};
export const initialFilterState: FilterReducerState = {
[ResourceType.dashboard]: initialDashboardFilterState,
[ResourceType.table]: initialTableFilterState,
};
......
......@@ -197,12 +197,14 @@ export function updateSearchState({
filters,
resource,
updateUrl,
submitSearch,
}: UpdateSearchStatePayload): UpdateSearchStateRequest {
return {
payload: {
filters,
resource,
updateUrl,
submitSearch,
},
type: UpdateSearchState.REQUEST,
};
......
......@@ -114,9 +114,11 @@ export function* submitSearchResourceWatcher(): SagaIterator {
export function* updateSearchStateWorker(
action: UpdateSearchStateRequest
): SagaIterator {
const { filters, resource, updateUrl } = action.payload;
const { filters, resource, updateUrl, submitSearch } = action.payload;
const state = yield select(getSearchState);
if (updateUrl) {
if (filters && submitSearch) {
yield put(searchAll(SearchType.FILTER, '', undefined, 0, true));
} else if (updateUrl) {
updateSearchUrl({
resource: resource || state.resource,
term: state.search_term,
......
......@@ -285,6 +285,21 @@ describe('search sagas', () => {
filters: searchState.filters,
});
});
it('it updates filters and executes search', () => {
const action = updateSearchState({
filters: { [ResourceType.table]: { database: { bigquery: true } } },
submitSearch: true,
});
const { search_term, resource } = searchState;
testSaga(Sagas.updateSearchStateWorker, action)
.next()
.select(SearchUtils.getSearchState)
.next(searchState)
.put(searchAll(SearchType.FILTER, '', undefined, 0, true))
.next()
.isDone();
});
});
describe('updateSearchStateWatcher', () => {
......
......@@ -153,6 +153,7 @@ export type UpdateSearchStatePayload = {
filters?: FilterReducerState;
resource?: ResourceType;
updateUrl?: boolean;
submitSearch?: boolean;
};
export interface UpdateSearchStateRequest {
payload?: UpdateSearchStatePayload;
......
......@@ -6,8 +6,8 @@ from http import HTTPStatus
from unittest.mock import patch
from amundsen_application import create_app
from amundsen_application.api.search.v0 import SEARCH_DASHBOARD_ENDPOINT, SEARCH_TABLE_ENDPOINT, \
SEARCH_TABLE_FILTER_ENDPOINT, SEARCH_USER_ENDPOINT
from amundsen_application.api.search.v0 import SEARCH_DASHBOARD_ENDPOINT, SEARCH_DASHBOARD_FILTER_ENDPOINT, \
SEARCH_TABLE_ENDPOINT, SEARCH_TABLE_FILTER_ENDPOINT, SEARCH_USER_ENDPOINT
local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates')
......@@ -97,7 +97,7 @@ class SearchTable(unittest.TestCase):
'pageIndex': 1,
'filters': test_filters,
'searchType': 'test'})
transform_filter_mock.assert_called_with(filters=test_filters)
transform_filter_mock.assert_called_with(filters=test_filters, resource='table')
@responses.activate
@patch('amundsen_application.api.search.v0.transform_filters')
......@@ -375,6 +375,7 @@ class SearchDashboard(unittest.TestCase):
},
]
self.search_service_url = local_app.config['SEARCHSERVICE_BASE'] + SEARCH_DASHBOARD_ENDPOINT
self.search_service_filter_url = local_app.config['SEARCHSERVICE_BASE'] + SEARCH_DASHBOARD_FILTER_ENDPOINT
self.fe_flask_endpoint = '/api/search/v0/dashboard'
def test_fail_if_term_is_none(self) -> None:
......@@ -383,7 +384,7 @@ class SearchDashboard(unittest.TestCase):
:return:
"""
with local_app.test_client() as test:
response = test.get(self.fe_flask_endpoint, query_string=dict(page_index='0'))
response = test.post(self.fe_flask_endpoint, json={'pageIndex': '0'})
self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
def test_fail_if_page_index_is_none(self) -> None:
......@@ -392,32 +393,105 @@ class SearchDashboard(unittest.TestCase):
:return:
"""
with local_app.test_client() as test:
response = test.get(self.fe_flask_endpoint, query_string=dict(query='test'))
response = test.post(self.fe_flask_endpoint, json={'term': 'test'})
self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
@responses.activate
@patch('amundsen_application.api.search.v0.transform_filters')
def test_calls_transform_filters(self, transform_filter_mock) -> None:
"""
Test transform_filters is called with the filters from the request json
from the request_json
:return:
"""
test_filters = {}
responses.add(responses.POST,
self.search_service_url,
json=self.mock_results,
status=HTTPStatus.OK)
with local_app.test_client() as test:
test.post(self.fe_flask_endpoint,
json={
'term': 'hello',
'pageIndex': 1,
'filters': test_filters,
'searchType': 'test'})
transform_filter_mock.assert_called_with(filters=test_filters, resource='dashboard')
@responses.activate
@patch('amundsen_application.api.search.v0._search_dashboard')
def test_calls_search_dashboard_log_helper(self, search_dashboard_mock) -> None:
"""
Test _search_table helper method is called with correct arguments
Test _search_dashboard helper method is called wwith correct arguments for logging
from the request_json
:return:
"""
test_term = 'hello'
test_index = 1
test_search_type = 'test'
mock_filters = {}
responses.add(responses.GET,
self.search_service_url,
body=self.mock_results,
status=HTTPStatus.OK)
with local_app.test_client() as test:
test.get(self.fe_flask_endpoint, query_string=dict(query=test_term,
page_index=test_index,
search_type=test_search_type))
search_dashboard_mock.assert_called_with(page_index=test_index,
test.post(self.fe_flask_endpoint,
json={
'term': test_term,
'pageIndex': test_index,
'filters': mock_filters,
'searchType': test_search_type})
search_dashboard_mock.assert_called_with(filters=mock_filters,
page_index=test_index,
search_term=test_term,
search_type=test_search_type)
@responses.activate
@patch('amundsen_application.api.search.v0.transform_filters')
@patch('amundsen_application.api.search.v0.has_filters')
@patch('amundsen_application.api.search.v0.generate_query_json')
def test_calls_generate_query_json(self, mock_generate_query_json, has_filters_mock, transform_filter_mock) -> None:
"""
Test generate_query_json helper method is called with correct arguments
from the request_json if filters exist
:return:
"""
test_term = 'hello'
test_index = 1
responses.add(responses.POST,
self.search_service_filter_url,
json=self.mock_results,
status=HTTPStatus.OK)
has_filters_mock.return_value = True
mock_filters = {'group_name': 'test'}
transform_filter_mock.return_value = mock_filters
with local_app.test_client() as test:
test.post(self.fe_flask_endpoint,
json={'term': test_term, 'pageIndex': test_index, 'filters': {}})
mock_generate_query_json.assert_called_with(filters=mock_filters,
page_index=test_index,
search_term=test_term)
@responses.activate
@patch('amundsen_application.api.search.v0.has_filters')
@patch('amundsen_application.api.search.v0.generate_query_json')
def test_does_not_calls_generate_query_json(self, mock_generate_query_json, has_filters_mock) -> None:
"""
Test generate_query_json helper method is not called if filters do not exist
:return:
"""
test_term = 'hello'
test_index = 1
responses.add(responses.GET, self.search_service_url, json=self.mock_results, status=HTTPStatus.OK)
has_filters_mock.return_value = False
with local_app.test_client() as test:
test.post(self.fe_flask_endpoint, json={'term': test_term, 'pageIndex': test_index, 'filters': {}})
mock_generate_query_json.assert_not_called()
@responses.activate
def test_request_success(self) -> None:
"""
......@@ -430,7 +504,8 @@ class SearchDashboard(unittest.TestCase):
status=HTTPStatus.OK)
with local_app.test_client() as test:
response = test.get(self.fe_flask_endpoint, query_string=dict(query='hello', page_index='0'))
response = test.post(self.fe_flask_endpoint,
json={'term': 'hello', 'pageIndex': '0'})
data = json.loads(response.data)
self.assertEqual(response.status_code, HTTPStatus.OK)
......@@ -451,7 +526,8 @@ class SearchDashboard(unittest.TestCase):
status=HTTPStatus.BAD_REQUEST)
with local_app.test_client() as test:
response = test.get(self.fe_flask_endpoint, query_string=dict(query='hello', page_index='1'))
response = test.post(self.fe_flask_endpoint,
json={'term': 'hello', 'pageIndex': '1'})
data = json.loads(response.data)
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
self.assertEqual(data.get('msg'), 'Encountered error: Search request failed')
......@@ -32,7 +32,8 @@ class SearchUtilsTest(unittest.TestCase):
Verify that the given filters are correctly transformed
:return:
"""
self.assertEqual(transform_filters(filters=self.test_filters), self.expected_transformed_filters)
self.assertEqual(transform_filters(filters=self.test_filters, resource='table'),
self.expected_transformed_filters)
def test_generate_query_json(self) -> None:
"""
......@@ -54,13 +55,13 @@ class SearchUtilsTest(unittest.TestCase):
Returns true if called with a dictionary that has values for a valid filter category
:return:
"""
self.assertTrue(has_filters(filters=self.expected_transformed_filters))
self.assertTrue(has_filters(filters=self.expected_transformed_filters, resource='table'))
def test_has_filters_return_false(self) -> None:
"""
Returns false if called with a dictionary that has no values for a valid filter category
:return:
"""
self.assertFalse(has_filters(filters={'fake_category': ['db1']}))
self.assertFalse(has_filters(filters={'tag': []}))
self.assertFalse(has_filters(filters={'fake_category': ['db1']}, resource='table'))
self.assertFalse(has_filters(filters={'tag': []}, resource='table'))
self.assertFalse(has_filters())
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