Problem with change in Aliases list sorting (Redmine #14015)
-
OK I split this off and moved it to the 23.05 category since that change was put in after 23.01.
-
Sorry I added a footnote to my previous post. Apparently we were both adding comments at the same time and you moved the thread.
so that said: with or without the msort the initial display of the table always matches the order of the records in the config.
I can then click on any of the headers, and the sort changes, with the id's remaining correct as the records move change on the displaySo if your expectation is that the msort is changing the initial data order of the data displayed, that is not happening here.
with the msort as part of the foreach construct the ids are wrong (and the list is not in name order originally anyway)
with the msort above the foreach the ids are correct but the list still isn't in 'name' order
remove the msort line completely and the ids are also correct.Not the end of the world, the list (in my case is small enough) and I can sort it by clicking on the headers if I want and that works. So as long as the id's are correct in the first place it will open the correct record. It is all good.
Let me know if there is something specific you want me to try.
now I want to go back to the original asort, as I can't remember the order of the initial list on load, I believe it was order of the config as well. Thinking about it, the records in the config are in the same order as originally created. The order in the config has never changed.
Thanks
-
And on each of mine the IDs always match the entries no matter what I've done so far.
I just started a fresh system and made three aliases: aaaa, cccc, then bbbb.
The order in the list is aaaa, bbbb, cccc, and the IDs to edit are 0, 2, 1.
So it's still not clear to me why yours don't match when any test I'm running comes out OK.
-
Both the asort and msort give no indication that the list has been sorted.
and clearly in my case msort not giving the results as expected anyway, the initial load of the list is not sorted.
Might it be better to just load the list, (and the ids are correct) then generate a event required to order the list by name (as if clicking on the header) ??
on initial load the table looks like this (in all cases, asort, msort)
After clicking on the header (the appropriate header is changed as)
I could see if there are people with much larger lists that sorting would be important. But then at the same time if (for example) I've sorted that long list on Description, then made a change and the screen refreshes and the default sort goes back to name, that might be annoying too.
In my case not worth the effort to care as long as the ids match what is being loaded, I don't care about the sort order, I can seem them all right there.
So I think for right now, I am going to just revert the existing patch back to the original asort, so as not mess something up if a revised patch "fixes" it at some point down the road.
Thanks
-
@jrey said in Problem with change in Aliases list sorting (Redmine #14015):
now I want to go back to the original asort, as I can't remember the order of the initial list on load,
so I've gone back to the original (asort code)
and the ids still match what I had originally posted as the order of id's 3, 4, 5, 6, 7, 9, 8, 1, 2, 0 and everything work
->> and now that I'm looking at the complete list of 10, they are in alpha order by name (which is why the id's are out of order on the display).So....in short for me, if it ain't broke don't fix it ... no patch.
Again let me know if there is something else you want me to specifically look at.
JR
-
If you think it's correct you must not have any mixed case alias named (some start with uppercase, some lowercase).
asort
is case sensitive and will put all uppercase before all lowercase instead of putting them in actual alphabetical order.asort
:msort
:Unfortunately,
asort
doesn't have a flag to do a case-insensitive sort (that actually works for this -- note there isSORT_FLAG_CASE
but it does work as expected on its own or when used with the natural or string sorts). -
This seems to do the trick:
uasort($aliases, function ($a, $b) { return strnatcasecmp($a['name'], $b['name']); });
-
I pushed a fix for #14015 (again) on 23.05 that fixes it there in a better way, but I also added a new patch for the system patches package on 23.01 that is a minimalist application of the sorting change alone.
I had to do it that way because the code for aliases in 23.05 diverged too much after 23.01 to apply the 23.05 fix directly.
- 23.05 fix: https://github.com/pfsense/pfsense/commit/5c4a6ada1867a7d6ec13461680d8309d154c90b1
- 23.01 fix: https://raw.githubusercontent.com/pfsense/FreeBSD-ports/devel/sysutils/pfSense-pkg-System_Patches/files/usr/local/pkg/patches/4342d179afe732baf3a73e2db4381b57b1aa7703.p2.patch
Updating the system patches package to 2.2.1 when it's ready will also get the fix, you can just apply the new patch on top of the old patch and it will do the right thing.
-
-
-
-
-
Okay so I reapplied the first patch to setup for "P2" version and installed that.
All good,
Much better now that editing loads the record you expect, and bonus (I guess) they are in order.
Thanks for looking into this so promptly,
(the only rules with lower case letters at the start are pfB aliases, and 1 alias that I had created for testing myTest
JR
-
@steveits said in Problem with change in Aliases list sorting (Redmine #14015):
I am not seeing a problem
Just for the record I did eventually notice this on a (different) router, in our data center. Fixed by the patch part 2.
-
@steveits said in Problem with change in Aliases list sorting (Redmine #14015):
Just for the record I did eventually notice this on a (different) router, in our data center. Fixed by the patch part 2.
See I'm only half crazy - lol - which I think with your confirmation now makes you the other half.
Thanks for the confirmation - good to know I'm not the only one.and I for one am really glad it is "sorted" out.