netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Try NetBox Cloud free: https://netboxlabs.com/free-netbox-cloud/
http://netboxlabs.com/oss/netbox/
Apache License 2.0
16.11k stars 2.58k forks source link

Locations are not sorted naturally #6198

Closed abrahamvegh closed 5 months ago

abrahamvegh commented 3 years ago

NetBox version

2.11.0

Python version

3.7

Steps to Reproduce

  1. Create a Location named 'Floor 2'
  2. Create a Location named 'Floor 10'

Expected Behavior

In areas where Locations are displayed (e.g. /dcim/locations, /dcim/sites/N/), I would expect Floor 2 to be sorted prior to Floor 10.

Observed Behavior

Floor 10 is sorted prior to Floor 2.

abrahamvegh commented 3 years ago

This is in a similar vein to #6179, and I support that too. Pretty much anywhere in the UI, natural sorting is what I expect.

jeremystretch commented 3 years ago

Pretty much anywhere in the UI, natural sorting is what I expect.

Easier said than done, I'm afraid. The solution I came up with in NetBox is NaturalOrderingField, which stores a "naturalized" copy of a name in the database alongside the original. The naturalized version is used for database ordering rather than the actual name. Of course, it would get rather messy trying to add this to every model with a name field.

abrahamvegh commented 3 years ago

Easier said than done, I'm afraid.

Ah, that is a pain. Well, I think I can justify requesting it for least Locations, since they are a higher-touch model (at least for me). 😅

I’m surprised PG doesn’t have any better solutions for this, I would naively assume this is a common issue.

candlerb commented 3 years ago

Minor correction: currently, "Floor 1" does sort before "Floor 10". However, "Floor 10" sorts before "Floor 2" :-(

abrahamvegh commented 3 years ago

Thanks @candlerb, I’ve corrected my report. 😆

jeremystretch commented 3 years ago

Reclassifying this as a feature request simply because RackGroup (now known as Location) was not naturally ordered in previous releases.

jeremystretch commented 3 years ago

This might actually take a bit of effort, since Location is an MPTT-enabled model. We need to change order_insertion_by from name to _name to ensure the proper ordering within the tree, however it doesn't seem like _name is populated in time for reference by MPTT under the current implementation.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

jeremystretch commented 3 years ago

Marking this as blocked by #6587

bellwood commented 3 years ago

Not sure if it's been suggested or not, however, I find the following Python implementation adequate:

# Based on the Perl implementation of Dave Koelle's Alphanum algorithm
# Beau Gunderson <beau@beaugunderson.com>, 2007
# http://www.bylandandsea.org/
# http://www.beaugunderson.com/

#
# Released under the MIT License - https://opensource.org/licenses/MIT
#
# Permission is hereby granted, free of charge, to any person obtaining
# a copy of this software and associated documentation files (the "Software"),
# to deal in the Software without restriction, including without limitation
# the rights to use, copy, modify, merge, publish, distribute, sublicense,
# and/or sell copies of the Software, and to permit persons to whom the
# Software is furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included
# in all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
# USE OR OTHER DEALINGS IN THE SOFTWARE.
#

test_strings = [ "1,3,5", "2,4,6", "7,9,11", "8,10,12", "13,15,17", "14,16,18", "20,22,24", "21,23,25", "1000X Radonius Maximus", "10X Radonius", "200X Radonius", "20X Radonius", "20X Radonius Prime", "30X Radonius", "40X Radonius", "Allegia 50 Clasteron", "Allegia 500 Clasteron", "Allegia 51 Clasteron", "Allegia 51B Clasteron", "Allegia 52 Clasteron", "Allegia 60 Clasteron", "Alpha 100", "Alpha 2", "Alpha 200", "Alpha 2A", "Alpha 2A-8000", "Alpha 2A-900", "Callisto Morphamax", "Callisto Morphamax 500", "Callisto Morphamax 5000", "Callisto Morphamax 600", "Callisto Morphamax 700", "Callisto Morphamax 7000", "Callisto Morphamax 7000 SE", "Callisto Morphamax 7000 SE2", "QRS-60 Intrinsia Machine", "QRS-60F Intrinsia Machine", "QRS-62 Intrinsia Machine", "QRS-62F Intrinsia Machine", "Xiph Xlater 10000", "Xiph Xlater 2000", "Xiph Xlater 300", "Xiph Xlater 40", "Xiph Xlater 5", "Xiph Xlater 50", "Xiph Xlater 500", "Xiph Xlater 5000", "Xiph Xlater 58" ]

import re

re_chunk = re.compile("([\D]+|[\d]+)")
re_letters = re.compile("\D+")
re_numbers = re.compile("\d+")

def getchunk(item):
    itemchunk = re_chunk.match(item)

    # Subtract the matched portion from the original string
    # if there was a match, otherwise set it to ""
    item = (item[itemchunk.end():] if itemchunk else "")
    # Don't return the match object, just the text
    itemchunk = (itemchunk.group() if itemchunk else "")

    return (itemchunk, item)

def alphanum(a, b):
    n = 0

    while (n == 0):
        # Get a chunk and the original string with the chunk subtracted
        (ac, a) = getchunk(a)
        (bc, b) = getchunk(b)

        # Both items contain only letters
        if (re_letters.match(ac) and re_letters.match(bc)):
            n = cmp(ac, bc)
        else:
            # Both items contain only numbers
            if (re_numbers.match(ac) and re_numbers.match(bc)):
                n = cmp(int(ac), int(bc))
            # One item has letters and one item has numbers, or one item is empty
            else:
                n = cmp(ac, bc)

                # Prevent deadlocks
                if (n == 0):
                    n = 1

    return n

test_strings.sort(cmp=alphanum)

for (v) in test_strings:
    print v

Results:

1,3,5
2,4,6
7,9,11
8,10,12
10X Radonius
13,15,17
14,16,18
20,22,24
20X Radonius
20X Radonius Prime
21,23,25
30X Radonius
40X Radonius
200X Radonius
1000X Radonius Maximus
Allegia 50 Clasteron
Allegia 51 Clasteron
Allegia 51B Clasteron
Allegia 52 Clasteron
Allegia 60 Clasteron
Allegia 500 Clasteron
Alpha 2
Alpha 2A
Alpha 2A-900
Alpha 2A-8000
Alpha 100
Alpha 200
Callisto Morphamax
Callisto Morphamax 500
Callisto Morphamax 600
Callisto Morphamax 700
Callisto Morphamax 5000
Callisto Morphamax 7000
Callisto Morphamax 7000 SE
Callisto Morphamax 7000 SE2
QRS-60 Intrinsia Machine
QRS-60F Intrinsia Machine
QRS-62 Intrinsia Machine
QRS-62F Intrinsia Machine
Xiph Xlater 5
Xiph Xlater 40
Xiph Xlater 50
Xiph Xlater 58
Xiph Xlater 300
Xiph Xlater 500
Xiph Xlater 2000
Xiph Xlater 5000
Xiph Xlater 10000
jeremystretch commented 3 years ago

Unfortunately we can't rely on sorting in Python: It needs to be done at the database level. We do have some natural ordering in place for other models as mentioned above, however the use of MPTT for locations complicates things a bit. If we do end up removing MPTT under #6587 that should unblock this issue.

benwa commented 2 years ago

Would using Collation help?

From that StackOverflow post in the beginning of this thread, it looks promising.

jeremystretch commented 1 year ago

Relates to #11279

jeremystretch commented 1 year ago

Blocked by #12552

jeremystretch commented 10 months ago

Unblocking this. Let's see if we can come up with a solution that's compatible with MPTT.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

github-actions[bot] commented 5 months ago

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.