Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Locations are not sorted naturally #6198

Closed
abrahamvegh opened this issue Apr 19, 2021 · 17 comments
Closed

Locations are not sorted naturally #6198

abrahamvegh opened this issue Apr 19, 2021 · 17 comments
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application

Comments

@abrahamvegh
Copy link
Contributor

abrahamvegh commented Apr 19, 2021

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 abrahamvegh added the type: bug A confirmed report of unexpected behavior in the application label Apr 19, 2021
@abrahamvegh
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@abrahamvegh
Copy link
Contributor Author

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

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application and removed type: bug A confirmed report of unexpected behavior in the application labels Apr 20, 2021
@jeremystretch jeremystretch self-assigned this Apr 20, 2021
@jeremystretch
Copy link
Member

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

@jeremystretch
Copy link
Member

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.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Apr 22, 2021
@jeremystretch jeremystretch removed their assignment Apr 22, 2021
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jun 22, 2021
@jeremystretch jeremystretch added status: blocked Another issue or external requirement is preventing implementation and removed pending closure Requires immediate attention to avoid being closed for inactivity status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jun 22, 2021
@jeremystretch
Copy link
Member

Marking this as blocked by #6587

@bellwood
Copy link
Contributor

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 <[email protected]>, 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
Copy link
Member

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
Copy link

benwa commented May 23, 2022

Would using Collation help?

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

@jeremystretch
Copy link
Member

Relates to #11279

@jeremystretch
Copy link
Member

Blocked by #12552

@jeremystretch
Copy link
Member

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

@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation and removed status: blocked Another issue or external requirement is preventing implementation labels Dec 27, 2023
Copy link
Contributor

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 github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Mar 27, 2024
Copy link
Contributor

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.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

5 participants