Skip to content

dnsmasq: leases sorting fixes - #10790

Open
Greelan wants to merge 2 commits into
opnsense:masterfrom
Greelan:dnsmasq
Open

dnsmasq: leases sorting fixes#10790
Greelan wants to merge 2 commits into
opnsense:masterfrom
Greelan:dnsmasq

Conversation

@Greelan

@Greelan Greelan commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Important notices

Before you submit a pull request, we ask you kindly to acknowledge the following:

If AI was used, please disclose:

  • Model used: Claude Opus 4.8
  • Extent of AI involvement: Identified the source of the issues and suggested solutions.

Describe the problem

Ordering of various Dnsmasq lease fields (IP Address, MAC Address, DUID, Lease Type) currently does not work properly since the existing code does not take proper account of the underlying data structure (array) or type (hex numbers).


Describe the proposed solution

Ensure proper ordering.

(Note that the same issues likely affect the KEA leases sorting, however I am focusing this PR on Dnsmasq for the time being.)


Related issue

https://forum.opnsense.org/index.php?topic=52794.0

@AdSchellevis

Copy link
Copy Markdown
Member

it's probably better to change this in the template / form, eg use a sortable field (sort_address) in:

<th data-column-id="address" data-identifier="true" data-type="string" data-formatter="overflowformatter">{{ lang._('IP Address') }}</th>

and add a formatter to show the address field instead of the sort_address one.

@Greelan

Greelan commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. I take it the preference is to keep this out of ApiControllerBase.php and confine all the changes to the module?

Noting that there will need to be four sortable fields because this doesn't only impact address.

@AdSchellevis

Copy link
Copy Markdown
Member

given the values we do want to sort are already in the dataset received by searchRecordsetBase(), I would prefer using actual fieldnames indeed (instead of adding a mapper in between).

@Greelan

Greelan commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thinking about it further, is there a case for a shared helper or helpers, such as in OPNsense\Firewall\Util, that sort IPs and hex codes? Then it wouldn't need to be replicated in each spot where it is needed (currently at least two, Dnsmasq and Kea).

@AdSchellevis

Copy link
Copy Markdown
Member

I had the idea some time ago to add types on data in the search function (dad956e), but haven't found time to work on it further I'm afraid.

Ideally you should be able to user filter logic based on the type of data that's inside the container, same would logically apply for sorting in that case too.

The main reason for wanting this is the filter page, which would be very practical to have a simple "impact assessment" for clauses provided (e.g. searching which rules impact 192.168.1.1 should logically also return 192.168.1.0/24)
Currently this only works partially (locate aliases which match the search criteria)

This is more of a longer term solution I'm afraid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants