Copilot commented on code in PR #14570:
URL: https://github.com/apache/gravitino/pull/14570#discussion_r4474820293
##########
web-v2/web/src/app/compliance/policies/CreatePolicyDialog.js:
##########
@@ -46,99 +49,100 @@ const defaultValues = {
export default function CreatePolicyDialog({ ...props }) {
const { open, setOpen, metalake, editPolicy } = props
const [confirmLoading, setConfirmLoading] = useState(false)
+ const [isLoading, setIsLoading] = useState(false)
const [cacheData, setCacheData] = useState()
- const loadedRef = useRef(false)
const [form] = Form.useForm()
const values = Form.useWatch([], form)
const dispatch = useAppDispatch()
+ const roles = useAppSelector(state => state.roles)
+ const isAccessControl = values?.policyType === 'system_access_control'
useResetFormOnCloseModal({
form,
open
})
useEffect(() => {
- if (open && editPolicy && !loadedRef.current) {
- loadedRef.current = true
+ setCacheData(undefined)
+ setIsLoading(open && !!editPolicy)
+ if (!open || !editPolicy) return
+ let cancelled = false
- const init = async () => {
- const { payload: policy } = await dispatch(getPolicyDetails({
metalake, policy: editPolicy }))
- setCacheData(policy)
- form.setFieldValue('name', policy.name)
- form.setFieldValue('enabled', policy.enabled)
- form.setFieldValue('comment', policy.comment)
- form.setFieldValue('policyType', policy.policyType)
- form.setFieldValue('supportedObjectTypes',
policy.content?.supportedObjectTypes || [])
- let ruleIndex = 0
- Object.entries(policy.content?.customRules || {}).forEach(([key,
value]) => {
- if (key !== 'color') {
- form.setFieldValue(['rules', ruleIndex, 'ruleName'], key)
- form.setFieldValue(['rules', ruleIndex, 'ruleContent'], value)
- ruleIndex++
- }
- })
- let index = 0
- Object.entries(policy.content?.properties || {}).forEach(([key,
value]) => {
- if (key !== 'color') {
- form.setFieldValue(['properties', index, 'key'], key)
- form.setFieldValue(['properties', index, 'value'], value)
- index++
- }
- })
- }
- init()
+ const init = async () => {
+ const { payload: policy } = await dispatch(getPolicyDetails({ metalake,
policy: editPolicy }))
+ if (cancelled) return
+ setIsLoading(false)
+ if (!policy) return
+ setCacheData(policy)
+ form.setFieldsValue({
+ name: policy.name,
+ enabled: policy.enabled,
+ comment: policy.comment,
+ policyType: policy.policyType,
+ privileges: policy.content?.privileges || [],
+ applicableRoles: policy.content?.applicableRoles || [],
+ supportedObjectTypes: policy.content?.supportedObjectTypes || [],
+ rules: Object.entries(policy.content?.customRules || {})
+ .filter(([key]) => key !== 'color')
+ .map(([ruleName, ruleContent]) => ({ ruleName, ruleContent })),
+ properties: Object.entries(policy.content?.properties || {})
+ .filter(([key]) => key !== 'color')
+ .map(([key, value]) => ({ key, value }))
+ })
}
+ init()
- // Reset loadedRef when dialog closes
- if (!open) {
- loadedRef.current = false
+ return () => {
+ cancelled = true
}
- }, [open, editPolicy, metalake])
+ }, [open, editPolicy, metalake, dispatch, form])
+
+ useEffect(() => {
+ if (open && isAccessControl) dispatch(fetchRoles({ metalake }))
+ }, [open, isAccessControl, metalake, dispatch])
- const handleSubmit = e => {
+ const handleSubmit = async e => {
e.preventDefault()
- form
- .validateFields()
- .then(async () => {
- setConfirmLoading(true)
+ try {
+ const values = await form.validateFields()
+ setConfirmLoading(true)
- const submitData = {
- name: values.name.trim(),
- comment: values.comment,
- enabled: values.enabled,
- policyType: values.policyType,
- content: {
- customRules: values.rules.reduce((acc, item) => {
- if (item.ruleName) {
- acc[item.ruleName] = item.ruleContent
- }
+ const submitData = {
+ name: values.name.trim(),
+ comment: values.comment,
+ enabled: values.enabled,
+ policyType: values.policyType,
+ content:
+ values.policyType === 'system_access_control'
+ ? { privileges: values.privileges, applicableRoles:
values.applicableRoles }
Review Comment:
The new access-control editor has no automated coverage; the added tests
only exercise assignment filtering. Please add dialog tests for rejecting empty
roles/privileges, creating the access-control content, editing it with the
correct updateContent policyType, and retaining input after a failed save.
Similar create/edit/retry tests already exist in
CreateSemanticModelDialog.test.jsx, so these flows can be covered with the
existing Vitest setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
