Commit 1945ca62 authored by Mikhail Ivanov's avatar Mikhail Ivanov Committed by Daniel

Improve table & columns description formatting (#98) (#298)

* Add support for React-Markdown to editable text fields
* Add support for windows via cross-env 
parent 560b89db
import json
import logging import logging
from http import HTTPStatus from http import HTTPStatus
...@@ -259,13 +260,12 @@ def put_table_description() -> Response: ...@@ -259,13 +260,12 @@ def put_table_description() -> Response:
table_key = get_query_param(args, 'key') table_key = get_query_param(args, 'key')
description = get_query_param(args, 'description') description = get_query_param(args, 'description')
description = ' '.join(description.split())
src = get_query_param(args, 'source') src = get_query_param(args, 'source')
url = '{0}/{1}/description/{2}'.format(table_endpoint, table_key, description) url = '{0}/{1}/description'.format(table_endpoint, table_key)
_log_put_table_description(table_key=table_key, description=description, source=src) _log_put_table_description(table_key=table_key, description=description, source=src)
response = request_metadata(url=url, method='PUT') response = request_metadata(url=url, method='PUT', json=json.dumps({'description': description}))
status_code = response.status_code status_code = response.status_code
if status_code == HTTPStatus.OK: if status_code == HTTPStatus.OK:
...@@ -295,14 +295,13 @@ def put_column_description() -> Response: ...@@ -295,14 +295,13 @@ def put_column_description() -> Response:
column_name = get_query_param(args, 'column_name') column_name = get_query_param(args, 'column_name')
description = get_query_param(args, 'description') description = get_query_param(args, 'description')
description = ' '.join(description.split())
src = get_query_param(args, 'source') src = get_query_param(args, 'source')
url = '{0}/{1}/column/{2}/description/{3}'.format(table_endpoint, table_key, column_name, description) url = '{0}/{1}/column/{2}/description'.format(table_endpoint, table_key, column_name)
_log_put_column_description(table_key=table_key, column_name=column_name, description=description, source=src) _log_put_column_description(table_key=table_key, column_name=column_name, description=description, source=src)
response = request_metadata(url=url, method='PUT') response = request_metadata(url=url, method='PUT', json=json.dumps({'description': description}))
status_code = response.status_code status_code = response.status_code
if status_code == HTTPStatus.OK: if status_code == HTTPStatus.OK:
......
...@@ -15,13 +15,15 @@ def get_query_param(args: Dict, param: str, error_msg: str = None) -> str: ...@@ -15,13 +15,15 @@ def get_query_param(args: Dict, param: str, error_msg: str = None) -> str:
def request_metadata(*, # type: ignore def request_metadata(*, # type: ignore
url: str, url: str,
method: str = 'GET', method: str = 'GET',
timeout_sec: int = 0): timeout_sec: int = 0,
json: str = '{}'):
""" """
Helper function to make a request to metadata service. Helper function to make a request to metadata service.
Sets the client and header information based on the configuration Sets the client and header information based on the configuration
:param method: DELETE | GET | POST | PUT :param method: DELETE | GET | POST | PUT
:param url: The request URL :param url: The request URL
:param timeout_sec: Number of seconds before timeout is triggered. :param timeout_sec: Number of seconds before timeout is triggered.
:param json: Optional request payload
:return: :return:
""" """
if app.config['REQUEST_HEADERS_METHOD']: if app.config['REQUEST_HEADERS_METHOD']:
...@@ -32,19 +34,22 @@ def request_metadata(*, # type: ignore ...@@ -32,19 +34,22 @@ def request_metadata(*, # type: ignore
url=url, url=url,
client=app.config['METADATASERVICE_REQUEST_CLIENT'], client=app.config['METADATASERVICE_REQUEST_CLIENT'],
headers=headers, headers=headers,
timeout_sec=timeout_sec) timeout_sec=timeout_sec,
json=json)
def request_search(*, # type: ignore def request_search(*, # type: ignore
url: str, url: str,
method: str = 'GET', method: str = 'GET',
timeout_sec: int = 0): timeout_sec: int = 0,
json: str = '{}'):
""" """
Helper function to make a request to search service. Helper function to make a request to search service.
Sets the client and header information based on the configuration Sets the client and header information based on the configuration
:param method: DELETE | GET | POST | PUT :param method: DELETE | GET | POST | PUT
:param url: The request URL :param url: The request URL
:param timeout_sec: Number of seconds before timeout is triggered. :param timeout_sec: Number of seconds before timeout is triggered.
:param json: Optional request payload
:return: :return:
""" """
if app.config['REQUEST_HEADERS_METHOD']: if app.config['REQUEST_HEADERS_METHOD']:
...@@ -55,11 +60,12 @@ def request_search(*, # type: ignore ...@@ -55,11 +60,12 @@ def request_search(*, # type: ignore
url=url, url=url,
client=app.config['SEARCHSERVICE_REQUEST_CLIENT'], client=app.config['SEARCHSERVICE_REQUEST_CLIENT'],
headers=headers, headers=headers,
timeout_sec=timeout_sec) timeout_sec=timeout_sec,
json=json)
# TODO: Define an interface for envoy_client # TODO: Define an interface for envoy_client
def request_wrapper(method: str, url: str, client, headers, timeout_sec: int): # type: ignore def request_wrapper(method: str, url: str, client, headers, timeout_sec: int, json: str = '{}'): # type: ignore
""" """
Wraps a request to use Envoy client and headers, if available Wraps a request to use Envoy client and headers, if available
:param method: DELETE | GET | POST | PUT :param method: DELETE | GET | POST | PUT
...@@ -67,6 +73,7 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int): ...@@ -67,6 +73,7 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int):
:param client: Optional Envoy client :param client: Optional Envoy client
:param headers: Optional Envoy request headers :param headers: Optional Envoy request headers
:param timeout_sec: Number of seconds before timeout is triggered. Not used with Envoy :param timeout_sec: Number of seconds before timeout is triggered. Not used with Envoy
:param json: Optional request payload
:return: :return:
""" """
# If no timeout specified, use the one from the configurations. # If no timeout specified, use the one from the configurations.
...@@ -78,9 +85,9 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int): ...@@ -78,9 +85,9 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int):
elif method == 'GET': elif method == 'GET':
return client.get(url, headers=headers, raw_response=True) return client.get(url, headers=headers, raw_response=True)
elif method == 'POST': elif method == 'POST':
return client.post(url, headers=headers, raw_response=True) return client.post(url, headers=headers, raw_response=True, json=json)
elif method == 'PUT': elif method == 'PUT':
return client.put(url, headers=headers, raw_response=True) return client.put(url, headers=headers, raw_response=True, json=json)
else: else:
raise Exception('Method not allowed: {}'.format(method)) raise Exception('Method not allowed: {}'.format(method))
else: else:
...@@ -90,8 +97,8 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int): ...@@ -90,8 +97,8 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int):
elif method == 'GET': elif method == 'GET':
return s.get(url, headers=headers, timeout=timeout_sec) return s.get(url, headers=headers, timeout=timeout_sec)
elif method == 'POST': elif method == 'POST':
return s.post(url, headers=headers, timeout=timeout_sec) return s.post(url, headers=headers, timeout=timeout_sec, json=json)
elif method == 'PUT': elif method == 'PUT':
return s.put(url, headers=headers, timeout=timeout_sec) return s.put(url, headers=headers, timeout=timeout_sec, json=json)
else: else:
raise Exception('Method not allowed: {}'.format(method)) raise Exception('Method not allowed: {}'.format(method))
...@@ -354,7 +354,6 @@ export class TableDetail extends React.Component<TableDetailProps & RouteCompone ...@@ -354,7 +354,6 @@ export class TableDetail extends React.Component<TableDetailProps & RouteCompone
!data.is_view && <WatermarkLabel watermarks={ data.watermarks }/> !data.is_view && <WatermarkLabel watermarks={ data.watermarks }/>
} }
<TableDescEditableText <TableDescEditableText
maxLength={ 750 }
value={ data.table_description } value={ data.table_description }
editable={ data.is_editable } editable={ data.is_editable }
/> />
......
import * as React from 'react'; import * as React from 'react';
import ReactDOM from 'react-dom'; import ReactDOM from 'react-dom';
import * as ReactMarkdown from 'react-markdown';
import { Overlay, Popover, Tooltip } from 'react-bootstrap'; import { Overlay, Popover, Tooltip } from 'react-bootstrap';
import autosize from 'autosize';
// TODO: Use css-modules instead of 'import' // TODO: Use css-modules instead of 'import'
// TODO: Outdated approach (lines 148, 168). Replace with React.createRef(). See more at https://reactjs.org/docs/refs-and-the-dom.html
import './styles.scss'; import './styles.scss';
export interface StateFromProps { export interface StateFromProps {
...@@ -36,7 +39,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState> ...@@ -36,7 +39,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
public static defaultProps: EditableTextProps = { public static defaultProps: EditableTextProps = {
editable: true, editable: true,
maxLength: 250, maxLength: 4000,
onSubmitValue: null, onSubmitValue: null,
getLatestValue: null, getLatestValue: null,
value: '', value: '',
...@@ -61,6 +64,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState> ...@@ -61,6 +64,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
componentDidUpdate() { componentDidUpdate() {
const { isDisabled, inEditMode, refreshValue, value } = this.state; const { isDisabled, inEditMode, refreshValue, value } = this.state;
if (inEditMode) { if (inEditMode) {
autosize(this.textAreaTarget);
if (refreshValue && refreshValue !== value && !isDisabled) { if (refreshValue && refreshValue !== value && !isDisabled) {
// disable the component if a refresh is needed // disable the component if a refresh is needed
this.setState({ isDisabled: true }) this.setState({ isDisabled: true })
...@@ -113,7 +117,9 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState> ...@@ -113,7 +117,9 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
if (!this.state.editable) { if (!this.state.editable) {
return ( return (
<div id='editable-container' className='editable-container'> <div id='editable-container' className='editable-container'>
<div id='editable-text' className='editable-text'>{ this.state.value }</div> <div id='editable-text' className='editable-text'>
<ReactMarkdown source={this.state.value}/>
</div>
</div> </div>
); );
} }
...@@ -135,7 +141,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState> ...@@ -135,7 +141,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
</Tooltip> </Tooltip>
</Overlay> </Overlay>
<div id='editable-text' className={"editable-text"}> <div id='editable-text' className={"editable-text"}>
{ this.state.value } <ReactMarkdown source={this.state.value}/>
<a className={ "edit-link" + (this.state.value ? "" : " no-value") } <a className={ "edit-link" + (this.state.value ? "" : " no-value") }
href="JavaScript:void(0)" href="JavaScript:void(0)"
onClick={ this.enterEditMode } onClick={ this.enterEditMode }
......
import * as React from 'react'; import * as React from 'react';
import * as ReactMarkdown from 'react-markdown';
import { shallow } from 'enzyme'; import { shallow } from 'enzyme';
...@@ -13,7 +14,7 @@ describe('EditableText', () => { ...@@ -13,7 +14,7 @@ describe('EditableText', () => {
beforeEach(() => { beforeEach(() => {
props = { props = {
editable: true, editable: true,
maxLength: 250, maxLength: 4000,
onSubmitValue: jest.fn(), onSubmitValue: jest.fn(),
getLatestValue: jest.fn(), getLatestValue: jest.fn(),
refreshValue: 'newValue', refreshValue: 'newValue',
...@@ -27,7 +28,7 @@ describe('EditableText', () => { ...@@ -27,7 +28,7 @@ describe('EditableText', () => {
props.editable = false; props.editable = false;
/* Note: Do not copy this pattern, for some reason setProps is not updating the content in this case */ /* Note: Do not copy this pattern, for some reason setProps is not updating the content in this case */
subject = shallow(<EditableText {...props} />); subject = shallow(<EditableText {...props} />);
expect(subject.find('div#editable-text').text()).toEqual(props.value); expect(subject.find('div#editable-text').find(ReactMarkdown).prop('source')).toEqual(props.value);
}); });
describe('renders correctly if !this.state.inEditMode', () => { describe('renders correctly if !this.state.inEditMode', () => {
...@@ -35,7 +36,7 @@ describe('EditableText', () => { ...@@ -35,7 +36,7 @@ describe('EditableText', () => {
subject.setState({ inEditMode: false }); subject.setState({ inEditMode: false });
}); });
it('renders value as first child', () => { it('renders value as first child', () => {
expect(subject.find('#editable-text').props().children[0]).toEqual(props.value); expect(subject.find('#editable-text').children().first().prop('source')).toEqual(props.value);
}); });
it('renders edit link to enterEditMode', () => { it('renders edit link to enterEditMode', () => {
......
...@@ -8,16 +8,16 @@ ...@@ -8,16 +8,16 @@
"url": "https://github.com/lyft/amundsenfrontendlibrary" "url": "https://github.com/lyft/amundsenfrontendlibrary"
}, },
"scripts": { "scripts": {
"build": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -p --progress --config webpack.prod.ts", "build": "cross-env 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": "cross-env TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts",
"test": "TZ=UTC jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}", "test": "cross-env TZ=UTC jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}",
"test-nocov": "TZ=UTC jest", "test-nocov": "cross-env TZ=UTC jest",
"watch": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts --watch", "watch": "cross-env 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",
"eslint": "eslint --ignore-path=.eslintignore --ext .js,.jsx .", "eslint": "eslint --ignore-path=.eslintignore --ext .js,.jsx .",
"eslint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx .", "eslint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx .",
"test:watch": "TZ=UTC jest --watch", "test:watch": "cross-env TZ=UTC jest --watch",
"tsc": "tsc", "tsc": "tsc",
"tslint": "tslint --project .", "tslint": "tslint --project .",
"tslint-fix": "tslint --fix --project ." "tslint-fix": "tslint --fix --project ."
...@@ -44,6 +44,7 @@ ...@@ -44,6 +44,7 @@
"babel-preset-stage-0": "^6.0.15", "babel-preset-stage-0": "^6.0.15",
"bootstrap-sass": "^3.3.7", "bootstrap-sass": "^3.3.7",
"clean-webpack-plugin": "^0.1.19", "clean-webpack-plugin": "^0.1.19",
"cross-env": "^5.2.1",
"css-loader": "^0.28.11", "css-loader": "^0.28.11",
"enzyme": "^3.7.0", "enzyme": "^3.7.0",
"enzyme-adapter-react-16": "^1.6.0", "enzyme-adapter-react-16": "^1.6.0",
...@@ -84,6 +85,7 @@ ...@@ -84,6 +85,7 @@
"webworkify-webpack": "2.1.0" "webworkify-webpack": "2.1.0"
}, },
"dependencies": { "dependencies": {
"autosize": "^4.0.2",
"axios": "0.19.0", "axios": "0.19.0",
"form-serialize": "^0.7.2", "form-serialize": "^0.7.2",
"jquery": "^3.3.1", "jquery": "^3.3.1",
...@@ -95,6 +97,7 @@ ...@@ -95,6 +97,7 @@
"react-dom": "^16.3.1", "react-dom": "^16.3.1",
"react-js-pagination": "^3.0.2", "react-js-pagination": "^3.0.2",
"react-linkify": "^0.2.2", "react-linkify": "^0.2.2",
"react-markdown": "^4.2.2",
"react-redux": "^5.0.7", "react-redux": "^5.0.7",
"react-router-dom": "^4.2.2", "react-router-dom": "^4.2.2",
"react-sanitized-html": "^2.0.0", "react-sanitized-html": "^2.0.0",
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
"moduleResolution": "node", "moduleResolution": "node",
"noResolve": false, "noResolve": false,
"removeComments": true, "removeComments": true,
"allowSyntheticDefaultImports": true,
"types": ["jest"], "types": ["jest"],
"baseUrl": "js", "baseUrl": "js",
"paths": { "paths": {
......
...@@ -34,7 +34,7 @@ requirements_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'r ...@@ -34,7 +34,7 @@ requirements_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'r
with open(requirements_path) as requirements_file: with open(requirements_path) as requirements_file:
requirements = requirements_file.readlines() requirements = requirements_file.readlines()
__version__ = '1.0.7' __version__ = '1.0.8'
setup( setup(
......
...@@ -397,7 +397,7 @@ class MetadataTest(unittest.TestCase): ...@@ -397,7 +397,7 @@ class MetadataTest(unittest.TestCase):
Test successful put_table_description request Test successful put_table_description request
:return: :return:
""" """
url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + '/db://cluster.schema/table/description/test' url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + '/db://cluster.schema/table/description'
responses.add(responses.PUT, url, json={}, status=HTTPStatus.OK) responses.add(responses.PUT, url, json={}, status=HTTPStatus.OK)
with local_app.test_client() as test: with local_app.test_client() as test:
...@@ -463,7 +463,7 @@ class MetadataTest(unittest.TestCase): ...@@ -463,7 +463,7 @@ class MetadataTest(unittest.TestCase):
:return: :return:
""" """
url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + \ url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + \
'/db://cluster.schema/table/column/col/description/test' '/db://cluster.schema/table/column/col/description'
responses.add(responses.PUT, url, json={}, status=HTTPStatus.OK) responses.add(responses.PUT, url, json={}, status=HTTPStatus.OK)
with local_app.test_client() as test: with local_app.test_client() as test:
......
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