From 29576eb451b893d63fb2bb56b99e6bef2d509c97 Mon Sep 17 00:00:00 2001 From: Tejas Rajopadhye Date: Mon, 31 Mar 2025 16:57:15 -0500 Subject: [PATCH] Adding critical mf bugfixes --- .../attached_metadata_form_service.py | 8 ++ .../components/AttachedFormCard.js | 2 +- .../MetadataFormAttachedEntities.js | 34 +++++++- .../components/MetadataFormEnforcement.js | 77 ++++++++++++++----- .../components/MetadataFormFields.js | 10 +-- .../Metadata_Forms/components/fields.js | 4 +- .../components/metadataAttachment.js | 65 ++++++++++++++-- .../components/renderedMetadataForm.js | 25 +++--- .../Metadata_Forms/views/MetadataFormView.js | 38 ++++++++- 9 files changed, 215 insertions(+), 48 deletions(-) diff --git a/backend/dataall/modules/metadata_forms/services/attached_metadata_form_service.py b/backend/dataall/modules/metadata_forms/services/attached_metadata_form_service.py index 91ae4a504..b4c0ff895 100644 --- a/backend/dataall/modules/metadata_forms/services/attached_metadata_form_service.py +++ b/backend/dataall/modules/metadata_forms/services/attached_metadata_form_service.py @@ -1,3 +1,4 @@ +import logging from dataall.base.context import get_context from dataall.base.db import exceptions, paginate from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService @@ -5,6 +6,8 @@ from dataall.modules.metadata_forms.services.metadata_form_access_service import MetadataFormAccessService from dataall.modules.metadata_forms.services.metadata_form_permissions import ATTACH_METADATA_FORM +log = logging.getLogger(__name__) + class AttachedMetadataFormValidationService: @staticmethod @@ -47,10 +50,13 @@ def create_or_update_attached_metadata_form(uri, data): if data.get('attachedUri'): with get_context().db_engine.scoped_session() as session: existingAMF = MetadataFormRepository.get_attached_metadata_form(session, data.get('attachedUri')) + log.info(f'Found an existing metadata form with uri: {existingAMF.uri}') if existingAMF and new_form: + log.info(f'Deleting older existing metadata form attachement with uri: {existingAMF.uri}') session.delete(existingAMF) return new_form except Exception as e: + log.error(f'Error occurred while creating / updating attached metadata forms due to: {e}') if new_form: AttachedMetadataFormService.delete_attached_metadata_form(uri=new_form.uri) raise e @@ -74,8 +80,10 @@ def create_attached_metadata_form(uri, data): MetadataFormRepository.create_attached_metadata_form_field( session, amf.uri, f.get('field'), f.get('value') ) + log.info(f'Returning new attached metadata forms with uri: {amf.uri}') return amf except Exception as e: + log.error(f'Error while creating attached meta form fields due to: {e}') AttachedMetadataFormService.delete_attached_metadata_form(uri=amf.uri) raise e diff --git a/frontend/src/modules/Metadata_Forms/components/AttachedFormCard.js b/frontend/src/modules/Metadata_Forms/components/AttachedFormCard.js index b6b0f5512..9b341a85f 100644 --- a/frontend/src/modules/Metadata_Forms/components/AttachedFormCard.js +++ b/frontend/src/modules/Metadata_Forms/components/AttachedFormCard.js @@ -72,7 +72,7 @@ export const AttachedFormCard = (props) => { ml: 5 }} > - {field.value} + {field.value?.toString()} ))} diff --git a/frontend/src/modules/Metadata_Forms/components/MetadataFormAttachedEntities.js b/frontend/src/modules/Metadata_Forms/components/MetadataFormAttachedEntities.js index daf674d97..b030cf08b 100644 --- a/frontend/src/modules/Metadata_Forms/components/MetadataFormAttachedEntities.js +++ b/frontend/src/modules/Metadata_Forms/components/MetadataFormAttachedEntities.js @@ -25,6 +25,7 @@ import { DataGrid } from '@mui/x-data-grid'; import { AttachedFormCard } from './AttachedFormCard'; import CircularProgress from '@mui/material/CircularProgress'; import { useTheme } from '@mui/styles'; +import { DeleteObjectWithFrictionModal } from '../../../design'; export const MetadataFormAttachedEntities = (props) => { const { metadataForm, userRolesMF } = props; @@ -44,6 +45,7 @@ export const MetadataFormAttachedEntities = (props) => { page: 0 }); const [selectedEntity, setSelectedEntity] = useState(null); + const [isDeleteRoleModalOpen, setIsDeleteRoleModalOpen] = useState(false); const header = [ { field: 'entityType', width: 200, headerName: 'Type', editable: false }, @@ -51,6 +53,15 @@ export const MetadataFormAttachedEntities = (props) => { { field: 'entityOwner', width: 200, headerName: 'Owner', editable: false } ]; + const handleDeleteRoleModalOpen = (uri) => { + setIsDeleteRoleModalOpen(true); + setSelectedVersion(uri); + }; + const handleDeleteRoleModalClosed = () => { + setIsDeleteRoleModalOpen(false); + setSelectedVersion(null); + }; + const fetchVersions = async () => { setLoadingVersions(true); const response = await client.query( @@ -99,7 +110,8 @@ export const MetadataFormAttachedEntities = (props) => { } }, [client, dispatch]); - const deleteVersion = async (version) => { + const deleteVersion = async () => { + const version = selectedVersion; setLoading(true); const response = await client.mutate( deleteMetadataFormVersion(version.metadataFormUri, version.version) @@ -109,6 +121,7 @@ export const MetadataFormAttachedEntities = (props) => { response.data && response.data.deleteMetadataFormVersion !== null ) { + handleDeleteRoleModalClosed(); await fetchVersions(); enqueueSnackbar('Version deleted', { anchorOrigin: { @@ -243,7 +256,7 @@ export const MetadataFormAttachedEntities = (props) => { onMouseOut={(e) => { e.currentTarget.style.opacity = 0.5; }} - onClick={() => deleteVersion(version)} + onClick={() => handleDeleteRoleModalOpen(version)} /> )} @@ -341,6 +354,23 @@ export const MetadataFormAttachedEntities = (props) => { ))} +
+ + + Are you sure you want to delete this version of metadata form ?{' '} + + + } + open={isDeleteRoleModalOpen} + isAWSResource={false} + deleteFunction={deleteVersion} + /> +
); }; diff --git a/frontend/src/modules/Metadata_Forms/components/MetadataFormEnforcement.js b/frontend/src/modules/Metadata_Forms/components/MetadataFormEnforcement.js index a02421ce3..7601bcc0b 100644 --- a/frontend/src/modules/Metadata_Forms/components/MetadataFormEnforcement.js +++ b/frontend/src/modules/Metadata_Forms/components/MetadataFormEnforcement.js @@ -15,7 +15,12 @@ import { TextField, Typography } from '@mui/material'; -import { Defaults, Label, PlusIcon } from 'design'; +import { + Defaults, + DeleteObjectWithFrictionModal, + Label, + PlusIcon +} from 'design'; import React, { useEffect, useState } from 'react'; import DoNotDisturbAltOutlinedIcon from '@mui/icons-material/DoNotDisturbAltOutlined'; import { fetchEnums, listValidEnvironments, useClient } from 'services'; @@ -434,6 +439,8 @@ export const MetadataFormEnforcement = (props) => { pageSize: 5, page: 0 }); + const [selectedRuleUri, setSelectedRuleUri] = useState(null); + const [isDeleteRoleModalOpen, setIsDeleteRoleModalOpen] = useState(false); const [selectedEntity, setSelectedEntity] = useState([]); const [loading, setLoading] = useState(true); const [loadingAffected, setLoadingAffected] = useState(true); @@ -457,6 +464,15 @@ export const MetadataFormEnforcement = (props) => { } ]; + const handleDeleteRoleModalOpen = (uri) => { + setIsDeleteRoleModalOpen(true); + setSelectedRuleUri(uri); + }; + const handleDeleteRoleModalClosed = () => { + setIsDeleteRoleModalOpen(false); + setSelectedRuleUri(null); + }; + const fetchEntityTypesWithScope = async () => { const response = await client.query(listEntityTypesWithScope()); if ( @@ -495,11 +511,13 @@ export const MetadataFormEnforcement = (props) => { setLoading(false); }; - const deleteRule = async (rule_uri) => { + const deleteRule = async () => { + const rule_uri = selectedRuleUri; const response = await client.mutate( deleteMetadataFormEnforcementRule(metadataForm.uri, rule_uri) ); if (!response.errors) { + handleDeleteRoleModalClosed(); if (selectedRule.uri === rule_uri) { setSelectedRule(null); setAffectedEntities([]); @@ -673,19 +691,21 @@ export const MetadataFormEnforcement = (props) => { {canEdit && ( - { - e.currentTarget.style.opacity = 1; - }} - onMouseOut={(e) => { - e.currentTarget.style.opacity = 0.5; - }} - onClick={(e) => { - e.stopPropagation(); - deleteRule(rule.uri); - }} - /> + <> + { + e.currentTarget.style.opacity = 1; + }} + onMouseOut={(e) => { + e.currentTarget.style.opacity = 0.5; + }} + onClick={(e) => { + e.stopPropagation(); + handleDeleteRoleModalOpen(rule.uri); + }} + /> + )} @@ -716,18 +736,16 @@ export const MetadataFormEnforcement = (props) => { { setPaginationModel({ ...paginationModel, - pageSize: newPageSize + pageSize: newPageSize, + page: 0 }); - await fetchAffectedEntities( - selectedRule, - paginationModel.page, - newPageSize - ); + await fetchAffectedEntities(selectedRule, 0, newPageSize); }} page={paginationModel.page} onPageChange={async (newPage) => { @@ -776,6 +794,23 @@ export const MetadataFormEnforcement = (props) => { onCancel={() => setShowCreateRuleModal(false)} /> )} +
+ + + Are you sure you want to delete this enforcement rule ?{' '} + + + } + open={isDeleteRoleModalOpen} + isAWSResource={false} + deleteFunction={deleteRule} + /> +
); }; diff --git a/frontend/src/modules/Metadata_Forms/components/MetadataFormFields.js b/frontend/src/modules/Metadata_Forms/components/MetadataFormFields.js index 914085918..d50b3862f 100644 --- a/frontend/src/modules/Metadata_Forms/components/MetadataFormFields.js +++ b/frontend/src/modules/Metadata_Forms/components/MetadataFormFields.js @@ -95,7 +95,7 @@ const EditTable = (props) => { const addField = () => { localFields.push({ id: uuidv4(), - name: 'New Field', + name: '', required: false, metadataFormUri: formUri, type: fieldTypeOptions[0].value, @@ -178,8 +178,8 @@ const EditTable = (props) => { { + value={field.name} + onChange={(event) => { updateField(index, 'name', event.target.value); }} sx={{ width: '100%' }} @@ -211,9 +211,9 @@ const EditTable = (props) => { { + onChange={(event) => { updateField(index, 'description', event.target.value); }} /> diff --git a/frontend/src/modules/Metadata_Forms/components/fields.js b/frontend/src/modules/Metadata_Forms/components/fields.js index f58c91826..edc8b12df 100644 --- a/frontend/src/modules/Metadata_Forms/components/fields.js +++ b/frontend/src/modules/Metadata_Forms/components/fields.js @@ -37,7 +37,9 @@ export const BooleanField = (props) => { control={ onChange(checked)} /> } diff --git a/frontend/src/modules/Metadata_Forms/components/metadataAttachment.js b/frontend/src/modules/Metadata_Forms/components/metadataAttachment.js index 97f49b3ed..f790af90a 100644 --- a/frontend/src/modules/Metadata_Forms/components/metadataAttachment.js +++ b/frontend/src/modules/Metadata_Forms/components/metadataAttachment.js @@ -21,7 +21,7 @@ import { listAttachedMetadataForms, listEntityMetadataForms } from '../services'; -import { Defaults, PlusIcon } from 'design'; +import { Defaults, DeleteObjectWithFrictionModal, PlusIcon } from 'design'; import CircularProgress from '@mui/material/CircularProgress'; import { useClient } from 'services'; import { RenderedMetadataForm } from './renderedMetadataForm'; @@ -46,6 +46,7 @@ export const MetadataAttachment = (props) => { const [editMode, setEditMode] = useState(false); const [values, setValues] = useState({}); const [attachedMFUri, setAttachedMFUri] = useState(-1); + const [isDeleteRoleModalOpenId, setIsDeleteRoleModalOpen] = useState(0); const [filter] = useState({ ...Defaults.filter, pageSize: 20, @@ -59,7 +60,7 @@ export const MetadataAttachment = (props) => { const fetchAvailableForms = async () => { const response = await client.query( listEntityMetadataForms({ - ...Defaults.filter, + ...Defaults.selectListFilter, entityType: entityType, entityUri: entityUri, hideAttached: true @@ -125,6 +126,11 @@ export const MetadataAttachment = (props) => { response.data && response.data.getMetadataForm !== null ) { + if (response.data.getMetadataForm.fields.length === 0) { + const error = 'Metadata form with no fields cannot be attached'; + dispatch({ type: SET_ERROR, error }); + setSelectedForm(null); + } setFields(response.data.getMetadataForm.fields); } else { const error = response.errors @@ -134,6 +140,16 @@ export const MetadataAttachment = (props) => { } setLoadingFields(false); }; + + const conditionAndSetFields = (fieldsData) => { + fieldsData.forEach((field, index) => { + if (field.field.type === 'Boolean' && field.value !== undefined) { + field.value = JSON.parse(field.value); + } + }); + setFields(fieldsData); + }; + const fetchAttachedFields = async (uri) => { setLoadingFields(true); const response = await client.query(getAttachedMetadataForm(uri)); @@ -142,7 +158,7 @@ export const MetadataAttachment = (props) => { response.data && response.data.getAttachedMetadataForm !== null ) { - setFields(response.data.getAttachedMetadataForm.fields); + conditionAndSetFields(response.data.getAttachedMetadataForm.fields); } else { const error = response.errors ? response.errors[0].message @@ -181,6 +197,13 @@ export const MetadataAttachment = (props) => { } }; + const handleDeleteRoleModalOpen = (id) => { + setIsDeleteRoleModalOpen(id); + }; + const handleDeleteRoleModalClosed = (id) => { + setIsDeleteRoleModalOpen(0); + }; + useEffect(() => { if (client) { fetchList().catch((e) => dispatch({ type: SET_ERROR, error: e.message })); @@ -389,11 +412,41 @@ export const MetadataAttachment = (props) => { onMouseOut={(e) => { e.currentTarget.style.opacity = 0.5; }} - onClick={() => deleteAttachedForm(attachedForm.uri)} + onClick={() => + handleDeleteRoleModalOpen(attachedForm.uri) + } /> )} + <> + + handleDeleteRoleModalClosed(attachedForm.uri) + } + onClose={() => + handleDeleteRoleModalClosed(attachedForm.uri) + } + deleteMessage={ + <> + + Are you sure you want to delete this Metadata form ?{' '} + Deleting attached form will permanently delete the + data on this form. Once deleted, data on this attached + metadata form cannot be recovered. + + + } + open={isDeleteRoleModalOpenId === attachedForm.uri} + isAWSResource={false} + deleteFunction={() => deleteAttachedForm(attachedForm.uri)} + /> + )) ) : ( @@ -430,9 +483,7 @@ export const MetadataAttachment = (props) => { onCancel={() => { setAddNewForm(false); setSelectedForm(null); - setFields([]); setEditMode(false); - setValues({}); setAttachedMFUri(-1); }} entityUri={entityUri} @@ -442,7 +493,7 @@ export const MetadataAttachment = (props) => { setSelectedAttachedForm(attachedForm); setEditMode(false); setValues({}); - setFields(attachedForm.fields); + conditionAndSetFields(attachedForm.fields); fetchList().catch((e) => dispatch({ type: SET_ERROR, error: e.message }) ); diff --git a/frontend/src/modules/Metadata_Forms/components/renderedMetadataForm.js b/frontend/src/modules/Metadata_Forms/components/renderedMetadataForm.js index 7509ee2ed..845982d54 100644 --- a/frontend/src/modules/Metadata_Forms/components/renderedMetadataForm.js +++ b/frontend/src/modules/Metadata_Forms/components/renderedMetadataForm.js @@ -48,16 +48,24 @@ export const RenderedMetadataForm = (props) => { metadataForm.versions ? metadataForm.versions[0] : 0 ); - localFields.forEach((field, index) => { - if (field.type === 'Boolean' && field.value === undefined) { - field.value = false; + useEffect(() => { + if (fields) { + updateFieldValues(); } - if (field.type === 'Boolean' || field.type === 'Integer') { - if (field.value) { - field.value = JSON.parse(field.value); + }, [fields]); + + const updateFieldValues = () => { + localFields.forEach((field, index) => { + if (field.type === 'Boolean' && field.value === undefined) { + field.value = false; } - } - }); + if (field.type === 'Boolean' || field.type === 'Integer') { + if (field.value) { + field.value = JSON.parse(field.value); + } + } + }); + }; const updateFields = (index, value) => { const updatedFields = [...localFields]; @@ -140,7 +148,6 @@ export const RenderedMetadataForm = (props) => { })) }; if (attachedUri && attachedUri !== -1) input.attachedUri = attachedUri; - const response = await client.mutate( createAttachedMetadataForm(metadataForm.uri, input) ); diff --git a/frontend/src/modules/Metadata_Forms/views/MetadataFormView.js b/frontend/src/modules/Metadata_Forms/views/MetadataFormView.js index 67a93b638..91b218cbc 100644 --- a/frontend/src/modules/Metadata_Forms/views/MetadataFormView.js +++ b/frontend/src/modules/Metadata_Forms/views/MetadataFormView.js @@ -18,7 +18,11 @@ import { Tabs, Typography } from '@mui/material'; -import { ChevronRightIcon, useSettings } from 'design'; +import { + ChevronRightIcon, + DeleteObjectWithFrictionModal, + useSettings +} from 'design'; import { FaTrash } from 'react-icons/fa'; import { MetadataFormInfo, @@ -49,6 +53,7 @@ const MetadataFormView = () => { const [visibilityDict, setVisibilityDict] = useState({}); const [fieldTypeOptions, setFieldTypeOptions] = useState([]); const [userRolesMF, setUserRolesMFDict] = useState({}); + const [isDeleteRoleModalOpen, setIsDeleteRoleModalOpen] = useState(false); const handleTabsChange = (event, value) => { setCurrentTab(value); @@ -117,6 +122,13 @@ const MetadataFormView = () => { } }; + const handleDeleteRoleModalOpen = () => { + setIsDeleteRoleModalOpen(true); + }; + const handleDeleteRoleModalClosed = () => { + setIsDeleteRoleModalOpen(false); + }; + useEffect(() => { if (client) { fetchMetadataForm().catch((e) => @@ -208,12 +220,34 @@ const MetadataFormView = () => { color="primary" startIcon={} sx={{ mt: 1 }} - onClick={deleteForm} + onClick={handleDeleteRoleModalOpen} type="button" variant="outlined" > Delete + handleDeleteRoleModalClosed(false)} + onClose={() => handleDeleteRoleModalClosed(false)} + deleteMessage={ + <> + + Are you sure you want to delete this Metadata form ? + Deleting a metadata form will remove the attached + form, from all the entities. Once deleted, attached + metadata forms cannot be recovered. + + + } + open={isDeleteRoleModalOpen} + isAWSResource={false} + deleteFunction={deleteForm} + /> )}