New plugin architecture patch

Project:ProjectPier
Version:0.8.5.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:patch - code needs review
Description

This patch adds an extensible plugin architecture to PP.

It includes:
1) Plugin manager to allow actions and filters to be added/removed by plugins
2) Plugin manager to execute actions and apply filters
3) Administration interface to allow plugins to be activated/de-activated
4) A few hooks
5) A demonstration plugin

Patch made on SVN trunk rev.147

AttachmentSize
plugin-arch.patch88.19 KB

This looks great!!
I'm trying it out now.
First thing I noticed though, is that your patch contains your database password and username

Ok,
First off there is a comma in your sql, which is causing an error at installation:

CREATE TABLE `<?php echo $table_prefix ?>plugins` (
  `plugin_id` int(10) unsigned NOT NULL default '0',
  `title` varchar(100) <?php echo $default_collation ?> NOT NULL default '',
) ENGINE=InnoDB <?php echo $default_charset ?>;

When I removed the comma after default '' (Line 147) it installed ok.

Next, for some reason there were two copied of the same code in each of your files, which was causing 'class already been declared' errors.
At first I thought this was me accidentally patching it too many times (I'm new to patching), but when I look in the patch file I see:

===================================================================
--- application/models/plugins/base/BasePlugin.class.php (revision 0)
+++ application/models/plugins/base/BasePlugin.class.php (revision 0)
.......
.....
===================================================================
--- application/models/plugins/base/BasePlugins.class.php (revision 0)
+++ application/models/plugins/base/BasePlugins.class.php (revision 0)
.......
.....
===================================================================
--- application/models/plugins/base/BasePlugin.class.php (revision 0)
+++ application/models/plugins/base/BasePlugin.class.php (revision 0)
.......
.....
===================================================================
--- application/models/plugins/base/BasePlugins.class.php (revision 0)
+++ application/models/plugins/base/BasePlugins.class.php (revision 0)
.......
......

Again, I don't know much about patches, so this might be normal....

I went through most of the files, and deleted the duplicate code.
I went to the admin tab and tried to enable the links plugin, and get this error:

Plugins failed to activate: Project Links (Query failed with message 'Unknown column 'name' in 'field list'')

Apart from the above, it looks promising :)

EDIT:
Ok, I changed the table column name from title to name - That solved the plugins' failed to activate error

Now it says plugins' activated, but with an error flash, and the radio button for Project links is set at deactivated.
However, it has created the db table pp_project_links but the project tabs (forms etc.) are not hidden.

Ok, for some reason my patches are giving exactly the same results as yours(the files you made have duplicated code).
I found an error in the your table structure though. It should be:

CREATE TABLE `<?php echo $table_prefix ?>plugins` (
  `plugin_id` int(10) unsigned NOT NULL auto_increment,
  `title` varchar(100) <?php echo $default_collation ?> NOT NULL default '',
  PRIMARY KEY  (`plugin_id`)
) ENGINE=InnoDB <?php echo $default_charset ?>;

It needs a primary key in order to work, else no plugins can be enabled.

Thanks for the feedback Alex. Fixes from above have been applied and new FULL patch uploaded. For some reason TortoiseSVN insists on creating doubles of some files.

AttachmentSize
plugin-arch.ver2.patch77.68 KB

Nice patch. ProjectPier was in dire need of a plugin system, and i think this pretty much fills in that gap. It's also simple and to the point, which i imagine will encourage third party development.

One criticism i have about this patch however is it does not appear to handle new permissions in the UI. For example if i had a plugin "Cake" i might want to add a permission "Can eat cake" so i could restrict cake eating in a project to those who deserve it.

(I'd imagine one would have to play about with the ProjectUsers class to make it more extendable)

Still, great work! :)

Thanks. The supplied patch is meant as a starting point to get the discussion moving on where to place hooks throughout the code. So far, as a proof of concept, I have only placed hooks in the menu areas of PP. Hooks could very well be placed in the permissions config area of PP etc.

The sample plugin does have user protection to make sure that only members of the owner company or an administrator can create, update or delete while all project members can view links associated with the project.

I found from writing the sample that it was more involved than writing for something like Wordpress and probably more like writing for Mambo/Joomla, in that the plugin files were spread across multiple existing folders (i.e. controllers, models, views). In presenting this plugin framework as a starting point, maybe we can move toward an easier structure for plugin writers to write for. This could be an uploader and unpacker that distributes files to their correct locations or maybe some mods that would allow each plugin to maintain all files in their plugin folder and allow PP to pick up whats needed in each folder.

Not sure what the way to go is and I am sure I know much less about the framework than others here but lets get talking.

New FULL patch version to include auto_increment on the plugins table.

AttachmentSize
plugin-arch.ver3.patch77.68 KB

Just curious if you're part of the development mailing list or might come chat in the IRC channel.

Version:» 0.8.5.x-dev

Great start! I have some ideas to toss in...

I vote for finding a way to combine all the related plugin files into one folder. I also think that folder should be entirely separate from the application tree. Something like this:

[pp-root]/plugins/
+- Project_Links/
. . +- index.php <-- "plugin.project_links.php"
. . +- controllers/
. . | . +- LinkController.class.php
. . +- models/
. . | . +- base/
. . | . | . +- BaseProjectLinks.class.php
. . | . | . +- BaseProjectLink.class.php
. . | . +- ProjectLink.class.php
. . | . +- ProjectLinks.class.php
. . +- views/
. . | . +- link/
. . | . +- index.php
. . | . +- edit_link.php
. . +- language/
. . . . +- en_us/
. . . . . . +- project_links.php

This way all the plugin files are grouped together. This makes it easy for users to remove plugins cleanly. Thoughts?

All the source files for deactivated plugins should be excluded from the AutoLoader.

The Plugin system should gracefully deactivate (perhaps only temporarily) any missing plugins.

I really like the WP plugin mechanism that identifies plugins by a description section. Maybe this system could include an "identity.xml" or "identity.txt" to help describe the plugin to the user. Fields would include Plugin Name, Version, Homepage, Description, Author, etc. It could also include an Upgrade URL in preparation for future automated plugin installation/upgrades. (Define the tag now, so authors will include it for the future.)

Anyone have ideas on how to handle errant plugins? If a plugin causes problems, the user should have simple problem resolution options. One idea comes to mind: A "PP safe mode" URL (administrators only) where all plugins are disabled and the admin can disable plugins. It could be linked from the login screen (which might be designated a "plugin free zone").

Moved this to the list thread.