I get this error when going to the menu tab within a group.

Fatal error: Call to undefined function og_get_group_ids() in sites\all\modules\og_menu\og_menu.pages.inc on line 12

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jgraham’s picture

Title: Doesn't work for version version 2 of OG » update og_menu to support og 7.x-2.x

og 7.x-2.x is nearly a complete rewrite to see #1342632: Deprecate OG group entity for more details, but this means that og_menu will need to be adjuted appropriately.

jgraham’s picture

Status: Active » Needs review
FileSize
6.43 KB

First pass at a patch to update this to work with og 7.x-2.x-dev.

rv0’s picture

Will it still work with og 1.x ?

pontus_nilsson’s picture

Just something I saw in the patch

-        'menu_name' => 'menu-og-' . $og->gid,
+        'menu_name' => 'menu-og-' . $node->nid,

Are we changing so that a menu is associated to a node instead of a group? I haven't followed OG 2.x just wondering :)

jgraham’s picture

Will it still work with og 1.x ?

Nope. The underlying architecture in og is considerably different in 2.x, and is build on top of entity reference.

Are we changing so that a menu is associated to a node instead of a group? I haven't followed OG 2.x just wondering :)

I believe this shift in code is correct. Previously there was an og entity, now the group gid is the nid. The intent of the patch was to shift the loading context to load via the nid of the passed group. That said, I may have messed up and made an incorrect assumption about the calling context of some of those functions. Where it uses nid the assumption is that the node in question *is* the group. From a cursory review of the code that seemed like a fair assumption.

edit: This should be be applied to a new branch as it will not work with og 1.x.

jgraham’s picture

Updated patch fixes several additional issues with the migration.

jgraham’s picture

Updated patch fixes issues with og_menu_get_menus()

jgraham’s picture

Re-rolled patch to account for the several commits from 2012/2/6

jgraham’s picture

Ignore the patch in the last comment.

rv0’s picture

Nice to see all the efforts here.

I am willing to start a 7.x-3.x branch for it, but I'd rather put my efforts rethinking this module from scratch in a 7.x-4.x branch.
jgraham, would you have time and interest to maintain the port?

jgraham’s picture

Updated patch with additional fixes.

Re; 3.x branch & 4.x branch, and rewrite

From writing this patch I have some concerns regarding some of the logic/architecture in the current codebase. I would rather spend my time helping to move a re-architected solution forward than supporting the legacy codebase.

That said, it seems like this should remain as a patch against 2.x as a stopgap for support against og 2.x and a complete rewrite should happen ASAP. Then when the rewrite is done we can immediately deprecate all previous versions in favor of the new og_menu version.

So yeah I will be minimally maintaining this patch against 2.x until a rewrite is available. I think it is in everyone's best interest to concentrate our efforts around the rewritten module rather than draining resources supporting a version that will be soon deprecated.

rv0’s picture

@jgraham
There's a lot of things that could be done much better with our current options. Am willing to team up for a full rewrite, but will take time, and maybe even D8 (got some ideas)
Anyhow I see no real problem in a 3.x branch that offers same functionality as the 2.x branch. We'll just have to cooperate a bit to sync them up. There's some small things i wanna do before 2.1, but other than that its mainly fixing the bugs that arise, no big changes planned in current branch

jgraham’s picture

@rv0
Are you saying that you want to do the rewrite work in 4.x initially targetting d8? That seems premature given that og hasn't stabilized with a release in the 7.x-2.x branch yet. I might be misunderstanding what you're suggesting in 12 though.

Attached patch has several more fixes.

rv0’s picture

@jgraham
Basicly what I'm suggesting is:
OG Menu 7.x-2.x -> compatible with OG 7.x-1.x
OG Menu 7.x-3.x -> compatible with OG 7.x-2.x ( = the patches you are writing)

Both would offer exactly same functionality, and improvements in either branches can be ported forward as well as backward.

OG Menu 7.x-4x -> compatible with OG 7.x-2.x, complete rewrite, targgetted at D8, takes advantage of changes in core that are not in D7.
For example this would be a great improvement: #33122: It is unclear how to delete a menu block from the block administration pages

bonobo’s picture

The compatibility between the 2.x and 3.x branches of OG Menu is not an option we have an interest in supporting.

A rewrite (or upgrade) within D7 is something we'll be doing regardless, as it's needed due to the changes in OG. We'd love to work with other folks on this.

Whether this is the 3.x or 4.x branch of OG Menu, or whether it makes more sense for that to be an entirely separate project, is an open question, but the one thing that is off the table (as far as our time) is supporting backports from a rewrite into the existing (and soon to be obsolete) 2.x branch of OG Menu.

(and jgraham and I work together, fyi).

rv0’s picture

We have a fair amount of sites based on the old OG platform, and all of them use OG Menu. Updating those sites to the new OG platform would just be too costly. I'm not sure if we'll already risk using the unstable OG version on new projects. So OG Menu 2.x will be supported at least by me for quite a while. And new features may be added. I'm not asking for help on backporting new functionality from 3.x to 2.x branch at all.

jgraham’s picture

Updated patch fixes

Notice: Array to string conversion in DatabaseStatementBase->execute() (line 2139 of /srv/www/vbdev/html/includes/database/database.inc).
bulldozer2003’s picture

@jgraham Your patch is most helpful, thank you. By the discussion, I presume og_menu-7.x-3.x will be og_menu-7.x-2.x with these and any further patches to support og-7.x-2.x?

Jackinloadup’s picture

changed creation of menu from node form to use
hook_group_insert()
opposed to
hook_node_insert()

this allows us to record the correct gids and such

Jackinloadup’s picture

More fixes. Looks like its at a usable state to me.

There is a small quark still in editing with admin/structure/og_menu and getting dropped into admin/structure/menu

Also the "Enable menu for this group" checkbox needs to allow users to delete the menu when its unchecked.

zipymonkey’s picture

Thanks for your work on this. Had a pretty major issue with this patch where all I get is an error message and nothing else whenever I try to edit a group or have the menu block display. The error message is below.

EntityMetadataWrapperException: Invalid data value given. Be sure it matches the required data type and format. in EntityMetadataWrapper->set() (line 122 of /Users/me/Sites/acquia-drupal/sites/all/modules/entity/includes/entity.wrapper.inc).

  • entity 7.x-1.0-rc1
  • Entityreference 7.x-1.0-rc1
  • og 7.x-2.0-alpha2
  • og_menu 7.x-2.x-dev with og_menu-og-2.x-1423564-20.patch
Jackinloadup’s picture

#21 ah yes. I didn't look into that issue. It got really late. I know from the #20 patch you can add and edit menus

I guess i lied when i said it was ready.

TODO: verify visual interface
TODO: integrate with menu_block?

Jackinloadup’s picture

here is a patch that fixes

og_menu single and og_menu multiple blocks
though og_menu multiple shows all menus under one block with no titles atm

adding og_menu's via the group->menus->add menu now associates the group instead of the node id

Jackinloadup’s picture

fixed bug that allowed group administrators to add more menus then allowed in og_menu_max_menus_per_group
NOTE: the setting in og_menu_max_menus_per_group does not apply to users with the site wide 'administer menu' permission as it currently is setup.

Also changed the wording on the site wide permission 'administer og menu' to be more clear that it allows admin access to ALL menus.

jgraham’s picture

Re-roll of 24 removed call to og_load_multiple() in og_menu_form_menu_edit_menu_alter().

jgraham’s picture

Patches in 23,24, and 25 are broken there was some regression from 17/20 where og_get_group() was re-introduced. That function no longer exists in og 2.x

Looking to re-roll now.

jgraham’s picture

Jackinloadup are you using og 2.x? gid as well as og_get_group() no longer exists in og 2.x.

The only version of the patches in this issue that work for me is patch 17. Patches 20-25 all re-introduce og_get_group() which is no longer available in og 2.x.

re-roll of patch in 17 that resolves the following issue;

Error message
Notice: Undefined property: stdClass::$menu_title in og_menu_edit_item_form() (line 164 of /srv/www/vbdev/html/profiles/julio/modules/og_menu/og_menu.pages.inc).
zipymonkey’s picture

Thanks @jgraham. I was also started working on a patch for this since I was thinking things went a bit amiss after patch 17... probably should have been watching the thread :/

I just install patch 25 and I found a few things.

  1. To make menu title link back to group node. At line 217 of og_menu.module
    -        $block['subject'] = l($menu->title, 'node/' . $context->etid);
      +        $block['subject'] = l($menu->title, 'node/' . $context['gid']);
  2. It looks like the .group-audience class is no longer added to group-audience fields so the js is breaking.
  3. Not sure if this is relate do the js issue.. but when I edit a node content type I started to get duplicate menu entries.
jgraham’s picture

zipymonkey please try the patch in 27 (a re-roll of 17), the one in 25 is a re-roll of 24 which has numerous issues.

zipymonkey’s picture

Typo. I installed 27.

jgraham’s picture

Status: Needs review » Needs work
FileSize
13.59 KB

Re-roll of 27 addresses 28.1

@zipmonkey
28.2: Can you give more details on how to reproduce, what you are expecting to happen and what isn't happening?
28.3: Can you give steps to reproduce as well? Are you editing the group node or content in that group?

zipymonkey’s picture

Re 28.2 - To reproduce try to create a new group content item as a group member (not group admin). OG groups are not populated in the parent item select list. I got this to work using the following... pretty sure this isn't the ideal solution.

og_menu.module. @ line 187

+  $settings['field_names'] = og_get_group_audience_fields();

og_menu.js  @ line 14

+      var selectors = '';
+
+      for( var i in Drupal.settings.og_menu.field_names) {
+    	selectors += 'input[name^="' + i + '"], select[name^="' + i + '"],'; 
+      }

@ line 64
-      if ($('select.group-audience').size()) {
-        $('select.group-audience').change(toggleSelect).ready(toggleSelect);
+      if ($(selectors).size()) {
+        $(selectors).change(toggleSelect).ready(toggleSelect);

@ line 69 
-      toggle($('select.group-audience').val());
+      toggle($(selectors).val());

Re 28.3: After making the changes above I could not duplicate this. Otherwise a new menu items was create each time you updated a node that was a group content type and checked the 'Provide a menu link' option.

rv0’s picture

An issue summary would come in handy.
It would be good to see the difference between new features/bug fixes/porting changes.

I am planning to commit a few changes and release a 2.1 version soon of OG Menu, which will probably break the patches in this thread.
And after that, I would like to create a new branch (3.x) based on the patches in this topic (co-maintainers for this branch would be nice!)

Btw, as some people may have noticed, there's some fixes in D6 Og Menu that never made it into D7 version. I don't know how that happened, had something to do with security issues and CVS which happened way before my time.
An example of an older issue that never made it into D7 codebase:
#793854: og_menu inadvertently implements hook_get_menus()
It appears that 6.x-2.4 has more recent code than 6.x-2.x-dev.
Those issues should be tackled too if still relevant today.

zipymonkey’s picture

I found another issue where I was getting an error of The menu you chose does not belong to the selected groups. when group members attempt to add an item to an og_menu.

@line 580 of og_menu.module

-  if (isset($form_state['values']['group_audience'][LANGUAGE_NONE])) {
-    foreach ($form_state['values']['group_audience'][LANGUAGE_NONE] as $item => $gid) {
-  	  $gids[] = $gid['gid']; 
+  if (isset($form_state['values']['og_group_ref']['und'])) {
+    foreach ($form_state['values']['og_group_ref']['und'] as $item => $gid) {	 
+    	$gids[] = $gid['target_id']; 
rv0’s picture

above is wrong. you should use [LANGUAGE_NONE]

zipymonkey’s picture

Thanks @rv0

jgraham’s picture

Re-roll of 31, addresses 34.

This still doesn't address the details in 28 as further detailed in 32 I need to get an installation setup to test this.

bulldozer2003’s picture

I think you should really start a 3.x branch for these, it seems tedious to post patches for every change and re-patch every time the repo updates.

rv0’s picture

@bulldozer2003
agreed.
I do believe it's kinda sad that my earlier call to cooperate and keeping the features in sync for 2.x and 3.x branch has been somewhat declined
As can be seen in git, some changes have been made to the code the past days, and I got a some stuff pipelined in local code.
I'm planning to get this in the git codebase asap and release a 2.1 version. Planning a feature freeze and "bug fix only" releases for the 2.x branch after that.

The short answer: yes I will create 3.x branch, but please just have a little bit more patience till I get 2.x finalized, the end is near ;)

jgraham’s picture

I think there is/was some confusion in the dialogue in comments 10-16.

rv0, can you follow up here once your feature freeze for 2.x is complete? At that point I think we should assess how to handle the 3.x branch and associated patches here.

zipymonkey’s picture

A 3.x release sounds like the way to go. I can definitely help out testing patches.

Looks like the OG Permission name changed (see line 386 or og_menu.module).

-     if (user_access('administer organic groups')) {
+    if (user_access('administer group')) {

As for #32, it looks like there is a bigger issue here if any of the og_group_ref fields are using the autocomplete widget. The javascript change event around line 65 of og_menu.js is grab the value enter by the user not the value selected using autocomplete. If I'm reading this (http://drupal.org/node/365241) right there is current not an event that fires on an autocomplete select.

jgraham’s picture

This is a different approach I re-rolled these by hand to itemize the changes by items they are addressing. Hopefully this will help improve the process and discussion around the patch process.

  1. 1-og_load_multiple.patch: Remove use of og_load_multiple()
  2. 2-og_get_group.patch: Remove use of og_get_group()
  3. 3-administer_group_permission.patch: Change og group permission name
  4. 4-use-of-etid-rather-than-gid.patch: Remove use of etid
zipymonkey’s picture

Howdy, uploading a patch to fix a couple of issues. Hopefully this is helpful.

  1. Replaced og_get_context_by_url with og_context
  2. Replaced group->gid with group['gid'] in several places
  3. Update og_menu_form_node_form_alter and og_menu_block_view forms call to og_get_entity_groups and og_context
  4. Replace og_user_access_by_entity function call with og_user_access_entity
  5. Updated JS to trigger on change to new field name (starts with 'og_group_ref')

This patch should have all the changes from #42 and is based on the newest release 2.x dev release.

rv0’s picture

@ zipymonkey

Replaced og_get_context_by_url with og_context

Why?
This makes sense on node creation, when using group based permissions.

zipymonkey’s picture

Unless I'm mistaken the og_get_context_by_url function has been removed from OG 2.x. My understanding is that populating group information based on the URL should be handled using the entityreference_prepopulate module.

rv0’s picture

@zipymonkey,
my bad. seems you are right about entityreference_prepopulate
But you'll still want the group context to provide the group based og_menu permissions..
Maybe this: og_context_handler_url()

rv0’s picture

btw, committed the last bits of code today.. I think appart from bugfixes, no fancy changes will be made to the current branch.
(read: feature freeze, bugfixes only)
so an OG Menu 7.x-2.1 soon-ish if no big bugs arise in the first few days.

I can create a 3.x branch with the patches here, but I hope someone will take on maintainership for it as I wont have much time to support it.

Anyone?

zipymonkey’s picture

Thanks @rv0. I looks like that handler (if enabled) should be run by og_context and that it'll return FALSE if entityreference_prepopulate is not enabled and configured for the group_audience field. I'll see about getting that enabled and try it out.

zipymonkey’s picture

#46 Looks like the entityreference_prepopulate module allows group context to be passed through the URL on node create using the og_context function (does that make sense?). So I think the previous patch should work.

#47 I've not helped maintain a module before so I would defer to someone with more experience, but will if no one else is able.

Also, I've attached another patch that replaces references to the og table with og_membership table.

jgraham’s picture

FileSize
12.63 KB

I couldn't get the patch in 49 to apply, it looks like it was rolled prior to commit04ad452

Attached is a re-roll of 49 that applies to 7.x-2.x HEAD

jgraham’s picture

FileSize
13.08 KB

Re-roll of 50 that resolves the following errors:

Notice: Undefined variable: og in og_menu_node_update() (line 605 of /webroot/julio-7.x-1.x-dev/profiles/julio/modules/og_menu/og_menu.module).
Notice: Trying to get property of non-object in og_menu_node_update() (line 605 of /webroot/julio-7.x-1.x-dev/profiles/julio/modules/og_menu/og_menu.module).
pgillis’s picture

Patch in #51 works for me. I assume it you will want to wait for more feedback before saying it was "reviewed & tested by community"

jide’s picture

A working-but-still-with-bugs version could definitely be in a -dev branch. That's what they are here for, and it seems that work being done here is advanced enough for it. My opinion :)

zipymonkey’s picture

Just installed patch in #51. I think it is a very good start and I agree that we should package this up as 3.x-dev branch.

ditcheva’s picture

I'm using the 7.x-2.x version of og and am noticing that the 'OG Menu : multiple' block just displays all the menu items for a certain group in a single block, and does not separate them out by menu as is implied in the project description.

Just wanted to mention this in case it is a bug.

I have two menus for my group, but the 'OG Menu: multiple' block just lists all their menu items in one single list, rather than separated out by menu...

Perhaps this issue is related to the update?

rv0’s picture

imo, OG Menu:multiple block should be deprecated and replaced by something better

@jide
Haven't had the time to do it myself, but I'm also +1 for the new branch.
You still have commit access, right?

ditcheva’s picture

Re the OG Menu: multiple block.

It's important for groups to be able to have multiple menus. Groups can get big and complicated and organizing links in multiple sections will be necessary. The way it stands with the 2.x branch, however, is that there's no way to display the various menus for a group once these multiple menus are created.

I mean, you can enable the said block and that displays the links from all the menus together in an un-organized bundle (not separated by menus). Even if the menus weren't displayed in separate blocks, I was hoping their links will be separated from each other via a title or something if the links are within the same block.

In any case, I'm still unclear on whether that's a bug or the intended behavior, but am gathering that this won't be just a quick bug fix if folks are for a re-write of that block's functionality. Is that true?

rv0’s picture

Status: Needs review » Fixed

Dev branch created.

Please use the issue queue for new issues.

pgillis’s picture

Can you add this branch to the list of available downloads so that drush can get it? Thanks.

rv0’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev

Snapshot release needed time.
You're lucky, it has just become available :)

jide’s picture

Yay ! @rv0 : Sorry, didn't see your comment about my commit access.

Status: Fixed » Closed (fixed)

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