feat: Allow specifying Data Product URN via UI #9386
Conversation
|
|
anshbansal
force-pushed
the
feat/PRD-815
branch
from
f085b46
to
3c0c228
Compare
metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java
Show resolved
Hide resolved
| @@ -47,6 +73,37 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat | |||
| <Form.Item label={<Typography.Text strong>Description</Typography.Text>}> | |||
| <StyledEditor doNotFocus content={builderState.description} onChange={updateDescription} /> | |||
| </Form.Item> | |||
| <Collapse ghost> | |||
There was a problem hiding this comment.
minor: this seems like a good chunk of code to break into its own component! just trying to keep react components tighter in general.
There was a problem hiding this comment.
created new component for Data Product Id and PR update
...src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java
Outdated
Show resolved
Hide resolved
| @@ -11054,6 +11054,10 @@ input CreateDataProductInput { | |||
| The primary key of the Domain | |||
| """ | |||
| domainUrn: String! | |||
| """ | |||
| An id for the DataProduct | |||
There was a problem hiding this comment.
nitpick: let's say
"An optional id for the new data product"
| """ | ||
| An id for the DataProduct | ||
| """ | ||
| id : String |
There was a problem hiding this comment.
please remove space between id and :
| @@ -32,6 +32,7 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on | |||
| variables: { | |||
| input: { | |||
| domainUrn: domain.urn, | |||
| id: builderState.id || '', | |||
There was a problem hiding this comment.
not || ''
this will cause an empty string id to be created.
let's just say builderState.id.
Undefined | null should be provided if an id is not
| color: #373d44; | ||
| `; | ||
|
|
||
| export function DataProductAdvanceOption({builderState, updateBuilderState }:DataProductBuilderFormProps){ |
There was a problem hiding this comment.
naming nitpick:
Let's call this DataProductAdvancedOption
| color: #373d44; | ||
| `; | ||
|
|
||
| export function DataProductAdvanceOption({builderState, updateBuilderState }:DataProductBuilderFormProps){ |
There was a problem hiding this comment.
lint fix: add a space between : and DataProductBuilderFormProps.
Please always pay attention to the small details!
| data-testid="data-product-id" | ||
| placeholder="engineering" | ||
| value={builderState.id} | ||
| onChange={(e) => updateDataProductId(e.target.value)} |
There was a problem hiding this comment.
overall, nice component!
| throw new IllegalArgumentException("This Data product already exists!"); | ||
| } | ||
| } catch (RemoteInvocationException e) { | ||
| throw new RuntimeException("Unable to check for existence of Data Product!"); |
There was a problem hiding this comment.
let's include the underlying context in this exception:
throw new RuntimeException("Unable to check for existence of Data Product!", e)
I have resolved all comments |
Co-authored-by: Aseem Bansal <asmbansal2@gmail.com>
Checklist