carverauto/serviceradar

Fork 0

2290 bug devices page crashing #2667

Merged

mfreeman451 merged 4 commits from refs/pull/2667/head into staging

2026-01-14 08:51:37 +00:00

mfreeman451 commented

2026-01-14 08:46:46 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2296
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2296
Original created: 2026-01-14T08:46:46Z
Original updated: 2026-01-14T08:51:48Z
Original head: carverauto/serviceradar:2290-bug-devices-page-crashing
Original base: staging
Original merged: 2026-01-14T08:51:37Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Bug fix, Tests

Description

Fix devices page crash when sysmon profile data missing or malformed
Add nil-safe handling for profile name extraction with fallback labels
Display "Unassigned" label instead of rendering nothing or crashing
Add regression test for devices with missing sysmon data

Diagram Walkthrough

flowchart LR
  A["Sysmon Profile Data"] --> B["sysmon_profile_label Helper"]
  B --> C["Nil-Safe Extraction"]
  C --> D["Profile Name or Nil"]
  D --> E["sysmon_profile_badge Component"]
  E --> F["Direct/Default/Missing Label"]
  F --> G["Rendered UI with Fallback"]

File Walkthrough

Relevant files

Bug fix

index.ex

Add nil-safe sysmon profile extraction with fallback labels

web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex

Extract profile name extraction logic into new sysmon_profile_label/1
helper function with nil-safe handling
Handle both atom and string keys when accessing profile name field
Trim whitespace and treat empty strings as nil
Replace boolean is_direct flag with semantic :direct, :default,
:missing source atoms
Always render badge div and display "Unassigned" fallback label when
profile data is missing
Remove conditional rendering that skipped the entire badge when label
was nil

+26/-11

Tests

device_live_test.exs

Add regression test for missing sysmon data

web-ng/test/serviceradar_web_ng_web/live/device_live_test.exs

Add new test case for devices with missing sysmon profile data
Insert test device without sysmon profile assignment
Assert that devices list renders with "Unassigned" fallback label
Verify page renders without crashing when profile data is absent

+20/-0

Documentation

proposal.md `Document sysmon badge crash fix proposal` openspec/changes/fix-devices-page-sysmon-badge/proposal.md Document root cause: page crashes when sysmon profile data missing or malformed Outline solution: nil-safe rendering with fallback labels List affected components: DeviceLive devices list sysmon badge helper Specify impact scope: build-web-ui spec	+13/-0
spec.md `Add spec requirement for missing sysmon data handling` openspec/changes/fix-devices-page-sysmon-badge/specs/build-web-ui/spec.md Add requirement for rendering devices list even with missing sysmon data Define scenario for missing sysmon profile data with "Unassigned" fallback Specify that page must render without error when profile/status fields absent Document expected UI behavior for direct, default, and missing profile states	+31/-0
tasks.md `Track implementation task completion status` openspec/changes/fix-devices-page-sysmon-badge/tasks.md Mark implementation tasks 1.1-1.3 as complete Task 1.1: Identify and add nil-safe handling for profile/status fields Task 1.2: Render fallback label when sysmon data missing Task 1.3: Add devices list tests for missing sysmon data Leave task 1.4 (staging verification) pending	+5/-0

Imported from GitHub pull request. Original GitHub pull request: #2296 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2296 Original created: 2026-01-14T08:46:46Z Original updated: 2026-01-14T08:51:48Z Original head: carverauto/serviceradar:2290-bug-devices-page-crashing Original base: staging Original merged: 2026-01-14T08:51:37Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Fix devices page crash when sysmon profile data missing or malformed - Add nil-safe handling for profile name extraction with fallback labels - Display "Unassigned" label instead of rendering nothing or crashing - Add regression test for devices with missing sysmon data ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Sysmon Profile Data"] --> B["sysmon_profile_label Helper"] B --> C["Nil-Safe Extraction"] C --> D["Profile Name or Nil"] D --> E["sysmon_profile_badge Component"] E --> F["Direct/Default/Missing Label"] F --> G["Rendered UI with Fallback"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>index.ex</strong><dd><code>Add nil-safe sysmon profile extraction with fallback labels</code></dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex <ul><li>Extract profile name extraction logic into new <code>sysmon_profile_label/1</code> <br>helper function with nil-safe handling<br> <li> Handle both atom and string keys when accessing profile name field<br> <li> Trim whitespace and treat empty strings as nil<br> <li> Replace boolean <code>is_direct</code> flag with semantic <code>:direct</code>, <code>:default</code>, <br><code>:missing</code> source atoms<br> <li> Always render badge div and display "Unassigned" fallback label when <br>profile data is missing<br> <li> Remove conditional rendering that skipped the entire badge when label <br>was nil</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2296/files#diff-261a01f4876e5984e1d9e9b38a3540675dfb0272abc30e6bdb2a4fa610353cc7">+26/-11</a>  </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>device_live_test.exs</strong><dd><code>Add regression test for missing sysmon data</code>                            </dd></summary> <hr> web-ng/test/serviceradar_web_ng_web/live/device_live_test.exs <ul><li>Add new test case for devices with missing sysmon profile data<br> <li> Insert test device without sysmon profile assignment<br> <li> Assert that devices list renders with "Unassigned" fallback label<br> <li> Verify page renders without crashing when profile data is absent</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2296/files#diff-8130c6c6897063cc952a326ac4e9d26373cfabd3413d181574c0e2427aba533e">+20/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>proposal.md</strong><dd><code>Document sysmon badge crash fix proposal</code>                                  </dd></summary> <hr> openspec/changes/fix-devices-page-sysmon-badge/proposal.md <ul><li>Document root cause: page crashes when sysmon profile data missing or <br>malformed<br> <li> Outline solution: nil-safe rendering with fallback labels<br> <li> List affected components: DeviceLive devices list sysmon badge helper<br> <li> Specify impact scope: build-web-ui spec</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2296/files#diff-814fc7dff0667ab2e42c95527da8c2bbb6222299ae5f3342768dc83630785ee6">+13/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>spec.md</strong><dd><code>Add spec requirement for missing sysmon data handling</code>        </dd></summary> <hr> openspec/changes/fix-devices-page-sysmon-badge/specs/build-web-ui/spec.md <ul><li>Add requirement for rendering devices list even with missing sysmon <br>data<br> <li> Define scenario for missing sysmon profile data with "Unassigned" <br>fallback<br> <li> Specify that page must render without error when profile/status fields <br>absent<br> <li> Document expected UI behavior for direct, default, and missing profile <br>states</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2296/files#diff-d6b20ab7cf5dbb43f32e9346fec19039a622c8bcf08b61b62f50c6b80bdfc8da">+31/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>tasks.md</strong><dd><code>Track implementation task completion status</code>                            </dd></summary> <hr> openspec/changes/fix-devices-page-sysmon-badge/tasks.md <ul><li>Mark implementation tasks 1.1-1.3 as complete<br> <li> Task 1.1: Identify and add nil-safe handling for profile/status fields<br> <li> Task 1.2: Render fallback label when sysmon data missing<br> <li> Task 1.3: Add devices list tests for missing sysmon data<br> <li> Leave task 1.4 (staging verification) pending</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2296/files#diff-15cf98ddd51c6d8c6742eeae6bc1d5ce2f5ae78a2fb3fcc0ed0763cec556d6da">+5/-0</a>      </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-14 08:47:29 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2296#issuecomment-3748455627
Original created: 2026-01-14T08:47:29Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2290

🟢	Fix the `/devices/` page so it no longer crashes with an Internal Server Error when rendering the devices list.
🟢	Ensure the Devices UI/Inventory page renders successfully in the web UI when sysmon profile data is missing or malformed.
⚪	Ensure the Devices UI/Inventory page renders successfully in the web UI (including staging) when sysmon profile data is missing or malformed.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2296#issuecomment-3748455627 Original created: 2026-01-14T08:47:29Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2290>#2290</a></summary> <table width='100%'><tbody> <tr><td rowspan=2>🟢</td> <td>Fix the <code>/devices/</code> page so it no longer crashes with an Internal Server Error when <br>rendering the devices list.<br></td></tr> <tr><td>Ensure the Devices UI/Inventory page renders successfully in the web UI when sysmon <br>profile data is missing or malformed.<br></td></tr> <tr><td rowspan=1>⚪</td> <td>Ensure the Devices UI/Inventory page renders successfully in the web UI (including <br>staging) when sysmon profile data is missing or malformed.<br></td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=6>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-14 08:48:38 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2296#issuecomment-3748459345
Original created: 2026-01-14T08:48:38Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ ~~Add test ID for label span~~ Suggestion Impact: The sysmon profile label was updated to include data-testid="sysmon-profile-label", matching the suggestion. (Additional unrelated refactor to sysmon_profile_label/1 was also included.) code diff: `- <span class={[ + <span data-testid="sysmon-profile-label" class={[ "text-xs truncate max-w-[8rem]", if(@source == :direct, do: "font-medium text-base-content", else: "text-base-content/60") ]}>` Add a `data-testid` attribute to the sysmon profile label's `span` element to create a stable selector for automated tests. web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex [893-898] `-<span class={[ +<span data-testid="sysmon-profile-label" class={[ "text-xs truncate max-w-[8rem]", if(@source == :direct, do: "font-medium text-base-content", else: "text-base-content/60") ]}> {@label} </span>` `[Suggestion processed]` Suggestion importance[1-10]: 6 __ Why: The suggestion improves testability by adding a `data-testid` attribute, which allows for more robust and less brittle UI test selectors.	Low
General	✅ ~~Refactor helper function for clarity~~ Suggestion Impact: The sysmon_profile_label/1 helper was refactored from a case/if structure to the suggested with-based validation flow, returning nil via else when validation fails. code diff: `defp sysmon_profile_label(profile) when is_map(profile) do - case Map.get(profile, :name) \|\| Map.get(profile, "name") do - name when is_binary(name) -> - name = String.trim(name) - if name == "", do: nil, else: name - - _ -> - nil + with name when is_binary(name) <- Map.get(profile, :name) \|\| Map.get(profile, "name"), + trimmed_name = String.trim(name), + true <- trimmed_name != "" do + trimmed_name + else + _ -> nil end` Refactor the `sysmon_profile_label/1` function to use a `with` statement for improved clarity and idiomatic style when handling multi-step validation. web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex [910-919] `defp sysmon_profile_label(profile) when is_map(profile) do - case Map.get(profile, :name) \|\| Map.get(profile, "name") do - name when is_binary(name) -> - name = String.trim(name) - if name == "", do: nil, else: name - - _ -> - nil + with name when is_binary(name) <- Map.get(profile, :name) \|\| Map.get(profile, "name"), + trimmed_name = String.trim(name), + true <- trimmed_name != "" do + trimmed_name + else + _ -> nil end end` `[Suggestion processed]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes using an idiomatic Elixir `with` statement to refactor the function, which improves code readability and maintainability.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2296#issuecomment-3748459345 Original created: 2026-01-14T08:48:38Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>✅ <s>Add test ID for label span</s></summary> ___ <details><summary><b>Suggestion Impact:</b></summary>The sysmon profile label <span> was updated to include data-testid="sysmon-profile-label", matching the suggestion. (Additional unrelated refactor to sysmon_profile_label/1 was also included.) code diff: ```diff - <span class={[ + <span data-testid="sysmon-profile-label" class={[ "text-xs truncate max-w-[8rem]", if(@source == :direct, do: "font-medium text-base-content", else: "text-base-content/60") ]}> ``` </details> ___ **Add a <code>data-testid</code> attribute to the sysmon profile label's <code>span</code> element to create <br>a stable selector for automated tests.** [web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex [893-898]](https://github.com/carverauto/serviceradar/pull/2296/files#diff-261a01f4876e5984e1d9e9b38a3540675dfb0272abc30e6bdb2a4fa610353cc7R893-R898) ```diff -<span class={[ +<span data-testid="sysmon-profile-label" class={[ "text-xs truncate max-w-[8rem]", if(@source == :direct, do: "font-medium text-base-content", else: "text-base-content/60") ]}> {@label} </span> ``` `[Suggestion processed]` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion improves testability by adding a `data-testid` attribute, which allows for more robust and less brittle UI test selectors. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>✅ <s>Refactor helper function for clarity</s></summary> ___ <details><summary><b>Suggestion Impact:</b></summary>The sysmon_profile_label/1 helper was refactored from a case/if structure to the suggested with-based validation flow, returning nil via else when validation fails. code diff: ```diff defp sysmon_profile_label(profile) when is_map(profile) do - case Map.get(profile, :name) || Map.get(profile, "name") do - name when is_binary(name) -> - name = String.trim(name) - if name == "", do: nil, else: name - - _ -> - nil + with name when is_binary(name) <- Map.get(profile, :name) || Map.get(profile, "name"), + trimmed_name = String.trim(name), + true <- trimmed_name != "" do + trimmed_name + else + _ -> nil end ``` </details> ___ **Refactor the <code>sysmon_profile_label/1</code> function to use a <code>with</code> statement for <br>improved clarity and idiomatic style when handling multi-step validation.** [web-ng/lib/serviceradar_web_ng_web/live/device_live/index.ex [910-919]](https://github.com/carverauto/serviceradar/pull/2296/files#diff-261a01f4876e5984e1d9e9b38a3540675dfb0272abc30e6bdb2a4fa610353cc7R910-R919) ```diff defp sysmon_profile_label(profile) when is_map(profile) do - case Map.get(profile, :name) || Map.get(profile, "name") do - name when is_binary(name) -> - name = String.trim(name) - if name == "", do: nil, else: name - - _ -> - nil + with name when is_binary(name) <- Map.get(profile, :name) || Map.get(profile, "name"), + trimmed_name = String.trim(name), + true <- trimmed_name != "" do + trimmed_name + else + _ -> nil end end ``` `[Suggestion processed]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly proposes using an idiomatic Elixir `with` statement to refactor the function, which improves code readability and maintainability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>