From cbca3b0ab20c67ab33b30c470ce16a69a4d262ab Mon Sep 17 00:00:00 2001 From: freshavocado7 Date: Wed, 25 Sep 2024 13:36:55 +0200 Subject: [PATCH] feat(frontend): Display better error messages during model comparison --- capella_model_explorer/backend/explorer.py | 11 +- capella_model_explorer/backend/model_diff.py | 2 +- frontend/src/components/CommitInformation.jsx | 46 +++ frontend/src/components/ModelDiff.jsx | 368 +++++++++--------- 4 files changed, 231 insertions(+), 196 deletions(-) create mode 100644 frontend/src/components/CommitInformation.jsx diff --git a/capella_model_explorer/backend/explorer.py b/capella_model_explorer/backend/explorer.py index c56c541..e324c22 100644 --- a/capella_model_explorer/backend/explorer.py +++ b/capella_model_explorer/backend/explorer.py @@ -286,7 +286,9 @@ async def post_compare(commit_range: CommitRange): self.model, commit_range.head, commit_range.prev ) self.diff["lookup"] = create_diff_lookup(self.diff["objects"]) - return {"success": True} + if self.diff["lookup"]: + return {"success": True} + return {"success": False, "error": "No model changes to show"} except Exception as e: LOGGER.exception("Failed to compare versions") return {"success": False, "error": str(e)} @@ -301,8 +303,11 @@ async def post_object_diff(object_id: ObjectDiffID): @self.router.get("/api/commits") async def get_commits(): - result = model_diff.populate_commits(self.model) - return result + try: + result = model_diff.populate_commits(self.model) + return result + except Exception as e: + return {"error": str(e)} @self.router.get("/api/diff") async def get_diff(): diff --git a/capella_model_explorer/backend/model_diff.py b/capella_model_explorer/backend/model_diff.py index c4f7cce..dad95c2 100644 --- a/capella_model_explorer/backend/model_diff.py +++ b/capella_model_explorer/backend/model_diff.py @@ -168,7 +168,7 @@ def _get_revision_info( .strip() .split("\x00") ) - subject = description.splitlines()[0] + subject = description.splitlines()[0] if description.splitlines() else "" try: tag = subprocess.check_output( ["git", "tag", "--points-at", revision], diff --git a/frontend/src/components/CommitInformation.jsx b/frontend/src/components/CommitInformation.jsx new file mode 100644 index 0000000..ebad513 --- /dev/null +++ b/frontend/src/components/CommitInformation.jsx @@ -0,0 +1,46 @@ +// Copyright DB InfraGO AG and contributors +// SPDX-License-Identifier: Apache-2.0 + +const CommitInformation = ({ + commitDetails, + isExpanded, + toggleExpand, + section +}) => { + return ( +
+

+ Hash: {commitDetails.hash} +

+ {commitDetails.tag && ( +

+ Tag: {commitDetails.tag} +

+ )} +

+ Author: {commitDetails.author} +

+

+ Description:{' '} + {isExpanded + ? commitDetails.description + : `${commitDetails.description.split('\n')[0]}${ + commitDetails.description.includes('\n') ? '' : '' + }`} +

+ {commitDetails.description.includes('\n') && ( + + )} +

+ Date: {commitDetails.date} +

+
+ ); +}; + +export default CommitInformation; diff --git a/frontend/src/components/ModelDiff.jsx b/frontend/src/components/ModelDiff.jsx index 0c086a9..c62ee41 100644 --- a/frontend/src/components/ModelDiff.jsx +++ b/frontend/src/components/ModelDiff.jsx @@ -4,65 +4,72 @@ import { API_BASE_URL, ROUTE_PREFIX } from '../APIConfig'; import { useState } from 'react'; import { Spinner } from './Spinner'; +import CommitInformation from './CommitInformation'; export const ModelDiff = ({ onRefetch, hasDiffed }) => { - const [isLoading, setIsLoading] = useState(false); - const [completeLoading, setCompleteLoading] = useState(false); - const [commitDetails, setCommitDetails] = useState({}); + const [loadingState, setLoadingState] = useState('idle'); + const [commitDetails, setCommitDetails] = useState([]); const [selectionOptions, setSelectionOptions] = useState([]); const [isPopupVisible, setIsPopupVisible] = useState(false); - const [selectedDetails, setSelectedDetails] = useState(''); + const [selectedDetails, setSelectedDetails] = useState(null); const [error, setError] = useState(''); const [selectedOption, setSelectedOption] = useState(''); - const [isExpandedHead, setIsExpandedHead] = useState(false); - const [isExpanded, setIsExpanded] = useState(false); + const [isExpanded, setIsExpanded] = useState({ + head: false, + details: false + }); + const [diffSuccess, setDiffSuccess] = useState(null); const handleSelectChange = (e) => { - const option = e.target.value; - setSelectedOption(option); - const selectedValue = JSON.parse(e.target.value); - setSelectedDetails(selectedValue); - setIsExpanded(false); + const option = JSON.parse(e.target.value); + setSelectedOption(e.target.value); + setSelectedDetails(option); + setIsExpanded((prev) => ({ ...prev, details: false })); }; const handleGenerateDiff = async () => { - if (!commitDetails[0].hash || !selectedDetails.hash) { + const headCommit = commitDetails[0]?.hash; + const selectedCommit = selectedDetails?.hash; + + if (!headCommit || !selectedCommit) { alert('Please select a version.'); return; } - setCompleteLoading(false); - setIsLoading(true); + + setLoadingState('loading'); + setError(''); + try { - const url = API_BASE_URL + '/compare'; - const response = await postData(url, { - head: commitDetails[0].hash, - prev: selectedDetails.hash + const response = await postData(`${API_BASE_URL}/compare`, { + head: headCommit, + prev: selectedCommit }); const data = await response.json(); + if (data.error) { throw new Error(data.error); } + + setDiffSuccess(true); + setLoadingState('complete'); } catch (error) { - console.error('Error:', error); + setDiffSuccess(false); + setError(error.message); + setLoadingState('error'); } finally { - setIsLoading(false); - setCompleteLoading(true); onRefetch(); } }; - const postData = async (url = '', data = {}) => { + const postData = async (url, data) => { try { const response = await fetch(url, { method: 'POST', - headers: { - 'Content-Type': 'application/json' - }, + headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(data) }); - if (!response.ok) { - throw new Error('Network response was not ok'); - } + + if (!response.ok) throw new Error('Network response was not ok'); return response; } catch (error) { console.error('Error in postData:', error); @@ -70,20 +77,32 @@ export const ModelDiff = ({ onRefetch, hasDiffed }) => { } }; - async function openModelCompareDialog() { + const openModelCompareDialog = async () => { try { - const response = await fetch(API_BASE_URL + '/commits'); + setError(''); + setSelectedOption(''); + setSelectedDetails(null); + setDiffSuccess(null); + setLoadingState('idle'); + + const response = await fetch(`${API_BASE_URL}/commits`); + if (!response.ok) { - throw new Error( - 'Failed to fetch commits info: ' + response.statusText - ); + const err = await response.json(); + throw new Error(err.error || 'Internal server error.'); } + const data = await response.json(); if (data === null) { - alert('No commits found.'); - throw new Error('No commits found.'); + throw new Error('Not a git repo'); + } else if (data.length < 2) { + throw new Error('Not enough commits to compare.'); + } else if (data.error) { + throw new Error('Internal server error: ' + data.error); } + setCommitDetails(data); + const options = data.map((commit) => ({ value: JSON.stringify(commit), label: `${commit.hash.substring(0, 7)} ${ @@ -94,16 +113,22 @@ export const ModelDiff = ({ onRefetch, hasDiffed }) => { setSelectionOptions(options); setIsPopupVisible(true); } catch (err) { - setError(err.message); + alert(err.message || 'An error occurred while fetching commits.'); } - } + }; - const toggleExpand = () => { - setIsExpanded(!isExpanded); + const closeModelCompareDialog = () => { + setIsPopupVisible(false); + setLoadingState('idle'); + setSelectedOption(''); + setSelectedDetails(null); }; - const toggleExpandHead = () => { - setIsExpandedHead(!isExpandedHead); + const toggleExpand = (section) => { + setIsExpanded((prev) => ({ + ...prev, + [section]: !prev[section] + })); }; return ( @@ -121,12 +146,9 @@ export const ModelDiff = ({ onRefetch, hasDiffed }) => {
{ - if (!isLoading) { - setIsPopupVisible(false); - setCompleteLoading(false); - } - }}>
+ onClick={ + loadingState !== 'loading' ? closeModelCompareDialog : null + }>
- {error ? ( -
-

- Cannot generate model diff: {error} -

-
- ) : ( - <> - -
-

- Hash:{' '} - {commitDetails[0].hash} -

- {commitDetails[0].tag && ( -

- Tag:{' '} - {commitDetails[0].tag} -

- )} -

- Author:{' '} - {commitDetails[0].author} -

-

- Description:{' '} - {isExpandedHead - ? commitDetails[0].description - : `${commitDetails[0].description.split('\n')[0]}${commitDetails[0].description.includes('\n') ? '' : ''}`} -

- {commitDetails[0].description.includes('\n') && ( - - )} -

- Date:{' '} - {commitDetails[0].date} -

+ + + + + + + {selectedDetails && ( + + )} +
+ {loadingState === 'loading' && } +
+
+ {loadingState === 'loading' && ( +
+ Comparing versions...
- - {selectedDetails && ( -
-

- Hash:{' '} - {selectedDetails.hash} -

- {selectedDetails.tag && ( -

- Tag:{' '} - {selectedDetails.tag} -

- )} -

- Author:{' '} - {selectedDetails.author} -

-

- Description:{' '} - {isExpanded - ? selectedDetails.description - : `${selectedDetails.description.split('\n')[0]}${ - selectedDetails.description.includes('\n') - ? '' - : '' - }`} -

- {selectedDetails.description.includes('\n') && ( - - )} -

- Date:{' '} - {selectedDetails.date.substring(0, 10)} -

+ )} + {loadingState === 'complete' && diffSuccess && ( + <> +
+ Successfully compared versions ✓
- )} - {isLoading && ( -
- + + + + )} + {loadingState === 'error' && ( + <> +
+ {error !== 'No model changes to show' && ( + Error: + )}{' '} + {error}
- )} -
- {completeLoading && ( - - )} -
- - )} + + )} + {loadingState === 'idle' && ( + + )} +