Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the visit UI in the edit page to support inline editing of visit labels and descriptions. The changes centralize schema definitions and add a new form handler for updating visit details.
- Adds inline form inputs for visit label and description fields with a Save button
- Moves reusable schema classes from
app.pytoschemas.pyfor better organization - Implements
ui_update_visitendpoint to handle visit updates from the edit page
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/soa_builder/web/templates/edit.html | Adds inline editing form for visit label/description and updates CSS to align visit actions |
| src/soa_builder/web/schemas.py | Moves schema definitions from app.py and adds new schema classes for various operations |
| src/soa_builder/web/app.py | Removes inline schema definitions, imports from schemas.py, adds ui_update_visit handler, and updates visit query to include description field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception: | ||
| # Let redirect proceed; detailed errors will appear in API logs | ||
| pass |
There was a problem hiding this comment.
The bare except Exception silently swallows all errors without providing any feedback to the user. Consider logging the error or displaying a user-friendly error message before redirecting.
| name: Optional[str] = Form(None), | ||
| label: Optional[str] = Form(None), | ||
| description: Optional[str] = Form(None), | ||
| ): | ||
| """Form handler to update a Visit's mutable fields (name/label/description).""" | ||
| # Build payload with provided fields; blanks should clear values | ||
| payload = VisitUpdate( | ||
| name=name, |
There was a problem hiding this comment.
The function accepts a name parameter but the inline form in edit.html doesn't provide a name input field, so name will always be None. This creates an inconsistency where the API could unintentionally clear the visit name. Consider removing the name parameter from this handler since it's not used in the UI.
| name: Optional[str] = Form(None), | |
| label: Optional[str] = Form(None), | |
| description: Optional[str] = Form(None), | |
| ): | |
| """Form handler to update a Visit's mutable fields (name/label/description).""" | |
| # Build payload with provided fields; blanks should clear values | |
| payload = VisitUpdate( | |
| name=name, | |
| label: Optional[str] = Form(None), | |
| description: Optional[str] = Form(None), | |
| ): | |
| """Form handler to update a Visit's mutable fields (label/description).""" | |
| # Build payload with provided fields; blanks should clear values | |
| payload = VisitUpdate( |
spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception: | ||
| # Let redirect proceed; detailed errors will appear in API logs | ||
| pass |
There was a problem hiding this comment.
Catching all exceptions with a bare except Exception and silently passing provides no user feedback when updates fail. Consider logging the exception or providing user-facing error feedback instead of silently redirecting.
| except Exception: | |
| # Let redirect proceed; detailed errors will appear in API logs | |
| pass | |
| except Exception as exc: | |
| # Log the exception but still redirect to the edit page | |
| logging.exception( | |
| "Failed to update visit %s for SOA %s via UI handler: %s", | |
| visit_id, | |
| soa_id, | |
| exc, | |
| ) |
Updated visit-section in edit.html to allow inline editing of label ans description
Updated to align fields