Commit 7df1cd20 authored by Tamika Tannis's avatar Tamika Tannis Committed by Tao Feng

Unit test coverage for SearchPage & nested components (#118)

* Tests for SearchList

* Tests for SearchBar

* Cleanup

* Tests for SearchPage

* Cleanup

* Move SearchPage hardcoded text to constants file

* Complete SearchPage coverage + update doc and test scripts

* Typos

* Add some doc

* nits
parent f0007ae2
module.exports = { module.exports = {
coverageThreshold: { coverageThreshold: {
'./js/config': { './js/config': {
branches: 0, // ?? branches: 50, // 100
functions: 0, // ?? functions: 0, // 100
lines: 0, // ?? lines: 50, // 100
statements: 0, // ?? statements: 50, // 100
}, },
'./js/components': { './js/components': {
branches: 15, // 75 branches: 30, // 75
functions: 25, // 75 functions: 30, // 75
lines: 25, // 75 lines: 35, // 75
statements: 25, // 75 statements: 35, // 75
}, },
'./js/ducks': { './js/ducks': {
branches: 0, // 75 branches: 0, // 75
......
// TODO: Hard-coded text strings should be translatable/customizable
export const ERROR_CLASSNAME = 'error';
export const PLACEHOLDER_DEFAULT = 'search for data resources...';
export const SUBTEXT_DEFAULT = `Search within a category using the pattern with wildcard support 'category:*searchTerm*', e.g. 'schema:*core*'.
Current categories are 'column', 'schema', 'table', and 'tag'.`;
export const SYNTAX_ERROR_CATEGORY = `Advanced search syntax only supports searching one category. Please remove all extra ':'`;
export const SYNTAX_ERROR_PREFIX = 'Did you mean ';
export const SYNTAX_ERROR_SPACING_SUFFIX = ` ? Please remove the space around the ':'.`;
...@@ -3,10 +3,16 @@ import * as React from 'react'; ...@@ -3,10 +3,16 @@ import * as React from 'react';
// TODO: Use css-modules instead of 'import' // TODO: Use css-modules instead of 'import'
import './styles.scss'; import './styles.scss';
const DEFAULT_SUBTEXT = `Search within a category using the pattern with wildcard support 'category:*searchTerm*', e.g. 'schema:*core*'. import {
Current categories are 'column', 'schema', 'table', and 'tag'.`; ERROR_CLASSNAME,
PLACEHOLDER_DEFAULT,
interface SearchBarProps { SUBTEXT_DEFAULT,
SYNTAX_ERROR_CATEGORY,
SYNTAX_ERROR_PREFIX,
SYNTAX_ERROR_SPACING_SUFFIX,
} from './constants';
export interface SearchBarProps {
handleValueSubmit: (term: string) => void; handleValueSubmit: (term: string) => void;
placeholder?: string; placeholder?: string;
searchTerm?: string; searchTerm?: string;
...@@ -14,58 +20,52 @@ interface SearchBarProps { ...@@ -14,58 +20,52 @@ interface SearchBarProps {
} }
interface SearchBarState { interface SearchBarState {
optionalSubTextClass: string; subTextClassName: string;
searchTerm: string; searchTerm: string;
subText: string; subText: string;
} }
class SearchBar extends React.Component<SearchBarProps, SearchBarState> { class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
private inputRef: React.RefObject<HTMLInputElement>; public static defaultProps: Partial<SearchBarProps> = {
placeholder: PLACEHOLDER_DEFAULT,
public static defaultProps: SearchBarProps = {
handleValueSubmit: () => undefined,
placeholder: 'search for data resources...', // TODO: Hard-coded text strings should be translatable/customizable
searchTerm: '', searchTerm: '',
subText: DEFAULT_SUBTEXT, subText: SUBTEXT_DEFAULT,
}; };
constructor(props) { constructor(props) {
super(props); super(props);
this.state = { this.state = {
optionalSubTextClass: '', subTextClassName: '',
searchTerm: this.props.searchTerm, searchTerm: this.props.searchTerm,
subText: this.props.subText, subText: this.props.subText,
}; };
this.inputRef = React.createRef();
} }
static getDerivedStateFromProps(nextProps, prevState) { static getDerivedStateFromProps(props, state) {
const { searchTerm } = nextProps; const { searchTerm } = props;
return { searchTerm }; return { searchTerm };
} }
handleValueSubmit = (event: React.FormEvent<HTMLFormElement>) => { handleValueChange = (event: React.SyntheticEvent<HTMLInputElement>) : void => {
this.setState({ searchTerm: (event.target as HTMLInputElement).value.toLowerCase() });
};
handleValueSubmit = (event: React.FormEvent<HTMLFormElement>) : void => {
event.preventDefault(); event.preventDefault();
if (this.isFormValid()) { if (this.isFormValid()) {
const inputElement = this.inputRef.current; this.props.handleValueSubmit(this.state.searchTerm);
this.props.handleValueSubmit(inputElement.value.toLowerCase());
} }
}; };
handleValueChange = (event: React.SyntheticEvent<HTMLInputElement>) => { isFormValid = () : boolean => {
this.setState({ searchTerm: (event.target as HTMLInputElement).value.toLowerCase() });
};
isFormValid = () => {
const searchTerm = this.state.searchTerm; const searchTerm = this.state.searchTerm;
const hasAtMostOneCategory = searchTerm.split(':').length <= 2; const hasAtMostOneCategory = searchTerm.split(':').length <= 2;
if (!hasAtMostOneCategory) { if (!hasAtMostOneCategory) {
this.setState({ this.setState({
subText: "Advanced search syntax only supports searching one category. Please remove all extra ':'", subText: SYNTAX_ERROR_CATEGORY,
optionalSubTextClass: "error" subTextClassName: ERROR_CLASSNAME,
}); });
return false; return false;
} }
...@@ -75,19 +75,18 @@ class SearchBar extends React.Component<SearchBarProps, SearchBarState> { ...@@ -75,19 +75,18 @@ class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
(colonIndex >= 1 && searchTerm.charAt(colonIndex+1) !== " " && searchTerm.charAt(colonIndex-1) !== " "); (colonIndex >= 1 && searchTerm.charAt(colonIndex+1) !== " " && searchTerm.charAt(colonIndex-1) !== " ");
if (!hasNoSpaceAroundColon) { if (!hasNoSpaceAroundColon) {
this.setState({ this.setState({
subText: `Did you mean '${searchTerm.substring(0,colonIndex).trim()}:${searchTerm.substring(colonIndex+1).trim()}' ? subText: `${SYNTAX_ERROR_PREFIX}'${searchTerm.substring(0,colonIndex).trim()}:${searchTerm.substring(colonIndex+1).trim()}'${SYNTAX_ERROR_SPACING_SUFFIX}`,
Please remove the space around the ':'.`, subTextClassName: ERROR_CLASSNAME,
optionalSubTextClass: "error"
}); });
return false; return false;
} }
this.setState({ subText: DEFAULT_SUBTEXT, optionalSubTextClass: "" }); this.setState({ subText: SUBTEXT_DEFAULT, subTextClassName: "" });
return true; return true;
}; };
render() { render() {
const subTextClass = `subtext ${this.state.optionalSubTextClass}`; const subTextClass = `subtext ${this.state.subTextClassName}`;
return ( return (
<div id="search-bar" className="col-xs-12 col-md-offset-1 col-md-10"> <div id="search-bar" className="col-xs-12 col-md-offset-1 col-md-10">
<form className="search-bar-form" onSubmit={ this.handleValueSubmit }> <form className="search-bar-form" onSubmit={ this.handleValueSubmit }>
...@@ -99,7 +98,6 @@ class SearchBar extends React.Component<SearchBarProps, SearchBarState> { ...@@ -99,7 +98,6 @@ class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
aria-label={ this.props.placeholder } aria-label={ this.props.placeholder }
placeholder={ this.props.placeholder } placeholder={ this.props.placeholder }
autoFocus={ true } autoFocus={ true }
ref={ this.inputRef }
/> />
<button className="btn btn-flat-icon search-bar-button" type="submit"> <button className="btn btn-flat-icon search-bar-button" type="submit">
<img className="icon icon-search" /> <img className="icon icon-search" />
......
import * as React from 'react';
import { shallow } from 'enzyme';
import SearchBar, { SearchBarProps } from '../';
import {
ERROR_CLASSNAME,
SUBTEXT_DEFAULT,
SYNTAX_ERROR_CATEGORY,
SYNTAX_ERROR_PREFIX,
SYNTAX_ERROR_SPACING_SUFFIX,
} from '../constants';
describe('SearchBar', () => {
const valueChangeMockEvent = { target: { value: 'Data Resources' } };
const submitMockEvent = { preventDefault: jest.fn() };
const setStateSpy = jest.spyOn(SearchBar.prototype, 'setState');
const setup = (propOverrides?: Partial<SearchBarProps>) => {
const props: SearchBarProps = {
handleValueSubmit: jest.fn(),
...propOverrides
};
const wrapper = shallow<SearchBar>(<SearchBar {...props} />)
return { props, wrapper };
};
describe('constructor', () => {
const searchTerm = 'data';
const subText = 'I am some text';
let wrapper;
beforeAll(() => {
wrapper = setup({ searchTerm, subText }).wrapper;
});
it('sets the searchTerm state from props', () => {
expect(wrapper.state().searchTerm).toEqual(searchTerm);
});
it('sets the subText state from props', () => {
expect(wrapper.state().subText).toEqual(subText);
});
});
describe('getDerivedStateFromProps', () => {
it('sets searchTerm on state from props', () => {
const { props, wrapper } = setup();
const prevState = wrapper.state();
props.searchTerm = 'newTerm';
wrapper.setProps(props);
expect(wrapper.state()).toMatchObject({
...prevState,
searchTerm: 'newTerm',
});
});
});
describe('handleValueChange', () => {
it('calls setState on searchTerm with event.target.value.toLowerCase()', () => {
const { props, wrapper } = setup();
// @ts-ignore: mocked events throw type errors
wrapper.instance().handleValueChange(valueChangeMockEvent);
expect(setStateSpy).toHaveBeenCalledWith({ searchTerm: valueChangeMockEvent.target.value.toLowerCase() });
});
});
describe('handleValueSubmit', () => {
let props;
let wrapper;
beforeAll(() => {
const setupResult = setup();
props = setupResult.props;
wrapper = setupResult.wrapper;
});
it('calls event.preventDefault', () => {
// @ts-ignore: mocked events throw type errors
wrapper.instance().handleValueSubmit(submitMockEvent);
expect(submitMockEvent.preventDefault).toHaveBeenCalled();
});
it('submits with correct props if isFormValid()', () => {
// @ts-ignore: mocked events throw type errors
wrapper.instance().handleValueSubmit(submitMockEvent);
expect(props.handleValueSubmit).toHaveBeenCalledWith(wrapper.state().searchTerm);
});
it('does not submit if !isFormValid()', () => {
const { props, wrapper } = setup({ searchTerm: 'tag:tag1 tag:tag2' });
// @ts-ignore: mocked events throw type errors
wrapper.instance().handleValueSubmit(submitMockEvent);
expect(props.handleValueSubmit).not.toHaveBeenCalled();
});
});
describe('isFormValid', () => {
describe('if searchTerm has more than one category', () => {
let wrapper;
beforeAll(() => {
wrapper = setup({ searchTerm: 'tag:tag1 tag:tag2' }).wrapper;
})
it('returns false', () => {
expect(wrapper.instance().isFormValid()).toEqual(false);
});
it('sets state.subText correctly', () => {
expect(wrapper.state().subText).toEqual(SYNTAX_ERROR_CATEGORY);
});
it('sets state.subTextClassName correctly', () => {
expect(wrapper.state().subTextClassName).toEqual(ERROR_CLASSNAME);
});
});
describe('if searchTerm has incorrect colon syntax', () => {
let wrapper;
beforeAll(() => {
wrapper = setup({ searchTerm: 'tag : tag1' }).wrapper;
})
it('returns false', () => {
expect(wrapper.instance().isFormValid()).toEqual(false);
});
it('sets state.subText correctly', () => {
expect(wrapper.state().subText).toEqual(`${SYNTAX_ERROR_PREFIX}'tag:tag1'${SYNTAX_ERROR_SPACING_SUFFIX}`);
});
it('sets state.subTextClassName correctly', () => {
expect(wrapper.state().subTextClassName).toEqual(ERROR_CLASSNAME);
});
});
describe('if searchTerm has correct syntax', () => {
let wrapper;
beforeAll(() => {
wrapper = setup({ searchTerm: 'tag:tag1' }).wrapper;
})
it('returns true', () => {
expect(wrapper.instance().isFormValid()).toEqual(true);
});
it('sets state.subText correctly', () => {
expect(wrapper.state().subText).toEqual(SUBTEXT_DEFAULT);
});
it('sets state.subTextClassName correctly', () => {
expect(wrapper.state().subTextClassName).toEqual('');
});
});
});
describe('render', () => {
let props;
let wrapper;
beforeAll(() => {
const setupResult = setup();
props = setupResult.props;
wrapper = setupResult.wrapper;
});
describe('form', () => {
it('renders with correct props', () => {
expect(wrapper.find('form').props()).toMatchObject({
className: 'search-bar-form',
onSubmit: wrapper.instance().handleValueSubmit,
});
});
it('renders input with correct default props', () => {
expect(wrapper.find('form').find('input').props()).toMatchObject({
'aria-label': SearchBar.defaultProps.placeholder,
autoFocus: true,
className: 'search-bar-input form-control',
id: 'search-input',
onChange: wrapper.instance().handleValueChange,
placeholder: SearchBar.defaultProps.placeholder,
value: wrapper.state().searchTerm,
});
});
it('renders input with correct given props', () => {
const { props, wrapper } = setup({ placeholder: 'Type something to search', searchTerm: 'data' });
expect(wrapper.find('form').find('input').props()).toMatchObject({
'aria-label': props.placeholder,
autoFocus: true,
className: 'search-bar-input form-control',
id: 'search-input',
onChange: wrapper.instance().handleValueChange,
placeholder: props.placeholder,
value: wrapper.state().searchTerm,
});
});
describe('submit button', () => {
it('renders button with correct props', () => {
expect(wrapper.find('form').find('button').props()).toMatchObject({
className: 'btn btn-flat-icon search-bar-button',
type: 'submit',
});
});
it('renders button img with correct props', () => {
expect(wrapper.find('form').find('button').find('img').props()).toMatchObject({
className: 'icon icon-search',
});
});
});
});
describe('subtext', () =>{
it('renders div with correct class', () => {
expect(wrapper.children().at(1).props()).toMatchObject({
className: `subtext ${wrapper.state().subTextClassName}`,
});
});
it('renders correct text', () => {
expect(wrapper.children().at(1).text()).toEqual(wrapper.state().subText);
});
});
});
});
...@@ -2,13 +2,12 @@ import * as React from 'react'; ...@@ -2,13 +2,12 @@ import * as React from 'react';
import ResourceListItem from 'components/common/ResourceListItem'; import ResourceListItem from 'components/common/ResourceListItem';
import { Resource } from 'components/common/ResourceListItem/types'; import { Resource } from 'components/common/ResourceListItem/types';
export interface SearchListProps {
interface SearchListProps {
results?: Resource[]; results?: Resource[];
params?: SearchListParams; params?: SearchListParams;
} }
interface SearchListParams { export interface SearchListParams {
source?: string; source?: string;
paginationStartIndex?: number; paginationStartIndex?: number;
} }
......
import * as React from 'react';
import { shallow } from 'enzyme';
import { Resource, ResourceType } from 'components/common/ResourceListItem/types';
import ResourceListItem from 'components/common/ResourceListItem';
import SearchList, { SearchListProps, SearchListParams } from '../';
describe('SearchList', () => {
const setup = (propOverrides?: Partial<SearchListProps>) => {
const props: SearchListProps = {
results: [
{ type: ResourceType.table },
{ type: ResourceType.user },
],
params: {
source: 'testSource',
paginationStartIndex: 0,
},
...propOverrides
};
const wrapper = shallow(<SearchList {...props} />);
return { props, wrapper };
};
describe('render', () => {
let props;
let wrapper;
beforeAll(() => {
const setupResult = setup();
props = setupResult.props;
wrapper = setupResult.wrapper;
});
it('renders unordered list', () => {
expect(wrapper.type()).toEqual('ul');
});
it('renders unordered list w/ correct className', () => {
expect(wrapper.props()).toMatchObject({
className: 'list-group',
});
});
it('renders a ResourceListItem for each result', () => {
const content = wrapper.find(ResourceListItem);
expect(content.length).toEqual(props.results.length);
});
it('passes correct props to each ResourceListItem', () => {
const content = wrapper.find(ResourceListItem);
content.forEach((contentItem, index) => {
expect(contentItem.props()).toMatchObject({
item: props.results[index],
logging: {
source: props.params.source,
index: props.params.paginationStartIndex + index,
}
})
});
});
});
});
export const PAGINATION_PAGE_RANGE = 10;
export const RESULTS_PER_PAGE = 10;
// TODO: Hard-coded text strings should be translatable/customizable
export const DOCUMENT_TITLE_SUFFIX = ' - Amundsen Search';
export const PAGE_INDEX_ERROR_MESSAGE = 'Page index out of bounds for available matches';
export const POPULAR_TABLES_INFO_TEXT = 'These are some of the most commonly accessed tables within your organization.';
export const POPULAR_TABLES_LABEL = 'Popular Tables';
export const POPULAR_TABLES_SOURCE_NAME = 'popular_tables';
export const SEARCH_INFO_TEXT = `Ordered by the relevance of matches within a resource's metadata, as well as overall usage.`;
export const SEARCH_SOURCE_NAME = 'search_results';
export const SEARCH_ERROR_MESSAGE_INFIX = ' - did not match any ';
export const SEARCH_ERROR_MESSAGE_PREFIX = 'Your search - ';
export const SEARCH_ERROR_MESSAGE_SUFFIX = ' result';
export const TABLE_RESOURCE_TITLE = 'Tables';
...@@ -28,12 +28,25 @@ import { GetPopularTablesRequest } from 'ducks/popularTables/types'; ...@@ -28,12 +28,25 @@ import { GetPopularTablesRequest } from 'ducks/popularTables/types';
// TODO: Use css-modules instead of 'import' // TODO: Use css-modules instead of 'import'
import './styles.scss'; import './styles.scss';
const RESULTS_PER_PAGE = 10; import {
DOCUMENT_TITLE_SUFFIX,
PAGE_INDEX_ERROR_MESSAGE,
PAGINATION_PAGE_RANGE,
POPULAR_TABLES_INFO_TEXT,
POPULAR_TABLES_LABEL,
POPULAR_TABLES_SOURCE_NAME,
RESULTS_PER_PAGE,
SEARCH_ERROR_MESSAGE_INFIX,
SEARCH_ERROR_MESSAGE_PREFIX,
SEARCH_ERROR_MESSAGE_SUFFIX,
SEARCH_INFO_TEXT,
SEARCH_SOURCE_NAME,
TABLE_RESOURCE_TITLE,
} from './constants';
export interface StateFromProps { export interface StateFromProps {
searchTerm: string; searchTerm: string;
popularTables: TableResource[]; popularTables: TableResource[];
tables: TableSearchResults; tables: TableSearchResults;
dashboards: DashboardSearchResults dashboards: DashboardSearchResults
users: UserSearchResults; users: UserSearchResults;
...@@ -45,35 +58,14 @@ export interface DispatchFromProps { ...@@ -45,35 +58,14 @@ export interface DispatchFromProps {
getPopularTables: () => GetPopularTablesRequest; getPopularTables: () => GetPopularTablesRequest;
} }
type SearchPageProps = StateFromProps & DispatchFromProps; export type SearchPageProps = StateFromProps & DispatchFromProps;
interface SearchPageState { interface SearchPageState {
selectedTab: ResourceType; selectedTab: ResourceType;
} }
export class SearchPage extends React.Component<SearchPageProps, SearchPageState> { export class SearchPage extends React.Component<SearchPageProps, SearchPageState> {
public static defaultProps: SearchPageProps = { public static defaultProps: Partial<SearchPageProps> = {};
searchAll: () => undefined,
searchResource: () => undefined,
getPopularTables: () => undefined,
searchTerm: '',
popularTables: [],
dashboards: {
page_index: 0,
results: [],
total_results: 0,
},
tables: {
page_index: 0,
results: [],
total_results: 0,
},
users: {
page_index: 0,
results: [],
total_results: 0,
}
};
constructor(props) { constructor(props) {
super(props); super(props);
...@@ -87,19 +79,19 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -87,19 +79,19 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
this.props.getPopularTables(); this.props.getPopularTables();
const params = qs.parse(window.location.search); const params = qs.parse(window.location.search);
const { searchTerm, pageIndex, selectedTab} = params; const { searchTerm, pageIndex, selectedTab } = params;
const validTab = this.validateTab(selectedTab); const currentTab = this.getSelectedTabByResourceType(selectedTab);
this.setState({ selectedTab: validTab }); this.setState({ selectedTab: currentTab });
if (searchTerm && searchTerm.length > 0) { if (searchTerm && searchTerm.length > 0) {
const index = pageIndex || 0; const index = pageIndex || 0;
this.props.searchAll(searchTerm, this.getSearchOptions(index, validTab)); this.props.searchAll(searchTerm, this.createSearchOptions(index, currentTab));
// Update the page URL with validated parameters. // Update the page URL with validated parameters.
this.updatePageUrl(searchTerm, validTab, index); this.updatePageUrl(searchTerm, currentTab, index);
} }
} }
validateTab = (newTab) => { getSelectedTabByResourceType = (newTab: ResourceType): ResourceType => {
switch(newTab) { switch(newTab) {
case ResourceType.table: case ResourceType.table:
case ResourceType.user: case ResourceType.user:
...@@ -110,7 +102,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -110,7 +102,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
} }
}; };
getSearchOptions = (pageIndex, selectedTab) => { createSearchOptions = (pageIndex: number, selectedTab: ResourceType) => {
return { return {
dashboardIndex: (selectedTab === ResourceType.dashboard) ? pageIndex : 0, dashboardIndex: (selectedTab === ResourceType.dashboard) ? pageIndex : 0,
userIndex: (selectedTab === ResourceType.user) ? pageIndex : 0, userIndex: (selectedTab === ResourceType.user) ? pageIndex : 0,
...@@ -118,7 +110,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -118,7 +110,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
}; };
}; };
getPageIndex = (tab) => { getPageIndexByResourceType = (tab: ResourceType): number => {
switch(tab) { switch(tab) {
case ResourceType.table: case ResourceType.table:
return this.props.tables.page_index; return this.props.tables.page_index;
...@@ -130,41 +122,39 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -130,41 +122,39 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
return 0; return 0;
}; };
onSearchBarSubmit = (searchTerm: string) => { onSearchBarSubmit = (searchTerm: string): void => {
this.props.searchAll(searchTerm); this.props.searchAll(searchTerm);
this.updatePageUrl(searchTerm, this.state.selectedTab,0); this.updatePageUrl(searchTerm, this.state.selectedTab,0);
}; };
onPaginationChange = (pageNumber) => { onPaginationChange = (pageNumber: number): void => {
// subtract 1 : pagination component indexes from 1, while our api is 0-indexed
const index = pageNumber - 1; const index = pageNumber - 1;
this.props.searchResource(this.state.selectedTab, this.props.searchTerm, index); this.props.searchResource(this.state.selectedTab, this.props.searchTerm, index);
this.updatePageUrl(this.props.searchTerm, this.state.selectedTab, index); this.updatePageUrl(this.props.searchTerm, this.state.selectedTab, index);
}; };
onTabChange = (tab: ResourceType) => { onTabChange = (tab: ResourceType): void => {
const validTab = this.validateTab(tab); const currentTab = this.getSelectedTabByResourceType(tab);
this.setState({ selectedTab: validTab }); this.setState({ selectedTab: currentTab });
this.updatePageUrl(this.props.searchTerm, validTab, this.getPageIndex(validTab)); this.updatePageUrl(this.props.searchTerm, currentTab, this.getPageIndexByResourceType(currentTab));
}; };
updatePageUrl = (searchTerm, tab, pageIndex) => { updatePageUrl = (searchTerm: string, tab: ResourceType, pageIndex: number): void => {
const pathName = `/search?searchTerm=${searchTerm}&selectedTab=${tab}&pageIndex=${pageIndex}`; const pathName = `/search?searchTerm=${searchTerm}&selectedTab=${tab}&pageIndex=${pageIndex}`;
window.history.pushState({}, '', `${window.location.origin}${pathName}`); window.history.pushState({}, '', `${window.location.origin}${pathName}`);
}; };
renderPopularTables = () => { renderPopularTables = () => {
const searchListParams = { const searchListParams = {
source: 'popular_tables', source: POPULAR_TABLES_SOURCE_NAME,
paginationStartIndex: 0, paginationStartIndex: 0,
}; };
return ( return (
<div className="col-xs-12 col-md-offset-1 col-md-10"> <div className="col-xs-12 col-md-offset-1 col-md-10">
<div className="search-list-container"> <div className="search-list-container">
<div className="popular-tables-header"> <div className="popular-tables-header">
<label>Popular Tables</label> <label>{POPULAR_TABLES_LABEL}</label>
<InfoButton infoText={ "These are some of the most commonly accessed tables within your organization." }/> <InfoButton infoText={POPULAR_TABLES_INFO_TEXT}/>
</div> </div>
<SearchList results={ this.props.popularTables } params={ searchListParams }/> <SearchList results={ this.props.popularTables } params={ searchListParams }/>
</div> </div>
...@@ -175,9 +165,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -175,9 +165,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
renderSearchResults = () => { renderSearchResults = () => {
const tabConfig = [ const tabConfig = [
{ {
title: `Tables (${ this.props.tables.total_results })`, title: `${TABLE_RESOURCE_TITLE} (${ this.props.tables.total_results })`,
key: ResourceType.table, key: ResourceType.table,
content: this.getTabContent(this.props.tables, 'tables'), content: this.getTabContent(this.props.tables, TABLE_RESOURCE_TITLE),
}, },
// TODO PEOPLE - Add users tab // TODO PEOPLE - Add users tab
]; ];
...@@ -194,14 +184,11 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -194,14 +184,11 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
); );
}; };
// TODO: Hard-coded text strings should be translatable/customizable
getTabContent = (results, tabLabel) => { getTabContent = (results, tabLabel) => {
const { searchTerm } = this.props; const { searchTerm } = this.props;
const { page_index, total_results } = results; const { page_index, total_results } = results;
const startIndex = (RESULTS_PER_PAGE * page_index) + 1; const startIndex = (RESULTS_PER_PAGE * page_index) + 1;
const endIndex = RESULTS_PER_PAGE * ( page_index + 1); const endIndex = RESULTS_PER_PAGE * (page_index + 1);
// TODO - Move error messages into Tab Component // TODO - Move error messages into Tab Component
// Check no results // Check no results
...@@ -209,7 +196,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -209,7 +196,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
return ( return (
<div className="search-list-container"> <div className="search-list-container">
<div className="search-error"> <div className="search-error">
Your search - <i>{ searchTerm }</i> - did not match any { tabLabel } result {SEARCH_ERROR_MESSAGE_PREFIX}<i>{ searchTerm }</i>{SEARCH_ERROR_MESSAGE_INFIX}{tabLabel.toLowerCase()}{SEARCH_ERROR_MESSAGE_SUFFIX}
</div> </div>
</div> </div>
) )
...@@ -220,7 +207,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -220,7 +207,7 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
return ( return (
<div className="search-list-container"> <div className="search-list-container">
<div className="search-error"> <div className="search-error">
Page index out of bounds for available matches. {PAGE_INDEX_ERROR_MESSAGE}
</div> </div>
</div> </div>
) )
...@@ -231,9 +218,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -231,9 +218,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
<div className="search-list-container"> <div className="search-list-container">
<div className="search-list-header"> <div className="search-list-header">
<label>{ title }</label> <label>{ title }</label>
<InfoButton infoText={ "Ordered by the relevance of matches within a resource's metadata, as well as overall usage." }/> <InfoButton infoText={SEARCH_INFO_TEXT}/>
</div> </div>
<SearchList results={ results.results } params={ {source: 'search_results', paginationStartIndex: 0 } }/> <SearchList results={ results.results } params={ {source: SEARCH_SOURCE_NAME, paginationStartIndex: 0 } }/>
<div className="search-pagination-component"> <div className="search-pagination-component">
{ {
total_results > RESULTS_PER_PAGE && total_results > RESULTS_PER_PAGE &&
...@@ -241,11 +228,11 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -241,11 +228,11 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
activePage={ page_index + 1 } activePage={ page_index + 1 }
itemsCountPerPage={ RESULTS_PER_PAGE } itemsCountPerPage={ RESULTS_PER_PAGE }
totalItemsCount={ total_results } totalItemsCount={ total_results }
pageRangeDisplayed={ 10 } pageRangeDisplayed={ PAGINATION_PAGE_RANGE }
onChange={ this.onPaginationChange } onChange={ this.onPaginationChange }
/> />
} }
</div> </div>
</div> </div>
); );
}; };
...@@ -261,9 +248,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState ...@@ -261,9 +248,9 @@ export class SearchPage extends React.Component<SearchPageProps, SearchPageState
</div> </div>
</div> </div>
); );
if (searchTerm !== undefined && searchTerm.length > 0) { if (searchTerm.length > 0) {
return ( return (
<DocumentTitle title={ searchTerm + " - Amundsen Search" }> <DocumentTitle title={ `${searchTerm}${DOCUMENT_TITLE_SUFFIX}` }>
{ innerContent } { innerContent }
</DocumentTitle> </DocumentTitle>
); );
......
import { GlobalState } from 'ducks/rootReducer'; import { GlobalState } from 'ducks/rootReducer';
import { SendingState } from 'components/Feedback/types'; import { SendingState } from 'components/Feedback/types';
import { ResourceType } from 'components/common/ResourceListItem/types';
const globalState: GlobalState = { const globalState: GlobalState = {
announcements: { announcements: {
...@@ -17,9 +19,28 @@ const globalState: GlobalState = { ...@@ -17,9 +19,28 @@ const globalState: GlobalState = {
feedback: { feedback: {
sendState: SendingState.IDLE, sendState: SendingState.IDLE,
}, },
popularTables: [], popularTables: [
{
cluster: 'testCluster',
database: 'testDatabase',
description: 'I have a lot of users',
key: 'testDatabase://testCluster.testSchema/testName',
name: 'testName',
schema_name: 'testSchema',
type: ResourceType.table,
},
{
cluster: 'testCluster',
database: 'testDatabase',
description: 'I also have a lot of users',
key: 'testDatabase://testCluster.testSchema/otherName',
name: 'otherName',
schema_name: 'testSchema',
type: ResourceType.table,
}
],
search: { search: {
search_term: '', search_term: 'testName',
dashboards: { dashboards: {
page_index: 0, page_index: 0,
results: [], results: [],
...@@ -27,8 +48,19 @@ const globalState: GlobalState = { ...@@ -27,8 +48,19 @@ const globalState: GlobalState = {
}, },
tables: { tables: {
page_index: 0, page_index: 0,
results: [], results: [
total_results: 0, {
cluster: 'testCluster',
database: 'testDatabase',
description: 'I have a lot of users',
key: 'testDatabase://testCluster.testSchema/testName',
last_updated_epoch: 946684799,
name: 'testName',
schema_name: 'testSchema',
type: ResourceType.table,
},
],
total_results: 1,
}, },
users: { users: {
page_index: 0, page_index: 0,
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
"build": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -p --progress --config webpack.prod.ts", "build": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -p --progress --config webpack.prod.ts",
"dev-build": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts", "dev-build": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts",
"test": "jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}", "test": "jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}",
"test-nocov": "jest",
"watch": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts --watch", "watch": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts --watch",
"lint": "npm run eslint && npm run tslint", "lint": "npm run eslint && npm run tslint",
"lint-fix": "npm run eslint-fix && npm run tslint-fix", "lint-fix": "npm run eslint-fix && npm run tslint-fix",
......
...@@ -25,7 +25,8 @@ $ python3 wsgi.py ...@@ -25,7 +25,8 @@ $ python3 wsgi.py
## Contributing ## Contributing
### Python ### Testing
#### Python
If changes were made to any python files, run the python unit tests, linter, and type checker. Unit tests are run with `py.test`. They are located in `tests/unit`. Type checks are run with `mypy`. Linting is `flake8`. There are friendly `make` targets for each of these tests: If changes were made to any python files, run the python unit tests, linter, and type checker. Unit tests are run with `py.test`. They are located in `tests/unit`. Type checks are run with `mypy`. Linting is `flake8`. There are friendly `make` targets for each of these tests:
```bash ```bash
...@@ -36,7 +37,17 @@ make mypy # type checks ...@@ -36,7 +37,17 @@ make mypy # type checks
``` ```
Fix all errors before submitting a PR. Fix all errors before submitting a PR.
### JS Assets #### JS Assets
By default, the build commands that are run to verify local changes -- `npm run build` and `npm run dev-build` -- also conduct linting and type checking. During development be sure to fix all errors before submitting a PR.
Run unit tests by executing `npm run test`. Fix all failures before submitting a PR. ##### Type Checking
The build commands `npm run build` and `npm run dev-build` also conduct type checking. Run either of these commands and fix all failed checks before submitting a PR.
##### Lint
`npm run lint` runs the linter. Fix all lint errors before submitting a PR. `npm run lint-fix` can help auto-fix most common errors.
##### Unit Tests
`npm run test` runs unit tests. Add unit tests to cover new code additions and fix any test failures before submitting a PR.
To run specific tests, run `npm run test-nocov -t <regex>`, where `<regex>` is any pattern that matches the names of the test blocks that you want to run.
See our recommendations for writing unit tests [here](https://github.com/lyft/amundsenfrontendlibrary/blob/master/docs/recommended-practices.md).
# Recommended Practices
This document serves as reference for current practices and patterns that we want to standardize across Amundsen's frontend application code. Below, we provide high-level guidelines targeted towards new contributors or any contributor who does not yet have domain knowledge in a particular framework or core library. This document is **not** intended to provide an exhaustive checklist for completing certain tasks.
We aim to maintain a reasonably consistent code base through these practices and welcome PRs to update and improve these recommendations.
## React Application
### Unit Testing
We use [Jest](https://jestjs.io/) as our test framework and leverage utility methods from [Enzyme](https://airbnb.io/enzyme/) to test React components.
#### Recommendations
1. Leverage TypeScript to prevent bugs in unit tests and ensure that components are tested with data that matches their defined interfaces. Adding and updating test [fixtures](https://github.com/lyft/amundsenfrontendlibrary/tree/master/amundsen_application/static/js/fixtures) helps to provide re-useable pieces of typed test data for our code.
2. Enzyme provides 3 different utilities for rendering React components for testing. We recommend using `shallow` rendering to start off. If a component has a use case that requires full DOM rendering, those cases will become apparent. See Enzyme's [api documentation](https://airbnb.io/enzyme/docs/api/) to read more about the recommendations for each option.
3. Create a re-useable `setup()` function that will take any arguments needed to test conditional logic.
4. Look for opportunities to organize tests a way such that one `setup()` can be used to test assertions that occur under the same conditions. For example, a test block for a method that has no conditional logic should only have one `setup()`. However, it is **not** recommended to share a `setup()` result across tests for different methods because we risk propagating side effects from one test block to another.
5. Leverage `beforeAll()`/`beforeEach()` for test setup when applicable. Leverage `afterAll()`/`afterEach` for test teardown to remove any side effects of the test block. For example if a mock implementation of a method was created in `beforeAll()`, the original implementation should be restored in `afterAll()`. See Jest's [setup-teardown documentation](https://jestjs.io/docs/en/setup-teardown) for further understanding.
6. Use descriptive title strings. To assist with debugging we should be able to understand what a test is checking for, and under what conditions.
7. Consider refactoring components or other files if they become burdensome to test. Potential options include (but are not limited to):
* Creating subcomponents for large components, or breaking down large functions.
* Export constants from a separate file for hardcoded values and import them into the relevant source files and test files. This is especially helpful for strings.
8. Code coverage is important to track but it only informs us of what code was actually run and executed during the test. The onus is on the developer to make sure that right assertions are run and that logic is adequately tested.
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