feat: Update prog_desc configuration + UI (#527)

* Update prog_desc configuration + UI

* Lint

* Lint

* mypy

* Component cleanup; Add types

* Lint fix

* Clean up and fix bug

* Update doc

* Update docs

* Update docs

* Update test

* Lint

* Update python logic

amundsen_application/api/utils/metadata_utils.py

@@ -113,8 +113,7 @@ def marshall_table_full(table_dict: Dict) -> Dict:
 
     # We follow same style as column stat order for arranging the programmatic descriptions
     prog_descriptions = results['programmatic_descriptions']
-    if prog_descriptions:
-        _update_prog_descriptions(prog_descriptions)
+    results['programmatic_descriptions'] = _convert_prog_descriptions(prog_descriptions)
 
     return results
 
@@ -150,16 +149,44 @@ def marshall_dashboard_full(dashboard_dict: Dict) -> Dict:
     return dashboard_dict
 
 
-def _update_prog_descriptions(prog_descriptions: List) -> None:
-    # We want to make sure there is a display title that is just source
-    for desc in prog_descriptions:
-        source = desc.get('source')
-        if not source:
-            logging.warning("no source found in: " + str(desc))
-    prog_display_config = app.config['PROGRAMMATIC_DISPLAY']
-    if prog_display_config and prog_descriptions:
-        # If config is defined for programmatic disply we look to see what configuration is being used
-        prog_descriptions.sort(key=lambda x: _sort_prog_descriptions(prog_display_config, x))
+def _convert_prog_descriptions(prog_descriptions: List = None) -> Dict:
+    """
+    Apply the PROGRAMMATIC_DISPLAY configuration to convert to the structure.
+    :param prog_descriptions: A list of objects representing programmatic descriptions
+    :return: A dictionary with organized programmatic_descriptions
+    """
+    left = []  # type: List
+    right = []  # type: List
+    other = []  # type: List
+    updated_descriptions = {}
+
+    if prog_descriptions:
+        # We want to make sure there is a display title that is just source
+        for desc in prog_descriptions:
+            source = desc.get('source')
+            if not source:
+                logging.warning("no source found in: " + str(desc))
+
+        # If config is defined for programmatic disply we organize and sort them based on the configuration
+        prog_display_config = app.config['PROGRAMMATIC_DISPLAY']
+        if prog_display_config:
+            left_config = prog_display_config.get('LEFT', {})
+            left = [x for x in prog_descriptions if x.get('source') in left_config]
+            left.sort(key=lambda x: _sort_prog_descriptions(left_config, x))
+
+            right_config = prog_display_config.get('RIGHT', {})
+            right = [x for x in prog_descriptions if x.get('source') in right_config]
+            right.sort(key=lambda x: _sort_prog_descriptions(right_config, x))
+
+            other_config = dict(filter(lambda x: x not in ['LEFT', 'RIGHT'], prog_display_config.items()))
+            other = list(filter(lambda x: x.get('source') not in left_config and x.get('source')
+                                not in right_config, prog_descriptions))
+            other.sort(key=lambda x: _sort_prog_descriptions(other_config, x))
+
+    updated_descriptions['left'] = left
+    updated_descriptions['right'] = right
+    updated_descriptions['other'] = other
+    return updated_descriptions
 
 
 def _sort_prog_descriptions(base_config: Dict, prog_description: Dict) -> int:

amundsen_application/static/js/components/TableDetail/ExploreButton/index.spec.tsx

@@ -53,7 +53,7 @@ describe('ExploreButton', () => {
         source: { source: '', source_type: '' },
         resource_reports: [],
         watermarks: [],
-        programmatic_descriptions: [],
+        programmatic_descriptions: {},
         ...tableDataOverrides,
       },
     };

amundsen_application/static/js/components/TableDetail/index.tsx

@@ -42,7 +42,11 @@ import TableIssues from 'components/TableDetail/TableIssues';
 import WatermarkLabel from 'components/TableDetail/WatermarkLabel';
 import WriterLink from 'components/TableDetail/WriterLink';
 import TagInput from 'components/Tags/TagInput';
-import { ResourceType, TableMetadata } from 'interfaces';
+import {
+  ProgrammaticDescription,
+  ResourceType,
+  TableMetadata,
+} from 'interfaces';
 
 import EditableSection from 'components/common/EditableSection';
 
@@ -140,6 +144,23 @@ export class TableDetail extends React.Component<
     return `${params.database}://${params.cluster}.${params.schema}/${params.table}`;
   }
 
+  renderProgrammaticDesc = (descriptions: ProgrammaticDescription[]) => {
+    if (!descriptions) {
+      return null;
+    }
+
+    return descriptions.map((d) => (
+      <EditableSection key={`prog_desc:${d.source}`} title={d.source} readOnly>
+        <EditableText
+          maxLength={999999}
+          value={d.text}
+          editable={false}
+          onSubmitValue={null}
+        />
+      </EditableSection>
+    ));
+  };
+
   renderTabs(editText, editUrl) {
     const tabInfo = [];
 
@@ -285,6 +306,9 @@ export class TableDetail extends React.Component<
                     <div className="section-title title-3">Frequent Users</div>
                     <FrequentUsers readers={data.table_readers} />
                   </section>
+                  {this.renderProgrammaticDesc(
+                    data.programmatic_descriptions.left
+                  )}
                 </section>
                 <section className="right-panel">
                   <EditableSection title="Tags">
@@ -296,28 +320,14 @@ export class TableDetail extends React.Component<
                   <EditableSection title="Owners">
                     <OwnerEditor />
                   </EditableSection>
+                  {this.renderProgrammaticDesc(
+                    data.programmatic_descriptions.right
+                  )}
                 </section>
               </section>
-              {data.programmatic_descriptions.length > 0 && (
-                <>
-                  <div className="programmatic-title title-4">
-                    {PROGRMMATIC_DESC_HEADER}
-                  </div>
-                  <hr className="programmatic-hr hr1" />
-                </>
+              {this.renderProgrammaticDesc(
+                data.programmatic_descriptions.other
               )}
-              {data.programmatic_descriptions.map((d) => (
-                <section key={d.source} className="column-layout-2">
-                  <EditableSection title={d.source} readOnly>
-                    <EditableText
-                      maxLength={999999}
-                      value={d.text}
-                      editable={false}
-                      onSubmitValue={null}
-                    />
-                  </EditableSection>
-                </section>
-              ))}
             </aside>
             <main className="right-panel">
               {this.renderTabs(editText, editUrl)}

amundsen_application/static/js/config/tests/index.spec.ts

@@ -196,7 +196,7 @@ describe('generateExploreUrl', () => {
     source: { source: '', source_type: '' },
     resource_reports: [],
     watermarks: [],
-    programmatic_descriptions: [],
+    programmatic_descriptions: {},
   };
 
   it('calls `exploreUrlGenerator` with table metadata', () => {

amundsen_application/static/js/ducks/tableMetadata/reducer.ts

@@ -282,7 +282,7 @@ export const initialTableDataState: TableMetadata = {
   source: { source: '', source_type: '' },
   resource_reports: [],
   watermarks: [],
-  programmatic_descriptions: [],
+  programmatic_descriptions: {},
 };
 
 export const initialState: TableMetadataReducerState = {

amundsen_application/static/js/fixtures/globalState.ts

@@ -172,7 +172,7 @@ const globalState: GlobalState = {
       source: { source: '', source_type: '' },
       resource_reports: [],
       watermarks: [],
-      programmatic_descriptions: [],
+      programmatic_descriptions: {},
     },
     tableOwners: {
       isLoading: true,

amundsen_application/static/js/fixtures/metadata/table.ts

@@ -106,7 +106,7 @@ export const tableMetadata: TableMetadata = {
     key: 'ds',
     value: '2020-03-05',
   },
-  programmatic_descriptions: [],
+  programmatic_descriptions: {},
   schema: 'base',
   source: {
     source:

amundsen_application/static/js/interfaces/TableMetadata.ts

@@ -73,6 +73,11 @@ export interface ProgrammaticDescription {
   source: string;
   text: string;
 }
+export interface TableProgrammaticDescriptions {
+  left?: ProgrammaticDescription[];
+  right?: ProgrammaticDescription[];
+  other?: ProgrammaticDescription[];
+}
 
 export interface ResourceReport {
   name: string;
@@ -97,7 +102,7 @@ export interface TableMetadata {
   source: TableSource;
   resource_reports: ResourceReport[];
   watermarks: Watermark[];
-  programmatic_descriptions: ProgrammaticDescription[];
+  programmatic_descriptions: TableProgrammaticDescriptions;
 }
 
 export interface UpdateOwnerPayload {

docs/flask_config.md

@@ -58,7 +58,7 @@ settings in `config.py` that should be set in order to use this feature.
 Here are the settings and what they should be set to
 ```python
     ISSUE_LABELS = []  # type: List[str] (Optional labels to be set on the created tickets)
-    ISSUE_TRACKER_URL = None  # type: str (Your JIRA environment, IE 'https://jira.net') 
+    ISSUE_TRACKER_URL = None  # type: str (Your JIRA environment, IE 'https://jira.net')
     ISSUE_TRACKER_USER = None  # type: str (Recommended to be a service account)
     ISSUE_TRACKER_PASSWORD = None  # type: str
     ISSUE_TRACKER_PROJECT_ID = None  # type: int (Project ID for the project you would like JIRA tickets to be created in)
@@ -67,11 +67,10 @@ Here are the settings and what they should be set to
     ISSUE_TRACKER_MAX_RESULTS = None  # type: int (Max issues to display at a time)
 
 ```
-
 ## Programmatic Descriptions
 Amundsen supports configuring other mark down supported non-editable description boxes on the table page.
-This can be useful if you have multiple writers which want to write different pieces of information to amundsen
-that are either very company specific and thus would never be directly integrated into amundsen or require long form text
+This can be useful if you have multiple writers which want to write different pieces of information to Amundsen
+that are either very company specific and thus would never be directly integrated into Amundsen or require long form text
 to properly convey the information.
 
 What are some more specific examples of what could be used for this?
@@ -81,44 +80,45 @@ report to provide context.
 - You have extended table information that is applicable to your datastore which you want to scrape and provide in the
 table page
 
-Programmatic Descriptions are referred to by a "description source" which is a unique identifier.
-You can then configure the descriptions to have a custom order in the config.py file like so:
+Programmatic descriptions are referred to by a "description source" which is a unique identifier.
+In the UI, they will appear on the table page under structured metadata.
+
+In config.py you can then configure the descriptions to have a custom order, as well as whether or not they should exist in the left column or right column.
 ```    
 PROGRAMMATIC_DISPLAY = {
-           "s3_crawler": {
-               "display_order": 0
-           },
-           "quality_service": {
-               "display_order": 1
-           },
-           "doesnt_exist": {
-               "display_order": 2
-           }
-       }
+    'RIGHT': {
+      "test3" : {},
+      "test2" : { "display_order": 0 }
+    },
+    'LEFT': {
+      "test1" : { "display_order": 1 },
+      "test0" : { "display_order": 0 },
+    },
+    'test4': {"display_order": 0},
+}
 ```
-description sources not mentioned in the configuration will be alphabetically placed at the end of the above list. If `PROGRAMMATIC_DISPLAY` is left at `None` all added fields are still showing up, so that display is entirely dynamically data-driven without configuration. Meaning configuration merely adds the (nice) benefit of setting display order.
+Description sources not mentioned in the configuration will be alphabetically placed at the bottom of the list. If `PROGRAMMATIC_DISPLAY` is left at `None` all added fields will show up in the order in which they were returned from the backend. Here is a screenshot of what it would look like in the bottom left:
 
-Here is a screenshot of what it would look like in the bottom left here:
-![programmatic_description](img/programmatic_descriptions.png)
+<img src='img/programmatic_descriptions.png' width='50%' />
 
 ## Uneditable Table Descriptions
-Amundsen supports configuring table and column description to be non-editable for selective tables. You may want to make table 
+Amundsen supports configuring table and column description to be non-editable for selective tables. You may want to make table
 descriptions non-editable due to various reasons such as table already has table description from source of truth.
-You can define matching rules in  [config.py](https://github.com/lyft/amundsenfrontendlibrary/blob/master/amundsen_application/config.py) for selecting tables. This configuration is useful as table selection criteria can 
-be company specific which will not directly integrated with Amundsen. 
+You can define matching rules in  [config.py](https://github.com/lyft/amundsenfrontendlibrary/blob/master/amundsen_application/config.py) for selecting tables. This configuration is useful as table selection criteria can
+be company specific which will not directly integrated with Amundsen.
 You can use different combinations of schema and table name for selecting tables.
 
-Here are some examples when this feature can be used: 
+Here are some examples when this feature can be used:
 1. You want to set all tables with a given schema or schema pattern as un-editable.
 2. You want to set all tables with a specific table name pattern in a given schema pattern as un-editable.
 3. You want to set all tables with a given table name pattern as un-editable.
 
 Amundsen has two variables in `config.py` file which can be used to define match rules:
 1. `UNEDITABLE_SCHEMAS` : Set of schemas where all tables should be un-editable. It takes exact schema name.
-2. `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` : List of MatchRuleObject, where each MatchRuleObject consists of regex for 
+2. `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` : List of MatchRuleObject, where each MatchRuleObject consists of regex for
 schema name or regex for table name or both.
 
-Purpose of `UNEDITABLE_SCHEMAS` can be fulfilled by `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` but we are keeping both 
+Purpose of `UNEDITABLE_SCHEMAS` can be fulfilled by `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` but we are keeping both
 variables for backward compatibility.
 If you want to restrict tables from a given schemas then you can use `UNEDITABLE_SCHEMAS` as follows:
 ```python
@@ -126,8 +126,8 @@ UNEDITABLE_SCHEMAS = set(['schema1', 'schema2'])
 ```
 After above configuration, all tables in 'schema1' and 'schema2' will have non-editable table and column descriptions.
 
-If you have more complex matching rules you can use `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES`. It provides you more flexibility 
-and control as you can create multiple match rules and use regex for matching schema nad table names.
+If you have more complex matching rules you can use `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES`. It provides you more flexibility
+and control as you can create multiple match rules and use regex for matching schema and table names.
 
 You can configure your match rules in `config.py` as follow:
 ```python

tests/unit/utils/test_metadata_utils.py

@@ -3,9 +3,7 @@
 
 import unittest
 
-from unittest.mock import patch, Mock
-
-from amundsen_application.api.utils.metadata_utils import _update_prog_descriptions, _sort_prog_descriptions, \
+from amundsen_application.api.utils.metadata_utils import _convert_prog_descriptions, _sort_prog_descriptions, \
     _parse_editable_rule
 from amundsen_application.config import MatchRuleObject
 from amundsen_application import create_app
@@ -17,26 +15,50 @@ class ProgrammaticDescriptionsTest(unittest.TestCase):
     def setUp(self) -> None:
         pass
 
-    @patch('amundsen_application.api.utils.metadata_utils._sort_prog_descriptions')
-    def test_update_prog_descriptions(self, sort_mock) -> None:
+    def test_convert_prog_descriptions(self) -> None:
         with local_app.app_context():
+            # mock config
+            test_config = {
+                'RIGHT': {
+                    'test3': {},
+                    'test2': {'display_order': 0},
+                },
+                'LEFT': {
+                    'test1': {'display_order': 1},
+                    'test0': {'display_order': 0},
+                },
+                'test4': {'display_order': 0},
+            }
+            # test data
             test_desc = [
-                {'source': 'c_1', 'text': 'description c'},
-                {'source': 'a_1', 'text': 'description a'},
-                {'source': 'b_1', 'text': 'description b'}
-            ]
-            # Pretend config exists
-            local_app.config['PROGRAMMATIC_DISPLAY'] = Mock()
-            # Mock the effects of the sort method
-            sort_mock.side_effect = [1, 0, 1]
-            # Expected order based on mocked side effect
-            expected_programmatic_desc = [
-                {'source': 'a_1', 'text': 'description a'},
-                {'source': 'c_1', 'text': 'description c'},
-                {'source': 'b_1', 'text': 'description b'}
+                {'source': 'test0', 'text': 'test'},
+                {'source': 'test1', 'text': 'test'},
+                {'source': 'test2', 'text': 'test'},
+                {'source': 'test3', 'text': 'test'},
+                {'source': 'test5', 'text': 'test'},
+                {'source': 'test4', 'text': 'test'},
             ]
-            _update_prog_descriptions(test_desc)
-            self.assertEqual(test_desc, expected_programmatic_desc)
+            # expected order based on mock
+            expected_programmatic_desc = {
+                'left': [
+                    {'source': 'test0', 'text': 'test'},
+                    {'source': 'test1', 'text': 'test'},
+                ],
+                'right': [
+                    {'source': 'test2', 'text': 'test'},
+                    {'source': 'test3', 'text': 'test'},
+                ],
+                'other': [
+                    {'source': 'test4', 'text': 'test'},
+                    {'source': 'test5', 'text': 'test'},
+                ]
+            }
+            local_app.config['PROGRAMMATIC_DISPLAY'] = test_config
+
+            result = _convert_prog_descriptions(test_desc)
+            self.assertEqual(result.get('left'), expected_programmatic_desc.get('left'))
+            self.assertEqual(result.get('right'), expected_programmatic_desc.get('right'))
+            self.assertEqual(result.get('other'), expected_programmatic_desc.get('other'))
 
     def test_sort_prog_descriptions_returns_value_from_config(self) -> None:
         """