Fixes: #19917 - Fix MAC address pagination duplicates by adding 'pk' to model ordering

[jseifeddine]

Fixes: #19917

Fix MAC address pagination by ensuring deterministic ordering
Add pk to MACAddress model ordering to ensure deterministic results
when multiple MAC addresses have the same value. This prevents the same
MAC address from appearing on multiple pages during pagination.

The issue occurred because Django's default ordering by 'mac_address'
alone is non-deterministic when multiple records share the same MAC
address value, causing inconsistent pagination results.

[jseifeddine]

Thanks @DanSheps

Done jseifeddine@83b312a

[jseifeddine]

Thanks @DanSheps

Done jseifeddine@83b312a

sorry, just saw the failed builds,
Will address this

[jseifeddine]

should be good to go... thanks

[jseifeddine]

Cleaned up the tests and able to reproduce in both dcim.tests.test_filtersets and dcim.tests.test_api

System check identified no issues (0 silenced).
test_mac_address_pagination_duplicates (dcim.tests.test_filtersets.MACAddressTestCase.test_mac_address_pagination_duplicates) ... FAIL
test_mac_address_pagination_duplicates (dcim.tests.test_api.MACAddressTest.test_mac_address_pagination_duplicates) ... FAIL

======================================================================
FAIL: test_mac_address_pagination_duplicates (dcim.tests.test_filtersets.MACAddressTestCase.test_mac_address_pagination_duplicates)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/netbox/netbox/dcim/tests/test_filtersets.py", line 6959, in test_mac_address_pagination_duplicates
    self.assertEqual(len(all_ids & page_ids), 0)
AssertionError: 1 != 0

======================================================================
FAIL: test_mac_address_pagination_duplicates (dcim.tests.test_api.MACAddressTest.test_mac_address_pagination_duplicates)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/netbox/netbox/dcim/tests/test_api.py", line 2696, in test_mac_address_pagination_duplicates
    self.assertEqual(len(all_mac_ids & page_ids), 0)
AssertionError: 1 != 0

----------------------------------------------------------------------
Ran 2 tests in 0.271s

FAILED (failures=2)

with the proposed pk order fix:

test_mac_address_pagination_duplicates (dcim.tests.test_filtersets.MACAddressTestCase.test_mac_address_pagination_duplicates) ... ok
test_mac_address_pagination_duplicates (dcim.tests.test_api.MACAddressTest.test_mac_address_pagination_duplicates) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.421s

OK

[jseifeddine]

@jeremystretch

I've removed the tests completely just keeping the actual fix,

netbox (19917-fix-mac-address-pagination-duplicates) ✔ ruff check netbox/ 
All checks passed!

[jseifeddine]

OK @jeremystretch

I added the appropriate test, with 10 identical mac address values;

It now fails without the pk ordering fix;

Test that MAC addresses with the same value are ordered by their primary key (pk). ... FAIL

======================================================================
FAIL: test_mac_address_ordering_with_duplicates (dcim.tests.test_models.MACAddressTestCase.test_mac_address_ordering_with_duplicates)
Test that MAC addresses with the same value are ordered by their primary key (pk).
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/netbox/netbox/dcim/tests/test_models.py", line 82, in test_mac_address_ordering_with_duplicates
    self.assertLess(
AssertionError: 12 not less than 1 : MAC addresses with value 12:34:56:78:90:AB are not ordered by primary key

----------------------------------------------------------------------
Ran 1 test in 0.027s

and passes with the pk ordering fix;

Test that MAC addresses with the same value are ordered by their primary key (pk). ... ok

----------------------------------------------------------------------
Ran 1 test in 0.028s

OK

[jeremystretch]

Thanks @jseifeddine. I've removed the test as I don't feel it's necessary; there are many other models with a similar pattern for which we don't include an explicit regression test for ordering. With pk added to the model's default ordering this shouldn't be an issue.