Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to associate elements with transition rules through dropdown selectors and introduces a USDM JSON export generator for elements. The changes update the UI to use select dropdowns instead of text inputs for transition rules, refactor element handling to use API routes consistently, and add a new generator module for exporting element data in USDM format.
Key changes:
- Added select dropdowns in the UI for associating elements with transition start/end rules
- Created new
generate_elements.pymodule to export element USDM JSON - Refactored element CRUD operations to route through API endpoints for consistency
- Removed test for monotonic element ID behavior after deletion
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_element_id_monotonic.py | Removed test case for monotonic element ID incrementing after deletion |
| src/usdm/generate_encounters.py | Fixed typo in transitionEndRule field marker (changed from " to ?) |
| src/usdm/generate_elements.py | New generator module for building and exporting USDM Elements JSON |
| src/soa_builder/web/templates/edit.html | Updated UI with dropdown selectors for element transition rules and refactored form structure |
| src/soa_builder/web/routers/elements.py | Updated element queries, added validation for transition rules, and deprecated reorder endpoint |
| src/soa_builder/web/app.py | Added new UI endpoints for element transition rule association and refactored element handlers to use API routes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button style="background:#1976d2;color:#fff;border:none;padding:4px 10px;border-radius:4px;cursor:pointer;">Add Element</button> | ||
| <form method="post" action="/ui/soa/{{ soa_id }}/add_element" style="margin-top:6px;"> | ||
| <input name="name" placeholder="Element Name" required /> | ||
| <input name="label" placeholder="Label" placeholder="Label (optional)" /> |
There was a problem hiding this comment.
The input element has duplicate placeholder attributes. Remove one of them.
| <input name="label" placeholder="Label" placeholder="Label (optional)" /> | |
| <input name="label" placeholder="Label (optional)" /> |
| conn.close() | ||
| raise HTTPException(400, "Invalid transition_rule id for this SOA") | ||
|
|
||
| if payload.testrl is not None: |
There was a problem hiding this comment.
The condition checks payload.testrl but the query uses payload.teenrl at line 333. This should check payload.teenrl is not None instead.
| if payload.testrl is not None: | |
| if payload.teenrl is not None: |
| conn = _connect() | ||
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT name,label,description,element_id,testrl,teenrl FROM element WHERE soa_id=? ORDER BY order_index", |
There was a problem hiding this comment.
The code constructs id_by_index at line 101 but never uses it. Consider removing this unused dictionary.
src/soa_builder/web/app.py
Outdated
| teenrl=teenrl_uid, | ||
| ) | ||
|
|
||
| # Create the element wia the API helper to ensure audits and ordering |
There was a problem hiding this comment.
Corrected spelling of 'wia' to 'via'.
| # Create the element wia the API helper to ensure audits and ordering | |
| # Create the element via the API helper to ensure audits and ordering |
src/soa_builder/web/app.py
Outdated
| element_transition_end_rule_uid: str = Form(...), | ||
| ): | ||
| """Form handler for associating a Transition End Rule with an Element""" | ||
| if not soa_exists: |
There was a problem hiding this comment.
Missing function call parentheses. Should be if not soa_exists(soa_id): to actually invoke the function.
| if not soa_exists: | |
| if not soa_exists(soa_id): |
src/soa_builder/web/app.py
Outdated
| pass | ||
|
|
||
| return HTMLResponse( | ||
| f"<script>window.location='/ui/soa{int(soa_id)}/edit';</script>" |
There was a problem hiding this comment.
Missing forward slash before {int(soa_id)}. Should be /ui/soa/{int(soa_id)}/edit to match the URL pattern.
| f"<script>window.location='/ui/soa{int(soa_id)}/edit';</script>" | |
| f"<script>window.location='/ui/soa/{int(soa_id)}/edit';</script>" |
src/soa_builder/web/app.py
Outdated
| "update", | ||
| element_id, | ||
| before=before, | ||
| after={**after, "updated_field": updated_fields}, |
There was a problem hiding this comment.
The key should be updated_fields (plural) to match the pattern used elsewhere in the code (e.g., line 4665). Change updated_field to updated_fields.
| after={**after, "updated_field": updated_fields}, | |
| after={**after, "updated_fields": updated_fields}, |
| @DeprecationWarning | ||
| @router.post("/elements/reorder", response_class=JSONResponse) |
There was a problem hiding this comment.
Using @DeprecationWarning as a decorator is incorrect. To mark a function as deprecated, use @deprecated from the warnings module or add a deprecation docstring. The current usage has no effect.
Added select dropdowns to associate element with transitionStartRule and transitionEndRule
Added generator to export element USDM JSON