Opened 13 years ago
Closed 13 years ago
#3474 closed defect (bug) (fixed)
Active 3rd party components are listed as Decativated in the BuddyPress->Pages screen
Reported by: | sbrajesh | Owned by: | |
---|---|---|---|
Milestone: | 1.5 | Priority: | minor |
Severity: | normal | Version: | 1.5 |
Component: | Administration | Keywords: | |
Cc: | sbrajesh, johnjamesjacoby |
Description
As some changes were made in #3422 and #3428, I have noticed something strange.
Now a component must be associated with a page by the site admin, that's all good. The problem is, even if the page is associated and the component is functional, it is not updated in $bp->active_components. Reason, $bp->active_components is populated from the site meta 'bp-active-components' which only contains information about the BuddyPress core components(saved via BuddyPress->Components screen).
As a 3rd party plugin developer, are we supposed to hook to the filter 'bp_active_components' to show our components active in the list. regardless of the notice(deactivated), the 3rd party plugins work.
I am concerned about the feelings of a Site admin and the no. of questions It will cause on the forums.
I have attached a screenshot to saw what it looks like.
Any thought about it ?
Attachments (3)
Change History (16)
#2
@
13 years ago
- Cc johnjamesjacoby added
- Priority changed from normal to minor
- Type changed from enhancement to defect
#3
@
13 years ago
Our team noticed that the action 'bp_active_external_components' is fired twice in bp-core-admin.php, at lines 427 and 461. Because of the way this code operates, there's no way for external components to determine state and only inject their HTML on one of the actions.
A plugin hooking on the function would have their form fields included twice. I suspect this would confuse the processor function and prevent the component from being treated as an active component by BP.
I suggest the included in the patch file I've attached.
F
#5
@
13 years ago
Good spot about the duplicated action name. However, I think those do_actions should just be removed as that admin screen is only meant to toggle core components on and off. Third party plugins will hook into somewhere like bp_include and thus be activated when that plugin is activated!
If these are taken out, perhaps 'bp_admin_optional_components' and 'bp_admin_required_components' should be removed, too.
#6
@
13 years ago
To expand on what DJPaul says: we don't really want to allow plugins to put themselves on the Components screen, because at the moment there is nothing built into BuddyPress that would allow it to "disable" a third-party plugin (without actually deactivating the WP plugin). In the future we might have a system whereby plugins can more formally register as BP-dependent, so that BP will be able to disable them without deactivation, but for now I'm with DJPaul that these hooks probably shouldn't be there.
#7
@
13 years ago
I'd vote for removing those hooks too. If you don't want people to use it, don't leave it in the code. Patch attached.
#9
@
13 years ago
Thanks for the patches, foxly, and sorry for neglecting to give you props on that last commit
#11
@
13 years ago
During last week's dev chat, after much discussion, it was arrived at that the 'deactivated' message on this page should probably just be removed altogether. When the 'deactivated' message was first introduced, the Pages panel was populated by a hardcoded list of components (Members, Groups, Activity, etc). In r4290, this was changed: in order to support non-core components on the Pages interface, we swapped out the hardcoded list, and looped through $bp->loaded_components instead (where BP_Component extensions register themselves automatically, and where legacy plugins are loaded by bp_core_add_root_component()). One result of this change is that there will never be a deactivated component on the Pages interface. For that reason, the message can be removed.
#12
@
13 years ago
As part of the fix for this ticket, I'm also going to remove the "Some of your WordPress pages are linked to BuddyPress Components that are disabled" message that appears in the Dashboard when you have a WP page linked to a component that is not activated. The 'Repair' button that appears in that message points to the Pages panel, but deactivated components don't appear there, so there's no way to "repair" it. Which is fine, IMO - the only reason why it matters if the pages are "orphaned" like this is that it might continue to show up in the nav, but this can be easily solved by unpublishing the page.
The only thing that $bp->active_components really does is tell BP which of its core components should be loaded during the bootstrap process. This is not necessary for plugins, because they are not part of the BP bootstrap (they get loaded independently and directly by WordPress's plugin architecture). Thus, there's no good reason to require that plugins load themselves into $bp->active_components. (The other place where active_components are referenced is in bp_is_active(), which is why you're seeing the 'deactivated' message. But this is not really a meaningful way to check whether a component - other than a root component - is active.)
That said, it appears that at one time, this *was* the recommended behavior. See the BuddyPress Skeleton Component: http://plugins.trac.wordpress.org/browser/buddypress-skeleton-component/trunk/includes/bp-example-core.php#L90 Plugins that are based on the BPSC (like Achievements, which was one of the main plugins I used during plugin tests - which explains why I didn't see this issue earlier) will probably not exhibit the problem, since they mostly have just copied what the Skeleton Component, and indeed the old core components, have always done.
On the other hand, the new BP_Core_Component class registers components in the $bp->loaded_components array. That means that plugins based on BP_Core_Component (which in the future should be most plugins) will exhibit the problem.
There are a bunch of ways we could go with this. My thinking is that to maximize backpat and also to support the (far improved) architecture of BP_Core_Component (which keeps activated components separate from those that have successfully loaded, which is IMO as it should be), we should adopt a two-tined strategy:
1) In bp_is_active(), we should check $bp->loaded_components in addition to $bp->active_components. Thus BP_Core_Component plugins won't have the problem.
2) We recommend that plugin authors register themselves manually in the $bp->active_components array, if they're using the old method.
I would like some feedback from another core dev on this, especially JJJ as I think he was the one who juggled $bp->active_components and $bp->loaded_components and so will be in a better position to say how they should be differentiated.