Questions about licensing and some suggested refactoring
|Reported by:||rjollos||Owned by:||rjollos|
May I suggest a few changes (which I'm happy to push myself given approval from the author)?
- Clarify which of the GPL licenses is used. Better yet, would you consider changing the license of this plugin and your other plugins (PrivateCommentPlugin and HideableQueryPlugin) to BSD 3-Clause, which is what the Trac core uses, and what I consider to be the most "friendly" license for Trac development? :)
- Add text of the license in a COPYING file.
- Add license headers to each file.
- Change tabs to spaces source files (per the PEP-008 recommendation). The template contains a mix of tabs and spaces.
- As in , we don't have to close the cursor, and as in  it isn't necessary to print the SQL statements to the debug log since the Trac API handles this.
- As in , we can get the report id from the request args and simplify the code in pre_process_request. reg.args.get('id') will return the ID number when on the /report/id page, and will return None when on the /report page. Actually, you do something very similar in render_admin_panel.
- Allow a user with REPORT_ADMIN and PERMISSION_ADMIN to grant/revoke permissions for a report, as opposed to requiring TRAC_ADMIN.
I haven't thought about this for too long, so maybe I'm overlooking the obvious, but what is the use-case for having admin-defined custom permissions for each report as opposed to just generating a VIEW permission for each report that exists REPORT_VIEW_1, REPORT_VIEW_2, ...? Making this change could eliminate the need for the admin panel. I don't think we'd even need a database table, since all of the permissions could be generated in IPermissionRequestion.get_permission_actions, given the existing report numbers.
Actually, all of this can be done anyway with t:TracFineGrainedPermissions (the accuracy of that statement should be checked given the existence of defects such as t:#11069), so maybe the plugin should just be a nicer interface to editing an authz file for reports? There is currently a FineGrainedPageAuthzEditorPlugin that we plan to integrate into AccountManagerPlugin in #9947. A friendly interface to the authz file could allow a user to make specific reports private without requiring a specialized plugin.
Change History (21)
comment:4 Changed 8 months ago by rjollos
- Owner changed from mhenke to rjollos
- Status changed from new to assigned
comment:20 Changed 8 months ago by rjollos
- Resolution set to fixed
- Status changed from assigned to closed