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

Correct sorting of IP addresses to be numeric not alpha #3888

Merged
merged 4 commits into from Oct 28, 2020

Conversation

JamesTilt
Copy link
Contributor

The IP column sorting function on the automation-> discovered devices is not working when single sorting the column after the page has been loaded once. I was able to see that this is the area of the code that was dropping the INET_ATON from the order by query.


Notes from wrong commit to develop instead of 1.2.x:

This change allows for sort to work properly when sorting on the Automation-> Discovered Devices page using the IP address column. What would happen is when sorting by the IP address field the sort would look like the following:

  • 192.168.2.10
  • 192.168.2.100
  • 192.168.2.22

The INET_ATON was not getting applied when sorting just by the IP address column. It would work when first going to the page after sorting by IP but if attempting to sort while staying on the page it would drop the INET_ATON(ip) to be just ip. It was also working properly when multi-sorting. This seems to only be an issue when clicking on the sort of the IP address column.

I attempted to follow the other places where this code looks to have been run to come up with a similar solutions.

TheWitness replied:

This isn't quite right, you have to inspect the entirety of the sort list which becomes problematic with multi-column sort. This has to be given some thought. It should also be against the 1.2.x branch an not develop.


In response I can see that the multisort happens in the next if statement below the one that I am editing. From what I can tell that if statement where I did make the changes would not work with a multisort as I think it is only when the columns are reset or only doing one column sort action at a time.

image

I have a couple print statements with the sort query and the full SQL query above the table. I also have a flag to let me know which if statement I am in.

That image is being sorted on the hostname column but it is the same idea. As you can see the INET_ATON(hostname) is not there. Which in a multisort or the initial load of the page it drops down to the third if statement in the block of code from 779-813.

Thanks

The IP column sorting function on the automation-> discovered devices is not working when single sorting the column after the page has been loaded once. I was able to see that this is the area of the code that was dropping the INET_ATON from the order by query.
@TheWitness
Copy link
Member

Can you create a CHANGELOG entry and update your pull request.

@JamesTilt
Copy link
Contributor Author

I saw the code documentation guidelines after I put this in and saw that I should have put in an issue before putting in the pull request. I have now put in the issue.

Is that you are referring to with the CHANGELOG entry or do you have a instructions on how you want the CHANGELOG handled.

Thanks

@JamesTilt
Copy link
Contributor Author

Let me know if that is how you wanted it. Thank you for your patience as this is my first time going through this process.

We use hard tabs
@TheWitness
Copy link
Member

I made a light change to switch your tabs to "hard" ones.

@TheWitness TheWitness merged commit ef765df into Cacti:1.2.x Oct 28, 2020
@netniV netniV changed the title Update html_utility.php to fix issue with IP sort Correct sorting of IP addresses to be numeric not alpha Nov 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants