Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create TableHeaderBullets component #397

Merged
merged 2 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as React from 'react';

import { getDisplayNameByResource, getDatabaseDisplayName } from 'config/config-utils';

import { ResourceType } from 'interfaces/Resources';

export interface TableHeaderBulletsProps {
cluster: string;
database: string;
}

const TableHeaderBullets: React.SFC<TableHeaderBulletsProps> = ({ cluster, database }) => {
return (
<ul className="header-bullets">
<li>{ getDisplayNameByResource(ResourceType.table)}</li>
<li>{ getDatabaseDisplayName(database) }</li>
<li>{ cluster }</li>
</ul>
);
};

TableHeaderBullets.defaultProps = {
cluster: '',
database: '',
};

export default TableHeaderBullets;
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import * as React from 'react';

import { mocked } from 'ts-jest/utils';
import { shallow } from 'enzyme';

import TableHeaderBullets, { TableHeaderBulletsProps } from '../';

import { ResourceType } from 'interfaces/Resources';

const MOCK_RESOURCE_DISPLAY_NAME = 'Test';
const MOCK_DB_DISPLAY_NAME = 'AlsoTest';

jest.mock('config/config-utils', () => ({
getDisplayNameByResource: jest.fn(),
getDatabaseDisplayName: jest.fn()
}));
import { getDatabaseDisplayName, getDisplayNameByResource } from 'config/config-utils';

describe('TableHeaderBullets', () => {
const setup = (propOverrides?: Partial<TableHeaderBulletsProps>) => {
const props: TableHeaderBulletsProps = {
database: 'hive',
cluster: 'main',
...propOverrides
};
const wrapper = shallow(<TableHeaderBullets {...props} />);
return { props, wrapper };
};

describe('render', () => {
let props: TableHeaderBulletsProps;
let wrapper;
let listElement;
beforeAll(() => {
mocked(getDatabaseDisplayName).mockImplementation(() => MOCK_DB_DISPLAY_NAME);
mocked(getDisplayNameByResource).mockImplementation(() => MOCK_RESOURCE_DISPLAY_NAME);
const setupResult = setup();
props = setupResult.props;
wrapper = setupResult.wrapper;
listElement = wrapper.find('ul');
});

it('renders a list with correct class', () => {
expect(listElement.props().className).toEqual('header-bullets');
});

it('renders a list with resource display name', () => {
expect(getDisplayNameByResource).toHaveBeenCalledWith(ResourceType.table);
expect(listElement.find('li').at(0).text()).toEqual(MOCK_RESOURCE_DISPLAY_NAME);
});

it('renders a list with database display name', () => {
expect(getDatabaseDisplayName).toHaveBeenCalledWith(props.database);
expect(listElement.find('li').at(1).text()).toEqual(MOCK_DB_DISPLAY_NAME);
});

it('renders a list with cluster', () => {
expect(listElement.find('li').at(2).text()).toEqual(props.cluster);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import OwnerEditor from 'components/TableDetail/OwnerEditor';
import ReportTableIssue from 'components/TableDetail/ReportTableIssue';
import SourceLink from 'components/TableDetail/SourceLink';
import TableDescEditableText from 'components/TableDetail/TableDescEditableText';
import TableHeaderBullets from 'components/TableDetail/TableHeaderBullets';
import TableIssues from 'components/TableDetail/TableIssues';
import WatermarkLabel from 'components/TableDetail/WatermarkLabel';
import WriterLink from 'components/TableDetail/WriterLink';
Expand All @@ -32,7 +33,7 @@ import { TableMetadata } from 'interfaces/TableMetadata';

import { EditableSection } from 'components/TableDetail/EditableSection';

import { getDisplayNameByResource, getDatabaseDisplayName, getDatabaseIconClass, issueTrackingEnabled, notificationsEnabled } from 'config/config-utils';
import { getDatabaseIconClass, issueTrackingEnabled, notificationsEnabled } from 'config/config-utils';

import { ResourceType } from 'interfaces/Resources';
import { formatDateTimeShort } from 'utils/dateUtils';
Expand Down Expand Up @@ -142,11 +143,10 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
</h3>
<BookmarkIcon bookmarkKey={ this.props.tableData.key }/>
<div className="body-2">
<ul className="header-bullets">
<li>{ getDisplayNameByResource(ResourceType.table) }</li>
<li>{ getDatabaseDisplayName(data.database) }</li>
<li>{ data.cluster }</li>
</ul>
<TableHeaderBullets
database={ data.database }
cluster={ data.cluster }
/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought about overwriting the TableHeaderBullets -- If we wanted to add another field from the table metadata, we'd have to overwrite the component file and the template that uses the component. On the other hand, I can see how extracting specific fields and passing it to the component is cleaner and easier to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that. As our use cases for overwriting components internally expose themselves, I will lean towards whatever makes maintenance the least painful.

{
data.badges.length > 0 &&
<BadgeList badges={ data.badges } />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';

import { mocked } from 'ts-jest/utils';
import { shallow } from 'enzyme';

import SearchItemList, { SearchItemListProps } from '../';
Expand Down Expand Up @@ -82,8 +83,7 @@ describe('SearchItemList', () => {

describe('renders ResourceType.user SearchItem based on config', () =>{
it('when indexUsersEnabled = true, renders SearchItem', () => {
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
indexUsersEnabled.mockImplementation(() => true);
mocked(indexUsersEnabled).mockImplementation(() => true);
setUpResult = setup();
props = setUpResult.props;
wrapper = setUpResult.wrapper;
Expand All @@ -101,8 +101,7 @@ describe('SearchItemList', () => {
});

it('when indexUsersEnabled = false, does not render SearchItem', () => {
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
indexUsersEnabled.mockImplementation(() => false);
mocked(indexUsersEnabled).mockImplementation(() => false);
wrapper = setup().wrapper;
const item = wrapper.find('SearchItem').findWhere(item => item.prop('resourceType') === ResourceType.user);
expect(item.exists()).toBe(false)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';

import { mocked } from 'ts-jest/utils';
import { shallow } from 'enzyme';

import { InlineSearchResults, InlineSearchResultsProps, mapStateToProps } from '../';
Expand Down Expand Up @@ -210,8 +211,7 @@ describe('InlineSearchResults', () => {
});
it('returns the results of getDatabaseIconClass for ResourceType.table', () => {
const mockClass = 'test-class';
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
getDatabaseIconClass.mockImplementation(() => mockClass);
mocked(getDatabaseIconClass).mockImplementation(() => mockClass);
const givenTable = props.tables.results[0];
const output = wrapper.instance().getSuggestedResultIconClass(ResourceType.table, givenTable);
expect(output).toEqual(mockClass);
Expand Down Expand Up @@ -285,8 +285,7 @@ describe('InlineSearchResults', () => {
});
it('returns the results of getDatabaseDisplayName for ResourceType.table', () => {
const mockName = 'Hive';
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
getDatabaseDisplayName.mockImplementation(() => mockName);
mocked(getDatabaseDisplayName).mockImplementation(() => mockName);
const givenTable = props.tables.results[0];
const output = wrapper.instance().getSuggestedResultType(ResourceType.table, givenTable);
expect(output).toEqual(mockName);
Expand Down Expand Up @@ -401,16 +400,14 @@ describe('InlineSearchResults', () => {

describe('calls renderResultsByResource for ResourceType.user based on config', () => {
it('does not call if indexUsersEnabled() = false', () => {
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
indexUsersEnabled.mockImplementation(() => false);
mocked(indexUsersEnabled).mockImplementation(() => false);
renderResultsByResourceSpy.mockClear();
wrapper.instance().renderResults();
expect(renderResultsByResourceSpy).not.toHaveBeenCalledWith(ResourceType.user);
});

it('calls if indexUsersEnabled() = true', () => {
// @ts-ignore: Known issue but can't find solution: https://github.com/kulshekhar/ts-jest/issues/661
indexUsersEnabled.mockImplementation(() => true);
mocked(indexUsersEnabled).mockImplementation(() => true);
renderResultsByResourceSpy.mockClear();
wrapper.instance().renderResults();
expect(renderResultsByResourceSpy).toHaveBeenCalledWith(ResourceType.user);
Expand Down