refactor: Implement more review comments

capella_model_explorer/backend/explorer.py

@@ -15,7 +15,7 @@
 import markupsafe
 import prometheus_client
 import yaml
-from fastapi import APIRouter, FastAPI, Request
+from fastapi import APIRouter, FastAPI, HTTPException, Request
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.responses import HTMLResponse
 from fastapi.staticfiles import StaticFiles
@@ -45,11 +45,8 @@ class CommitRange(BaseModel):
     prev: str
 
 
-class DiffData(BaseModel):
-    display_name: str
-    change: str
-    attributes: dict[str, t.Any]
-    children: dict[str, t.Any]
+class ObjectDiffID(BaseModel):
+    uuid: str
 
 
 @dataclasses.dataclass
@@ -305,17 +302,18 @@ async def post_compare(commit_range: CommitRange):
                 self.diff = model_diff.get_diff_data(
                     self.model, commit_range.head, commit_range.prev
                 )
+                self.diff["lookup"] = create_diff_lookup(self.diff["objects"])
                 return {"success": True}
             except Exception as e:
                 return {"success": False, "error": str(e)}
 
         @self.app.post("/api/object-diff")
-        async def post_object_diff(diff_data: DiffData):
-            try:
-                self.object_diff = diff_data.model_dump()
-                return {"success": True}
-            except Exception as e:
-                return {"success": False, "error": str(e)}
+        async def post_object_diff(object_id: ObjectDiffID):
+            if object_id.uuid not in self.diff["lookup"]:
+                raise HTTPException(status_code=404, detail="Object not found")
+
+            self.object_diff = self.diff["lookup"][object_id.uuid]
+            return {"success": True}
 
         @self.app.get("/api/commits")
         async def get_commits():
@@ -360,3 +358,24 @@ def index_templates(
                 template, templates, templates_grouped, filename=idx
             )
     return templates_grouped, templates
+
+
+def create_diff_lookup(data, lookup=None):
+    if lookup is None:
+        lookup = {}
+    try:
+        if isinstance(data, dict):
+            for _, obj in data.items():
+                if "uuid" in obj:
+                    lookup[obj["uuid"]] = {
+                        "uuid": obj["uuid"],
+                        "display_name": obj["display_name"],
+                        "change": obj["change"],
+                        "attributes": obj["attributes"],
+                    }
+                if "children" in obj:
+                    if obj["children"]:
+                        create_diff_lookup(obj["children"], lookup)
+    except Exception:
+        pass
+    return lookup

capella_model_explorer/backend/model_diff.py

@@ -11,7 +11,7 @@
 import capellambse
 import capellambse.model as m
 import capellambse.model.common as c
-import diff_match_patch  # type: ignore
+import diff_match_patch
 import typing_extensions as te
 from capellambse.filehandler import git, local
 
@@ -30,6 +30,8 @@ class RevisionInfo(te.TypedDict, total=False):
     """The time and date of the revision."""
     description: str
     """The description of the revision, i.e. the commit message."""
+    subject: str
+    """The subject of the commit."""
     tag: str | None
     """The tag of the commit."""
 
@@ -97,40 +99,28 @@ class ChangeSummaryDocument(te.TypedDict):
     objects: ObjectChanges
 
 
-def init_model(model: capellambse.MelodyModel):
+def init_model(model: capellambse.MelodyModel) -> t.Optional[str]:
+    """Initialize the model and return the path if it's a git repository."""
     file_handler = model.resources["\x00"]
-    path = str(file_handler.path)
-    model_data: dict = {
-        "metadata": {"model": {"path": path, "entrypoint": None}},
-        "diagrams": {
-            "created": {},
-            "modified": {},
-            "deleted": {},
-        },
-        "objects": {
-            "created": {},
-            "modified": {},
-            "deleted": {},
-        },
-    }
+    path = file_handler.path
 
     if isinstance(file_handler, git.GitFileHandler):
-        path = str(file_handler.cache_dir)
+        path = file_handler.cache_dir
     elif (
         isinstance(file_handler, local.LocalFileHandler)
         and file_handler.rootdir.joinpath(".git").is_dir()
     ):
         pass
     else:
-        return {"error": "Not a git repo"}, model_data
-    return path, model_data
+        return None
+    return str(path)
 
 
 def populate_commits(model: capellambse.MelodyModel):
-    result, _ = init_model(model)
-    if "error" in result:
-        return result
-    commits = get_commit_hashes(result)
+    path = init_model(model)
+    if not path:
+        return path
+    commits = get_commit_hashes(path)
     return commits
 
 
@@ -145,8 +135,10 @@ def _serialize_obj(obj: t.Any) -> t.Any:
 
 
 def get_diff_data(model: capellambse.MelodyModel, head: str, prev: str):
-    path, _ = init_model(model)
-    path = pathlib.Path(path).resolve()
+    path = init_model(model)
+    if not path:
+        return None
+    path = str(pathlib.Path(path).resolve())
     old_model = capellambse.MelodyModel(path=f"git+{path}", revision=prev)
 
     metadata: Metadata = {
@@ -177,25 +169,30 @@ def _get_revision_info(
         .strip()
         .split("\x00")
     )
+    subject = description.splitlines()[0]
     try:
         tag = subprocess.check_output(
-            ["git", "describe", "--tags", revision],
+            ["git", "tag", "--points-at", revision],
             cwd=repo_path,
             encoding="utf-8",
+            stderr=subprocess.DEVNULL,
         ).strip()
     except subprocess.CalledProcessError:
         tag = None
+
     return {
         "hash": revision,
         "revision": revision,
         "author": author,
         "date": datetime.datetime.fromisoformat(date_str),
         "description": description.rstrip(),
-        "tag": tag,
+        "subject": subject,
+        "tag": tag if tag else None,
     }
 
 
-def get_commit_hashes(path: str):
+def get_commit_hashes(path: str) -> list[RevisionInfo]:
+    """Return the commit hashes of the given model."""
     commit_hashes = subprocess.check_output(
         ["git", "log", "-n", NUM_COMMITS, "--format=%H"],
         cwd=path,
@@ -386,7 +383,7 @@ def _handle_direct_accessors(
                 }
 
 
-def _traverse_and_diff(data):
+def _traverse_and_diff(data) -> dict[str, t.Any]:
     """Traverse the data and perform diff on text fields.
 
     This function recursively traverses the data and performs an HTML
@@ -433,14 +430,14 @@ def _traverse_and_diff(data):
     return data
 
 
-def _diff_text(previous, current):
+def _diff_text(previous, current) -> str:
     dmp = diff_match_patch.diff_match_patch()
     diff = dmp.diff_main("\n".join(previous), "\n".join(current))
     dmp.diff_cleanupSemantic(diff)
     return dmp.diff_prettyHtml(diff)
 
 
-def _diff_objects(previous, current):
+def _diff_objects(previous, current) -> str:
     return (
         f"<del>{previous['display_name']}</del> → " if previous else ""
     ) + f"<ins>{current['display_name']}</ins>"
@@ -466,7 +463,9 @@ def _diff_lists(previous, current):
     return out
 
 
-def _diff_description(previous, current):
+def _diff_description(
+    previous, current
+) -> t.Tuple[str, str] | t.Tuple[None, None]:
     if previous == current == None:
         return None, None
     dmp = diff_match_patch.diff_match_patch()

frontend/src/App.jsx

@@ -21,7 +21,10 @@ function App() {
           path="/:templateName/:objectID"
           element={<TemplateView endpoint={`${API_BASE_URL}/views/`} />}
         />
-        <Route path="/model-comparison" element={<ModelComparisonView />} />
+        <Route
+          path="/model-comparison"
+          element={<ModelComparisonView endpoint={`${API_BASE_URL}/views/`} />}
+        />
       </Routes>
     </Router>
   );

frontend/src/components/DiffExplorer.jsx

@@ -17,7 +17,6 @@ import {
 export const DiffExplorer = ({
   node,
   setObjectID,
-  setDiffData,
   searchTerm,
   filterStatus
 }) => {
@@ -70,7 +69,6 @@ export const DiffExplorer = ({
 
   const handleLeafClick = (node) => {
     setObjectID(node.uuid);
-    setDiffData(node);
   };
 
   const flattenNodes = (node) => {
@@ -142,7 +140,6 @@ export const DiffExplorer = ({
                 key={childId}
                 node={node.children[childId]}
                 setObjectID={setObjectID}
-                setDiffData={setDiffData}
                 searchTerm={searchTerm}
                 filterStatus={filterStatus}
               />

frontend/src/components/DiffView.jsx

@@ -5,67 +5,80 @@ import { SVGDisplay } from './SVGDisplay';
 import { API_BASE_URL } from '../APIConfig';
 import { Spinner } from './Spinner';
 
-export const DiffView = ({ objectID, endpoint, diffData }) => {
+export const DiffView = ({ objectID, endpoint }) => {
   const [details, setDetails] = useState([]);
   const [isLoading, setIsLoading] = useState(false);
 
   useEffect(() => {
-    const fetchTemplate = () => {
+    const fetchTemplate = async () => {
       setIsLoading(true);
-      fetch(`${API_BASE_URL}/object-diff`, {
-        method: 'POST',
-        headers: {
-          'Content-Type': 'application/json'
-        },
-        body: JSON.stringify(diffData)
-      })
-        .then((response) => response.json())
-        .then(() => {
-          let url;
-          if (diffData.change) {
-            url = `${endpoint}object_comparison/${objectID}`;
+
+      try {
+        const postResponse = await fetch(`${API_BASE_URL}/object-diff`, {
+          method: 'POST',
+          headers: {
+            'Content-Type': 'application/json'
+          },
+          body: JSON.stringify({ uuid: objectID })
+        });
+
+        if (!postResponse.ok) {
+          throw new Error(
+            `Failed to post diff data: ${postResponse.statusText}`
+          );
+        }
+
+        let url;
+        if (objectID) {
+          url = `${endpoint}object_comparison/${objectID}`;
+        } else {
+          throw new Error('Object ID is missing or invalid');
+        }
+
+        const getResponse = await fetch(url, {
+          method: 'GET',
+          headers: {
+            'Content-Type': 'text/html'
           }
-          fetch(url, {
-            method: 'GET',
-            headers: {
-              'Content-Type': 'text/html'
-            }
-          })
-            .then((response) => response.text())
-            .then((data) => {
-              const parser = new DOMParser();
-              const doc = parser.parseFromString(data, 'text/html');
-              const contentItems = [];
+        });
+
+        if (!getResponse.ok) {
+          throw new Error(
+            `Failed to fetch object comparison: ${getResponse.statusText}`
+          );
+        }
 
-              doc.body.childNodes.forEach((node) => {
-                if (node.nodeType === Node.ELEMENT_NODE) {
-                  if (node.tagName === 'svg') {
-                    contentItems.push({
-                      type: 'SVGDisplay',
-                      content: node.outerHTML
-                    });
-                  } else {
-                    contentItems.push({
-                      type: 'HTML',
-                      content: node.outerHTML
-                    });
-                  }
-                }
+        const data = await getResponse.text();
+
+        const parser = new DOMParser();
+        const doc = parser.parseFromString(data, 'text/html');
+        const contentItems = [];
+
+        doc.body.childNodes.forEach((node) => {
+          if (node.nodeType === Node.ELEMENT_NODE) {
+            if (node.tagName === 'svg') {
+              contentItems.push({
+                type: 'SVGDisplay',
+                content: node.outerHTML
+              });
+            } else {
+              contentItems.push({
+                type: 'HTML',
+                content: node.outerHTML
               });
-              setDetails(contentItems);
-              setIsLoading(false);
-            })
-            .catch((error) => {
-              setDetails([
-                { type: 'Error', content: `Error fetching data: ${error}` }
-              ]);
-              setIsLoading(false);
-            });
-        })
-        .catch((error) => {
-          console.error('Error posting diff data:', error);
-          setIsLoading(false);
+            }
+          }
         });
+
+        setDetails(contentItems);
+      } catch (error) {
+        console.error('Error fetching template:', error);
+        setDetails([
+          { type: 'Error', content: `Error fetching data: ${error.message}` }
+        ]);
+      } finally {
+        setIsLoading(false);
+      }
     };
 
     if (objectID) {