Conversation
|
@TsotneMikadze |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces comprehensive user experience improvements including a new toast notification system, enhanced modals for creating vSUMs, better error handling in auth services, and additional API functionality for user management.
- Adds a global toast notification system with success, error, and info message types
- Replaces inline vSUM creation form with a dedicated modal interface
- Improves authentication flow to fetch authoritative user data from the API
- Simplifies error message formatting in authentication services
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/auth.ts | Simplified error message formatting by removing status codes |
| src/services/api.ts | Added getUserInfo endpoint and enhanced createVsum to accept description |
| src/index.tsx | Wrapped app with ToastProvider for global notifications |
| src/contexts/AuthContext.tsx | Enhanced user data fetching to prefer API over JWT parsing |
| src/components/ui/VsumsPanel.tsx | Replaced inline form with modal-based vSUM creation |
| src/components/ui/ToastProvider.tsx | New toast notification system implementation |
| src/components/ui/MetaModelsPanel.tsx | Added delete functionality for meta models |
| src/components/ui/CreateVsumModal.tsx | New modal component for vSUM creation |
| src/components/ui/CreateModelModal.tsx | Moved progress indicator to bottom of modal |
| src/components/index.ts | Exported new ToastProvider components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const fallback = response.statusText || 'Request failed'; | ||
| throw new Error(errorMessage || fallback); |
There was a problem hiding this comment.
The simplified error handling removes valuable debugging information. HTTP status codes help developers and users understand the nature of the failure (e.g., 401 Unauthorized vs 500 Server Error). Consider preserving the status code while simplifying the format, such as throw new Error(errorMessage ? ${response.status}: ${errorMessage}:${response.status} ${fallback}`);
| AuthService.setCurrentUser(mapped); | ||
| setUser(mapped); | ||
| } catch (e) { | ||
| // Fallback: try to parse from token if API not available |
There was a problem hiding this comment.
[nitpick] The comment should be 'Fallback: Try to parse from token if API is not available' or 'Fallback: try to parse from token if API call fails' for better clarity and grammar.
| // Fallback: try to parse from token if API not available | |
| // Fallback: Try to parse from token if API is not available |
| setUser(mapped); | ||
| return; | ||
| } catch (e) { | ||
| // Fallback: parse JWT |
There was a problem hiding this comment.
This comment is too brief and doesn't explain why JWT parsing is a fallback. Consider expanding to 'Fallback: parse JWT token if API call fails to get user info' for better code documentation.
| // Fallback: parse JWT | |
| // Fallback: parse JWT token if API call fails to get user info |
| try { | ||
| const res = await apiService.createVsum({ name: trimmedName, description: description.trim() || undefined }); | ||
| onSuccess?.(res.data); | ||
| showSuccess('Vsum successfully created'); |
There was a problem hiding this comment.
The success message should be 'vSUM successfully created' to match the consistent capitalization used throughout the codebase (as seen in the modal title and other references).
| showSuccess('Vsum successfully created'); | |
| showSuccess('vSUM successfully created'); |
summery of your works
@TsotneMikadze