Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#1737 closed enhancement (fixed)

Refactor how custom reports are stored/queried into a separate class

Reported by: coling Owned by: bobbysmith007
Priority: normal Component: TimingAndEstimationPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.10

Description

Hi,

As you know from private email I've been working on a refactor to make it simple to incorporate reports for T&E into other plugins (e.g. my ClientsPlugin).

Well I have now done this and I attach the results :)

Here are the diffstats

 setup.py                                       |   11 -
 timingandestimationplugin/api.py               |  220 ++++++++-----------------
 timingandestimationplugin/dbhelper.py          |   38 ----
 timingandestimationplugin/migrate/upgrade4.py  |   10 -
 timingandestimationplugin/reportmanager.py     |  142 ++++++++++++++++
 timingandestimationplugin/reports.py           |    9 +
 timingandestimationplugin/templates/billing.cs |    4 
 timingandestimationplugin/usermanual.py        |   17 +
 timingandestimationplugin/webui.py             |   43 +---
 9 files changed, 258 insertions(+), 236 deletions(-)

I'll walk through the major changes:

  • Introduced a new reportmanager.py file/class
    • This file is designed to be stand alone and independent (e.g. freely copyable to other plugins etc. It maintains it's own db schema version and will raise an exception if two plugins use different version on a given install. It provides methods to add/update reports and retrieve listings of reports etc. It has a concept of maingroup and subgroup for reports. This allows me to use the plugin name ('Timing and Estimation Plugin') as the maingroup and the two categories of report for the subgroup. These subgroups are purely cosmetic.
  • I introduced the concept of each custom report having a uuid.
    • Although not strictly speaking necessary in this plugin, some plugins may require to reference specific reports and a uuid gives a way for them to do this in code. I generated uuids for each of your reports.
  • I significantly simplified the code in api.py
    • This was possible due to the fact that the report stuff was refactored. The various tests for upgrades required were also significantly simplified as this is now based purely on a version stored in the system table. This was used somewhat before, but not overly wholistically, so this refactor changes that. This in turn has resulted in the removal of some methods from dbhelper.py that are now no longer called.
  • Simplified DB Schema tracking.
    • As the reportmanager.py introduced a simple DB schema version tracking, I converted your current migrate/upgrade?.py system to use a similar model. This is a much simpler implementation of schema tracking and keeps the schema representation in one place which can aid visualisation. I appreciate you may not like this but as reportmanager.py has to be self contained I figured using a similar implementation here would be advantageous.
  • Efficiency Improvements
    • As reports are accessed 100% from the database at runtime, I only import the reports.py file dynamically at the relevant part of the upgrade. This should save some resources on every page hit as api.py has to be parsed. If the version numbers are migraged from usermanual.py to api.py another efficiency saving could also be had, but this is beyond the scope of my patch.
  • Template Update
    • I updated the billing.cs template appropriately and changed slightly how the data was passed to this template as a result of refactoring the code to the reportmanager.py class.

I think that's about it, but if you have any questions please just shout and I'll do my best.

Hopefully this is acceptable to you and I've not wasted my time!

The patch applies cleanly on the trac0.10 branch but a few hunks fail on the trac0.11 branch. It should be easy enough to make it work and it may infact work if the changes on 0.10 branch are merged to the 0.11 branch first (not sure if the 0.11 branch has been synced from the 0.10 one for a while).

I tested this on a clean install and on an upgrade. Both worked fine.

I have not tested the SQL on postgres or mysql but I can't see any reason as to why it would not work as the SQL is all fairly simplistic.

Let me know if you have any comments.

Cheers

Col

Attachments (1)

reportmanager.diff (31.2 KB) - added by coling 7 years ago.
Convert to general purpose report management

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by coling

Convert to general purpose report management

comment:1 Changed 7 years ago by coling

As I discovered recently on upstream trac's bug tracker, Trac's built in diff viewer for attachments is broken :(

The raw version works as expected which is the important thing.

comment:2 Changed 7 years ago by bobbysmith007

Its a holiday here, but I wanted to let you know that I am looking at this and will try to get this applied in the next day or two.

Russ

comment:3 Changed 7 years ago by bobbysmith007

ps, sounds good so far

comment:4 Changed 7 years ago by coling

Great :)

There is one small comment in the code that no longer applies: It's in api.py and it concerns the upgrade to schema 5. It says "all custom reports will be lost (deleted)". This is not true as they will not be deleted, they will just be de-associated with this plugin (they were deleted in a previous revision but I made it slightly nicer). Migrating properly would have been a pain, so I figured that not many users would have added their own reports yet, so this shouldn't a major blow!

Speak later. I'll work on some other patches - just some general UI improvements (e.g. Javascript date picker etc.).

comment:5 Changed 7 years ago by bobbysmith007

  • Resolution set to fixed
  • Status changed from new to closed

(In [2390]) closes #1737

Applied Colin Guthrie's patch to make managing custom reports easier.

Thanks for the patch!

comment:6 Changed 7 years ago by coling

Thanks Russ :D

comment:7 Changed 7 years ago by coling

(In [2393]) Refs #1737. Adds TimingAndEstimationPlugin reports and provides automatic setup.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from bobbysmith007. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.