I am requesting to become a co-maintainer for the OG Subgroups module. I am a full time Drupal developer at Arizona State University's Applied Learning Technologies Institute. Over the past two years of working with Drupal I have been using the OG suite of modules, which includes OG, OG Subgroups and OG User Roles, and I have become quite proficient with how to use them and their module code. I feel that my Drupal development skills have finally reached a point where I can contribute back to the community in a bigger way than just providing patches.

My desire to become a co-maintainer for og_subgroups stems from this issue here: http://drupal.org/node/337808. I posted a D5 to D6 port of the og_subgroups module at comment #30 (http://drupal.org/node/337808#comment-1783018). I am currently in development of another OG related module and I have a strong dependency on OG Subgroups. I have identified and fixed many issues with my privately maintained copy of OG Subgroups. It has been necessary for me to maintain my own copy of Subgroups because the development of that modules seems to have halted. Now I feel that I'm at a point where I can make contributions as a co-maintainer and contribute my work back into Subgroups.

I look forward to contributing back to the community that has given me so much already.

Thanks

Brian

CommentFileSizeAuthor
#7 ogs.diff13.39 KBezra-g
#4 og_subgroups.tar_.gz10.99 KBbschilt

Comments

ezra-g’s picture

ezra-g commented
Status: Active » Postponed (maintainer needs more info)

@bschilt -- Can you share the updated version of the module you have privately been maintaining?

OG_Subgroups could definitely use a new maintainer, but before we commit it to the project and make a release, I'd like to

a) Do a quick once-over code review for basic things like security and coding standards
b) Get some functional testing from someone, ideally more than one person. Given how many people have subscribed on the thread that seems possible, and at the very least auzigog can give it a functional review as part of his GSOC project.

Thanks!

izmeez’s picture

izmeez commented

subscribing, might be able to help testing.

Anonymous’s picture

Anonymous (not verified) commented

I wanted to comment on this.

As part of my GSoC project, http://activismlabs.org/ , I wanted to code a functional version of og_subgroups for D6. While doing research, I found that bschilt had already done the work for me! We just need to get him maintainer access so he can publish the work he has already been doing and maintaining.

I will let bschilt upload his code once it is cleaned up.

I have done a fair bit of functional tests on his og_subgroups code. I checked maybe 60% of the use cases in a very clean environment. I also tested it within Open Atrium and it worked great in there as well.

ezra-g, what would you consider a sufficient amount of functional testing? How would we know when enough has been done?

I'm really excited to see bschilt's other "complementary module to og_subgroups". From the sound of it, there are a bunch of feature additions and bug fixes in that module. It seems to me that this code could be published as part of og_subgroups, but could be enabled/disabled in the preferences. I'd have to see the code to be sure, but I'm excited non-the-less.

bschilt’s picture

bschilt commented
StatusFileSize
new og_subgroups.tar_.gz10.99 KB

I have attached the module that I have ported from 5 to 6. I removed my custom edits to the module that allowed it to integrate with the og_user_roles module. I figure we should start with the same functionality that version 5 had, then update it via the issue queue.

Anonymous’s picture

Anonymous (not verified) commented

I wanted to follow up on this issue.

Ezra, what exactly would you like to see before bschilt can have maintainer access? What specific testing? What code cleanup?

I'd love to wrap up this issue sometime this week. Do you think that's feasible?

Thanks!

ezra-g’s picture

ezra-g commented
Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

The code in #4 did splendidly in an analysis by Coder, and seems like mostly API changes from the 5.x-4.x version which is basically the most trusted branch of the module (I've attached a diff for reference).

auzigog and I discussed this in IRC and I forgot to followup here, but I'd be happy with functional testing that basically says, "Yes this provides the features that it claims to provide." Having looked at the code, I feel reasonably confident about that.

I'd say this is RTBC and would be thrilled to make bschilt a co-maintainer so we can finally get a D6 release (just in time for D7 core ;) ).

Thanks for your work here and to auzigog for bearing with me!

ezra-g’s picture

ezra-g commented
StatusFileSize
new ogs.diff13.39 KB

Diff is actually attached here ;).

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

Seems reasonable. From a quick look:

+++ og_subgroups/og_subgroups.module	2010-06-30 19:33:51.000000000 -0400
@@ -410,31 +329,33 @@ function og_subgroups_outline($nid) {
-      if ($form_values['parent']) {
-        og_subgroups_set_hierarchy('insert', $node, $form_values['parent'], FALSE, $form_values['log']);
+      if ($form_state['values']['parent']) {
+        db_query('INSERT INTO {og_subgroups} (gid, parent) VALUES (%d, %d)', $node->nid, $form_state['values']['parent']);
+        db_query("UPDATE {node_revisions} SET log = '%s' WHERE vid = %d", $form_state['values']['log'], $node->vid);
       }

This doesn't look so clean. A submit handler should use API functions, and not access directly the DB.

Powered by Dreditor.

ezra-g’s picture

ezra-g commented

Good point, Amitai.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

FYI, I granted bschilt access to this project repository.

bschilt’s picture

bschilt commented

I'm going to work on getting a new 6.x-1.x branch created and put version attached above in there. Then I will start responding to issues. I will also create an issue for using the og_subgroups_set_hierarchy() api.

ezra-g’s picture

ezra-g commented
Status: Reviewed & tested by the community » Fixed

Thanks, everyone! Marking as fixed.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

@bschilt,

Welcome! :)
Any chance you'll be working on the D7 version of og_subgroups?

bschilt’s picture

bschilt commented

I can work in the D7 realm in the coming weeks, gonna need to do it sooner or later anyway. But first I think the D6 version will need some immediate attention.

Anonymous’s picture

Anonymous (not verified) commented

I may be wrong, but after talk to Amitaibu, I'm pretty sure that subgroups is native in the groups for D7. Maybe not, though.

I can't wait to see a stable D6 release! And even more so, I can't wait to see the addition of the features you've already started working on, bschilt. Those sound awesome!

Status: Fixed » Closed (fixed)

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