Hi Ezra,
I have been reviewing subgroups module and found some issues I would like to address.
1. The module is using og’s table. When subgroups module is uninstalled it’s data still remains (also the variables should be deleted).
2. The module should be dealing with hierarchy and propagating. OG is already showing the members of every group, so in my opinion the members feature is out of the scope of this module. It should be at least move to another module.
3. Number #2 will also assist with removing the too many tabs added.
4. under ‘/edit/children’ it is always written ‘Check all subgroups.’ This leads to users banging their head ;)
5. It is very difficult to understand the hierarchy.

These are the major issues I’ve encountered. For this I’d like to suggest a rewrite of the module. a) You need to agree for this of course. b) We need to decide what would be the best approach.

Let’s define the target of the module:
1. Control and show the groups’ hierarchy.
2. Propagate content and users.

IMO we should implement it similar to the core book module.

As I believe in doing and not just renting, I place here a code I’ve been working on. It is surely not final, but it is showing the hierarchy (the propagation still not implemented).

To test:
1. Enable the module.
2. Create some groups
3. Through the 'Groups hierarchy' tab control the groups.
4. Enable the group hierarchy block, to see the hierarchy.

Comments

amitaibu’s picture

Title: Reviwe of the module and suggestion » Review of the module and a suggestion
ezra-g’s picture

Thanks for your comments and this patch. I'm unable to respond at this moment but will get back to you as soon as I can. I appreciate your involvement in this project!

amitaibu’s picture

Status: Active » Needs work

I'll soon upload a rewrite to the rewrite :)

amitaibu’s picture

Title: A complete rewrite of subgroups module and user/ content propagation » Review of the module and a suggestion
StatusFileSize
new17.49 KB
new5.71 KB
new18.34 KB
new21.79 KB
new21.37 KB

This one is already a working module, implementing:
1. Editing groups hierarchy.
2. Block with group hierarchy according to the group context.
3. Propagation of content
4. Propagation of users (you will have to patch og module for this).
5. Demotion of users.

Here are some screenshots for the lazy ones - you can see private groups are handeled according to user access permissions.
To use the module remove the .txt from the file and open it as ZIP.

amitaibu’s picture

Title: Review of the module and a suggestion » A complete rewrite of subgroups module and user/ content propagation
Status: Needs work » Needs review
amitaibu’s picture

Please note that the propagation occurs only when content is created/ edit or a user is subscribed/ unsubscribed. That means you shouldn't expect your content/ users to be propagated as the module is installed - this is a feature that IMO should be added only after a proper review of the community.

antipix’s picture

i have installed and tested it on a test site and it works well...
now that i run a new project, i wanted to re install organic group, og subgroups and og hierarchy but strangely it doesnt work anymore...
i cant' understand why it was working on a site and not another...both are 5.7 version, same module version...

can og hierarchy work alone with organic group or does it need to be used with subgroups ?
does it need to be installed in a special order ? some important steps ?

amitaibu’s picture

og_hierarchy's aim is to replace og_subgroups. However, I don't think ther is an issue for both of them to reside next to each other.

antipix’s picture

thanks ! i'm gonna test again with a new installation...

antipix’s picture

it works ! i think i had a problem with an old organic group version...
now i must implement the http://drupal.org/node/257180 patch in order to have a working user propagation...

moshe weitzman’s picture

looks pretty good to me ... note that book module changed a lot in D6. I hope this module makes the same transition in D6. The hierarchy is stored/managed in the menu system.

moshe weitzman’s picture

looks pretty good to me ... note that book module changed a lot in D6. I hope this module makes the same transition in D6. The hierarchy is stored/managed in the menu system.

amitaibu’s picture

10x Moshe for the review.
Others, please note that patch is already committed to OG DEV version.

ezra-g’s picture

Thank you, Moshe for your review!

@Amitaibu - Could you clarify which patch has been applied where?

amitaibu’s picture

@ezra-g,
http://drupal.org/cvs?commit=117789 (i.e user propagation will work without manually patching og module).

ezra-g’s picture

I've finally been able to take a look at this. Great job, Amitaibu! I like that you're using a separate table to store the ancestry. I'd like to do test this out this week, but it seems likely that we can replace OG_Subgroups with this. Fantastic that you've rewritten this. Thank you.

@Amitaibu - Would you like to be co-maintainer?

amitaibu’s picture

I'd be honored :)

ezra-g’s picture

@Amitaibu - The first thing to do would be to sign of for a Drupal CVS account. Reference this node in your application so that the CVS admin knows that I've approved you as co-maintainer.

Are you familiar with CVS? I think the best way to proceed would be to create a 5.x.4.x-Dev version\branch so that more people can test out your rewrite.

amitaibu’s picture

@ezra-g - I've got now a cvs account. I have some experience with CVS.

One concern I have is with the naming - module name is og_hierarchy which I think reflects better the functionality. What do you think should be done?

ezra-g’s picture

My view is that OG_Subgroups is a better name because it is more descriptive of the module's functionality than og_heirarchy.

Without a description, OG_Hierarchy doesn't describe to an unfamiliar user what this module does. One might ask, "a heirarchy of what?"

OG_Subgroups seems a bit more explanatory: OG = Organic Groups, Subgroups = Groups that are members of other organic groups. That's what this module does.

I'm totally open to reasons for OG_Hierarchy, though.

I agree with Moshe that it would be nice to see the 6.x version follow the Book module's lead by using the new menu system to store hierarchies.

@Amitaibu: I've given you CVS access to this project -- Would you like to create the new branch\dev release node or shall I do that? Then I'll go through the Queue and "Won't fix" most of the issues on the current 5.x release version. I'll also open a separate issue to discuss migrating between the two 5.x versions.

amitaibu’s picture

@ezra-g,
Please create the new dev release. I prefer not to mess with it yet...

Here's a Zip file with the right name (i.e. og_subgroups) and a README.txt . As I am not a native speaker a grammar review might be nice :)

btw, how about using snap 1 from #4 for the module main page?

amitaibu’s picture

StatusFileSize
new11.89 KB

Missed a few find and replace - here it is.

ezra-g’s picture

Title: Review of the module and a suggestion » A complete rewrite of subgroups module and user/ content propagation
Status: Needs review » Fixed

5.x-4.x Nightly Dev Snapshot is now available. Let's file issues against that version :).

amitaibu’s picture

10x all!

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

klezmer41’s picture

I have this issue when trying to edit subgroups:

Fatal error: Call to undefined function: og_node_groups_distinguish() in testsite.com/modules/og_subgroups/og_subgroups.module on line 264

amitaibu’s picture

klezmer41, Please open a new issue and make sure you are using the 5.x-4.0 version.

klezmer41’s picture

It completely hosed my Drupal installation. I had to go back to a version 1.5 months ago :( Lessons learned, backup code and database EVERY DAY. Luckly this isn't a going to take me a month and a half to get back to where I was.

ezra-g’s picture

@klezmer41: What do you mean it hosed our Drupal installation? I strongly doubt that this module caused damage to your site.