Conversation
Merge UI edit page changes
There was a problem hiding this comment.
Pull request overview
This PR introduces StudyCell relationships and associated functionality for managing clinical trial workflows. The implementation adds database tables for study cells and transition rules, updates the element audit system to use logical UIDs instead of row IDs, and improves UI feedback and terminology upload workflows.
Key changes:
- Added StudyCell relationships linking Arms, Epochs, and Elements with corresponding audit tables
- Introduced TransitionRule editor with CRUD operations and audit trails
- Refactored element auditing to use
StudyElement_Nidentifiers instead of row IDs - Updated edit.html to include study cells and transition rules sections with HTMX-driven interactions
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_ui_export_buttons.py | Deleted test file for export buttons validation |
| tests/test_element_id_monotonic.py | Added test verifying monotonic element ID generation after deletes |
| tests/test_element_audit_endpoint.py | Added test validating element audit endpoint returns create/update/delete actions |
| src/soa_builder/web/templates/transition_rules_list.html | New template for displaying and editing transition rules with inline forms |
| src/soa_builder/web/templates/protocol_terminology.html | Updated upload form layout and moved it above search filters |
| src/soa_builder/web/templates/element_audit_section.html | New collapsible section template for displaying element audit history |
| src/soa_builder/web/templates/edit.html | Added study cells and transition rules sections with HTMX form handling |
| src/soa_builder/web/templates/ddf_terminology.html | Updated upload form layout to match protocol terminology styling |
| src/soa_builder/web/routers/elements.py | Enhanced element operations to use monotonic StudyElement_N identifiers |
| src/soa_builder/web/migrate_database.py | Added migration for element_audit columns (before_json/after_json) |
| src/soa_builder/web/initialize_database.py | Created study_cell, transition_rule, and related audit tables |
| src/soa_builder/web/audit.py | Added study cell audit recording function with defensive table creation |
| src/soa_builder/web/app.py | Major refactor: added study cell and transition rule CRUD endpoints, refactored element audit to use UIDs |
Comments suppressed due to low confidence (1)
src/soa_builder/web/app.py:1
- The
else ifstatement is unnecessary here since the previous condition already handles the case whensaved === '0'. The default behavior when no saved state exists should be to leave the element's default state unchanged, not to explicitly set it totrue. Consider removing this line or adding a comment explaining why explicitly setting to true is needed.
from __future__ import annotations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/soa_builder/web/app.py
Outdated
| return f"StudyCell_{max_n + 1}" | ||
|
|
||
|
|
||
| def _next_element_identifier(soa_id: int) -> str: |
There was a problem hiding this comment.
The function _next_element_identifier is duplicated in this file. It appears on lines 4564-4602 and was also defined in src/soa_builder/web/routers/elements.py (lines 24-62). This duplication should be eliminated by importing the function from the router module or placing it in a shared utility module.
Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/soa_builder/web/app.py
Outdated
| if request.headers.get("HX-Request") == "true": | ||
| conn_ea = _connect() | ||
| cur_ea = conn_ea.cursor() | ||
| cur_ea.execute("PRAGMA table_info(element_audit)") | ||
| cols = [row[1] for row in cur_ea.fetchall()] | ||
| want = [ | ||
| "id", | ||
| "element_id", | ||
| "action", | ||
| "before_json", | ||
| "after_json", | ||
| "performed_at", | ||
| ] | ||
| available = [c for c in want if c in cols] | ||
| element_audits = [] | ||
| if available: | ||
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | ||
| cur_ea.execute(select_sql, (soa_id,)) | ||
| for r in cur_ea.fetchall(): | ||
| item = {} | ||
| for i, c in enumerate(available): | ||
| item[c] = r[i] | ||
| for k in want: | ||
| item.setdefault(k, None) | ||
| element_audits.append(item) | ||
| conn_ea.close() | ||
| html = templates.get_template("element_audit_section.html").render( | ||
| request=request, soa_id=soa_id, element_audits=element_audits | ||
| ) | ||
| return HTMLResponse(html) |
There was a problem hiding this comment.
This HTMX response block appears in the ui_refresh_concepts function but is unrelated to concept refreshing. The code should be removed or moved to the appropriate endpoint handler.
| if request.headers.get("HX-Request") == "true": | |
| conn_ea = _connect() | |
| cur_ea = conn_ea.cursor() | |
| cur_ea.execute("PRAGMA table_info(element_audit)") | |
| cols = [row[1] for row in cur_ea.fetchall()] | |
| want = [ | |
| "id", | |
| "element_id", | |
| "action", | |
| "before_json", | |
| "after_json", | |
| "performed_at", | |
| ] | |
| available = [c for c in want if c in cols] | |
| element_audits = [] | |
| if available: | |
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | |
| cur_ea.execute(select_sql, (soa_id,)) | |
| for r in cur_ea.fetchall(): | |
| item = {} | |
| for i, c in enumerate(available): | |
| item[c] = r[i] | |
| for k in want: | |
| item.setdefault(k, None) | |
| element_audits.append(item) | |
| conn_ea.close() | |
| html = templates.get_template("element_audit_section.html").render( | |
| request=request, soa_id=soa_id, element_audits=element_audits | |
| ) | |
| return HTMLResponse(html) | |
| # (Removed: unrelated Element Audit HTMX response block) |
src/soa_builder/web/app.py
Outdated
| if request.headers.get("HX-Request") == "true": | ||
| conn_ea = _connect() | ||
| cur_ea = conn_ea.cursor() | ||
| cur_ea.execute("PRAGMA table_info(element_audit)") | ||
| cols = [row[1] for row in cur_ea.fetchall()] | ||
| want = [ | ||
| "id", | ||
| "element_id", | ||
| "action", | ||
| "before_json", | ||
| "after_json", | ||
| "performed_at", | ||
| ] | ||
| available = [c for c in want if c in cols] | ||
| element_audits = [] | ||
| if available: | ||
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | ||
| cur_ea.execute(select_sql, (soa_id,)) | ||
| for r in cur_ea.fetchall(): | ||
| item = {} | ||
| for i, c in enumerate(available): | ||
| item[c] = r[i] | ||
| for k in want: | ||
| item.setdefault(k, None) | ||
| element_audits.append(item) | ||
| conn_ea.close() | ||
| html = templates.get_template("element_audit_section.html").render( | ||
| request=request, soa_id=soa_id, element_audits=element_audits | ||
| ) | ||
| return HTMLResponse(html) |
There was a problem hiding this comment.
This HTMX response block appears in the ui_add_activity function but returns element audit HTML instead of activity-related content. The code should be removed or moved to the appropriate endpoint handler.
| if request.headers.get("HX-Request") == "true": | |
| conn_ea = _connect() | |
| cur_ea = conn_ea.cursor() | |
| cur_ea.execute("PRAGMA table_info(element_audit)") | |
| cols = [row[1] for row in cur_ea.fetchall()] | |
| want = [ | |
| "id", | |
| "element_id", | |
| "action", | |
| "before_json", | |
| "after_json", | |
| "performed_at", | |
| ] | |
| available = [c for c in want if c in cols] | |
| element_audits = [] | |
| if available: | |
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | |
| cur_ea.execute(select_sql, (soa_id,)) | |
| for r in cur_ea.fetchall(): | |
| item = {} | |
| for i, c in enumerate(available): | |
| item[c] = r[i] | |
| for k in want: | |
| item.setdefault(k, None) | |
| element_audits.append(item) | |
| conn_ea.close() | |
| html = templates.get_template("element_audit_section.html").render( | |
| request=request, soa_id=soa_id, element_audits=element_audits | |
| ) | |
| return HTMLResponse(html) |
src/soa_builder/web/app.py
Outdated
| if request.headers.get("HX-Request") == "true": | ||
| conn_ea = _connect() | ||
| cur_ea = conn_ea.cursor() | ||
| cur_ea.execute("PRAGMA table_info(element_audit)") | ||
| cols = [row[1] for row in cur_ea.fetchall()] | ||
| want = [ | ||
| "id", | ||
| "element_id", | ||
| "action", | ||
| "before_json", | ||
| "after_json", | ||
| "performed_at", | ||
| ] | ||
| available = [c for c in want if c in cols] | ||
| element_audits = [] | ||
| if available: | ||
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | ||
| cur_ea.execute(select_sql, (soa_id,)) | ||
| for r in cur_ea.fetchall(): | ||
| item = {} | ||
| for i, c in enumerate(available): | ||
| item[c] = r[i] | ||
| for k in want: | ||
| item.setdefault(k, None) | ||
| element_audits.append(item) | ||
| conn_ea.close() | ||
| html = templates.get_template("element_audit_section.html").render( | ||
| request=request, soa_id=soa_id, element_audits=element_audits | ||
| ) | ||
| return HTMLResponse(html) |
There was a problem hiding this comment.
This HTMX response block appears in the ui_update_meta function but returns element audit HTML instead of metadata-related content. The code should be removed or moved to the appropriate endpoint handler.
| if request.headers.get("HX-Request") == "true": | |
| conn_ea = _connect() | |
| cur_ea = conn_ea.cursor() | |
| cur_ea.execute("PRAGMA table_info(element_audit)") | |
| cols = [row[1] for row in cur_ea.fetchall()] | |
| want = [ | |
| "id", | |
| "element_id", | |
| "action", | |
| "before_json", | |
| "after_json", | |
| "performed_at", | |
| ] | |
| available = [c for c in want if c in cols] | |
| element_audits = [] | |
| if available: | |
| select_sql = f"SELECT {', '.join(available)} FROM element_audit WHERE soa_id=? ORDER BY id DESC" | |
| cur_ea.execute(select_sql, (soa_id,)) | |
| for r in cur_ea.fetchall(): | |
| item = {} | |
| for i, c in enumerate(available): | |
| item[c] = r[i] | |
| for k in want: | |
| item.setdefault(k, None) | |
| element_audits.append(item) | |
| conn_ea.close() | |
| html = templates.get_template("element_audit_section.html").render( | |
| request=request, soa_id=soa_id, element_audits=element_audits | |
| ) | |
| return HTMLResponse(html) |
Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <input name="text" placeholder="Transition Rule text" required style="flex:2;min-width:160px;" /> | ||
| <button style="background:#1976d2;color:#fff;border:none;padding:4px 10px;border-radius:4px;cursor:pointer;">Add Transition Rule</button> | ||
| </form> | ||
| </details> |
There was a problem hiding this comment.
Trailing whitespace at end of line should be removed.
| </details> | |
| </details> |
src/soa_builder/web/app.py
Outdated
| # HTMX: return refreshed Element Audit section when requested | ||
| if request.headers.get("HX-Request") == "true": |
There was a problem hiding this comment.
This HTMX response block (lines 1744-1773) appears duplicated in three separate endpoint functions. Consider extracting this logic into a helper function to reduce code duplication.
src/soa_builder/web/app.py
Outdated
| # Fallback: plain form POST non-htmx redirect via script | ||
| # HTMX: return refreshed Element Audit section when requested |
There was a problem hiding this comment.
Comment on line 1742 indicates a fallback for non-HTMX requests, but the code that follows (lines 1744-1773) handles HTMX requests. The comment is misleading and should be updated or removed.
| # Fallback: plain form POST non-htmx redirect via script | |
| # HTMX: return refreshed Element Audit section when requested |
Removed trailing whitespace Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Use of the 'placeholder' attribute instead of 'value' for hint text Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added space after 'if' keyword as per JavaScript conventions Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/soa_builder/web/app.py
Outdated
| # HTMX: return refreshed Element Audit section when requested | ||
| if request.headers.get("HX-Request") == "true": |
There was a problem hiding this comment.
Duplicated code block for fetching element audits appears in three locations (lines 1744-1773, 2944-2973, 3062-3091). This logic should be extracted into a helper function to reduce duplication and improve maintainability.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/soa_builder/web/app.py
Outdated
| # HTMX: return refreshed Element Audit section when requested | ||
| if request.headers.get("HX-Request") == "true": |
There was a problem hiding this comment.
This HTMX response block appears in the ui_refresh_concepts function but the function's documented purpose is to refresh concepts, not element audits. This code seems misplaced and creates duplicated logic that also appears in ui_add_activity (lines 2944-2973) and ui_update_meta (lines 3062-3091). Consider extracting this into a helper function.
pendingintent
added
bug
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/soa_builder/web/app.py
Outdated
| "SELECT id, name, label, description, testrl, teenrl, order_index, NULL as element_id FROM element WHERE id=? AND soa_id=?", | ||
| (element_id, soa_id), | ||
| ) |
There was a problem hiding this comment.
The NULL as element_id cast is unnecessary in the else branch. Since the function already checks for the column's presence, you could simplify by selecting only the available columns and handling the missing element_id after fetching.
Remove redundant conditional check Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
_list_study_cells now returns epoch_name Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <fieldset style="display:inline-block; padding:0.5em 1em;"> | ||
| <legend>Upload Excel Workbook (.xls/.xlsx)</legend> | ||
| <label style="margin-left:1em;">File: <input type="file" name="file" accept=".xls,.xlsx" required></label> | ||
| <label>Sheet Name: <input type="text" name="sheet_name" value="Enter name of worksheet..." size="28"></label> |
There was a problem hiding this comment.
The 'value' attribute should not contain placeholder text. Use the 'placeholder' attribute instead to avoid submitting 'Enter name of worksheet...' as actual data.
| <label>Sheet Name: <input type="text" name="sheet_name" value="Enter name of worksheet..." size="28"></label> | |
| <label>Sheet Name: <input type="text" name="sheet_name" placeholder="Enter name of worksheet..." size="28"></label> |
src/soa_builder/web/app.py
Outdated
|
|
||
| Duplicate prevention enforced on (soa_id, arm_uid, epoch_uid, element_uid). | ||
| """ | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
The function '_soa_exists' is called but was removed from this file. It should be 'soa_exists' (without underscore prefix) to match the imported utility function.
src/soa_builder/web/app.py
Outdated
|
|
||
| Duplicate prevention enforced; if update causes a duplicate, no change is applied. | ||
| """ | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
Multiple calls to '_soa_exists' which should be 'soa_exists' to match the imported utility function from utils module.
src/soa_builder/web/app.py
Outdated
| @app.post("/ui/soa/{soa_id}/delete_study_cell", response_class=HTMLResponse) | ||
| def ui_delete_study_cell(request: Request, soa_id: int, study_cell_id: int = Form(...)): | ||
| """Delete a Study Cell by id.""" | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
Multiple calls to '_soa_exists' which should be 'soa_exists' to match the imported utility function from utils module.
src/soa_builder/web/app.py
Outdated
| text: str = Form(...), | ||
| ): | ||
| """Form handler to add a Transition Rule.""" | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
Multiple calls to '_soa_exists' which should be 'soa_exists' to match the imported utility function from utils module.
src/soa_builder/web/app.py
Outdated
| text: Optional[str] = Form(None), | ||
| ): | ||
| """Form handler to update an existing Transition Rule.""" | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
Multiple calls to '_soa_exists' which should be 'soa_exists' to match the imported utility function from utils module.
| if not _soa_exists(soa_id): | |
| if not soa_exists(soa_id): |
src/soa_builder/web/app.py
Outdated
| request: Request, soa_id: int, transition_rule_uid: str = Form(...) | ||
| ): | ||
| """Form handler to delete a transition rule""" | ||
| if not _soa_exists(soa_id): |
There was a problem hiding this comment.
Multiple calls to '_soa_exists' which should be 'soa_exists' to match the imported utility function from utils module.
…id and handle missing element_id
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT id,name,label,description,order_index,COALESCE(type,''),COALESCE(data_origin_type,'') FROM arm WHERE soa_id=? ORDER BY order_index", | ||
| "SELECT id,name,label,description,order_index,arm_uid,COALESCE(type,''),COALESCE(data_origin_type,'') FROM arm WHERE soa_id=? ORDER BY order_index", |
There was a problem hiding this comment.
The query selects 8 columns but the dictionary construction uses hardcoded indices that may not match. After adding arm_uid at position 5, type shifts to position 6 and data_origin_type to position 7, which matches the code. However, this is fragile. Consider using named columns or column position constants.
| if not first or first[1] is None: | ||
| return | ||
| assert first[1].startswith(PREFIX) | ||
| n1 = int(first[1][len(PREFIX) :]) |
There was a problem hiding this comment.
The slice operation [len(PREFIX) :] can be simplified using removeprefix() for better readability: first[1].removeprefix(PREFIX).
| assert last is not None | ||
| assert last[1] is not None | ||
| assert last[1].startswith(PREFIX) | ||
| n2 = int(last[1][len(PREFIX) :]) |
There was a problem hiding this comment.
The slice operation [len(PREFIX) :] can be simplified using removeprefix() for better readability: last[1].removeprefix(PREFIX).
| n2 = int(last[1][len(PREFIX) :]) | |
| n2 = int(last[1].removeprefix(PREFIX)) |
…from utils and removed the local _soa_exists helper
sheet name field is now marked as required Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use of 'placeholder' attribute instead of 'value' for hint text Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _get_element_uid(soa_id: int, row_id: int) -> Optional[str]: | ||
| """Return element.element_id (StudyElement_N) for row id if column exists, else None.""" | ||
| try: | ||
| conn = _connect() | ||
| cur = conn.cursor() | ||
| cur.execute("PRAGMA table_info(element)") | ||
| cols = {r[1] for r in cur.fetchall()} | ||
| if "element_id" not in cols: | ||
| conn.close() | ||
| return None | ||
| cur.execute( | ||
| "SELECT element_id FROM element WHERE id=? AND soa_id=?", | ||
| (row_id, soa_id), | ||
| ) | ||
| r = cur.fetchone() | ||
| conn.close() | ||
| return r[0] if r else None | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
The function silently swallows all exceptions. Consider logging exceptions to aid debugging, especially for database errors that might indicate schema issues or connection problems.
| } | ||
| _record_element_audit(soa_id, "create", eid, before=None, after=el) | ||
| # Audit with logical StudyElement_N when available | ||
| _record_element_audit(soa_id, "create", element_identifier, before=None, after=el) |
There was a problem hiding this comment.
The variable element_identifier may be None when the element_id column doesn't exist, but _record_element_audit expects a string or int for the element_id parameter. This could cause issues in the audit record.
| _record_element_audit(soa_id, "create", element_identifier, before=None, after=el) | |
| _record_element_audit(soa_id, "create", element_identifier if element_identifier is not None else eid, before=None, after=el) |
| cur = conn.cursor() | ||
| cur.execute( | ||
| "SELECT id,name,label,description,order_index,COALESCE(type,''),COALESCE(data_origin_type,'') FROM arm WHERE soa_id=? ORDER BY order_index", | ||
| "SELECT id,name,label,description,order_index,arm_uid,COALESCE(type,''),COALESCE(data_origin_type,'') FROM arm WHERE soa_id=? ORDER BY order_index", |
There was a problem hiding this comment.
The SELECT statement now retrieves 8 columns but the dictionary construction below still maps to the old 7-column structure. Column indices are now shifted - arm_uid is at index 5, type at 6, and data_origin_type at 7.
| soa_id, | ||
| "create", | ||
| eid, | ||
| element_identifier, |
There was a problem hiding this comment.
When element_id column doesn't exist (element_identifier is None), passing None to _record_element_audit may cause inconsistent audit records. Consider passing the row id as a fallback.
| element_identifier, | |
| element_identifier if element_identifier is not None else eid, |
| soa_id, | ||
| "update", | ||
| element_id, | ||
| element_uid_for_audit, |
There was a problem hiding this comment.
When the PRAGMA check fails or element_id column doesn't exist, element_uid_for_audit could be None. This should be handled by either falling back to the row id or ensuring the audit function accepts None gracefully.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "_record_epoch_audit failed soa_id=%s epoch_id=%s action=%s: %s", | ||
| soa_id, | ||
| epoch_id, | ||
| action, | ||
| e, |
There was a problem hiding this comment.
Using logger.exception() already includes the exception information, so the %s placeholder and e argument at the end are redundant. Remove the final %s from the format string and e from the arguments.
| "_record_epoch_audit failed soa_id=%s epoch_id=%s action=%s: %s", | |
| soa_id, | |
| epoch_id, | |
| action, | |
| e, | |
| "_record_epoch_audit failed soa_id=%s epoch_id=%s action=%s", | |
| soa_id, | |
| epoch_id, | |
| action, |
| "get_freeze JSON decode failed soa_id=%s freeze_id=%s: %s", | ||
| soa_id, | ||
| freeze_id, | ||
| e, |
There was a problem hiding this comment.
Using logger.exception() already includes the exception information, so the %s placeholder and e argument at the end are redundant. Remove the final %s from the format string and e from the arguments.
| "get_freeze JSON decode failed soa_id=%s freeze_id=%s: %s", | |
| soa_id, | |
| freeze_id, | |
| e, | |
| "get_freeze JSON decode failed soa_id=%s freeze_id=%s", | |
| soa_id, | |
| freeze_id, |
| logger.exception( | ||
| "_next_element_identifier scan elements failed for soa_id=%s: %s", | ||
| soa_id, | ||
| e, | ||
| ) |
There was a problem hiding this comment.
Using logger.exception() already includes the exception information, so the %s placeholder and e argument at the end are redundant. Remove the final %s from the format string and e from the arguments.
| logger.exception( | ||
| "_next_element_identifier scan element_audit failed for soa_id=%s: %s", | ||
| soa_id, | ||
| e, | ||
| ) |
There was a problem hiding this comment.
Using logger.exception() already includes the exception information, so the %s placeholder and e argument at the end are redundant. Remove the final %s from the format string and e from the arguments.
| return r[0] if r else None | ||
| except Exception as e: | ||
| logger.exception( | ||
| "_get_element_uid failed for soa_id=%s row_id=%s: %s", soa_id, row_id, e |
There was a problem hiding this comment.
Using logger.exception() already includes the exception information, so the %s placeholder and e argument at the end are redundant. Remove the final %s from the format string and e from the arguments.
| "_get_element_uid failed for soa_id=%s row_id=%s: %s", soa_id, row_id, e | |
| "_get_element_uid failed for soa_id=%s row_id=%s", soa_id, row_id |
Major changes: