Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for associating timing and transition rules with visits/encounters in the Schedule of Activities (SoA) system. The changes enable users to specify when a visit is scheduled and what rules govern the transitions into and out of that visit.
Key Changes:
- Added database query and UI support for timing, transition start rule, and transition end rule associations with visits
- Implemented USDM JSON generation logic to include timing and transition rule data in encounter output
- Created three new form endpoints for setting timing and transition rules via the web UI
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/usdm/generate_encounters.py | Added helper functions to retrieve timing and transition rule data from database, updated encounter generation to include scheduledAt and transition rules |
| src/soa_builder/web/templates/edit.html | Added dropdown selectors for timing, transition start rule, and transition end rule selection in the visit editing interface |
| src/soa_builder/web/app.py | Extended visit data fetching to include new fields, added three new POST endpoints for setting timing and transition rules, and provided timing/transition rule data to edit template |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/usdm/generate_encounters.py
Outdated
| soa_id, transition_start_rule_uid | ||
| ) | ||
|
|
||
| transition_end_rule_obj = _get_transition_start_rule( |
There was a problem hiding this comment.
The function called should be _get_transition_end_rule instead of _get_transition_start_rule when retrieving the transition end rule object.
| transition_end_rule_obj = _get_transition_start_rule( | |
| transition_end_rule_obj = _get_transition_end_rule( |
| def _get_transition_start_rule( | ||
| soa_id: int, transition_rule_uid: Optional[str] | ||
| ) -> Optional[Dict[str, Any]]: | ||
| if not transition_rule_uid: | ||
| return None | ||
| conn = _connect() | ||
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT tr.name, tr.label, tr.description, tr.text FROM transition_rule tr WHERE soa_id=? AND transition_rule_uid=?", | ||
| (soa_id, transition_rule_uid), | ||
| ) | ||
| row = cur.fetchone() | ||
| conn.close() | ||
| if not row: | ||
| return None | ||
| return { | ||
| "id": transition_rule_uid, | ||
| "extensionAttributes": [], | ||
| "name": row[0] or None, | ||
| "label": row[1] or None, | ||
| "description": row[2] or None, | ||
| "text": row[3] or None, | ||
| "instanceType": "TransitionRule", | ||
| } | ||
|
|
||
|
|
||
| def _get_transition_end_rule( | ||
| soa_id: int, transition_rule_uid: Optional[str] | ||
| ) -> Optional[Dict[str, Any]]: | ||
| if not transition_rule_uid: | ||
| return None | ||
| conn = _connect() | ||
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT tr.name, tr.label, tr.description, tr.text FROM transition_rule tr WHERE soa_id=? AND transition_rule_uid=?", | ||
| (soa_id, transition_rule_uid), | ||
| ) | ||
| row = cur.fetchone() | ||
| conn.close() | ||
| if not row: | ||
| return None | ||
| return { | ||
| "id": transition_rule_uid, | ||
| "extensionAttributes": [], | ||
| "name": row[0] or None, | ||
| "label": row[1] or None, | ||
| "description": row[2] or None, | ||
| "text": row[3] or None, | ||
| "instanceType": "TransitionRule", | ||
| } |
There was a problem hiding this comment.
The functions _get_transition_start_rule and _get_transition_end_rule contain identical implementation. Consider consolidating them into a single function _get_transition_rule to reduce code duplication and improve maintainability.
| """Form handler for associating a Timing with a Visit/Encounter""" | ||
| if not soa_exists(soa_id): | ||
| raise HTTPException(404, "SOA not found") | ||
| # Determing timing name |
There was a problem hiding this comment.
Corrected spelling of 'Determing' to 'Determining'.
| # Determing timing name | |
| # Determining timing name |
| (parsed_timing, visit_id), | ||
| ) | ||
| conn.commit() | ||
| # Fecth after record audit |
There was a problem hiding this comment.
Corrected spelling of 'Fecth' to 'Fetch'.
| # Fecth after record audit | |
| # Fetch after record audit |
Added visit timing
Added visit transition start rule
Added visit transition end rule
Added USDM JSON generator for encounters