Rework "group theme" feature

It does seem like this themeing shouldn't be part of the _init hook, since it keeps breaking things. I think I understand that the reason it's here is to be sure to establish the 'group_context' as early as possible. But I wonder if we could just slim down the _init code to do the minimal group_context calculation and leave all the more complex stuff (e.g., things that invoke the t() function, see: http://drupal.org/node/65801), till later?

I probably have to study the code some more to understand why a module is doing theming anyway.

But in reply to gerhard's suggestion - I love css too, but no one likes to be dependent on it (e.g. backward browser compatibility).

okay, just to get moshe involved here .. I took the advice of hook_init documentation and renamed og_init to _og_init and then called it from og_menu(!$maycache), and it seems to work. I assume that there are cases where this won't work or else moshe would have done this (though this reminds me of this suite of microsoft windows developer test tools that were supposed to check whether your code abided by their api rules, and of course all of the microsoft office suite failed abysmally ..).

Conclusion - hey, moshe, why did you put all that code in og_init! Or more specifically, which is the exact thing that you need to do in hook_init. Because your current code in og_init is calling node_load, which seems like a very bad thing indeed...

An issue/patch posted to move the theme code to hook menu: http://drupal.org/node/67106

i put it here because the views integration required it happen early in the request. the views stuff might be better fixed with module_weight but that may have other repurcussions.

the css only solution has some appeal, i admit. but thats not a total solution. the og_theme() function does a lot more now than setup a group specific theme.

I'm a little confused about the integration with views (not having used that module yet). In og_init, there are two different things done now: one is to set the theme, the other is to include the file "og_views.inc" if the views module is installed. I don't think including "og_views.inc" in og_init() causes any problems and in my patch I left this in og_init(). However, I'm trying to understand if you're saying that the interaction with the views module is also dependent on having the theme set?

Sorry, i looked at history and it was the group specific locale that caused me to move og_theme() to hook_init(). if we can figure out how to do the locale stuff later in the request, we can move the code out of init. the menu items need to be in the group specific locale which i think precludes us from putting the code in hook_menu(!$may_cache). not sure though.

Ok, I can see how it would be necessary to set the group-specific locale early in the process, but it's still true that the current arrangment causes problems with the use of local.module for a site-wide translation/substitution.

getting back to the theme issue, i think i will rework og_set_group_context() so that it also sets the theme and thus any module could just call this function and have all the right stuff happen. would that suffice?

I haven't looked yet in detail at your patch at http://drupal.org/node/65801#comment-105086 so I'm not sure how these proposals interact. Having a general-purpose og_set_group_context() certainly sounds useful for og-addon modules.

re #8: i still haven't understood everything going on in the init_hook, so I hope this isn't too dumb .. but I do like to do all my theme-related stuff within my theme. You're offering og_set_group_context() as a function for other modules to call, but wouldn't it make most sense for some of it to be left as a themeing helper function? As I understand it, because a posting could be a part of more than one group, you have this function as a way to decide which of the groups is setting the 'context', where context includes og menus/blocks as well as allowing for a custom theme (so you can make different group pages look different).

Conclusion - I think you need some of the group context figured out in the hook_menu(!$may_cache), but you can leave the custom theme selection to the theme, just providing a function that returns (presumably as a static var cache trick) the current 'group context' (i.e., gid).

this context function was born a long time ago

Was this actually fixed b/c I'm still having this issue on our site where you change the theme and it doesn't change for the group at all. Anyone else experiencing this?