Reports Project

Software improvement (Archived)
27

Reports Code Analysis

I did some general analysis of the existing reports code, looking at where the issues and complexities are and what would be required in a refactor/rewrite.

General Issues

  • the classes that define each report are scattered all over the codebase (it’s incredible how many different locations they reside in)
  • in some cases the structure and methods used are quite different from one report to the next, there are various approaches and styles
  • a lot of the code is obtuse and difficult to follow
  • the reports are quite inflexible, so for example if we wanted to add a nice UX feature like the ability to choose which columns were exported (show/hide) when downloading the report as a spreadsheet, it would be a nightmare
  • the code around permissions needs improving. It’s not reports-specific, it applies to the whole app

Grouping and Summarising

The primary point of code complexity is in the places where we apply rules for grouping results and adding “summary rows” for those groupings. So for example, a report might list line_items grouped by order, and after each set we inject a row that shows some summed/aggregated values for selected columns, like quantity total, before showing the next set, eg:

---------------------------------
| order   | item     | quantity |
---------------------------------
| order1  | carrots  | 5        |
| order1  | broccoli | 3        |
---------------------------------
| TOTAL   |          | 8        |
---------------------------------
| order2  | carrots  | 2        |
| order2  | parsnips | 1        |
---------------------------------
| TOTAL   |          | 3        |
---------------------------------

This “grouping results and injecting summary rows” is a key part of the report-generating process. If we can find a way to do it really nicely and cleanly, we can make everything a lot prettier.

Masking Customer Data

We have some really nasty code for conditionally hiding certain customer info in a few reports based on permissions. Summary of the current state:

  • The code for hiding customer data is really awful in terms of performance and gets run in every single report that queries line items (most reports).
  • There are only actually 5 reports out of 27 where customer data even potentially needs to be hidden.
  • Of these 5 reports, in some cases there is only a single field hidden. In other cases there are also other fields present in the report that should probably not be shown to these users with limited permissions either, but they are shown.
  • There are two separate versions of this same code for hiding the customer data, kept in two different places (see Reports::LineItems and OrderAndDistributorReport).

As with the “grouping and summarising”, if we handled this bit nicely we could could clean everything up and radically improve performance at the same time.

Broad Objectives

  • Report controllers should be slim and clean
  • Report classes where we define what data should be fetched and how the data should be grouped and aggregated (including “summary rows”) should be: slim, easy to read, easy to change and easy to create
  • Once a report object is created, it should contain a nice representation of the resulting dataset which is easy to manipulate and easy to render/convert to a broad range of formats
  • We should be able to render that object directly to (for example): a HTML table, a CSV spreadsheet, an Excel spreadsheet, an OpenOffice spreadhseet, JSON, etc
  • For UX purposes the flexibility of the report object is really important
2 Likes