the applied patch provides the following:

- add a permission to allow/disallow crossposting in groups
- add a setting for the number of target groups, when the widget changes from buttons to select-list
- add a setting the switch the audience widget to read-only when the audience is given in the request

The patch provides those features while changes as few codeslines as possible, if the general direction is appreciated there are other options for changing the gui, especially for the read-only case.

comments?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drupalzack’s picture

very useful options! any chance of backporting to 4.7?

moshe weitzman’s picture

1. nice feature
2. too much interface clutter. most sites will never touch this. use $conf in settings.php (or make it possible to do so)
3. isn't this what the audience checkboxes setting does?

ray007’s picture

@drupalzack: I don't have a single 4.7 system, so it won't be me doing the backport

@moshe
ad 1) thanx ;-)
ad 2) aren't most settings "set once and leave them that way"? of course we can take out the number from the gui and let users wanting to change it edit the variables, but the setting become rather invisible then ...
ad 3) correct me if I'm wrong, but current audience settings determine whether a target-group is mandatory or optional, while the new setting prevents the user from changing it, when the link leading to the content edit form supplied a group for the content to be in. Maybe the help-text should be more specific about that ...

do you have specific wishes for me to change something in this patch?

moshe weitzman’s picture

1) i would actually like to see a preference for 'maximum number of groups per post'. this would be set to 1 to avoid cross posting. the default should be 'unlimited'. the error message should be something like: "Please remove some groups from your post. A maximum of x are allowed.'
2) sorry, we disagree. please don't put a UI for this number
3) the audience checkboxes setting is different from the audience required setting. you aare duplicating audience checkboxes IMO. please drop this part of the patch.

thanks for your contribution.

moshe weitzman’s picture

make sure you are not an admin to actually see the audience checkboxes feature. also, that works on node add, not node edit.

ray007’s picture

Attached a new patch without gui settings and the read-only option.

I didn't yet change the crossposting stuff because
- having a number means a setting (per user/role/???) and validation instead of a simple permission and radios instead of checkboxes
- I think allow/disallow crossposting is good enough for most situations
- I don't have time right now to do it

If you want to I can change the check from user_access() to call a new special function evaluating a variable but I don't think it's worth it.

If I understand the $conf in settings.php, reading the limit between checkboxes/radios and a selectlist with variable_get() should already enable to set this value in settings.php, right?

moshe weitzman’s picture

Status: Needs review » Needs work

i really want the maximum number of groups instead of allow crossposting because thats whats needed on groups.drupal.org. just like audience checkboxes, admins will be immune from this restriction. no need for a new perm.

this variable is not well named: variable_get('og_button_limit', 20))

thanks.

ray007’s picture

1.) Do you have a suggested name for the variable instead of 'og_button_limit'?

2.) All right, I'll do something with a number. But the point on my side is, I don't want only admin to be exempt from that restriction, but other users (groups) of my choice as well. If we go that route we could even have a number per group (1 setting per group + a default for new groups), or just some way to define the groups being exempt (1 permission + 1 setting) ... any preferences on which way to go and how the admin interface for that should look like?

moshe weitzman’s picture

Title: audience settings » Enforce a maximum number of groups on a single post

1. og_audience_select_threshhold

2. like most of our other prefs, i want this to be global for all groups. to do otheriwse is to multiply the number of preference decisions unacceptably (IMO) ... if you really need group specific crossposting, switch $GLOBALS['conf']['og_crosspost_max'] on a page by page basis. Just stick code into hook_menu(!may_cache), for example

thnaks for your persistence.

markDrupal’s picture

Title: Enforce a maximum number of groups on a single post » issue with the dropdown select box

I have had problems where the public/private checkbox is not changeable when the group audience select box was changed into a drop down select box, or the click to select box. It works fine with the check box or the radio buttons.

is anyone else haveing this problem with this patch?

ray007’s picture

Status: Needs work » Needs review
FileSize
4.71 KB

Ok, instead of a permission I now have a variable telling how many groups a 'normal' user may post to.
I'm getting the number from a function call, so we may do something more fancy a bit later. Depending on the user, the user's roles, the selected groups ...

So, how does this patch look?

ray007’s picture

Version: 5.x-2.2 » master

Yeah, forgot, patch is of course against current DRUPAL-5 branch tip, that's version 5.x-3.x-dev, right?

moshe weitzman’s picture

Status: Needs review » Needs work

looking good. i didn't test it yet, but i would think admins would be exempt from this.

nit: "t('You may not post to more than %audience_max groups at once.')" ... should use format_plural() instead

moshe weitzman’s picture

yeah, tip of drupal-5 and head are equivalent for now. so all is well here ... i won't be using this strategy for D6 though - just patch off of HEAD.

ray007’s picture

by admins you mean, we should make a permission for it?
currently i have a check for user 1 in the function, but I think we could use a role-based scheme there once the basic functionality is there.

and yes, I know format_plural() is the right function for the final version ;-)

moshe weitzman’s picture

in this case, "admins" means: user_access('administer nodes')

ray007’s picture

will do.
anything else you want me to change on this patch?

moshe weitzman’s picture

any progress here?

Grayside’s picture

Status: Needs work » Closed (duplicate)

The meat of this has been duplicated and fixed long since.