Current architecture adds necessary data to $node object in hook_node_prepare(), which is then carried to hook_node_update(). If this data is not present, og_menu settings gets dropped after node save (menu might even get deleted?). This is not a problem when editing nodes normally using the node form.

When using custom node_load() and followed by node_save() functions this fails, because hook_node_prepare() is not triggered in this process. Easy workaround is to add og_menu_node_prepare(&$node) to custom code before calling node_save(), but considering the amount of other contrib modules using the node_save() method a more permanent fix might be better.

Maybe some of the necessary data population could be moved to hook_node_load()?

CommentFileSizeAuthor
#19 og_menu-node-update-1330550-19.patch712 bytesosopolar
#12 og_menu-nodeload-1330550-11.patch1.26 KBgmclelland
#11 og_menu-nodeload-1330550-11.patch1.27 KBmarkusbroman
#10 og_menu-nodeload-1330550-10.patch1.32 KBmarkusbroman
#7 og_menu-nodeload-1330550-7.patch2.63 KBrobeano

Comments

rv0’s picture

rv0 commented

I don't think this will result in data being dropped, because there is no data about og_menu saved in the node afaik

Which content are we talking about, groups or group-content?

juhaniemi’s picture

juhaniemi commented

I'm talking about the group node, where the "Enable menu for this group" checkbox is. At least in my site's case saving the group node with node_save() puts that checkbox disabled.

This code is related. Some of my notes are in comments, prefixed with NOTE.

/**
 * Implementation of hook_node_update()
 */
function og_menu_node_update($node) {
  if (og_is_group_type('node', $node->type)) {
    $og = og_get_group('node', $node->nid, FALSE, array(OG_STATE_ACTIVE, OG_STATE_PENDING));
    // NOTE: The following $node->og_menu is set in og_menu_node_prepare(), if a menu is enabled for this group
    if ($node->og_menu) { 
      $menu = og_menu_get_menus(array($og->gid));
      if (empty($menu)) {
        menu_save(array(
          'menu_name' => 'menu-og-' . $og->gid,
          'title' => $node->title,
          'description' => t('OG Menu for') . ' ' . check_plain($node->title),
        ));
        og_menu_update_menu('menu-og-' . $og->gid, $og->gid);
      }
    }
    else {
      // NOTE: When $node->og_menu not set
      // We can't assume that the menu name is 'menu-og-[gid]' - we need to look up menus associated with this group
      $result = db_query('SELECT menu_name FROM {og_menu} WHERE gid = :gid', array(':gid' => $og->gid))->fetchCol();
      foreach ($result as $menu_name) {
        // NOTE: The following is deleting all the menus that might have been assigned to this group?
        menu_delete(array(
          'menu_name' => $menu_name,
          'title' => $node->title,
          'description' => t('OG Menu for') . ' ' . check_plain($node->title),
        ));
        og_menu_update_menu($menu_name);
      }
    }
  }
}
rv0’s picture

rv0 commented

before I sort this out, I wanna see what kind of changes to the code #1313426: 'Enable menu for this group' setting does not appear in the 'Manage Fields' form would cause, because that might change a lot about this issue (not sure, just thinking out loud)

rv0’s picture

rv0 commented
Status: Active » Postponed
gmclelland’s picture

gmclelland commented

This might be what is causing this issue #1393958: Menus are lost when using Panelizer module. When I edit("Customize this page") my layout using Panels IPE, it blows away my og_menus and menu items.

Maybe Panels/Panelizer isn't using hook_node_prepare() when saving? Just trying to figure this out. :(

nicksanta’s picture

nicksanta commented

I'm experiencing this issue as well. I've fixed it for the time being by jumping in hook_node_update before og_menu and adding in the $node->og_menu flag. Use the following code in a custom module and replace all instances of 'HOOK' with your module's machine name.

<?php
/**
 * Implement hook_module_implements_alter
 */
function HOOK_module_implements_alter(&$implementations, $hook) {
  if ($hook == 'node_update') {
    if (isset($implementations['HOOK'])) {
      unset($implementations['HOOK']);
      $new_hooks = array('HOOK' => FALSE);
      foreach ($implementations as $module => $group) {
        $new_hooks[$module] = $group;
      }
      $implementations = $new_hooks;
    }
  }
}

/**
 * Implement hook_node_update
 */
function HOOK_node_update(&$node) {
  $group_content_type = 'group';
  if ($node->type == $group_content_type) {
    $node->og_menu = TRUE;
  }
}
?>
robeano’s picture

robeano commented
Priority: Normal » Minor
Status: Postponed » Needs work
StatusFileSize
new og_menu-nodeload-1330550-7.patch2.63 KB

I ran into this same issue when there was a call to node_save() from another module. A node is not always saved directly from the node edito form. In that case, og_menu can delete its own menu because it was never loaded with the node in the first place i.e. in og_menu_node_update() $node->og_menu is not set because og_menu_node_prepare was never called.

The following patch adds og_menu data to the node on load. I've tested this thoroughly with group nodes which want an og_menu. I've tested it a little with og groups which do not want an og_menu. It could use more testing.

The patch was created from 7.x-2.x version of og_menu

gmclelland’s picture

gmclelland commented

@robeano - This patch sounds great, but I can't get it to apply with git or drush make. I think the paths are wrong in the patch. Can you reroll from the module's directory?

rv0’s picture

rv0 commented

--- a/www/sites/default/modules/og_menu/og_menu.module
+++ b/www/sites/default/modules/og_menu/og_menu.module

is indeed the problem

thanks for the contribution!

markusbroman’s picture

markusbroman commented
Status: Needs work » Needs review
StatusFileSize
new og_menu-nodeload-1330550-10.patch1.32 KB
markusbroman’s picture

markusbroman commented
StatusFileSize
new og_menu-nodeload-1330550-11.patch1.27 KB

Sorry, made the same mistake

gmclelland’s picture

gmclelland commented
StatusFileSize
new og_menu-nodeload-1330550-11.patch1.26 KB

Had to fix the paths for it to apply. Let's review this one.

gmclelland’s picture

gmclelland commented

Ok, I'm a little confused. One patch is modifying hook_node_prepare() and the other patch is using hook_node_insert(). So which one should we use?

The patch I made to work in #12 is a reroll of #11. With the patch in #12 I get the following error when trying to create a group node.

Fatal error: Call to undefined function og_load_multiple() in /home/quickstart/websites/updater.dev/profiles/cmf/modules/contrib/og_menu/og_menu.module on line 663 Call Stack: 0.0007 328832 1. {main}() /home/quickstart/websites/updater.dev/index.php:0 0.0912 6244620 2. menu_execute_active_handler() /home/quickstart/websites/updater.dev/index.php:21 0.0915 6246336 3. call_user_func_array() /home/quickstart/websites/updater.dev/includes/menu.inc:517 0.0915 6246588 4. node_add() /home/quickstart/websites/updater.dev/includes/menu.inc:0 0.0923 6268800 5. drupal_get_form() /home/quickstart/websites/updater.dev/modules/node/node.pages.inc:66 0.0923 6269532 6. drupal_build_form() /home/quickstart/websites/updater.dev/includes/form.inc:123 0.1703 8826844 7. drupal_prepare_form() /home/quickstart/websites/updater.dev/includes/form.inc:332 0.1741 8969400 8. drupal_alter() /home/quickstart/websites/updater.dev/includes/form.inc:1058 0.1868 9028632 9. og_menu_form_node_form_alter() /home/quickstart/websites/updater.dev/includes/module.inc:1018 0.1870 9029048 10. og_menu_get_menus() /home/quickstart/websites/updater.dev/profiles/cmf/modules/contrib/og_menu/og_menu.module:303

gmclelland’s picture

gmclelland commented

Oh, sorry I was using the og-2.x branch. That's what was probably causing the errors.

robeano’s picture

robeano commented

@gmclelland Is there still confusion? As far as hook_node_load vs. hook_node_prepare(), it isn't clear to me that we can drop all of hook_node_prepare(). There's some additional code in that hook and I'm not really sure if it's appropriate to move it all to hook_node_load(). The minor change in hook_node_insert() was made because I was hitting an error there when I added hook_node_load(). Maybe there's a better way to handle it?

@markusbroman thanks for re-rolling!

rcaracaus’s picture

rcaracaus commented

Patch in #12 gave me error. Needed to change og_menu_get_menu to just menu_get_menu.

rv0’s picture

rv0 commented
Status: Needs review » Fixed

Fixed for 7.x-2.x-dev
http://drupalcode.org/project/og_menu.git/commit/3526827

Status: Fixed » Closed (fixed)

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

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
commented
Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Needs review
StatusFileSize
new og_menu-node-update-1330550-19.patch712 bytes

Here comes the patch for version 6.x-2.x-dev.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes
Status: Needs review » Closed (outdated)

Closing, since the Drupal 6 version of OG Menu is no longer supported. If this issue is still relevant for the currently supported versions (D7 and D8) then feel free to update the version and reopen the issue.