carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

rule builder WIP #2691

Merged
mfreeman451 merged 1 commit from refs/pull/2691/head into staging 2026-01-18 08:36:27 +00:00
Conversation 3 Commits 1 Files changed 10 +2651 -25
mfreeman451 commented 2026-01-18 08:35:14 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #2341
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2341
Original created: 2026-01-18T08:35:14Z
Original updated: 2026-01-18T08:37:17Z
Original head: carverauto/serviceradar:2338-feat-create-event-from-existing-log
Original base: staging
Original merged: 2026-01-18T08:36:27Z 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

Enhancement

Description

  • Add rule builder modal for creating log promotion rules directly from log details view

  • Implement attribute parsing to display structured log metadata instead of raw strings

  • Enable rule testing/preview against recent logs before saving via SRQL queries

  • Integrate rule builder with existing Rules management UI for editing and toggling rules

  • Add RBAC checks to restrict rule creation to operator/admin roles

Diagram Walkthrough

flowchart LR
  LogView["Log Details View"]
  RuleBuilder["Rule Builder Modal"]
  Preview["SRQL Preview Query"]
  LogPromoRule["LogPromotionRule Created"]
  RulesUI["Rules Settings UI"]
  
  LogView -->|"Click Create Event Rule"| RuleBuilder
  RuleBuilder -->|"Test Rule"| Preview
  Preview -->|"Match Count + Samples"| RuleBuilder
  RuleBuilder -->|"Save"| LogPromoRule
  LogPromoRule -->|"Appears in"| RulesUI
  RulesUI -->|"Edit/Toggle"| RuleBuilder

File Walkthrough
Relevant files
Enhancement
3 files
promotion_rule_builder.ex
New LiveComponent for building log promotion rules             
+876/-0 
show.ex
Add rule builder modal and attribute parsing to log details
 +155/-9 
index.ex
Enable rule builder for creating and editing promotion rules
 +200/-16
Tests
3 files
promotion_rule_builder_test.exs
Unit tests for rule builder parsing and query building     
+489/-0 
show_test.exs
LiveView tests for RBAC and rule builder integration         
+214/-0 
rules_live_test.exs
Add tests for promotion rule builder functionality             
+165/-0 
Documentation
4 files
design.md
Design document for log-to-event rule builder feature       
+285/-0 
proposal.md
Proposal document explaining feature motivation and scope
 +93/-0   
spec.md
Requirements specification for rule creation and attribute display
 +98/-0   
tasks.md
Implementation task checklist for feature development       
+76/-0   
Imported from GitHub pull request. Original GitHub pull request: #2341 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2341 Original created: 2026-01-18T08:35:14Z Original updated: 2026-01-18T08:37:17Z Original head: carverauto/serviceradar:2338-feat-create-event-from-existing-log Original base: staging Original merged: 2026-01-18T08:36:27Z 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** Enhancement ___ ### **Description** - Add rule builder modal for creating log promotion rules directly from log details view - Implement attribute parsing to display structured log metadata instead of raw strings - Enable rule testing/preview against recent logs before saving via SRQL queries - Integrate rule builder with existing Rules management UI for editing and toggling rules - Add RBAC checks to restrict rule creation to operator/admin roles ___ ### Diagram Walkthrough ```mermaid flowchart LR LogView["Log Details View"] RuleBuilder["Rule Builder Modal"] Preview["SRQL Preview Query"] LogPromoRule["LogPromotionRule Created"] RulesUI["Rules Settings UI"] LogView -->|"Click Create Event Rule"| RuleBuilder RuleBuilder -->|"Test Rule"| Preview Preview -->|"Match Count + Samples"| RuleBuilder RuleBuilder -->|"Save"| LogPromoRule LogPromoRule -->|"Appears in"| RulesUI RulesUI -->|"Edit/Toggle"| RuleBuilder ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><details><summary>3 files</summary><table> <tr> <td><strong>promotion_rule_builder.ex</strong><dd><code>New LiveComponent for building log promotion rules</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354">+876/-0</a>&nbsp; </td> </tr> <tr> <td><strong>show.ex</strong><dd><code>Add rule builder modal and attribute parsing to log details</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-4f9769353c55928a0d382cd7510379444445aea116e1ecdf7b8eb892d249ff26">+155/-9</a>&nbsp; </td> </tr> <tr> <td><strong>index.ex</strong><dd><code>Enable rule builder for creating and editing promotion rules</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-ce489a06ca328b705897a7b71749c6519594920a192aa0d033944a046d743ef0">+200/-16</a></td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>3 files</summary><table> <tr> <td><strong>promotion_rule_builder_test.exs</strong><dd><code>Unit tests for rule builder parsing and query building</code>&nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-4ff7f672a7acd6834d3e7eba25af462a2535e6ee010b511de8e57d12693703d4">+489/-0</a>&nbsp; </td> </tr> <tr> <td><strong>show_test.exs</strong><dd><code>LiveView tests for RBAC and rule builder integration</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3b">+214/-0</a>&nbsp; </td> </tr> <tr> <td><strong>rules_live_test.exs</strong><dd><code>Add tests for promotion rule builder functionality</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-dddd0d747bd725738a10e74770d765ed98fb22cdd5cd702bcbe98cc47c5af5d0">+165/-0</a>&nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>4 files</summary><table> <tr> <td><strong>design.md</strong><dd><code>Design document for log-to-event rule builder feature</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-f50f160873d72106253ab525dad60226699dd0f53ec4182be89e39ae4f39f659">+285/-0</a>&nbsp; </td> </tr> <tr> <td><strong>proposal.md</strong><dd><code>Proposal document explaining feature motivation and scope</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-4005d28f3aaac17b5b1f57892e21a2961ea23ee2d3b551626b4408fa8e15a473">+93/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Requirements specification for rule creation and attribute display</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-7d974543111469656418619b58b14bf94845021650602aa8ab0e3c3338f6a9c9">+98/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Implementation task checklist for feature development</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2341/files#diff-ee81afe728f07f39c4cd4f0da61e541d8a818cb2f8f24c83b81c952dc1c85098">+76/-0</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2026-01-18 08:36:06 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2341#issuecomment-3765059055
Original created: 2026-01-18T08:36:06Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build

Failed stage: Configure SRQL fixture database for tests [❌]

Failed test name: ""

Failure summary:

The action failed during environment/fixture validation because the required secret
SRQL_TEST_DATABASE_CA_CERT was not configured.
The workflow explicitly checks for this secret to
verify TLS for the SRQL test fixture and aborts when it is empty, printing:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. and exiting with
code 1.

Relevant error logs: 
1:  Runner name: 'arc-runner-set-hk6mk-runner-h4r6x'
2:  Runner group name: 'Default'
...

139:  ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m
140:  ^[[36;1m  sudo apt-get update^[[0m
141:  ^[[36;1m  sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m
142:  ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m
143:  ^[[36;1m  sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
144:  ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m
145:  ^[[36;1m  sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
146:  ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m
147:  ^[[36;1m  sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
148:  ^[[36;1melse^[[0m
149:  ^[[36;1m  echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m
150:  ^[[36;1m  exit 1^[[0m
151:  ^[[36;1mfi^[[0m
152:  ^[[36;1m^[[0m
153:  ^[[36;1mensure_pkg_config^[[0m
154:  ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m
155:  shell: /usr/bin/bash -e {0}
...

316:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
317:  env:
318:  BUILDBUDDY_ORG_API_KEY: ***
319:  SRQL_TEST_DATABASE_URL: ***
320:  SRQL_TEST_ADMIN_URL: ***
321:  SRQL_TEST_DATABASE_CA_CERT: 
322:  DOCKERHUB_USERNAME: ***
323:  DOCKERHUB_TOKEN: ***
324:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
325:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
326:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
327:  ##[endgroup]
328:  ##[group]Run : install rustup if needed
329:  ^[[36;1m: install rustup if needed^[[0m
330:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
331:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
332:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

472:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
473:  env:
474:  BUILDBUDDY_ORG_API_KEY: ***
475:  SRQL_TEST_DATABASE_URL: ***
476:  SRQL_TEST_ADMIN_URL: ***
477:  SRQL_TEST_DATABASE_CA_CERT: 
478:  DOCKERHUB_USERNAME: ***
479:  DOCKERHUB_TOKEN: ***
480:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
481:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
482:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
483:  CARGO_HOME: /home/runner/.cargo
484:  CARGO_INCREMENTAL: 0
485:  CARGO_TERM_COLOR: always
486:  ##[endgroup]
487:  ##[group]Run : work around spurious network errors in curl 8.0
488:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
489:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

540:  SRQL_TEST_DATABASE_CA_CERT: 
541:  DOCKERHUB_USERNAME: ***
542:  DOCKERHUB_TOKEN: ***
543:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
544:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
545:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
546:  CARGO_HOME: /home/runner/.cargo
547:  CARGO_INCREMENTAL: 0
548:  CARGO_TERM_COLOR: always
549:  ##[endgroup]
550:  Attempting to download 1.x...
551:  Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64
552:  Adding to the cache ...
553:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64
554:  Added bazelisk to the path
555:  ##[warning]Failed to restore: Cache service responded with 400
556:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

622:  env:
623:  BUILDBUDDY_ORG_API_KEY: ***
624:  SRQL_TEST_DATABASE_URL: ***
625:  SRQL_TEST_ADMIN_URL: ***
626:  SRQL_TEST_DATABASE_CA_CERT: 
627:  DOCKERHUB_USERNAME: ***
628:  DOCKERHUB_TOKEN: ***
629:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
630:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
631:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
632:  CARGO_HOME: /home/runner/.cargo
633:  CARGO_INCREMENTAL: 0
634:  CARGO_TERM_COLOR: always
635:  ##[endgroup]
636:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
637:  ##[error]Process completed with exit code 1.
638:  Post job cleanup.
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2341#issuecomment-3765059055 Original created: 2026-01-18T08:36:06Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Configure SRQL fixture database for tests](https://github.com/carverauto/serviceradar/actions/runs/21108833404/job/60704285843) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed during environment/fixture validation because the required secret <br><code>SRQL_TEST_DATABASE_CA_CERT</code> was not configured.<br> The workflow explicitly checks for this secret to <br>verify TLS for the SRQL test fixture and aborts when it is empty, printing: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code> and exiting with <br>code 1.<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-h4r6x' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 316: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 317: env: 318: BUILDBUDDY_ORG_API_KEY: *** 319: SRQL_TEST_DATABASE_URL: *** 320: SRQL_TEST_ADMIN_URL: *** 321: SRQL_TEST_DATABASE_CA_CERT: 322: DOCKERHUB_USERNAME: *** 323: DOCKERHUB_TOKEN: *** 324: TEST_CNPG_DATABASE: serviceradar_web_ng_test 325: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 326: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 327: ##[endgroup] 328: ##[group]Run : install rustup if needed 329: ^[[36;1m: install rustup if needed^[[0m 330: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 331: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 332: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 472: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 473: env: 474: BUILDBUDDY_ORG_API_KEY: *** 475: SRQL_TEST_DATABASE_URL: *** 476: SRQL_TEST_ADMIN_URL: *** 477: SRQL_TEST_DATABASE_CA_CERT: 478: DOCKERHUB_USERNAME: *** 479: DOCKERHUB_TOKEN: *** 480: TEST_CNPG_DATABASE: serviceradar_web_ng_test 481: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 482: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 483: CARGO_HOME: /home/runner/.cargo 484: CARGO_INCREMENTAL: 0 485: CARGO_TERM_COLOR: always 486: ##[endgroup] 487: ##[group]Run : work around spurious network errors in curl 8.0 488: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 489: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 540: SRQL_TEST_DATABASE_CA_CERT: 541: DOCKERHUB_USERNAME: *** 542: DOCKERHUB_TOKEN: *** 543: TEST_CNPG_DATABASE: serviceradar_web_ng_test 544: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 545: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 546: CARGO_HOME: /home/runner/.cargo 547: CARGO_INCREMENTAL: 0 548: CARGO_TERM_COLOR: always 549: ##[endgroup] 550: Attempting to download 1.x... 551: Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64 552: Adding to the cache ... 553: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64 554: Added bazelisk to the path 555: ##[warning]Failed to restore: Cache service responded with 400 556: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 622: env: 623: BUILDBUDDY_ORG_API_KEY: *** 624: SRQL_TEST_DATABASE_URL: *** 625: SRQL_TEST_ADMIN_URL: *** 626: SRQL_TEST_DATABASE_CA_CERT: 627: DOCKERHUB_USERNAME: *** 628: DOCKERHUB_TOKEN: *** 629: TEST_CNPG_DATABASE: serviceradar_web_ng_test 630: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 631: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 632: CARGO_HOME: /home/runner/.cargo 633: CARGO_INCREMENTAL: 0 634: CARGO_TERM_COLOR: always 635: ##[endgroup] 636: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 637: ##[error]Process completed with exit code 1. 638: Post job cleanup. ``` </details></td></tr></table>
qodo-code-review[bot] commented 2026-01-18 08:36:11 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2341#issuecomment-3765059117
Original created: 2026-01-18T08:36:11Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
SRQL injection

Description: User-controlled inputs are interpolated into an SRQL query string for preview
(build_preview_query/1 -> srql_module().query/1), and while quotes/backslashes are
escaped, other SRQL metacharacters/operators may still allow query manipulation depending
on SRQL parsing rules (needs verification against SRQL grammar).
promotion_rule_builder.ex [500-748]

Referred Code 
defp run_preview_query(socket) do
  # Build SRQL query from form data
  query = build_preview_query(socket.assigns.form)

  # Execute query with timeout
  task = Task.async(fn -> srql_module().query(query) end)

  case Task.yield(task, @preview_timeout) || Task.shutdown(task, :brutal_kill) do
    {:ok, {:ok, %{"results" => results, "total_count" => total}}} ->
      socket
      |> assign(:preview_state, :done)
      |> assign(:preview_result, %{
        match_count: total || length(results),
        sample_logs: Enum.take(results, 5)
      })

    {:ok, {:ok, %{"results" => results}}} ->
      socket
      |> assign(:preview_state, :done)
      |> assign(:preview_result, %{
        match_count: length(results),


 ... (clipped 228 lines)
Ticket Compliance
🟡
🎫 #2338
🟢 Provide a modal-based rule builder UI launched from the log details view.
Pre-populate the rule builder fields from the current log entry.
Let the user enable/disable individual match conditions (message contains, severity,
service name, attribute match).
Create a `LogPromotionRule` record for the current tenant/scope.
Integrate created rules into the existing Rules management UI (view, edit, toggle enabled,
delete).
Parse and display log attributes in a structured format; if unparseable, show raw
attributes without errors.
Provide an auto-alert toggle that sets `event.alert` on the created rule when enabled.
🔴 Allow rule testing/preview against recent logs before saving (query last hour, show match
count and sample matches).
In Settings rules UI, allow editing rule match conditions, event config, and priority.
⚪ Enforce RBAC so only operator/admin users can create rules (viewers can view log details
but not create rules).
From the log details UI, allow a user to turn a log into an event via an intuitive UI
(implemented as creating a log-promotion rule/event rule from the log).
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
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: 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: Secure Error Handling

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

Status: 🏷️
Internal details exposed: User-visible error strings embed inspect(reason) (including task exit reasons and other
internal structures), which can leak implementation details to the UI.

Referred Code 
  {:ok, {:error, reason}} ->
    socket
    |> assign(:preview_state, :error)
    |> assign(:preview_error, format_error(reason))

  nil ->
    socket
    |> assign(:preview_state, :error)
    |> assign(:preview_error, "Query timed out after 5 seconds. Try narrowing your conditions.")

  {:exit, reason} ->
    socket
    |> assign(:preview_state, :error)
    |> assign(:preview_error, "Query failed: #{inspect(reason)}")
end

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

⚪
Generic: Comprehensive Audit Trails

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

Status: 🏷️
Missing audit logging: The PR adds rule create/update operations (and related UI actions) without any explicit
audit log events that record acting user, action, and outcome, so audit coverage likely
depends on unseen Ash-level auditing.

Referred Code 
defp create_rule(params, socket) do
  match = build_match_map(params)
  event = build_event_map(params)

  attrs = %{
    name: String.trim(params["name"]),
    enabled: true,
    priority: 100,
    match: match,
    event: event
  }

  scope = socket.assigns.current_scope

  case Ash.create(LogPromotionRule, attrs, scope: scope) do
    {:ok, rule} ->
      send(self(), {:rule_created, rule})
      {:noreply, assign(socket, :saving, false)}

    {:error, %Ash.Error.Invalid{} = error} ->
      error_message = format_ash_error(error)


 ... (clipped 42 lines)

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: 🏷️
Unhandled SRQL exceptions: The preview execution calls srql_module().query(query) inside a task without a rescue, so
unexpected exceptions may surface as task exits and produce inconsistent/overly detailed
errors depending on runtime behavior.

Referred Code 
task = Task.async(fn -> srql_module().query(query) end)

case Task.yield(task, @preview_timeout) || Task.shutdown(task, :brutal_kill) do
  {:ok, {:ok, %{"results" => results, "total_count" => total}}} ->
    socket
    |> assign(:preview_state, :done)
    |> assign(:preview_result, %{
      match_count: total || length(results),
      sample_logs: Enum.take(results, 5)
    })

  {:ok, {:ok, %{"results" => results}}} ->
    socket
    |> assign(:preview_state, :done)
    |> assign(:preview_result, %{
      match_count: length(results),
      sample_logs: Enum.take(results, 5)
    })

  {:ok, {:error, reason}} ->
    socket


 ... (clipped 13 lines)

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: 🏷️
Missing RBAC enforcement: The settings UI exposes create/edit/toggle promotion-rule actions without any role checks
in the LiveView, so authorization relies on unseen Ash policies and cannot be verified
from the diff.

Referred Code 
def handle_event("toggle_promotion", %{"id" => id}, socket) do
  scope = socket.assigns.current_scope
  id = to_string(id)

  case Enum.find(socket.assigns.promotion_rules, &(to_string(&1.id) == id)) do
    nil ->
      {:noreply, socket}

    rule ->
      changeset =
        Ash.Changeset.for_update(rule, :update, %{enabled: !rule.enabled}, scope: scope)

      case Ash.update(changeset) do
        {:ok, _} ->
          {:noreply,
           socket
           |> assign(:promotion_rules, list_promotion_rules(scope))
           |> put_flash(:info, "Rule #{if rule.enabled, do: "disabled", else: "enabled"}")}

        {:error, _} ->
          {:noreply, put_flash(socket, :error, "Failed to toggle rule")}


 ... (clipped 78 lines)

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

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/2341#issuecomment-3765059117 Original created: 2026-01-18T08:36:11Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/bfdb41dd887dfe1fabb0a03665a56b68158a2723 --> 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 rowspan=1>⚪</td> <td><details><summary><strong>SRQL injection</strong></summary><br> <b>Description:</b> User-controlled inputs are interpolated into an SRQL query string for preview <br>(<code>build_preview_query/1</code> -> <code>srql_module().query/1</code>), and while quotes/backslashes are <br>escaped, other SRQL metacharacters/operators may still allow query manipulation depending <br>on SRQL parsing rules (needs verification against SRQL grammar).<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R500-R748'>promotion_rule_builder.ex [500-748]</a></strong><br> <details open><summary>Referred Code</summary> ```elixir defp run_preview_query(socket) do # Build SRQL query from form data query = build_preview_query(socket.assigns.form) # Execute query with timeout task = Task.async(fn -> srql_module().query(query) end) case Task.yield(task, @preview_timeout) || Task.shutdown(task, :brutal_kill) do {:ok, {:ok, %{"results" => results, "total_count" => total}}} -> socket |> assign(:preview_state, :done) |> assign(:preview_result, %{ match_count: total || length(results), sample_logs: Enum.take(results, 5) }) {:ok, {:ok, %{"results" => results}}} -> socket |> assign(:preview_state, :done) |> assign(:preview_result, %{ match_count: length(results), ... (clipped 228 lines) ``` </details></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/2338>#2338</a></summary> <table width='100%'><tbody> <tr><td rowspan=7>🟢</td> <td>Provide a modal-based rule builder UI launched from the log details view.</td></tr> <tr><td>Pre-populate the rule builder fields from the current log entry.</td></tr> <tr><td>Let the user enable/disable individual match conditions (message contains, severity, <br>service name, attribute match).</td></tr> <tr><td>Create a `LogPromotionRule` record for the current tenant/scope.</td></tr> <tr><td>Integrate created rules into the existing Rules management UI (view, edit, toggle enabled, <br>delete).</td></tr> <tr><td>Parse and display log attributes in a structured format; if unparseable, show raw <br>attributes without errors.</td></tr> <tr><td>Provide an auto-alert toggle that sets `event.alert` on the created rule when enabled.</td></tr> <tr><td rowspan=2>🔴</td> <td>Allow rule testing/preview against recent logs before saving (query last hour, show match <br>count and sample matches).</td></tr> <tr><td>In Settings rules UI, allow editing rule match conditions, event config, and priority.</td></tr> <tr><td rowspan=2>⚪</td> <td>Enforce RBAC so only operator/admin users can create rules (viewers can view log details <br>but not create rules).</td></tr> <tr><td>From the log details UI, allow a user to turn a log into an event via an intuitive UI <br>(implemented as creating a log-promotion rule/event rule from the log).</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=2>🟢</td><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: 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 rowspan=1>🔴</td> <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:** 🏷️<br><a href='https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R524-R538'><strong>Internal details exposed</strong></a>: User-visible error strings embed <code>inspect(reason)</code> (including task exit reasons and other <br>internal structures), which can leak implementation details to the UI.<br> <details open><summary>Referred Code</summary> ```elixir {:ok, {:error, reason}} -> socket |> assign(:preview_state, :error) |> assign(:preview_error, format_error(reason)) nil -> socket |> assign(:preview_state, :error) |> assign(:preview_error, "Query timed out after 5 seconds. Try narrowing your conditions.") {:exit, reason} -> socket |> assign(:preview_state, :error) |> assign(:preview_error, "Query failed: #{inspect(reason)}") end ``` </details> > 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 rowspan=3>⚪</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:** 🏷️<br><a href='https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R652-R714'><strong>Missing audit logging</strong></a>: The PR adds rule create/update operations (and related UI actions) without any explicit <br>audit log events that record acting user, action, and outcome, so audit coverage likely <br>depends on unseen Ash-level auditing.<br> <details open><summary>Referred Code</summary> ```elixir defp create_rule(params, socket) do match = build_match_map(params) event = build_event_map(params) attrs = %{ name: String.trim(params["name"]), enabled: true, priority: 100, match: match, event: event } scope = socket.assigns.current_scope case Ash.create(LogPromotionRule, attrs, scope: scope) do {:ok, rule} -> send(self(), {:rule_created, rule}) {:noreply, assign(socket, :saving, false)} {:error, %Ash.Error.Invalid{} = error} -> error_message = format_ash_error(error) ... (clipped 42 lines) ``` </details> > 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:** 🏷️<br><a href='https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R505-R538'><strong>Unhandled SRQL exceptions</strong></a>: The preview execution calls <code>srql_module().query(query)</code> inside a task without a rescue, so <br>unexpected exceptions may surface as task exits and produce inconsistent/overly detailed <br>errors depending on runtime behavior.<br> <details open><summary>Referred Code</summary> ```elixir task = Task.async(fn -> srql_module().query(query) end) case Task.yield(task, @preview_timeout) || Task.shutdown(task, :brutal_kill) do {:ok, {:ok, %{"results" => results, "total_count" => total}}} -> socket |> assign(:preview_state, :done) |> assign(:preview_result, %{ match_count: total || length(results), sample_logs: Enum.take(results, 5) }) {:ok, {:ok, %{"results" => results}}} -> socket |> assign(:preview_state, :done) |> assign(:preview_result, %{ match_count: length(results), sample_logs: Enum.take(results, 5) }) {:ok, {:error, reason}} -> socket ... (clipped 13 lines) ``` </details> > 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:** 🏷️<br><a href='https://github.com/carverauto/serviceradar/pull/2341/files#diff-ce489a06ca328b705897a7b71749c6519594920a192aa0d033944a046d743ef0R117-R215'><strong>Missing RBAC enforcement</strong></a>: The settings UI exposes create/edit/toggle promotion-rule actions without any role checks <br>in the LiveView, so authorization relies on unseen Ash policies and cannot be verified <br>from the diff.<br> <details open><summary>Referred Code</summary> ```elixir def handle_event("toggle_promotion", %{"id" => id}, socket) do scope = socket.assigns.current_scope id = to_string(id) case Enum.find(socket.assigns.promotion_rules, &(to_string(&1.id) == id)) do nil -> {:noreply, socket} rule -> changeset = Ash.Changeset.for_update(rule, :update, %{enabled: !rule.enabled}, scope: scope) case Ash.update(changeset) do {:ok, _} -> {:noreply, socket |> assign(:promotion_rules, list_promotion_rules(scope)) |> put_flash(:info, "Rule #{if rule.enabled, do: "disabled", else: "enabled"}")} {:error, _} -> {:noreply, put_flash(socket, :error, "Failed to toggle rule")} ... (clipped 78 lines) ``` </details> > 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"> <!-- placeholder --> <!-- /compliance --update_compliance=true --> </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-18 08:37:17 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2341#issuecomment-3765059906
Original created: 2026-01-18T08:37:17Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Implement promotion rule deletion

Implement the missing handle_event("delete_promotion", ...) callback to handle
the deletion of a LogPromotionRule when the delete button is clicked.

web-ng/lib/serviceradar_web_ng_web/live/settings/rules_live/index.ex [446-454]

-<button
-  type="button"
-  class="btn btn-ghost btn-xs text-error"
-  phx-click="delete_promotion"
-  phx-value-id={rule.id}
-  data-confirm="Are you sure you want to delete this rule?"
->
-  <.icon name="hero-trash" class="w-4 h-4" />
-</button>
+def handle_event("delete_promotion", %{"id" => id}, socket) do
+  scope = socket.assigns.current_scope
+  case Enum.find(socket.assigns.promotion_rules, &(to_string(&1.id) == id)) do
+    nil ->
+      {:noreply, socket}
+    rule ->
+      case Ash.destroy(rule, scope: scope) do
+        :ok ->
+          {:noreply,
+           socket
+           |> assign(:promotion_rules, list_promotion_rules(scope))
+           |> put_flash(:info, "Promotion rule deleted")}
+        {:error, _} ->
+          {:noreply, put_flash(socket, :error, "Failed to delete promotion rule")}
+      end
+  end
+end
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The PR adds a delete button with a phx-click="delete_promotion" event but fails to implement the corresponding handle_event callback, which would cause a runtime crash. This suggestion correctly identifies and fixes this critical bug.

High
General
Support dots/hyphens in attribute keys

Update the attribute parsing regex to support keys containing dots and hyphens
by changing \w+ to [^=,]+.

web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [793]

-~r/(\w+)=(\{[^}]*\}|[^,]+?)(?=,\w+=|$)/
+~r/([^=,]+)=(\{[^}]*\}|[^,]+?)(?=(?:,[^=,]+=)|$)/
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the attribute parsing regex is too restrictive, as it only allows word characters in keys. The proposed change to [^=,]+ correctly expands support to include common characters like dots and hyphens, making the parsing logic more robust and preventing silent failures for valid attribute formats.

Medium
Extract attribute parsing helper

Refactor the duplicated attribute parsing logic from promotion_rule_builder.ex
and show.ex into a single, shared helper module to improve maintainability.

web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [773-779]

-case Map.get(log, "attributes") do
-  nil -> %{}
-  "" -> %{}
-  value when is_map(value) -> value
-  value when is_binary(value) -> parse_attribute_string(value)
-  _ -> %{}
-end
+alias ServiceRadarWebNG.AttributeParser
+...
+parsed_attributes = AttributeParser.parse(log["attributes"])

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies duplicated attribute parsing logic between two files. Extracting this logic into a shared module would improve code maintainability and reduce redundancy, which is a valuable improvement.

Medium
  • More
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2341#issuecomment-3765059906 Original created: 2026-01-18T08:37:17Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Code Suggestions ✨ <!-- bfdb41d --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Implement promotion rule deletion</summary> ___ **Implement the missing <code>handle_event("delete_promotion", ...)</code> callback to handle <br>the deletion of a <code>LogPromotionRule</code> when the delete button is clicked.** [web-ng/lib/serviceradar_web_ng_web/live/settings/rules_live/index.ex [446-454]](https://github.com/carverauto/serviceradar/pull/2341/files#diff-ce489a06ca328b705897a7b71749c6519594920a192aa0d033944a046d743ef0R446-R454) ```diff -<button - type="button" - class="btn btn-ghost btn-xs text-error" - phx-click="delete_promotion" - phx-value-id={rule.id} - data-confirm="Are you sure you want to delete this rule?" -> - <.icon name="hero-trash" class="w-4 h-4" /> -</button> +def handle_event("delete_promotion", %{"id" => id}, socket) do + scope = socket.assigns.current_scope + case Enum.find(socket.assigns.promotion_rules, &(to_string(&1.id) == id)) do + nil -> + {:noreply, socket} + rule -> + case Ash.destroy(rule, scope: scope) do + :ok -> + {:noreply, + socket + |> assign(:promotion_rules, list_promotion_rules(scope)) + |> put_flash(:info, "Promotion rule deleted")} + {:error, _} -> + {:noreply, put_flash(socket, :error, "Failed to delete promotion rule")} + end + end +end ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The PR adds a delete button with a `phx-click="delete_promotion"` event but fails to implement the corresponding `handle_event` callback, which would cause a runtime crash. This suggestion correctly identifies and fixes this critical bug. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Support dots/hyphens in attribute keys</summary> ___ **Update the attribute parsing regex to support keys containing dots and hyphens <br>by changing <code>\w+</code> to <code>[^=,]+</code>.** [web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [793]](https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R793-R793) ```diff -~r/(\w+)=(\{[^}]*\}|[^,]+?)(?=,\w+=|$)/ +~r/([^=,]+)=(\{[^}]*\}|[^,]+?)(?=(?:,[^=,]+=)|$)/ ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies that the attribute parsing regex is too restrictive, as it only allows word characters in keys. The proposed change to `[^=,]+` correctly expands support to include common characters like dots and hyphens, making the parsing logic more robust and preventing silent failures for valid attribute formats. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Extract attribute parsing helper</summary> ___ **Refactor the duplicated attribute parsing logic from <code>promotion_rule_builder.ex</code> <br>and <code>show.ex</code> into a single, shared helper module to improve maintainability.** [web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [773-779]](https://github.com/carverauto/serviceradar/pull/2341/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R773-R779) ```diff -case Map.get(log, "attributes") do - nil -> %{} - "" -> %{} - value when is_map(value) -> value - value when is_binary(value) -> parse_attribute_string(value) - _ -> %{} -end +alias ServiceRadarWebNG.AttributeParser +... +parsed_attributes = AttributeParser.parse(log["attributes"]) ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies duplicated attribute parsing logic between two files. Extracting this logic into a shared module would improve code maintainability and reduce redundancy, which is a valuable improvement. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] More <!-- /improve --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2691
No description provided.