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

Fixes #18594: asn_count sort in Sites list #18634

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jnovinger
Copy link
Contributor

Fixes: #18594 - Site list can now be sorted by the asn_count column

Changes:

  • adds asn_count annotation
  • removes now unnecessary accessor argument for asn_count column

@jnovinger jnovinger requested a review from bctiemann February 12, 2025 20:16
Copy link
Contributor

@bctiemann bctiemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProviderListView needs this same fix.

@jeremystretch
Copy link
Member

@jnovinger could you please apply the same fix for the asn_count column on circuits.tables.ProviderTable?

@jnovinger
Copy link
Contributor Author

@jnovinger could you please apply the same fix for the asn_count column on circuits.tables.ProviderTable?

Fix committed, looking to see what testing opportunities we might have.

@jnovinger
Copy link
Contributor Author

Fix committed, looking to see what testing opportunities we might have.

Looks like we don't currently have any tests for various table subclasses, outside of one small example. While I think it's worthwhile to add tests, seems like it might be outside the scope for this ticket, especially since the change in behavior is not using an in-house customization. @jeremystretch , thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot Sort By asn_count Column For Sites Table And Providers View Table
3 participants