Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve table & columns description formatting (amundsen-io#98) (amun…
Browse files Browse the repository at this point in the history
…dsen-io#298)

* Add support for React-Markdown to editable text fields
* Add support for windows via cross-env
Mikhail-Ivanov authored and Hans Adriaans committed Jun 30, 2022
1 parent 6fbd15d commit 8aba96d
Showing 10 changed files with 418 additions and 60 deletions.
11 changes: 5 additions & 6 deletions frontend/amundsen_application/api/metadata/v0.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
import logging

from http import HTTPStatus
@@ -259,13 +260,12 @@ def _log_put_table_description(*, table_key: str, description: str, source: str)
table_key = get_query_param(args, 'key')

description = get_query_param(args, 'description')
description = ' '.join(description.split())
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)

response = request_metadata(url=url, method='PUT')
response = request_metadata(url=url, method='PUT', json=json.dumps({'description': description}))
status_code = response.status_code

if status_code == HTTPStatus.OK:
@@ -295,14 +295,13 @@ def _log_put_column_description(*, table_key: str, column_name: str, description

column_name = get_query_param(args, 'column_name')
description = get_query_param(args, 'description')
description = ' '.join(description.split())

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)

response = request_metadata(url=url, method='PUT')
response = request_metadata(url=url, method='PUT', json=json.dumps({'description': description}))
status_code = response.status_code

if status_code == HTTPStatus.OK:
25 changes: 16 additions & 9 deletions frontend/amundsen_application/api/utils/request_utils.py
Original file line number Diff line number Diff line change
@@ -15,13 +15,15 @@ def get_query_param(args: Dict, param: str, error_msg: str = None) -> str:
def request_metadata(*, # type: ignore
url: str,
method: str = 'GET',
timeout_sec: int = 0):
timeout_sec: int = 0,
json: str = '{}'):
"""
Helper function to make a request to metadata service.
Sets the client and header information based on the configuration
:param method: DELETE | GET | POST | PUT
:param url: The request URL
:param timeout_sec: Number of seconds before timeout is triggered.
:param json: Optional request payload
:return:
"""
if app.config['REQUEST_HEADERS_METHOD']:
@@ -32,19 +34,22 @@ def request_metadata(*, # type: ignore
url=url,
client=app.config['METADATASERVICE_REQUEST_CLIENT'],
headers=headers,
timeout_sec=timeout_sec)
timeout_sec=timeout_sec,
json=json)


def request_search(*, # type: ignore
url: str,
method: str = 'GET',
timeout_sec: int = 0):
timeout_sec: int = 0,
json: str = '{}'):
"""
Helper function to make a request to search service.
Sets the client and header information based on the configuration
:param method: DELETE | GET | POST | PUT
:param url: The request URL
:param timeout_sec: Number of seconds before timeout is triggered.
:param json: Optional request payload
:return:
"""
if app.config['REQUEST_HEADERS_METHOD']:
@@ -55,18 +60,20 @@ def request_search(*, # type: ignore
url=url,
client=app.config['SEARCHSERVICE_REQUEST_CLIENT'],
headers=headers,
timeout_sec=timeout_sec)
timeout_sec=timeout_sec,
json=json)


# 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
:param method: DELETE | GET | POST | PUT
:param url: The request URL
:param client: Optional Envoy client
:param headers: Optional Envoy request headers
:param timeout_sec: Number of seconds before timeout is triggered. Not used with Envoy
:param json: Optional request payload
:return:
"""
# 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):
elif method == 'GET':
return client.get(url, headers=headers, raw_response=True)
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':
return client.put(url, headers=headers, raw_response=True)
return client.put(url, headers=headers, raw_response=True, json=json)
else:
raise Exception('Method not allowed: {}'.format(method))
else:
@@ -90,8 +97,8 @@ def request_wrapper(method: str, url: str, client, headers, timeout_sec: int):
elif method == 'GET':
return s.get(url, headers=headers, timeout=timeout_sec)
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':
return s.put(url, headers=headers, timeout=timeout_sec)
return s.put(url, headers=headers, timeout=timeout_sec, json=json)
else:
raise Exception('Method not allowed: {}'.format(method))
Original file line number Diff line number Diff line change
@@ -354,7 +354,6 @@ export class TableDetail extends React.Component<TableDetailProps & RouteCompone
!data.is_view && <WatermarkLabel watermarks={ data.watermarks }/>
}
<TableDescEditableText
maxLength={ 750 }
value={ data.table_description }
editable={ data.is_editable }
/>
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import * as React from 'react';
import ReactDOM from 'react-dom';
import * as ReactMarkdown from 'react-markdown';
import { Overlay, Popover, Tooltip } from 'react-bootstrap';
import autosize from 'autosize';

// 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';

export interface StateFromProps {
@@ -36,7 +39,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>

public static defaultProps: EditableTextProps = {
editable: true,
maxLength: 250,
maxLength: 4000,
onSubmitValue: null,
getLatestValue: null,
value: '',
@@ -61,6 +64,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
componentDidUpdate() {
const { isDisabled, inEditMode, refreshValue, value } = this.state;
if (inEditMode) {
autosize(this.textAreaTarget);
if (refreshValue && refreshValue !== value && !isDisabled) {
// disable the component if a refresh is needed
this.setState({ isDisabled: true })
@@ -113,7 +117,9 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
if (!this.state.editable) {
return (
<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>
);
}
@@ -135,7 +141,7 @@ class EditableText extends React.Component<EditableTextProps, EditableTextState>
</Tooltip>
</Overlay>
<div id='editable-text' className={"editable-text"}>
{ this.state.value }
<ReactMarkdown source={this.state.value}/>
<a className={ "edit-link" + (this.state.value ? "" : " no-value") }
href="JavaScript:void(0)"
onClick={ this.enterEditMode }
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import * as ReactMarkdown from 'react-markdown';

import { shallow } from 'enzyme';

@@ -13,7 +14,7 @@ describe('EditableText', () => {
beforeEach(() => {
props = {
editable: true,
maxLength: 250,
maxLength: 4000,
onSubmitValue: jest.fn(),
getLatestValue: jest.fn(),
refreshValue: 'newValue',
@@ -27,15 +28,15 @@ describe('EditableText', () => {
props.editable = false;
/* Note: Do not copy this pattern, for some reason setProps is not updating the content in this case */
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', () => {
beforeEach(() => {
subject.setState({ inEditMode: false });
});
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', () => {
400 changes: 371 additions & 29 deletions frontend/amundsen_application/static/package-lock.json

Large diffs are not rendered by default.

15 changes: 9 additions & 6 deletions frontend/amundsen_application/static/package.json
Original file line number Diff line number Diff line change
@@ -8,16 +8,16 @@
"url": "https://github.com/lyft/amundsenfrontendlibrary"
},
"scripts": {
"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",
"test": "TZ=UTC jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}",
"test-nocov": "TZ=UTC jest",
"watch": "TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts --watch",
"build": "cross-env TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -p --progress --config webpack.prod.ts",
"dev-build": "cross-env TS_NODE_PROJECT='tsconfig-for-webpack.json' webpack -d --progress --config webpack.dev.ts",
"test": "cross-env TZ=UTC jest --coverage --collectCoverageFrom=js/**/*.{js,jsx,ts,tsx}",
"test-nocov": "cross-env TZ=UTC jest",
"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-fix": "npm run eslint-fix && npm run tslint-fix",
"eslint": "eslint --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",
"tslint": "tslint --project .",
"tslint-fix": "tslint --fix --project ."
@@ -44,6 +44,7 @@
"babel-preset-stage-0": "^6.0.15",
"bootstrap-sass": "^3.3.7",
"clean-webpack-plugin": "^0.1.19",
"cross-env": "^5.2.1",
"css-loader": "^0.28.11",
"enzyme": "^3.7.0",
"enzyme-adapter-react-16": "^1.6.0",
@@ -84,6 +85,7 @@
"webworkify-webpack": "2.1.0"
},
"dependencies": {
"autosize": "^4.0.2",
"axios": "0.19.0",
"form-serialize": "^0.7.2",
"jquery": "^3.3.1",
@@ -95,6 +97,7 @@
"react-dom": "^16.3.1",
"react-js-pagination": "^3.0.2",
"react-linkify": "^0.2.2",
"react-markdown": "^4.2.2",
"react-redux": "^5.0.7",
"react-router-dom": "^4.2.2",
"react-sanitized-html": "^2.0.0",
1 change: 1 addition & 0 deletions frontend/amundsen_application/static/tsconfig.json
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
"moduleResolution": "node",
"noResolve": false,
"removeComments": true,
"allowSyntheticDefaultImports": true,
"types": ["jest"],
"baseUrl": "js",
"paths": {
2 changes: 1 addition & 1 deletion frontend/setup.py
Original file line number Diff line number Diff line change
@@ -34,7 +34,7 @@ def build_js() -> None:
with open(requirements_path) as requirements_file:
requirements = requirements_file.readlines()

__version__ = '1.0.7'
__version__ = '1.0.8'


setup(
4 changes: 2 additions & 2 deletions frontend/tests/unit/api/metadata/test_v0.py
Original file line number Diff line number Diff line change
@@ -397,7 +397,7 @@ def test_put_table_description_success(self) -> None:
Test successful put_table_description request
: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)

with local_app.test_client() as test:
@@ -463,7 +463,7 @@ def test_put_column_description_success(self) -> None:
:return:
"""
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)

with local_app.test_client() as test:

0 comments on commit 8aba96d

Please sign in to comment.