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

Possibility to order reports #6973

Closed
maximumG opened this issue Aug 17, 2021 · 6 comments · Fixed by #7369
Closed

Possibility to order reports #6973

maximumG opened this issue Aug 17, 2021 · 6 comments · Fixed by #7369
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@maximumG
Copy link
Contributor

maximumG commented Aug 17, 2021

NetBox version

v2.11.7

Feature type

Change to existing functionality

Proposed functionality

State of the art

As of Netbox 2.11.x, reports are automatically discovered and rendered in the UI (extras/reports/). The order of the reports is based on alphabetical order.

As an exemple, if you have a python module named DCIM-report.py with 3 Reports subclasses named report_a, report_b and report_z, the order in which reports are rendered in the report list depends upon their name.

  1. report_a
  2. report_b
  3. report_z

The code handling this list can be found here:

def get_reports():
"""
Compile a list of all reports available across all modules in the reports path. Returns a list of tuples:
[
(module_name, (report, report, report, ...)),
(module_name, (report, report, report, ...)),
...
]
"""
module_list = []
# Iterate through all modules within the reports path. These are the user-created files in which reports are
# defined.
for importer, module_name, _ in pkgutil.iter_modules([settings.REPORTS_ROOT]):
module = importer.find_module(module_name).load_module(module_name)
report_list = [cls() for _, cls in inspect.getmembers(module, is_report)]
module_list.append((module_name, report_list))
return module_list

Proposal

Use case

We propose the possibility for the developer to order the report's listing inside a python module. This can possibly help Ops team to follow each report and correct corresponding errors one after another.

Implementation

The Report class should have a new attribute named order with a default value of -1. Developer would have the possibility to order their reports based on this attribute, the biggest number being the first report in the list.

# Report base class (netbox/extras/report.py)
Report(object):
[...]
    order = -1
    [...]

# some custom reports under netbox/reports/my_custom_report.py
MyCustomReport(Report)
[...]
    order = 999

MyCustomReport2(Report)
[...]
    order = 1000

Netbox source code should also take into account this attribute in order to reorder the report list inside each python module.

 def get_reports(): 
     """ 
     Compile a list of all reports available across all modules in the reports path. Returns a list of tuples: 
  
     [ 
         (module_name, (report, report, report, ...)), 
         (module_name, (report, report, report, ...)), 
         ... 
     ] 
     """ 
     module_list = [] 
  
     # Iterate through all modules within the reports path. These are the user-created files in which reports are 
     # defined. 
     for importer, module_name, _ in pkgutil.iter_modules([settings.REPORTS_ROOT]): 
         module = importer.find_module(module_name).load_module(module_name) 
         report_list = [cls() for _, cls in inspect.getmembers(module, is_report)]
         report_list.sort(key=lambda x: x.order, reverse=True)
         module_list.append((module_name, report_list)) 
  
     return module_list 

Use case

Ordering reports on the WebUI would guide Ops team to check each report and correct the pinpoint errors one after another.

Database changes

N/A

External dependencies

N/A

@maximumG maximumG added the type: feature Introduction of new functionality to the application label Aug 17, 2021
@jeremystretch
Copy link
Member

It might be a bit cleaner to allow the declaration of an ordered list within the module, akin to Django's field_order parameter for forms. Something like:

reports = ('report_a', 'report_b', 'report_z')

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Aug 17, 2021
@maximumG
Copy link
Contributor Author

It might be a bit cleaner to allow the declaration of an ordered list within the module

You are totally right about that. I would propose to have a variable named report_order within the module as you mentioned. This variable contains the ordered list of Report subclass at the end of the module.

Exemple with a mycustomreport.py module

class MySuperReportZ(Report):
[...]

class MySuperReportA(Report):
[...]

class MySuperReportC(Report):
[...]

report_order = (MySuperReportA, MySuperReportC, MySuperReportZ)

I would then modify the original get_reports function to implement ordering of Reports:

  1. Gather dynamically the report_order variable from within the module
  2. Gather dynamically all Reports inside the module (as it is currently done)
  3. Compare both lists above and append missing reports at the end of the report_order list
def get_reports():
    """ """
    module_list = []

    # Iterate through all modules within the reports path. These are the user-created files in which reports are
    # defined.
    for importer, module_name, _ in pkgutil.iter_modules([settings.REPORTS_ROOT]):
        module = importer.find_module(module_name).load_module(module_name)
        ordered_reports = set([report for report in getattr(module, "report_order", ()) if is_report(report)])
        available_reports = set([cls for _, cls in inspect.getmembers(module, is_report)])
        ordered_reports.update(available_reports - ordered_reports)
        report_list = [cls() for _, cls in ordered_reports]
        module_list.append((module_name, report_list))

    return module_list

If you agree with the implementation I can handle this feature with the code modification, test cases and documentation update.

@jeremystretch
Copy link
Member

Sorry, this fell off the radar. Please have it if you're still available @maximumG!

Also, you can likely simplify the above logic a bit:

  1. Get all reports dynamically as dynamic_reports (current behavior)
  2. Remove any reports included in module_list from dynamic_reports
  3. Return a new list of [*module_list, *dynamic_reports]

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Sep 16, 2021
@maximumG
Copy link
Contributor Author

Don't worry about the delay in your answer. We still need this feature internally so I will handle this and try to implement what you described above.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Sep 16, 2021
@maximumG
Copy link
Contributor Author

@jeremystretch We also have the same needs to order Script within a module. Do you want me to create another FR dedicated to this ?

jeremystretch added a commit that referenced this issue Sep 28, 2021
@jeremystretch
Copy link
Member

@maximumG Yes, let's handle that in a separate FR. Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants