Granulate available og_panels content by administrator type

One thing I forgot - there is a conceptual shift that I implemented here. Previously, you had to be both a group admin AND have the appropriate perm in order to edit group panels. Now, group admin status is sufficient...hmm.

OK, I've thought through this again, and please hold off on looking at the above patch. I need to make some changes.

Yep, that's the perm that had me up in a tizzy. The way I reworked things, I had a little bit too much 'collections' on the brain. I think I wrote the extra menu item in...bleh, well, doesn't matter.

Here's the more relevant question: what's the utility in having the 'manage OG panels pages' perm in addition to checking to see if the user is a group admin?

With the patch in my OP, there wasn't much point in separating these two. However, with the rest of what I'm working on now, it does make sense to have a perm - one that's not 'administer organic groups' - that determines whether or not the person sees certain 'advanced' options (i.e., CSS settings, title overrides, visibility by role, etc.) in the panels configuration screen. I separated them initially because I was thinking of 'manage OG panels pages' as that perm, then got distracted and lost my train of thought. woohoo. Anyway, I can come up with a third perm for that situation if we'd like to separate them out; it just seems redundant to me to require that a person have BOTH group admin status and the perm. Unless I've missed something.

Makes sense. Fair enough - I'll come up with a third perm and re-post the patch. Not like it's difficult to change in the future if this proves unnecessary.

I know that I'm coming at this a lot from the Collections perspective, but I think it also may be because I have something of a different picture in mind when it comes to the potential utility for OG panel pages. Purely personally speaking, if it's something that pertains to a group I'm in then I'd rather use an OG panels page than a panels page. I see it as a question of conceptual unity as well as administrative UI unity/simplicity, both for me as admin and for visitors in terms of finding that content.

Regardless, I'm hoping that the patch I'm working on now meets with both our expectations. It seems to me that if we can provide the flexibility to go either way without hindering performance or usability, then it's a win-win.

Yep, I think it is a question of the module's mission. I think that the idea behind og_collections (and these changes) does boil down to helping group admins build their groups, but for right now, that's neither here nor there.

As it pertains to controlling what appears on the panels ajax content config form, it requires what I think is a very small change to the panels API. I'm talking it over with Earl, but in the meantime, I'm writing this patch with those changes to the Panels API in mind; they should remain happily inert until those API changes go in, at which point they'll pick up without missing a beat. All they do is change various form widgets to 'hidden.'

I've attached another patch, this one with considerably more extensive changes. I believe I've fully taken into account the stuff you committed this morning.

As you'll see, I completely reorganized the groupcontent.inc (and the corresponding hook) so that all the og_panels content types are run through the same handler functions - with space provided for the needs of individual content types - so that we can easily do 'mass' operations, like the inclusion of the 'og_visibility' feature or the removal of certain form widgets (Those two are lines 103-113 of groupcontent.inc). I believe this is more in keeping with the general approach of blocks.inc.

Unfortunately, there's ONE piece of this code that isn't working yet. The problem is dug in...somewhere in the ajax modals. If you leave the og_visibility radio selector at its default value, the modal form submits just fine. If you change it, however, the form hangs, at least on my system. I've been trying to debug this for hours, but my javascript skills & debugging environment are still pretty rudimentary. Since we're working on this so actively, I wanted to get this patch up, especially because everything else is working, as far as I can tell. Plus, I thought you might have a better sense of how to attack that problem. I'm trying to catch merlin, too.

I've also implemented that third permission - 'advanced og panels editing'. I think it fits the bill with what I laid out above, performance-wise, at least - it only gets used in two places (line 407 of og_panels.module, and line 111 of groupcontent.inc), but those two places ought to be enough to fully allow for a simplified edit screen for 'noob' group admins, as well as the fully-featured one for others.

Update - fixed the patch. Disregard #8.

I swear, you put ONE db_query() inside a while loop...

So, what're your thoughts on this most recent patch?

One more thought here - currently, the 'visibility' function that's in the groupcontent.inc file is governing only pane visibility (and with panes, of course, visibility === access). However, there's no reason that it couldn't be moved & slightly generalized to support a similar system for controlling access to the og_panels pages themselves.

Some of the inelegance in that abstraction layer, the substr() bit, will go away once I stop abbreviating. I don't know why, but I keep worrying about there being a maximum function name length that I may bump into.

However, I do think that there are some important tasks the abstraction layer accomplishes. I know I've sketched em out in IRC, but here they are for clarity and posterity:

1. If we're to return only one $item in our implementation of hook_panels_content_types() in og_panels, the abstraction layer is unavoidable. The one clear argument I have for this (that's independent of the rest of these reasons) comes from the structure of the panels_pane table. If we implement them through a single $item & abstraction layer in the .inc file, then all of our content types will be grouped under a single type with varying subtypes. If we keep them separate, then circumstances could arise where we have difficulty tracking panes that are or are not og_panels panes. If we get to the point where we want to run operations against panels_pane where we work from listings of panes that are specific to og_panels, then the BEST case scenario is that that we'd have to run a considerably more complex query (and comparably more db-intensive) in order to approach the same level of accuracy as having them grouped under the same type. In the worst (and ultimately more likely) case, we'd be unable to guarantee at all that we'd be effectively targeting all the relevant panes.
   The clearest use case that comes to mind is actually one very much like the hook_update() in the #9 patch - what if there's some mass update operation that we need to run against all the og_panels panes b/c of a change we've introduced? We might be able to guarantee that on g.d.o we catch all the relevant panes, but if other folks have created their own og_panels content_type items, we'd never be able to catch those in our own update. It could be a nightmare in the queues.
   I'll freely admit that, at least in part, this is circuitous argumentation - if we don't HAVE anything that spans across all our og_panels content types, then there's unlikely to be any need for mass operations. But, I obviously see those as valuable, for the reasons that follow...
2. There are two main 'mass' operations that I've proposed in that code. One is the removal of certain items from the content config form via site vs. group admin, and the other is the visibility settings:
   1. Site/Group Admin: This wouldn't be terribly useful for implementations of og like g.d.o, but that's just one way that og gets used. I'm very keen on this one because I know for a fact that it's something that would be very useful in a social networking context like the one I'm working on. I need mechanisms that reduce interface complexity for most group admins, because I'm operating in a situation where group administrator status does not necessarily correlate technical expertise, and my site won't work if I'm effectively penalizing people without that expertise. I don't think I'm the only one with that problem.
      The interface can be made less complex and more intuitive than what's in the above patch, to the point where default operation could be just one admin 'type,' and only people who need the granularity will even need to know that it's there. Plus, since these restrictions only apply at the administrative level, they have no impact whatsoever on performance.
   2. Visibility - as you've pointed out, OG has its own complex and differing conception of node access even vis-a-vis core, and trepidation is most certainly in order if we're to think about making that system more complex. I agree entirely - I still haven't fully grokked OG's access system, and so in part am definitely deferring to your judgment on this one. At the same time, I know enough about the Panels API now to say fairly confidently that the visibility system should not conflict, and may not even overlap, with any access mechanisms that OG has designed:
      'Visibility' here is being defined strictly with respect to specific, individual panes. Because panes can only be accessed IN panels displays, which in turn have their own authentication mechanisms (that do intersect with OG's), we don't have to worry about people being able to sneak in the back end - simply closing the front door by removing it from view should be sufficient to guarantee that people can access only what they're supposed to access. Additionally, since the whole visibility determination process is taking place within the panels API which, as far as I can deduce, is its own self-contained process that does not touch ANY of the normal access mechanisms (unless they are explicitly called), I can't foresee any spillover.
      As a last point on the visibility issue, it's also pretty lightweight - even if a hundred panes are being rendered on a given page request, it still only has to make a single query in order to determine the appropriate visibility for each and every one of those panes. Moreover, it doesn't just hide those panes, but actually prevents them from being rendered, which is another potential performance gain (or at least, not a performance loss).
3. Other potential benefits arise from heading in this direction. For example, it would be pretty simple for us to actually create our a hook_og_panels_content_types() (or something like that), which would act like a reduced version of hook_panels_content_types(). Might seem crazy, but given all the interest that's been shown in this already, I don't think it's a big stretch to assume that people are going to want to create their own group content types, and it seems prudent to me to provide a way for them to do it that's as easy and as consistent with what we're doing as possible. Plus, those content types then get the benefit of being automatically included in any mass-ops (as in the above two) that we implement for the og_panels content types we put together.
4. In terms of performance, I don't think that we're sacrificing anything by running them all through the same render function. Any parts that aren't desired can easily be skipped, but perhaps more importantly, it's a way of caching data in static vars that we might otherwise be querying the database for for each individual pane.
5. A lot of this CAN be done using the approach to implementing content types that's currently in HEAD - just add helper functions with the shared functionality and call them if/when necessary in the content type-specific functions - but then we may start running into problems with #1, we lose the potential for a hook as in #3, and we start to see a lot of code duplication, not all of which will be needed.

So, I know you're quite busy, but I hope this clarifies why I think these types of changes are really potentially valuable to og_panels. I hope you'll also understand why I find your indicating that you're "not too fond of abstraction layers" to be a bit of a frustrating answer, as it doesn't give me much to work with in terms of reshaping it.

Just another version of the patch to keep up to date with the newest changes you've committed. Also, I noticed a dumb error in og_panels_update_5002()...db_query() instead of $ret = update_sql().