Skip to content

Refactor statistics - #1

Draft
CalamityC wants to merge 31 commits into
mainfrom
refactor-statistics
Draft

Refactor statistics#1
CalamityC wants to merge 31 commits into
mainfrom
refactor-statistics

Conversation

@CalamityC

Copy link
Copy Markdown
Collaborator
  • Add a statistics plugin for RDMO.
  • Provide project, user, and catalog statistics with Chart.js.
  • Integrate via a permission-protected view and theme navigation.
  • Support both RDMO 2.5.x and RDMO 3.x.

@CalamityC
CalamityC requested a review from MyPyDavid August 5, 2026 14:19
Comment thread rdmo_plugins_statistics/views.py Outdated
Comment thread rdmo_plugins_statistics/utils.py Outdated
'key': 'project',
'type': 'time',
'model': 'projects.Project',
'filters': {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please leave the model and the filters in the view, in the big chain of the queryset and explicitly one query for Project and one for User. Same for Catalog but it is already like that ;)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, you don't see what I'm up to.
I'd like to make the charts fully configurable. So if you have a requirement for a new chart, all you have to do is add a config entry.
So no, in that setup it does not belong to the view.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, it is nice to make everything hyper-configurable but a Plugin and it's code is also configurable by itself. What other use cases besides these do we have for which it would be relevant?
Also, the Project and User objects are sensitive data and it should be clear as to what happens with that data.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is very time consuming to fill that all into the code. It is one configuration entry per chart, that makes it much easier to maintain. And we should know about how to use that.
I do not understand your rejection.
Maybe we can discuss this in person.

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.

2 participants