Allow Multiple Mandatory Groups

regx - January 5, 2007 - 05:25
Project:Organic Groups Mandatory Group
Version:4.7.x-1.0
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review
Description

This patch allows multiple mandatory groups. The patch was created against :
// $Id: og_mandatory_group.module,v 1.5 2006/07/22 12:25:10 pwolanin Exp $

AttachmentSize
regx_og_mandatory_groups_01.patch.txt1.1 KB

#1

pwolanin - January 6, 2007 - 00:29
Status:needs review» needs work

this patch doesn't make the conversion complete- for example, the part in hook_form_alter.

I'm also not really interested in adding this additional complexity to the module.

#2

regx - January 7, 2007 - 16:46

The version I was patching from did not implement hook_form_alter.
I need the ability to have multiple mandatory groups for a project I am currently working on, and this patch does the trick. I can create a new patch against the latest version and flush out the hook_form alter stuff.

Others might also need this "added complexity". Should I start a new project - og_mandatory_groups, or maybe og_multi_mandatory_group? The latter would be less likely mistaken for og_mandatory_group.

#3

pwolanin - January 7, 2007 - 16:53

well, if you want to help maintain this module, I'll give you CVS access and we could add another branch for your development of a 4.7 version with multiple mandatory groups.

#4

rconstantine - January 10, 2007 - 02:04
Version:4.7.x-1.x-dev» HEAD
Status:needs work» needs review

I failed to see this thread before starting my own version of the problem. I'm attaching the entire file as I'm in a bit of a hurry tonight.

Features added:
-Select 0-N groups as mandatory via checkboxes.

This updates both the admin interface, user registration form, validation, submittal, and emailing all admins in batches.

I'll take a look at these other patches that have been submitted to see if I could have done anything different/easier/cleaner.

AttachmentSize
og_mandatory_group.module 14.53 KB

#5

pwolanin - January 10, 2007 - 02:29

here's the diff against HEAD

Can you explain the use case for having many mandatory groups?

Is this the best admin UI?

Does this still work with the "make user choose X more groups" feature?

AttachmentSize
rconstantine.diff 17.69 KB

#6

rconstantine - January 10, 2007 - 03:11

Use case for many mandatory groups:

I will be providing a web site with many areas, some of which are part of the site's 'core' and others are added by users. I need all users to be members of these 'core groups'. Each of the 'core groups' has similar functionality to each other, but is admin'ed by different people. These groups are 'selective = 3', I think. They can't unsubscribe. This feature will be even more important once I make additional contributions to other modules. There are quite a few that almost do what I need. I don't plan on taking away any features (of course), but adding to them - hopefully without making them too complicated.

I must admit that I'm not the most experienced programmer. Getting something to work is one thing that I can do. Getting it to work in a small footprint is something I need work on. Comments or changes to the code are welcome.

As for the admin UI, if there is a need, I can add a drop-down list option if the list of groups is too large - say, larger than 20; or maybe admin's discretion.

And as for the "make user choose X more groups" feature, I thought I was using HEAD, but your X seems to imply that the admin has the option of more than one more group - so maybe I had an earlier copy. Is that the case? My version currently has just the checkbox, but I was planning on adding a drop down which runs from 0 to 'group count' so that more 'minimum groups' can be selected. This can easily be added.

#7

pwolanin - January 10, 2007 - 03:19

There is a feature to require users to pick X groups from among the options in the registration form.

Anyhow, I'm partly asking about your use case, since if you don't need the fancy e-mail notices, admin interface, etc., you could write a tiny module and hard code all the values.

#8

rconstantine - January 10, 2007 - 03:31

Oh, I see. Well, I do actually want the admins to be emailed. That was a huge pain in the butt to get working. Hopefully, I did it right.

As for X groups from the form, even after looking at the diff you provided, I don't see where that is. Is it being worked on in another issue? The only thing I see is

"+ //TODO allow the admin to specify how many additional groups must be joined
$form['og_mandatory_additional_group'] = array(
'#type' => 'checkbox',
'#title' => t('Require new users to join at least one group in addition to any mandatory group'),
@@ -179,8 +277,46 @@ function og_mandatory_group_settings() {
return system_settings_form($form);
}"
and I didn't change a thing here.

I take it you've read my embedded TODOs. Would you rather I not do these (for the community)? This is your project, so I don't want to step on any toes.

#9

pwolanin - January 10, 2007 - 05:28

Ah, you're right- that feature was requested by someone else, and I'd forgotten the exact implementation that got committed. So, it's only an ability to require the choice of one additional group.

Anyhow, there now stand about three significant feature requests:

1) multiple mandatory groups
2) retroactive mandatory groups
3) a mandatory group for group admins

So, my thought at this point is to see about developing some or all of these features on a separate branch of the code (e.g. 5.x-2)

Also, I think the commenting in the full module attachment was excessive- Ideally, the functionality and effect should be pretty obvious from the code.

finally, you mentioned a stray element in the grops array (I think0 in the comments. Look at the 4.7.x version of this module- I had some code there to counter-act a bug in OG, but I thought that bug had been fixed.

#10

rconstantine - January 10, 2007 - 19:27

Regarding the comments, I assume you mean the in-code comments, not the comments before each function definition. The latter are meant for automated documentation via phpDocumentor and as they are meant to be seen without the code should be as descriptive as possible. The former I left in so you could see what I was thinking as I did some of it in case you had a better way - particularly in that last function with the really long comment. I fully intended either you or myself to remove it before inclusion into the main project.

I will take a look at the 4.7.x code to see what you're referring to. All I knew was that I was generating that output. It drove me crazy for a while. I think that it was in OG that it was being added, but I couldn't track down the exact spot it was happening.

As for those other features, I don't have a problem implementing them in the next couple of months if that's not too long of a wait. You'll notice that the various TODOs I had in the file encompass these features. My version of feature 3 from your list would allow the user to set different mandatory groups for each account 'type' - something I'll be adding either to a user-related module or a module I do from scratch which will extend the existing roles/permission system and make managing very large sets of users easier. However, I could do a test to see if my mystery module is present, and if not, switch to the simpler functionality you've described.

So if you've taken a good look at the code and comments, would you like me to remove the in-code comments now? Also, I made a patch myself, but I'll check it against yours. I think SVN patches may be different from CVS patches. If so, I'm not sure how you'd like me to get changes to you. Let me know your preferences. Maybe I'll just setup some CVS crap on my machine anyway...

#11

rconstantine - January 10, 2007 - 19:44

I checked out the 4.7.x code, and I essentially did the same kind of work-around, but only once in the og_mandatory_group_settings_validate function. I have other checks, in the functions that you had your temporary fixes in, that weed-out the OG problem as a side-effect of taking care of the new checkboxes case.

#12

pwolanin - January 10, 2007 - 19:47

The standard for Drupal is CVS patches. Also, take a look at the patch in http://drupal.org/node/107039

#13

pwolanin - January 10, 2007 - 19:55

The bug fixed for OG was supposedly applied: http://drupal.org/node/85724

However, it's possible it was never committed, or not committed to HEAD. If so, that issue should be re-opened.

#14

rconstantine - January 10, 2007 - 20:30

Hmm. I like the retroactive patch and am going to go ahead and incorporate it into my working copy to make sure it all works together. I may have to modify it just a bit.

As for the OG bug, it looks as though the patch was in fact incorporated into HEAD and the copy I worked from. So I must have found something else going on. I am using the latest official releases of everything. Oops - I see OG just came out with a new release on the 6th. I'll update to that and see what happens.

#15

rconstantine - January 10, 2007 - 23:26

First, let me apologize for not having setup CVS diff tools on my system yet - hence the whole file again.

Added retro-active user subscription of mandatory groups per http://drupal.org/node/107039.

Seems to work. I did have to change it to work with the multiple mandatory groups. As in the original author's patch, mine does not email the admins. I'm thinking that's a good thing as this code iterates over every user and doesn't check whether the user was already subscribed to the group or not. So if the admins were emailed, they'd get a complete list of all users, not just the new ones. That is, if we cut-and-pasted my current email implementation for new registrations.

I suppose we could add a checkbox asking whether to email or not. If not, run the simple code we have in place, otherwise, test whether the user is already subscribed and build emails based on the results. Kind of complicated and maybe not that useful.

If there's a better place I should have put this combined-feature file, let me know.

AttachmentSize
og_mandatory_group_0.module 16.5 KB

#16

rconstantine - January 11, 2007 - 00:23

Okay, last one for today.(?)

I updated the module to incorporate feature #3 from post 9, above. The site admin can now maintain a list of mandatory groups for group admins. The list is displayed below the list for users. Both lists are collapsible. Default view is for the user list to be expanded and the admin list to be collapsed. I also added an admin update subscriptions page exactly the same as the update for users. Each is a submenu of og_mandatory_group.

Everything seems to work together just fine.

AttachmentSize
og_mandatory_group_1.module 20.02 KB

#17

pwolanin - January 11, 2007 - 03:50

Ok, I haven't had time to look in detail today, but I'm imagining a different admin UI that might be better:

a drop down list of all groups. On each visit you can only add one new mandatory group (considering that one is the usual use case). The current mandatory groups are shown in a table above the drop down with a check box and actions (maybe "remove from mandatory list", "subscribe all users to this group", others?). I'm thinking something more like the /admin/content page.

#18

pwolanin - January 11, 2007 - 13:05

Or how about an autocomplete field, instead of a drop-down?

#19

rconstantine - January 11, 2007 - 19:00

I mean no offense, but why over-complicate a simple interface? How often do site admins change their mandatory groups? I would think that these are setup once upon site creation and that others MAY be added over time if the need arises, but most often, that won't be the case. Your suggestion to list the 'current mandatory groups' at the top is a good idea, but I don't see why we should separate them from the rest and make separate controls. As an admin, I don't want to add 7 or 8 mandatory groups one at a time as you suggest - even if I only have to do it once. To scroll down and check/uncheck what I want and then hit a single save button is nice.

As for the controls for the 'current mandatory groups' as you suggest - the checkboxes with the action drop-down - duplicate what a simple checkbox now does, so I don't get why that should be complicated. Add to that the fact that the groups are listed in alphabetical order, and I think they are fairly easy to find and maintain, even if there were 100-1000 groups. The admin just grabs the scroll bar and pulls hard :-) One easy (?) change would be to display the listings as two columns, so the admin wouldn't have to scroll as far.

What might be a compromise and hard to code, I think, would be to display a kind of auto-complete box as you suggest, but put it above the full list. Next to it, have a button for 'check' and another for 'uncheck'. What the user would do is search for the name of the group they want, and once it is in the box, click either 'check' or 'uncheck'. This would have the effect of checking/unchecking the actual box farther down on the screen/off screen, without the scrolling. Additionally, the full list could collapse. This way, the admin would have two choices of selecting their mandatory groups - direct checkboxing or autocomplete textfield.

How does that sound?

As for now, I think it is completely usable for most people as-is. If you like my modified UI idea, I could code it, but it will have to wait a few weeks while I finish the next project I started. Who knows, maybe my current project won't take as long as I think and I can do this one sooner.

Let me know and thanks for the comments.

BTW, have you installed it and played with it?

#20

rconstantine - January 11, 2007 - 19:05

BTW, I noticed that the feature request for the group admin mandatory groups want new group admins to be automatically subscribed when a new group is created. That makes sense and was an oversight on my part. If it can wait for a couple of weeks, I can add that as well. I imagine it will just take another form_alter for group creation.

#21

jwilde - January 19, 2007 - 22:28

Hey guys,

Has anybody updated og_mandatory (// $Id: og_mandatory_group.module,v 1.4.2.3 2006/10/24 00:39:49 pwolanin Exp $ ) with multiple groups selection for 4.7.5?

Thanks,

Jim

#22

pwolanin - January 20, 2007 - 01:18

@jwilde - I haven't had a chance to review well this proposed code yes, so it's not available as an opfficial release for any version. One of the initial patches was for 4.7.x, so you could try that.

#23

jwilde - January 20, 2007 - 11:38
Version:HEAD» 4.7.x-1.0

Thanks Pwolanin. I tried using the early 4.7 version but it didn't work all the way with your recent version. It doesn't hold the checked values.

Kind regards,

Jim

#24

jwilde - January 20, 2007 - 13:02

Hey Guys,

I got it partially working for multiple mandatory groups. The only problem I have is that the user can uncheck the mandatory groups on registration form.

This is for 4.7 using the latest og-mandatory (10-23).

I'm attaching the module in case anyone wants to add the final touches.

jim

AttachmentSize
og_mandatory_group_2.module 9.94 KB

#25

jwilde - January 20, 2007 - 13:49

This one works but does not update existing users.

AttachmentSize
og_mandatory_group_3.module 9.97 KB
 
 

Drupal is a registered trademark of Dries Buytaert.