Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrfelton’s picture

Status: Active » Needs work
FileSize
1.99 KB

Well, here is a start. I'm having a but of trouble with the action though. Rules seems to want to report that the group is empty, and so it won't fire the action properly. This is the Rules component that I'm testing with.

{ "rules_sac_apply_create_local_chapter" : {
    "LABEL" : "Create local chapter from Chapter Application\"",
    "PLUGIN" : "rule",
    "REQUIRES" : [ "rules", "og" ],
    "USES VARIABLES" : {
      "application_node" : { "label" : "Application node", "type" : "node" },
      "created_chapter_node" : { "label" : "Created Chapter node", "type" : "node", "parameter" : false }
    },
    "IF" : [
      { "entity_has_field" : {
          "entity" : [ "application-node" ],
          "field" : "field_sac_chapter_apply_name"
        }
      },
      { "user_has_role" : {
          "account" : [ "application-node:author" ],
          "roles" : { "value" : { "5" : "5" } }
        }
      }
    ],
    "DO" : [
      { "entity_create" : {
          "USING" : {
            "type" : "node",
            "param_type" : "sac_chapter",
            "param_title" : [ "application-node:field-sac-chapter-apply-name" ],
            "param_author" : [ "application-node:author" ]
          },
          "PROVIDE" : { "entity_created" : { "entity_created" : "Created entity" } }
        }
      },
      { "og_add_role" : {
          "account" : [ "application-node:author" ],
          "group" : [ "entity-created:group" ],
          "roles" : { "value" : { "4" : "4" } }
        }
      }
    ],
    "PROVIDES VARIABLES" : [ "created_chapter_node" ]
  }
}
blackclover’s picture

subscribe

mrfelton’s picture

Status: Needs work » Needs review

Infact - the above patch does infact work. My problem was that I was trying to assign the role to a user on a newly created group - I had to add an entity save action after creating the group, and then the add role action worked without issue.

blackclover’s picture

Works great!!! Thanks

fago’s picture

Status: Needs review » Needs work
+++ b/og.rules.inc
@@ -223,6 +223,36 @@ function og_rules_action_info() {
+        'save' => TRUE,

Saving the user account should not be needed, as og_role_grant writes directly to the database?
Else, the patch looks good to me!

FYI: I've created #1357956: handle actions that require saved entities.

fago’s picture

Title: Request for new OG Rules event - Assign a user to an OG Group with specific OG role » add rules action for adding og roles for a user per group

I've tested the patch without 'save' - that works fine. Still, I think the patch also misses a check whether the use is at least member of the group. If he is no member we shouldn't add an og role for him, I guess.

mrfelton’s picture

> I think the patch also misses a check whether the use is at least member of the group. If he is no member we shouldn't add an og role for him, I guess.

I'm not sure about that. Do you have to be a member of a group to have a role within it? I'm pretty sure that OG currently lets you remove the group owner from the group - whilst leaving as the owner.

fago’s picture

Yep, I think it also keeps the roles while you are not in the group. Thus, leaving it out for now should be fine.
However, what happens if we grant a role and user isn't member of the group yet? He shouldn't become member that way.

mrfelton’s picture

@fago - this works without save=TRUE, even though #1357956: handle actions that require saved entities is still an open issue?

mrfelton’s picture

> what happens if we grant a role and user isn't member of the group yet? He shouldn't become member that way.

Shouldn't the role just be granted without affecting the membership status, since OG is storing role based information independently of membership status? Is that not what happens already with this patch?

fago’s picture

Yep, I've just tested it and it's ok.

save=TRUE does not help with #1357956: handle actions that require saved entities either. Thus, it should be removed + then the actions do not have any cause to return FALSE.

fago’s picture

Status: Needs work » Needs review
FileSize
3.95 KB

ok, I've done so + improved the user roles options list to only show "writable" user roles, so the member and non-member role we cannot change is no listed for the action.

Then also, I've added a condition to check whether a user has a role in a group.

Status: Needs review » Needs work

The last submitted patch, og_roles.patch, failed testing.

amitaibu’s picture

+++ b/og.rules.incundefined
@@ -223,10 +223,51 @@ function og_rules_action_info() {
+      'account' => array(
+        'type' => 'user',
+        'label' => t('User'),
+        'description' => t('The user whose group roles should be changed.'),
+      ),
+      'group' => array(
+        'type' => 'group',
+        'label' => t('Group'),

Maybe instead of taking the user + group -- we can simply get the OG membership entity -- like this we know for sure the user belongs to the group?

amitaibu’s picture

Status: Needs work » Needs review
freddura’s picture

tinefin’s picture

Just saw that the former patch passed testing now.

However, since it's already done, here is a patch that should follow the slightly different approach of using og_membership entity, suggested in #14

mrfelton’s picture

Trying to test the patch in #17... How do I get a og_membership to pass to this?

mnlund’s picture

Me too wondering. Which selector to user as Membership?

tinefin’s picture

Sorry, didn't see the comments until now.

You can use it to react on events like "User has become a group member" "User membership is saved" etc. These provide the membership directly.

Otherwise you could get it in node and user events by including the membership in a condition? Anything that lets you select an og_membership entity.

Fabianx’s picture

While #17 works great, getting the membership is a real hassle and just adding it in a condition does not work. And I need it to work in node updated context.

I've such did go with #12.

blackclover’s picture

This was working great for me but now that I've migrated to OG 7.2x it no longer works. It's essential to my website so I would like to offer a bounty to anyone who could port it over to 7.2x. Just respond here or send me an email kelly@blackclover.ca

Cheers

blackclover’s picture

Please ignore my post above sticking with OG7.1x

akalam’s picture

Within og-7.x-2.0-beta1 I see the actions in the "add action" field, but when I select "Assign a group role to a member", I got the following ajax error:

StatusText: parsererror
ResponseText: Fatal error: Call to undefined function og_get_global_roles() in /path/to/website/sites/all/modules/og/og.rules.inc on line 243

akalam’s picture

changing the line 243 from:

    $roles = og_get_global_roles();

to

    $roles = og_og_default_roles();

Solves the ajax error for me, and gives me to the next screen, but I cannot see any rol in the "Group role" section...
Also, almost in og-2.0, you have roles for content-type, so there must be a way to choose, almost, the content-type from wich you will want to get the roles loaded...

I don't have enought time, so I will solve this for my purposes in a "hardcoded" way, but I will try to came later to help with the 2.0 implementation

akalam’s picture

Sorry, the previous comment have a mistake. Line 243, in og.rules.inc file must be:

   $roles = og_get_default_roles();

Now I got the default roles ('administrator member').

amitaibu’s picture

Status: Needs review » Active

Please provide a proper patch.

knsheely’s picture

Using the patch from #12 I am getting the following notice in the add a new action section:

Notice: Undefined index: label in RulesPluginUI::getOptions() (line 860 of /home/fourc/public_html/sites/all/modules/rules/ui/ui.core.inc).

The action labeled "add new group role to user" is now available along with another action that has no label.

amitaibu’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Active » Needs review

Please ignore #27 I confused it with another issue.

1.x is a bug fix branch, so moving to 2.x

pingers’s picture

In the meantime, #12 also worked great for me in 7.x-1.x. Thanks for the patch fago.
Use case was to react to user save (create + update) to sync a user role to an og role.

ioskevich’s picture

Here ia patch for OG 7.x-2.x-dev that adds actions to add/remove OG roles and condition to check particular OG role. Works with roles per bundle (node type) and only with NODE type groups. Was done for our particular needs so might not be universal enough...
Based on patch from #12

Status: Needs review » Needs work

The last submitted patch, 1327326_og_roles_rules.patch, failed testing.

ioskevich’s picture

Updated patch - fixed condition to check for OG roles.

pingers’s picture

Status: Needs work » Needs review

Run tests...

Status: Needs review » Needs work

The last submitted patch, 1327326_og_roles_rules_1.patch, failed testing.

ioskevich’s picture

Seems that my patch in #33 is for older 7.x-2.dev version than is currently available so patch doesn't apply. For those interested, I am attaching og.rules.in file itself...

Renee S’s picture

Status: Needs work » Needs review
FileSize
4.47 KB

I've re-rolled the patch against the latest dev. Works :)

amitaibu’s picture

+++ b/og.rules.incundefined
@@ -199,10 +199,97 @@ function og_rules_action_info() {
+  foreach($og_bundles['node'] as $bundle => $bundle_label){

Why hardcode to the node entity?

+++ b/og.rules.incundefined
@@ -199,10 +199,97 @@ function og_rules_action_info() {
+    ¶

Spaces here and other places.

+++ b/og.rules.incundefined
@@ -199,10 +199,97 @@ function og_rules_action_info() {
+  if ($account->uid && $account->uid != 1) {

Why have this if?

ioskevich’s picture

Node entity is hardcoded since I couldn't find quick solution how to make it work with all entity types - and since we needed that functionality urgently, I just uploaded the code the way it was. Sorry, I am not that good with PHP.

For user UID check - if I recall correctly, it was inherited from the older patch my code was based on. As for Drupal's superadmin's check - in our particular setup I was getting integrity constraint violation errors when trying to assign OG roles to uid=1.

I can take care of cleaning this patch (run it through coder at least) and getting rid of account uid check, but not sure I will be able to make it work with all entities in near future - help with this part would be appreciated.

amitaibu’s picture

Status: Needs review » Needs work

Setting to needs work, as the node hardcoding should be removed.

adamtong’s picture

the patch is #37 is not work in the lastest stable 7.x-2.x version

othermachines’s picture

Status: Needs work » Needs review
FileSize
4.21 KB

I wonder if we can expedite this by making things simpler.

Can we live without select lists? I would love to see this feature committed, and these can easily be added later. I also don't see a great need to check multiple roles in our condition.

If there's agreement, this patch might come in handy (patched against latest 7.x-2.x-dev).

Also just uploaded a patch that adds OG role-related events: http://drupal.org/node/1816800#comment-7058238

mrfelton’s picture

One of the problems with this is that the roles have numeric ids and not machine names, which means that when you transfer these rules from one site to another (eg, via features) they are likely to break as the role ids are likely to be different on the other site.

othermachines’s picture

@mrfelton Are you suggesting we use name instead of rid? I figured that wasn't a good idea because name is editable via the UI.

Had some free time this weekend so expanded on the last patch. Changes:

- Brought list type back in for both actions and the condition.
- List type returns group roles on all group entities.
- Check multiple roles on "User has group role" condition.

So, essentially the patch in #37 except with fix requested by Amitaibu in #40 plus a couple of minor nitpicks.

mrfelton’s picture

@othermachines - no, name is not a god candidate to use instead of rid. The og_role table and related module code needs to be reworked so that roles have machine names instead of numerical ID's. However, that is a much larger set of changes and would be out of scope for this patch. I just wanted to flag it here. Using rid is the best we can do for now. Will try and review your updated patch tomorrow, thanks.

othermachines’s picture

@mrfelton - I wouldn't mind seeing that changed. And a review would be fab. Thx -

askibinski’s picture

I applied the patch succesfully and see the new actions.

I'm trying to apply it for this rule:
- if new group created (node of type group)
- add the role "administrator member" to the author (assign OG role)

The rule is applied, (confirmed that) but it doesn't add it to current user or node:author.

I don't think that is possible with this action? The action itself is available but nothing is done, probably because it can only be added to events like noted in #20?

askibinski’s picture

I also tried it on an event "User has become a group member"
action: grant og role
user data selector: account (default)
group data selector: site:og-context--node (default)
and selected a role.

Rule is perfomed but after creating a group, the role is not assigned. Am I missing something perhaps? (using all the latest versions etc)

othermachines’s picture

Hi, @askibinski .

Can you post your exported rules?

I think [site:og-context--node] is not an entity, but the title of the current group node (not 100% sure). You may have to add "Entity is of type" as a condition to be able to access the entity data (see the pre-loaded "OG member subscribe" rule as an example).

askibinski’s picture

Okay, I got the example and used "Entity is of type", and now I have og-membership but still no role is added.

{ "rules_groepsbeheerder_rol_toewijzen_aan_auteur_nieuwe_groep" : {
    "LABEL" : "groepsbeheerder rol toewijzen aan auteur nieuwe groep",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "rules", "og" ],
    "ON" : [ "og_user_insert" ],
    "IF" : [
      { "entity_is_of_type" : { "entity" : [ "og-membership:group" ], "type" : "node" } }
    ],
    "DO" : [
      { "drupal_message" : { "message" : "debug: rule performed" } },
      { "og_grant_og_role" : {
          "account" : [ "og-membership:group:author" ],
          "group" : [ "og-membership:group" ],
          "roles" : { "value" : { "3" : "3" } }
        }
      }
    ]
  }
}

This should work right...?

othermachines’s picture

@askibinski:

For the grant role action, I believe your User should be set to 'account'. It needs an object/entity not just an id. You can bring author in by using the "Fetch entity by id" action prior to this one. (I'm assuming this is a test rule and you're not actually trying to grant the author/creator of the group a role when another user subscribes?)

Just as an aside, in admin/config/group/settings you can set a role to be granted by default when a new group is created. Is this different than what you are trying to do?

askibinski’s picture

@othermachines
yes this was just to test the rule, I'll try other data selectors..

I know there is a setting for this, and I used it but it doesn't work. Maybe there is something else doing evil in my setup. Will check it out on a vanilla install.

update: you're right, the default setting should work (just checked) so I've got a issue somewehere, probably in custom code ;)
Thanks!

othermachines’s picture

No problem at all, @askibinski.

Saoirse1916’s picture

I'm having the same problem as askibinski -- I've created a rule using the new options that fires when a Drupal administrator (has the default Drupal "administrator" role) is added to an OG group, and the user is indeed added but the "administrator member" role is not added. I assume the problem is that the action is trying to add site:current-user (which I assume is me, since I'm logged in) instead of the user who was just added to the group. However, I can't see an option to pick which will target the user that was just added. Am I missing something important here?

{ "rules_add_admin" : {
    "LABEL" : "add admin",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "rules", "og" ],
    "ON" : [ "og_membership_insert" ],
    "IF" : [
      { "user_has_role" : {
          "account" : [ "site:current-user" ],
          "roles" : { "value" : { "3" : "3" } }
        }
      }
    ],
    "DO" : [
      { "og_grant_og_role" : {
          "account" : [ "site:current-user" ],
          "group" : [ "site:og-context--node" ],
          "roles" : { "value" : { "3" : "3" } }
        }
      }
    ]
  }
}
othermachines’s picture

HI, @Saoirse1916.

Both Account and Group expect entities. See #49 and #51, above. I'm pretty sure (though not positive) that [site:og-context--node] is not an entity but a node title, only because I've seen it used to form URL aliases. The value of [site:current-user] depends on the context, and could be you want here, but try using [account] if it's available.

Saoirse1916’s picture

I'm not sure what you mean, I'm afraid -- I'm a bit new to rules and don't know quite what to pass to the actions to do what I want. My goal here is to grant the OG role "administrator member" whenever someone with the "administrator" Drupal role is added to a group. When I look at the Data Selectors for the User option in the action I don't see anything that looks like [account].

othermachines’s picture

I suspect [site:og-context--node] is your problem, but you'll have to troubleshoot that. Turn on Rules debugging and test that your conditions are passing, first. The Devel module has a "Debug value" action that can also be helpful. Second, debug your data... you need to make sure that the action you want to invoke will have the data it needs. Different actions require different data - in this case, it needs both the user and the group object - and this data isn't always available by default. As mentioned above, OG has some rules that install with the module which can be helpful as examples for people new to the module.

The price for Rules' flexibility is a pretty big learning curve, but it's worth taking the time to really wrap your head around how it works. Once you do, you realize you can make it do pretty much anything.

@askibinski did report that his problem wasn't with the patch. Hopefully he's still around somewhere and will give us an update. :)

Saoirse1916’s picture

Thanks @othermachines -- how can I turn on Rules debugging? I don't see that as a module option but perhaps I'm not looking in the right place. Do you know of any good Rules documentation, particularly how it relates to OG?

Thanks again!

othermachines’s picture

@Saoirse1916 No problem. Sorry, of course you need the Devel module to debug Rules. After you enable Devel, you should see the debugging settings on the Rules > Settings page.

drupal.org documentation is a bit lacking in this area. I'm sure there are plenty of helpful tutorials out in the beyond, though. Sorry I can't name one specifically.

Saoirse1916’s picture

Thanks -- I'll see what I can dig up with Devel.

Saoirse1916’s picture

I got debugging working and you're right, according to the log the problem is that the group parameter is not receiving any data. Unfortunately I have no idea how to get the data to it so I'm at a bit of a loss here.

" Reacting on event After saving a new og membership.
0 ms Reacting on event After saving a new og membership.
5.666 ms Evaluating conditions of rule add admin. [edit]
13.327 ms The condition user_has_role evaluated to TRUE [edit]
13.337 ms AND evaluated to TRUE.
" Rule add admin fires. [edit]
0 ms Rule add admin fires.
1.202 ms The variable or parameter group is empty.
3.434 ms Unable to evaluate action og_grant_og_role. [edit]
3.47 ms Rule add admin has fire

othermachines’s picture

Did you try what I suggested earlier?

You may have to add "Entity is of type" as a condition to be able to access the entity data (see the pre-loaded "OG member subscribe" rule as an example).

Saoirse1916’s picture

I added in the condition Entity is of Type using the data selector og-membership:group linked to entity type node. Devel reports The condition entity_is_of_type evaluated to TRUE but unfortunately it also shows the same problem: The variable or parameter group is empty.

supadits’s picture

I did apply patch #44
However, there is a change in function
function og_rules_group_roles_options_list($element)
instead of
$bundle_roles = og_roles($group_type, $label);
change to
$bundle_roles = og_roles($group_type, $bundle);

My case:
There is a user who responsible to create group.
However, I would like to configure User 1 to be part of every group and be Administrator of every group
Below is rule export

{ "rules_add_user1_to_group" : {
    "LABEL" : "Add User1 to Group",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "rules", "og" ],
    "ON" : [ "node_insert" ],
    "IF" : [
      { "node_is_of_type" : {
          "node" : [ "node" ],
          "type" : { "value" : { "community" : "community" } }
        }
      },
      { "og_entity_is_group" : { "entity" : [ "node" ] } }
    ],
    "DO" : [
      { "og_subcribe_user" : { "user" : "1", "group" : [ "node" ] } },
      { "og_grant_og_role" : {
          "account" : "1",
          "group" : [ "node" ],
          "roles" : { "value" : { "11" : "11" } }
        }
      }
    ]
  }
}

Seem everything work as expected
Thanks @othermachines for the patch

othermachines’s picture

Good catch, @supadits. Thanks a lot.

New patch attached.

othermachines’s picture

@Saoirse1916:

Sounds like you've got your head around the key part, which is that you need to find both a node entity and a user entity from your event ("After saving a new og membership") because that is what the action ("Grant OG role") requires. It also sounds like you found your node entity by checking "Entity is type" (og-membership:group), but the user entity remains elusive.

A quick 101, below. Some of it you probably already know, but bear with me.

An entity type defines a particular grouping of fields (e.g. "User", "Node"). A bundle is a subgroup of an entity (e.g. node type "Course" is a subgroup of "Node"). An entity is a single instance of an entity type (e.g. "Breadmaking").

OG introduces another entity type, "OG membership". This is used to connect users and content to a group.

In your event "After saving a new og membership", user "Bob" becomes a member of group "Breadmaking", or:

"Bob" (User) <----- [id] "2" (OG membership) ----> "Breadmaking" (Node)

So, you have three interconnected entities. In Rules, the condition "Entity is of type" enables us to zoom in on a specific entity, which makes its metadata available to any conditions and actions that follow. So where are these entities hidden? Remember that "2" (your OG membership entity) is the link between your User entity and Node entity, so that's where you will want to look.

og-membership:group = "Breadmaking"
og-membership:entity = "Bob"

Granted, the data selectors for og-membership can be really hard to understand. og-membership:group is described as "[OG Group] The OG group associated with the OG membership." OK, clear enough. But og-membership:entity is described as "[Entity] The entity that is a group content." Huh?! It helps to think of "group content" as anything being added to a group, including a user.

Wow, I really hope I'm getting all of this right! Anyway, give it a try and report back if you can.

othermachines’s picture

@Saoirse1916:

OK, I'm learning a few things, too.

I was wondering the difference between events "After saving a new og membership" and "User has become a group member". Turns out the only difference is in the data (parameters) passed in. Because OG membership is an entity entity type, the event option "After saving..." is automatically created by Entity API. The second event option "User has become..." is implemented by OG.

Wow, that's a wee bit confusing.

Anyway, choose "User has become a group member" instead. Then I believe you will find your user entity (account) readily available. You will still need to use "Entity is of type" to get your node (group) data.

Saoirse1916’s picture

@othermachines: Ahh ha! You know, I even thought about trying out User has become a group member but didn't, for some reason. I also couldn't figure out where everyone was getting account as an option -- now I see! Works great, thanks a million for working with me on this!

othermachines’s picture

@Saoirse1916 Awesome. Happy to help. As I said, I learned a thing or two myself.

jfarry’s picture

Hi guys,

I've tested the patch in $65 and it's working great for me :) HUGE thanks to everyone involved, especially othermachines!

Edit:
One thing that I will add though, is that if the node on which the group is created, DOESN'T have:

Group roles and permissions:
Use default roles and permissions

selected, then roles are not added. I'm afraid that I don't know enough about OG just yet to know if this is how things are meant to happen, but this is what is happening for me.

othermachines’s picture

Status: Needs review » Needs work

@pyrhoe - Sorry for the delayed response and thanks for the review!

Indeed, you are right, that if you are using the Group roles and permissions field, the action will only work if "Use default roles and permissions" is selected. I've traced this back to the role selection list in the action configuration, which will always contain the RIDs that belong to GID:0 (default). When "Override.." is selected, it looks like another set of roles is created in the {og_role} table pointing to the GID of the group node. I need to generate this list in og_rules_group_roles_options_list() in a way that takes this field setting into account.

I don't have a fix, yet. I am just talking out loud, here, so I'll remember when I come back to it later. :)

Thanks for the catch!

cgdrupalkwk’s picture

#65 is working great for me! This makes an already powerful module even better! Thanks!

murias’s picture

The patch in #65 seems broken now for the 7.2 dev release of June 10th. The patch itself did seem to apply without much issue, although there was a message, "Hmm... Ignoring the trailing garbage." before it stated done.

After I choose "Grant OG Role" from the actions select list all I get in result is a window with a save button. When I then hit save, just to see what happens, I get an error "Unknown action og_grant_og_role".

I then opted to rollback the module to its state without the patch, flushed caches, reapplied patch, flushed caches again, and this time there is not even an option in the actions for anything associated with OG Roles.

Any thoughts?

Cheers,
Murias

othermachines’s picture

@murias

I'll have a look. Thanks-

othermachines’s picture

Status: Needs work » Needs review
FileSize
43.87 KB
2.72 KB
6.47 KB

Re-rolling patch against latest dev.

This patch also includes a fix for problem described in #71. Select options for roles will now appear as such:

Roles options

That is, roles will be grouped by bundle name *and* whether the group is a default set or belongs to an entity id (the latter applies in circumstances where default roles and permissions are being overridden for a group entity). It will be up to the administrator to use this information to select the correct role. If the role selected is incompatible with the group being evaluated, the role will not be granted.

(First attempt at interdiff, so I hope I don't mess it up!)

murias’s picture

Just as everything seems to be running well, and integrated, wouldn't you know it, there's a Security Update. The Patch in #75 was working well for the July 24 Dev release. Yet today, I find that release to needing the security update to 2.3.

I took a gander at the release notes for 2.3 and found nothing that referenced this thread and rules for OG Roles. Sad. I know 2.3 was just released yesterday, but does anyone know yet for sure if a) this patch is already rolled into that release, or b) does this patch still work with 2.3. I am sure this question is going to be on the minds of many today.

Cheers
Murias

othermachines’s picture

@murias

b) does this patch still work with 2.3.

The patch should still apply cleanly. It only alters one file, and this file has not changed since July 24 dev and yesterday's security release.

Unfortunately we take on these annoyances when we choose to use fixes or features that are still up for review.

By the way, if you post on this thread that the patch has been tested and is working for you, that helps. :)

murias’s picture

@othermachines - I appreciate the information. Try to do my part in letting those know when patches work or not. Sad thing is that yesterday I had just finished all of my rules, and was testing them, to be able to make my post today on its working condition. Then freaked when I read the security update. I'll most likely do my update later tonight and report back.

Cheers

othermachines’s picture

@murias - Fantastic. Thanks!

murias’s picture

Wanted to report back that all seems a good to go with this patch and the 2.3 release of OG.

Renee S’s picture

Status: Needs review » Reviewed & tested by the community

Is working for me, too :)

jessehs’s picture

Beautiful! Works perfectly for me with 2.3.

jessehs’s picture

Let's get rid of the whitespace errors though. Forgive me, I don't mean to be haughty, this is just an exercise in my first interdiff. :)

othermachines’s picture

Thanks. If there are whitespace errors, obviously they are unintentional. I thought I'd solved it. Any suggestions?

jessehs’s picture

Absolutely! If you're not using Dreditor, you're not living! ;-) It shows you the whitespace errors as bright pink.

Many text editors can be configured to automatically trim them out when you save a file. Try googling "[your text editor] Drupal coding standards".

PHPCS/DrupalCS can integrate with many text editors as well as Git, and shows you where your code doesn't meet standards.

Also, Git can be configured to show you whitespace errors when you are running a "git diff " command. The following section of my .gitconfig file (at ~/.gitconfig) turns on color highlighting for diffs, and also makes whitespace errors bright red:

[color]
	branch = auto
	diff = true
	interactive = auto
	status = auto
	whitespace = red reverse
othermachines’s picture

Windows seems to make this a perpetual problem. I'll have another go at it, thanks. I am a bit of a stickler for detail myself.

checker’s picture

I tested patch #83 and it seems to work fine. I would suggest to add a t() function to "group" and "label".

+    'label' => t('Grant OG role'),
+    'group' => t('Organic groups'),
[..]
+    'label' => t('Revoke OG role'),
+    'group' => t('Organic groups'),
othermachines’s picture

Thanks @checker. Added t() functions as suggested.

tedbow’s picture

Status: Reviewed & tested by the community » Needs work

Needs re-reroll

othermachines’s picture

Status: Needs work » Needs review
FileSize
6.04 KB

Re-rolled against latest dev...

Status: Needs review » Needs work

The last submitted patch, og-rules_og_roles-1327326-6.patch, failed testing.

checker’s picture

Status: Needs work » Needs review

#90: og-rules_og_roles-1327326-6.patch queued for re-testing.

checker’s picture

Status: Needs review » Reviewed & tested by the community

I guess it is still rtbc?

WebSinPat’s picture

Thanks @othermachines for the work on the patch and esp for the careful explanations in this thread that helped me figure out what the heck was going on and be on my way toward creating the rules I need.

The patch in #90 seem to generally work.

However, for my case I am more interested in using the roles as a condition rather than action. And for the condition case, the select drop down menu does not seem to make much sense to me. What I want is to check whether the user has the selected role in the group entity the rule is firing for, rather than for a specific group nid.

That is, I'd like to check the role for the User: [og-membership:entity] and Group: [og-membership:group], so I'd like to see a dropdown that has just the roles to select from, similar to how the og permission condition works.

For the action of assigning a role, I can see the benefit of being able to select specific discrete groups via the existing dropdown, but less so for the condition. Actually though, wouldn't it also be useful to have the parameter [og-membership:group] as an option in the dropdown for the case of assigning a role as well, in addition to the hardcoded gid/nid?

thanks.

othermachines’s picture

Hi, @WebSinPat,

I'm not sure I follow what the problem is.

The condition will check the selected role against the group specified by the token in the "Group" data selector. In fact, it works almost identically to the "User has group permission" condition that you mentioned.

The "Group roles" field contains all roles, ordered by group. This is a solution to a problem which is described in this comment. Original problem report here.

For the condition to return TRUE the role has to belong to the group and the user. You will likely have to use Entity is of type in order to access the group entity. (I've been away from this for a bit so my brain is a bit fuzzy on these concepts.)

Thanks for the feedback.

WebSinPat’s picture

Thanks @othermachines. Sorry for not being able to be clearer.

I would like to set up a rule that on the event of updating an og_membership, has a condition that returns TRUE when the the user has a particular role in the group associated with that og_membership.
At the time that I am creating my rule, and using the "group roles" selector I do not know the nid of that og_membership. So I would like to be able to select "role XYZ" in the group specified in the "Group" data selector. Rather than "role XYZ"in "Group (nid:123)" with the hardcoded nid at the time i save the rule.

OK, i have hacked together some code that seems to accomplish what i want.
it's totally kludgey and i don't know php or drupal well enough to do things efficiently, but maybe helps get at it.
In the options_list function add the option to select a role for the group to be named later per the token.
In the user_has_role function now that we know the gid, get the rid of the role name selected above

new role selector screenshot

(also I am using commons, which does a lot of the OG configuration for me, so maybe my problem is not quite understanding what commons is doing with respect to overriding default roles, per the issue fixed in #75?)

othermachines’s picture

@WebSinPat - Can you please post:

- A screenshot (or copy and paste from HTML source) the options that are available under "Group roles" BEFORE your code changes are applied.
- The exported rule that is supposed to work with these code changes (og-rules_og_roles-1327326-kerenhack.txt)

Thx -

WebSinPat’s picture

sure.
1. my BEFORE "group roles" pretty much looked like the screenshot you posted in #75. (sorry, i meant to say that earlier) So the setup is Commons 3.3, OG 2.3, with the patch og-rules_og_roles-1327326-6.patch applied.
Here is the html, if this is the right bit that you are looking for:

 <select multiple="multiple" name="parameter[roles][settings][roles][]" id="edit-parameter-roles-settings-roles" class="form-select required"><optgroup label="Group (Defaults)"><option value="3">administrator member</option></optgroup><optgroup label="Group (node:327)"><option value="21">administrator member</option></optgroup><optgroup label="Group (node:331)"><option value="18">administrator member</option></optgroup><optgroup label="Group (node:332)"><option value="15">administrator member</option></optgroup><optgroup label="Group (node:333)"><option value="12">administrator member</option></optgroup><optgroup label="Group (node:334)"><option value="9">administrator member</option></optgroup><optgroup label="Group (node:336)"><option value="24">administrator member</option></optgroup><optgroup label="Group (node:390)"><option value="27">administrator member</option></optgroup><optgroup label="Group (node:392)"><option value="33">administrator member</option></optgroup><optgroup label="Group (node:393)"><option value="36">administrator member</option></optgroup><optgroup label="Group (node:394)"><option value="39">administrator member</option></optgroup><optgroup label="Group (node:399)"><option value="50">administrator member</option></optgroup><optgroup label="Group (node:400)"><option value="53">administrator member</option></optgroup><optgroup label="Group (node:402)"><option value="56">administrator member</option></optgroup></select>

2. here is the exported rule using my hacked code:

{ "rules_og_admin_get_membership_coordinator_role_2" : {
    "LABEL" : "og_admin_get_membership_coordinator_role_2",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "rules", "og" ],
    "ON" : [ "og_membership_insert", "og_membership_update" ],
    "IF" : [
      { "entity_is_of_type" : { "entity" : [ "og-membership:group" ], "type" : "node" } },
      { "entity_is_of_type" : { "entity" : [ "og-membership:entity" ], "type" : "user" } },
      { "og_user_has_role" : {
          "account" : [ "og-membership:entity" ],
          "group" : [ "og-membership:group" ],
          "roles" : { "value" : { "103" : "103" } }
        }
      }
    ],
    "DO" : [
      { "user_add_role" : {
          "account" : [ "og-membership:entity" ],
          "roles" : { "value" : { "5" : "5" } }
        }
      }
    ]
  }
}

[[3. strictly speaking, my rule is not yet working like I thought it would, because I'm running into another problem where if I change a user's og_role and save via the admin pages at group/node/327/admin/people/edit-membership/111, the event "After updating an existing og membership" triggers as I would expect, but the $account does not yet seem to have the new og_role information, so the condition still returns FALSE. If I then edit the user, keep the role information the same, and save it again, then the rule starts returning TRUE.
I think that's a totally separate issue to what we've been talking about, but figured I'd mention it in case.
Ah,I think I'm running into #1592076: Add or remove roles not working on event 'User has become group member'
]]

4. again @othermachines, thanks for taking the time.

othermachines’s picture

OK, I see the dilemma but what I don't get is why you need to override default roles for this set-up since you only have one role. Your options list should look like this:

<select>
  <optgroup label="Group (Defaults)">
    <option value="3">administrator member</option>
  </optgroup>
</select>

And that's it.

What you need to do is edit each group, changing "Group roles and permissions" to "Use default roles and permissions". A hassle, but it will simplify your life substantially. :)

WebSinPat’s picture

I cannot seem to find that "use default roles and permissions" setting.... I am posting in the Commons issue queue for help because I'm not sure if have misconfigured something or it's by design in Commons that the setting is not visible or configurable.

I will post back if I am able to resolve this and get the patch to work as desired.

I guess I'm still confused what happens with this rule functionality if a group needs to override group permissions, but hopefully it will not come to that.

Thanks again!

othermachines’s picture

@WebSinPat -

This is how I understand it to work:

When you use default roles, you can apply them to any group. When you override the defaults for a group, the roles assigned to that group can only belong to that one group, and each is assigned a unique role id. This is why you ended up with multiple copies of the same "administer member" role.

Having said that, it stands to reason that these overridden roles should only return TRUE when evaluated against the group to which they belong.

Yes, please let us know if you manage to get that sorted.

WebSinPat’s picture

Well, I give up. for my case.

drupal commons 3.3+ does override all the group roles/permissions by design #2121923: Where is the group "use default roles and permissions" setting?. Out of the box, the roles are the same as the default ones, but the permissions vary group to group depending on the privacy settings.
So I think for my use case the way I'm trying to check/assign roles is incompatible with having the defaults overridden.

And in any case I'm also running into #1592076: Add or remove roles not working on event 'User has become group member' where the order of user_save and og_membership_save does not always happen in a reliable order, so checking/assigning roles does not reliably give the expected results, which makes everything else moot.

None of which is a problem with the patch itself.

othermachines’s picture

@WebSinPat - Well, that is frustrating.

Obviously we can't always cover every use case, but since Commons has gone this route I'd be interested to hear from a maintainer on whether this is worth trying to address here. Off the top of my head I'm not sure any solution would be 100% reliable since you'd have to essentially fudge a match based on the role name.

You could set up a rule for each group but of course this isn't practical if there are a large number of groups or they are being created dynamically. Not sure how much experience you have with PHP but another option would be to check with some custom code inside the rule.

As for #1592076 I'm tempted to take a stab at fixing it but I'll have to wait until things are less busy.

Cheers -

WebSinPat’s picture

@othermachines, i posted a paraphrased version of your question in the commons issue queue https://drupal.org/comment/8136233#comment-8136233 So we can see what feedback those folks have.
And yeah, I can think of a number of workaround the overridden default roles issue for my case, but #1592076 really is the blocking issue because the user object just doesn't have the data that I expect in it yet. So here's a +1 for you having time to fix that one when you get a chance :)

othermachines’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
FileSize
4.99 KB
4.33 KB

@WebSinPat - I actually meant an og maintainer. No matter...

I've been mulling over this issue since learning Commons overrides all default roles. This is just going to keep coming up.

My conclusions:

a) Maybe I've been too fussy about not wanting to do look-ups based on editable data (e.g. role names instead of rid);

b) in the case where one wants to check against a SPECIFIC group, one can always add a separate condition.

SO, here's the new patch. What it does:

1. Provides an options list that contains ALL *distinct* OG roles, keyed by name;

2. retrieves the role id at evaluation time based on role name and the group being evaluated;

3. if the group being evaluated does not have a role by that name, it will do nothing.

Testers please!

WebSinPat’s picture

The patch in #105 seems to be working great for me, with commons 3.3 overwritten roles.

Renee S’s picture

The patch in #105 works perfectly for me. Thank you!

Renee S’s picture

Status: Needs review » Reviewed & tested by the community

Flipping back to RTBC - two of us are using the new one, and it was at RTBC before the othermachines' lookup changes.

amitaibu’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thanks.

othermachines’s picture

Yay! Thanks, everyone.

Status: Fixed » Closed (fixed)

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

martindilling’s picture

I'm still a newbie to most of the drupal-stuff, but is there anyway to see if this patch is going to be implemented in the real module? kinda need this functionality, but it would be ideal for me to have to patch OG every time I need to update the module :)

othermachines’s picture

@martindilling - It will be in the next version release. Timing depends on many factors. In the meantime you can opt to download the dev version (7.x-2.x-dev), which is the most up-to-date.