carverauto/serviceradar

Fork 0

sorting by is_available #2827

Merged

mfreeman451 merged 1 commit from refs/pull/2827/head into staging

2026-02-02 01:36:23 +00:00

mfreeman451 commented

2026-02-02 01:35:38 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2661
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2661
Original created: 2026-02-02T01:35:38Z
Original updated: 2026-02-02T01:37:26Z
Original head: carverauto/serviceradar:2643-featweb-ng-device-inventory-should-always-sort-devices-by-is_available-true
Original base: staging
Original merged: 2026-02-02T01:36:23Z 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 is_available as sortable field for devices in SRQL
Change default device sort from last_seen to is_available
Refactor ordering logic to support primary and secondary sort fields
Implement intelligent default ordering: is_available desc, then ip asc, then uid asc

Diagram Walkthrough

flowchart LR
  A["Device Sorting Request"] --> B["SRQL Builder"]
  B --> C["Allowed Sort Fields"]
  C --> D["is_available Added"]
  D --> E["Rust Query Builder"]
  E --> F["Primary Order Function"]
  E --> G["Secondary Order Function"]
  F --> H["Default: is_available desc"]
  G --> I["Then: ip asc, uid asc"]
  H --> J["Ordered Results"]
  I --> J

File Walkthrough

Relevant files

Configuration changes

catalog.ex

Change default device sort field

web-ng/lib/serviceradar_web_ng_web/srql/catalog.ex

Changed default sort field for devices from last_seen to is_available
Maintains backward compatibility with existing catalog structure

+1/-1

Enhancement

builder.ex

Add is_available to allowed sort fields

web-ng/lib/serviceradar_web_ng_web/srql/builder.ex

Added is_available to allowed sort fields for devices
Removed unnecessary blank line in normalize_sort_field function

+1/-2

devices.rs

Refactor ordering logic and add is_available support

rust/srql/src/query/devices.rs

Added Inet type import for IP address handling
Refactored apply_ordering function to support multiple sort clauses
with primary and secondary ordering
Extracted ordering logic into apply_primary_order and
apply_secondary_order helper functions
Implemented intelligent default ordering: is_available (desc), then ip
(asc), then uid (asc)
Added support for is_available and ip fields with proper SQL type
casting
Track which fields have been applied to avoid duplicate ordering

+139/-40

Imported from GitHub pull request. Original GitHub pull request: #2661 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2661 Original created: 2026-02-02T01:35:38Z Original updated: 2026-02-02T01:37:26Z Original head: carverauto/serviceradar:2643-featweb-ng-device-inventory-should-always-sort-devices-by-is_available-true Original base: staging Original merged: 2026-02-02T01:36:23Z 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 `is_available` as sortable field for devices in SRQL - Change default device sort from `last_seen` to `is_available` - Refactor ordering logic to support primary and secondary sort fields - Implement intelligent default ordering: `is_available` desc, then `ip` asc, then `uid` asc ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Device Sorting Request"] --> B["SRQL Builder"] B --> C["Allowed Sort Fields"] C --> D["is_available Added"] D --> E["Rust Query Builder"] E --> F["Primary Order Function"] E --> G["Secondary Order Function"] F --> H["Default: is_available desc"] G --> I["Then: ip asc, uid asc"] H --> J["Ordered Results"] I --> J ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>catalog.ex</strong><dd><code>Change default device sort field</code>                                                  </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/srql/catalog.ex <ul><li>Changed default sort field for devices from <code>last_seen</code> to <code>is_available</code><br> <li> Maintains backward compatibility with existing catalog structure</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2661/files#diff-80c07c25c17e48bf86860ec91db10256b7700a863d5371798e1893e741dc0e15">+1/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>builder.ex</strong><dd><code>Add is_available to allowed sort fields</code>                                    </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/srql/builder.ex <ul><li>Added <code>is_available</code> to allowed sort fields for devices<br> <li> Removed unnecessary blank line in <code>normalize_sort_field</code> function</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2661/files#diff-0eed4282479f6f8c4f3ed9ae60280a2acdd5ff9d52db06dde050ed49d3b6da20">+1/-2</a>      </td> </tr> <tr> <td> <details> <summary><strong>devices.rs</strong><dd><code>Refactor ordering logic and add is_available support</code>          </dd></summary> <hr> rust/srql/src/query/devices.rs <ul><li>Added <code>Inet</code> type import for IP address handling<br> <li> Refactored <code>apply_ordering</code> function to support multiple sort clauses <br>with primary and secondary ordering<br> <li> Extracted ordering logic into <code>apply_primary_order</code> and <br><code>apply_secondary_order</code> helper functions<br> <li> Implemented intelligent default ordering: <code>is_available</code> (desc), then <code>ip</code> <br>(asc), then <code>uid</code> (asc)<br> <li> Added support for <code>is_available</code> and <code>ip</code> fields with proper SQL type <br>casting<br> <li> Track which fields have been applied to avoid duplicate ordering</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2661/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5">+139/-40</a></td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-02-02 01:36:12 +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/2661#issuecomment-3832478759
Original created: 2026-02-02T01:36:12Z

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

🟡

🎫 #2643

🟢	Device inventory sorting should first prioritize devices where `is_available = true`.
⚪	Remaining devices should be sorted by CIDR/IP address (deterministic ordering by network/IP).

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: 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: Robust Error Handling and Edge Case Management

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

Status:
Inet cast failure: Ordering by ip uses NULLIF(ip, '')::inet which can raise a database error at
runtime if the stored ip value is non-empty but not a valid inet string, and this edge
case is not handled in the query logic.

Referred Code

        .order(sql::<Bool>("coalesce(is_available, false)").desc())
        .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc())
        .then_order_by(col_uid.asc());
} else if saw_is_available && !saw_ip {
    query = query
        .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc())
        .then_order_by(col_uid.asc());

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:
Raw SQL ordering: The new ordering logic introduces raw SQL fragments (e.g., coalesce(is_available, false)
and NULLIF(ip, '')::inet) and correctness/safety depends on upstream validation
of order fields and on column contents always being castable, which is not verifiable from
this diff alone.

Referred Code

            .order(sql::<Bool>("coalesce(is_available, false)").desc())
            .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc())
            .then_order_by(col_uid.asc());
    } else if saw_is_available && !saw_ip {
        query = query
            .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc())
            .then_order_by(col_uid.asc());
    } else {
        query = query.then_order_by(col_uid.asc());
    }

    query
}

fn apply_primary_order<'a>(
    query: DeviceQuery<'a>,
    clause: &OrderClause,
) -> (DeviceQuery<'a>, bool) {
    match clause.field.as_str() {
        "is_available" => (
            match clause.direction {


 ... (clipped 104 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/2661#issuecomment-3832478759 Original created: 2026-02-02T01:36:12Z --- ## 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/2643>#2643</a></summary> <table width='100%'><tbody> <tr><td rowspan=1>🟢</td> <td>Device inventory sorting should first prioritize devices where `is_available = true`. </td></tr> <tr><td rowspan=1>⚪</td> <td>Remaining devices should be sorted by CIDR/IP address (deterministic ordering by <br>network/IP).<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=4>🟢</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: 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 rowspan=2>⚪</td> <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/2661/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5R1119-R1125'><strong>Inet cast failure</strong></a>: Ordering by <code>ip</code> uses <code>NULLIF(ip, '')::inet</code> which can raise a database error at <br>runtime if the stored <code>ip</code> value is non-empty but not a valid inet string, and this edge <br>case is not handled in the query logic.<br> <details open><summary>Referred Code</summary> ```rust .order(sql::<Bool>("coalesce(is_available, false)").desc()) .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc()) .then_order_by(col_uid.asc()); } else if saw_is_available && !saw_ip { query = query .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc()) .then_order_by(col_uid.asc()); ``` </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/2661/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5R1119-R1243'><strong>Raw SQL ordering</strong></a>: The new ordering logic introduces raw SQL fragments (e.g., <code>coalesce(is_available, false)</code> <br>and <code>NULLIF(ip, '')::inet</code>) and correctness/safety depends on upstream validation <br>of order fields and on column contents always being castable, which is not verifiable from <br>this diff alone.<br> <details open><summary>Referred Code</summary> ```rust .order(sql::<Bool>("coalesce(is_available, false)").desc()) .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc()) .then_order_by(col_uid.asc()); } else if saw_is_available && !saw_ip { query = query .then_order_by(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet").asc()) .then_order_by(col_uid.asc()); } else { query = query.then_order_by(col_uid.asc()); } query } fn apply_primary_order<'a>( query: DeviceQuery<'a>, clause: &OrderClause, ) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( match clause.direction { ... (clipped 104 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">   </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-02-02 01:37:26 +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/2661#issuecomment-3832480686
Original created: 2026-02-02T01:37:26Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Only add UID if not seen Track if `uid` is used in sorting and conditionally add the default `uid` sort order only if it has not been specified, preventing duplicate clauses. rust/srql/src/query/devices.rs [1110-1128] `match clause.field.as_str() { "is_available" => saw_is_available = true, "ip" => saw_ip = true, + "uid" => saw_uid = true, _ => {} } ... -} else { +} else if !saw_uid { query = query.then_order_by(col_uid.asc()); }` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies and fixes a bug where a redundant `ORDER BY` clause for `uid` is added, improving the correctness and efficiency of the generated SQL.	Medium
High-level	Refactor duplicated Rust ordering logic Refactor the duplicated logic in `apply_primary_order` and `apply_secondary_order` into a single, generic function or macro to improve maintainability. The only difference between them is the call to `order()` versus `then_order_by()`. Examples: rust/srql/src/query/devices.rs [1133-1243] `fn apply_primary_order<'a>( query: DeviceQuery<'a>, clause: &OrderClause, ) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( match clause.direction { OrderDirection::Asc => query.order(sql::<Bool>("coalesce(is_available, false)").asc()), OrderDirection::Desc => query.order(sql::<Bool>("coalesce(is_available, false)").desc()), }, ... (clipped 101 lines)` Solution Walkthrough: Before: `fn apply_primary_order<'a>(query: DeviceQuery<'a>, clause: &OrderClause) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( query.order(...), true, ), "ip" => ( query.order(...), true, ), // ... other fields } } fn apply_secondary_order<'a>(query: DeviceQuery<'a>, clause: &OrderClause) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( query.then_order_by(...), true, ), "ip" => ( query.then_order_by(...), true, ), // ... other fields } }` After: macro_rules! apply_order_logic { ($query:ident, $clause:ident, $order_method:ident) => { match $clause.field.as_str() { "is_available" => ( $query.$order_method(sql::<Bool>("coalesce(is_available, false)")...), true, ), "ip" => ( $query.$order_method(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet")...), true, ), // ... other fields _ => ($query, false), } }; } fn apply_primary_order(...) { apply_order_logic!(query, clause, order) } fn apply_secondary_order(...) { apply_order_logic!(query, clause, then_order_by) } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies significant code duplication in the new `apply_primary_order` and `apply_secondary_order` functions, which impacts maintainability.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2661#issuecomment-3832480686 Original created: 2026-02-02T01:37:26Z --- ## 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=1>Possible issue</td> <td> <details><summary>Only add UID if not seen</summary> ___ **Track if <code>uid</code> is used in sorting and conditionally add the default <code>uid</code> sort order <br>only if it has not been specified, preventing duplicate clauses.** [rust/srql/src/query/devices.rs [1110-1128]](https://github.com/carverauto/serviceradar/pull/2661/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5R1110-R1128) ```diff match clause.field.as_str() { "is_available" => saw_is_available = true, "ip" => saw_ip = true, + "uid" => saw_uid = true, _ => {} } ... -} else { +} else if !saw_uid { query = query.then_order_by(col_uid.asc()); } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies and fixes a bug where a redundant `ORDER BY` clause for `uid` is added, improving the correctness and efficiency of the generated SQL. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>High-level</td> <td> <details><summary>Refactor duplicated Rust ordering logic</summary> ___ **Refactor the duplicated logic in <code>apply_primary_order</code> and <code>apply_secondary_order</code> <br>into a single, generic function or macro to improve maintainability. The only <br>difference between them is the call to <code>order()</code> versus <code>then_order_by()</code>.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2661/files#diff-3202f22fff6863ed7848a129c49e2323322462b379d896d3fca2e59aa6f7b4c5R1133-R1243">rust/srql/src/query/devices.rs [1133-1243]</a> </summary> ```rust fn apply_primary_order<'a>( query: DeviceQuery<'a>, clause: &OrderClause, ) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( match clause.direction { OrderDirection::Asc => query.order(sql::<Bool>("coalesce(is_available, false)").asc()), OrderDirection::Desc => query.order(sql::<Bool>("coalesce(is_available, false)").desc()), }, ... (clipped 101 lines) ``` </details> ### Solution Walkthrough: #### Before: ```rust fn apply_primary_order<'a>(query: DeviceQuery<'a>, clause: &OrderClause) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( query.order(...), true, ), "ip" => ( query.order(...), true, ), // ... other fields } } fn apply_secondary_order<'a>(query: DeviceQuery<'a>, clause: &OrderClause) -> (DeviceQuery<'a>, bool) { match clause.field.as_str() { "is_available" => ( query.then_order_by(...), true, ), "ip" => ( query.then_order_by(...), true, ), // ... other fields } } ``` #### After: ```rust macro_rules! apply_order_logic { ($query:ident, $clause:ident, $order_method:ident) => { match $clause.field.as_str() { "is_available" => ( $query.$order_method(sql::<Bool>("coalesce(is_available, false)")...), true, ), "ip" => ( $query.$order_method(sql::<Nullable<Inet>>("NULLIF(ip, '')::inet")...), true, ), // ... other fields _ => ($query, false), } }; } fn apply_primary_order(...) { apply_order_logic!(query, clause, order) } fn apply_secondary_order(...) { apply_order_logic!(query, clause, then_order_by) } ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies significant code duplication in the new `apply_primary_order` and `apply_secondary_order` functions, which impacts maintainability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>