-
Notifications
You must be signed in to change notification settings - Fork 305
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
fix(metadata): Disable updates to metadata #3376
Changes from all commits
54b4e38
853d4d1
b3bf5ad
66ced6d
bd22736
b00d3e0
ca8e8fc
255ebf5
3876b2e
ec98e5a
ae1efb8
91062a8
4a6a954
e56de3b
3c06a6a
9d109ee
496f857
8166c3a
8537652
1241fe7
c826ee1
f71d399
c55f430
d1a4709
b831683
8d8cbb7
85586c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,8 +52,9 @@ type Props = { | |
hasError: boolean, | ||
id: string, | ||
intl: Object, | ||
isCascadingPolicyApplicable?: boolean, | ||
isCascadingPolicyApplicable: boolean, | ||
isDirty: boolean, | ||
isFolderMetadata?: boolean, | ||
isOpen: boolean, | ||
onModification?: (id: string, isDirty: boolean, type?: string) => void, | ||
onRemove?: (id: string) => void, | ||
|
@@ -102,6 +103,8 @@ class Instance extends React.PureComponent<Props, State> { | |
static defaultProps = { | ||
data: {}, | ||
isDirty: false, | ||
isCascadingPolicyApplicable: false, | ||
tjuanitas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
isFolderMetadata: false, | ||
}; | ||
|
||
constructor(props: Props) { | ||
|
@@ -200,14 +203,7 @@ class Instance extends React.PureComponent<Props, State> { | |
* @return {void} | ||
*/ | ||
onSave = (): void => { | ||
const { | ||
cascadePolicy, | ||
data: originalData, | ||
id, | ||
isDirty, | ||
isCascadingPolicyApplicable, | ||
onSave, | ||
}: Props = this.props; | ||
const { cascadePolicy = {}, data: originalData, id, isDirty, onSave, isFolderMetadata }: Props = this.props; | ||
const { data: currentData, errors, isCascadingEnabled, isCascadingOverwritten }: State = this.state; | ||
|
||
if (!this.isEditing() || !isDirty || !onSave || Object.keys(errors).length) { | ||
|
@@ -219,19 +215,16 @@ class Instance extends React.PureComponent<Props, State> { | |
isEditing: false, | ||
shouldShowCascadeOptions: false, | ||
}); | ||
onSave( | ||
id, | ||
this.createJSONPatch(currentData, originalData), | ||
isCascadingPolicyApplicable | ||
? { | ||
canEdit: cascadePolicy ? cascadePolicy.canEdit : false, | ||
id: cascadePolicy ? cascadePolicy.id : undefined, | ||
isEnabled: isCascadingEnabled, | ||
overwrite: isCascadingOverwritten, | ||
} | ||
: undefined, | ||
cloneDeep(currentData), | ||
); | ||
|
||
const cascadePolicyData = { | ||
...cascadePolicy, | ||
isEnabled: isFolderMetadata ? isCascadingEnabled : false, | ||
overwrite: isCascadingOverwritten, | ||
}; | ||
|
||
const clonedCurrentData = cloneDeep(currentData); | ||
const JSONPatch = this.createJSONPatch(clonedCurrentData, originalData); | ||
onSave(id, JSONPatch, cascadePolicyData, clonedCurrentData); | ||
}; | ||
|
||
/** | ||
|
@@ -351,10 +344,10 @@ class Instance extends React.PureComponent<Props, State> { | |
* @return {Object} - react title element | ||
*/ | ||
getTitle(): React.Node { | ||
const { cascadePolicy = {}, hasError, isCascadingPolicyApplicable, template }: Props = this.props; | ||
const { cascadePolicy = {}, hasError, template }: Props = this.props; | ||
const isProperties = template.templateKey === TEMPLATE_CUSTOM_PROPERTIES; | ||
|
||
const type = isCascadingPolicyApplicable && cascadePolicy.id ? 'cascade' : 'default'; | ||
tjuanitas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const type = cascadePolicy?.id ? 'cascade' : 'default'; | ||
|
||
return ( | ||
<span className="metadata-instance-editor-instance-title"> | ||
|
@@ -363,6 +356,7 @@ class Instance extends React.PureComponent<Props, State> { | |
className={classNames('metadata-instance-editor-instance-title-text', { | ||
'metadata-instance-editor-instance-has-error': hasError, | ||
})} | ||
data-testid={hasError ? 'metadata-instance-has-error' : undefined} | ||
> | ||
{isProperties ? <FormattedMessage {...messages.customTitle} /> : template.displayName} | ||
</span> | ||
|
@@ -398,8 +392,8 @@ class Instance extends React.PureComponent<Props, State> { | |
* Get the delete confirmation message base on the template key | ||
*/ | ||
getConfirmationMessage(): React.Node { | ||
const { template, isCascadingPolicyApplicable }: Props = this.props; | ||
const isFile = !isCascadingPolicyApplicable; | ||
const { template, isFolderMetadata }: Props = this.props; | ||
const isFile = !isFolderMetadata; | ||
return this.renderDeleteMessage(isFile, template); | ||
} | ||
|
||
|
@@ -440,10 +434,7 @@ class Instance extends React.PureComponent<Props, State> { | |
* @return {boolean} true if cascading policy is enabled | ||
*/ | ||
isCascadingEnabled(props: Props) { | ||
if (props.cascadePolicy) { | ||
return !!props.cascadePolicy.id; | ||
} | ||
return false; | ||
return !!props?.cascadePolicy?.id; | ||
} | ||
|
||
/** | ||
|
@@ -531,6 +522,7 @@ class Instance extends React.PureComponent<Props, State> { | |
*/ | ||
canEdit(): boolean { | ||
const { canEdit, onModification, onRemove, onSave }: Props = this.props; | ||
|
||
return ( | ||
canEdit && | ||
typeof onRemove === 'function' && | ||
|
@@ -579,7 +571,7 @@ class Instance extends React.PureComponent<Props, State> { | |
}; | ||
|
||
render() { | ||
const { cascadePolicy = {}, isDirty, isCascadingPolicyApplicable, isOpen, template }: Props = this.props; | ||
const { isDirty, isFolderMetadata, isOpen, template }: Props = this.props; | ||
const { fields = [] } = template; | ||
const { | ||
data, | ||
|
@@ -590,8 +582,7 @@ class Instance extends React.PureComponent<Props, State> { | |
shouldShowCascadeOptions, | ||
isCascadingOverwritten, | ||
}: State = this.state; | ||
const isProperties = template.templateKey === TEMPLATE_CUSTOM_PROPERTIES; | ||
const isEditing = this.isEditing(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would imagine the performance impact is minor but consider that you are now calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree, I think the original is cleaner too |
||
const isCustomInstance = template.templateKey === TEMPLATE_CUSTOM_PROPERTIES; | ||
|
||
if (!template || isHidden(template)) { | ||
return null; | ||
|
@@ -601,7 +592,7 @@ class Instance extends React.PureComponent<Props, State> { | |
const animationDuration = (fields.length + 1) * 50; | ||
|
||
return ( | ||
<div ref={this.collapsibleRef}> | ||
<div ref={this.collapsibleRef} data-testid="metadata-instance"> | ||
<Collapsible | ||
animationDuration={animationDuration} | ||
buttonProps={{ | ||
|
@@ -626,27 +617,29 @@ class Instance extends React.PureComponent<Props, State> { | |
<LoadingIndicatorWrapper isLoading={isBusy}> | ||
<Form onValidSubmit={isDirty ? this.onSave : noop}> | ||
<div className="metadata-instance-editor-instance"> | ||
{isCascadingPolicyApplicable && ( | ||
{/* policies aren't enabled on custom instances but we show this anyway to display a message to inform the user */} | ||
{isFolderMetadata && ( | ||
<CascadePolicy | ||
canEdit={isEditing && !!cascadePolicy.canEdit} | ||
canEdit={this.isEditing()} | ||
isCascadingEnabled={isCascadingEnabled} | ||
isCascadingOverwritten={isCascadingOverwritten} | ||
isCustomMetadata={isProperties} | ||
isCustomMetadata={isCustomInstance} | ||
onCascadeModeChange={this.onCascadeModeChange} | ||
onCascadeToggle={this.onCascadeToggle} | ||
shouldShowCascadeOptions={shouldShowCascadeOptions} | ||
/> | ||
)} | ||
{isProperties ? ( | ||
|
||
{isCustomInstance ? ( | ||
<CustomInstance | ||
canEdit={isEditing} | ||
canEdit={this.isEditing()} | ||
data={data} | ||
onFieldChange={this.onFieldChange} | ||
onFieldRemove={this.onFieldRemove} | ||
/> | ||
) : ( | ||
<TemplatedInstance | ||
canEdit={isEditing} | ||
canEdit={this.isEditing()} | ||
data={data} | ||
errors={errors} | ||
onFieldChange={this.onFieldChange} | ||
|
@@ -655,7 +648,7 @@ class Instance extends React.PureComponent<Props, State> { | |
/> | ||
)} | ||
</div> | ||
{isEditing && ( | ||
{this.isEditing() && ( | ||
<Footer | ||
onCancel={this.onCancel} | ||
onRemove={this.onConfirmRemove} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I this intentional?
metadata.js
is still usingisCascadingPolicyApplicable?: boolean,