mchades commented on code in PR #12900:
URL: https://github.com/apache/gravitino/pull/12900#discussion_r4249707970


##########
web-v2/web/src/lib/store/metalakes/index.js:
##########
@@ -2332,6 +2374,136 @@ export const deleteView = createAsyncThunk(
   }
 )
 
+export const fetchSemanticModels = createAsyncThunk(
+  'appMetalakes/fetchSemanticModels',
+  async ({ init, metalake, catalog, schema }, { getState, dispatch }) => {
+    const [err, res] = await to(getSemanticModelsApi({ metalake, catalog, 
schema }, { errorMessageMode: 'none' }))
+
+    if (err || !res) {
+      // The catalog doesn't support semantic models (405), or the server 
predates the endpoint and
+      // has no route for it (404). Both mean "this schema has none" — return 
empty silently rather
+      // than failing every schema page against an older server.
+      if ([404, 405].includes(err?.response?.status)) {
+        return { semanticModels: [], init }
+      }
+      if (init) {
+        throw new Error(err)
+      }
+    }
+
+    const { identifiers = [] } = res || {}
+
+    const semanticModels = identifiers.map(semanticModelItem => {
+      return {
+        ...semanticModelItem,
+        node: 'semanticModel',
+        id: 
`{{${metalake}}}{{${catalog}}}{{${'relational'}}}{{${schema}}}{{${semanticModelItem.name}}}`,
+        key: 
`{{${metalake}}}{{${catalog}}}{{${'relational'}}}{{${schema}}}{{${semanticModelItem.name}}}`,

Review Comment:
   [P2] Include the entity type in Semantic Model node keys
   
   When a table and a Semantic Model in the same schema are both named 
`orders`, these keys are identical. `mergeWithSemanticModelNodes()` 
deduplicates by `key` and keeps the table first, so refreshing or creating the 
model removes its node from the tree. Initial schema expansion also produces 
duplicate keys. Give Semantic Models distinct keys/IDs in both builders and 
update the route-to-selection mapping consistently.



##########
web-v2/web/src/app/catalogs/rightContent/CreateSemanticModelDialog.js:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+'use client'
+
+import React, { useEffect, useRef, useState } from 'react'
+import { Form, Input, Modal, Spin, Typography } from 'antd'
+import { useScrolling } from 'react-use'
+import RenderPropertiesFormItem from '@/components/EntityPropertiesFormItem'
+import { validateMessages, dialogContentMaxHeigth, mismatchName } from 
'@/config'
+import { nameRegex } from '@/lib/utils/regex'
+import { useResetFormOnCloseModal } from '@/lib/hooks/use-reset'
+import { cn } from '@/lib/utils/tailwind'
+import { useAppDispatch } from '@/lib/hooks/useStore'
+import {
+  formatSemanticModelDefinition,
+  genSemanticModelUpdates,
+  parseSemanticModelDefinition
+} from '@/lib/utils/semanticModel'
+import { createSemanticModel, updateSemanticModel, getSemanticModelDetails } 
from '@/lib/store/metalakes'
+
+const { Paragraph } = Typography
+const { TextArea } = Input
+
+const definitionPlaceholder = `{
+  "datasets": [
+    {
+      "name": "orders",
+      "source": { "namespace": ["sales", "mart"], "name": "orders" },
+      "fields": []
+    }
+  ]
+}`
+
+const defaultValues = {
+  name: '',
+  comment: '',
+  definition: '',
+  properties: []
+}
+
+export default function CreateSemanticModelDialog({ ...props }) {
+  const { open, setOpen, metalake, catalog, schema, editSemanticModel } = props
+  const [confirmLoading, setConfirmLoading] = useState(false)
+  const [isLoading, setIsLoading] = useState(false)
+  const [cacheData, setCacheData] = useState()
+  const scrollRef = useRef(null)
+  const loadedRef = useRef(false)
+  const scrolling = useScrolling(scrollRef)
+  const [bottomShadow, setBottomShadow] = useState(false)
+  const [topShadow, setTopShadow] = useState(false)
+  const [form] = Form.useForm()
+  const values = Form.useWatch([], form)
+  const dispatch = useAppDispatch()
+
+  useResetFormOnCloseModal({
+    form,
+    open
+  })
+
+  const handScroll = () => {
+    if (scrollRef.current) {
+      const { scrollTop, scrollHeight, clientHeight } = scrollRef.current
+      if (scrollHeight > clientHeight + scrollTop) {
+        setTopShadow(true)
+        setBottomShadow(scrollTop > 0)
+      } else if (scrollHeight === clientHeight + scrollTop) {
+        setTopShadow(false)
+        setBottomShadow(scrollTop > 0)
+      }
+    }
+  }
+
+  useEffect(() => {
+    scrollRef.current && handScroll()
+  }, [scrolling])
+
+  useEffect(() => {
+    if (open && editSemanticModel && !loadedRef.current) {
+      loadedRef.current = true
+
+      const initLoad = async () => {
+        setIsLoading(true)
+        try {
+          const { payload } = await dispatch(
+            getSemanticModelDetails({ metalake, catalog, schema, 
semanticModel: editSemanticModel })
+          )
+          const loaded = payload?.semanticModel
+          setCacheData(loaded)
+          form.setFieldValue('name', loaded?.name)
+          form.setFieldValue('comment', loaded?.comment)
+          form.setFieldValue('definition', 
formatSemanticModelDefinition(loaded?.definition))
+          let index = 0
+          Object.entries(loaded?.properties || {}).forEach(([key, value]) => {
+            form.setFieldValue(['properties', index, 'key'], key)
+            form.setFieldValue(['properties', index, 'value'], value)
+            index++
+          })
+          setIsLoading(false)
+        } catch (e) {
+          setIsLoading(false)
+        }
+      }
+      initLoad()
+    }
+
+    // Reset loadedRef when dialog closes
+    if (!open) {
+      loadedRef.current = false
+    }
+  }, [open, editSemanticModel, metalake, catalog, schema])
+
+  const validateDefinition = (rule, value) => {
+    const { errors } = parseSemanticModelDefinition(value)
+
+    return errors.length ? Promise.reject(new Error(errors.join(' '))) : 
Promise.resolve()
+  }
+
+  const handleSubmit = e => {
+    e.preventDefault()
+    form
+      .validateFields()
+      .then(async () => {
+        const { definition } = parseSemanticModelDefinition(values.definition)
+
+        setConfirmLoading(true)
+
+        const submitData = {
+          name: values.name.trim(),
+          comment: values.comment,
+          definition,
+          properties: (values.properties || []).reduce((acc, item) => {
+            acc[item.key] = values[item.key] || item.value
+
+            return acc
+          }, {})
+        }
+
+        if (editSemanticModel) {
+          const updates = genSemanticModelUpdates(cacheData, submitData)
+          if (updates.length) {
+            await dispatch(
+              updateSemanticModel({
+                metalake,
+                catalog,
+                schema,
+                semanticModel: editSemanticModel,
+                data: { updates }
+              })
+            )
+          }
+        } else {
+          await dispatch(createSemanticModel({ data: submitData, metalake, 
catalog, schema }))
+        }
+
+        setConfirmLoading(false)
+        setOpen(false)

Review Comment:
   [P2] Preserve the editor input when saving fails
   
   Both write thunks return `{ err: true }` when the API rejects, for example 
with HTTP 400 for an invalid dataset source. This handler ignores that result 
and always closes the dialog; `SchemaDetailsPage` then unmounts it, discarding 
the JSON and other edits even though nothing was saved. Check the action 
result, as `CreateTableDialog` does, and close only on success. Reset 
`confirmLoading` on both paths.



-- 
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]

Reply via email to