#1737 closed enhancement (fixed)
Refactor how custom reports are stored/queried into a separate class
Reported by: | Colin Guthrie | Owned by: | Russ Tyndall |
---|---|---|---|
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
andsubgroup
for reports. This allows me to use the plugin name ('Timing and Estimation Plugin') as themaingroup
and the two categories of report for thesubgroup
. Thesesubgroups
are purely cosmetic.
- 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
- 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 fromdbhelper.py
that are now no longer called.
- 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
- Simplified DB Schema tracking.
- As the
reportmanager.py
introduced a simple DB schema version tracking, I converted your currentmigrate/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 asreportmanager.py
has to be self contained I figured using a similar implementation here would be advantageous.
- As the
- 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 fromusermanual.py
toapi.py
another efficiency saving could also be had, but this is beyond the scope of my patch.
- As reports are accessed 100% from the database at runtime, I only import the
- 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 thereportmanager.py
class.
- I updated the
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)
Change History (8)
Changed 17 years ago by
Attachment: | reportmanager.diff added |
---|
comment:1 Changed 17 years ago by
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 17 years ago by
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:4 Changed 17 years ago by
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 17 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:7 Changed 17 years ago by
(In [2393]) Refs #1737. Adds TimingAndEstimationPlugin reports and provides automatic setup.
Convert to general purpose report management