Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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
Comment | File | Size | Author |
---|---|---|---|
#51 | og_menu-1423564-51.patch | 13.08 KB | jgraham |
#50 | og_menu-1423564-50.patch | 12.63 KB | jgraham |
#49 | og_menu-2.x-1423564-og_membership_table.patch | 12.72 KB | zipymonkey |
#43 | og_menu-2.x-1423564-42_plus_misc.patch | 11.11 KB | zipymonkey |
#42 | og_menu-2.x-1423564-42-1-og_load_multiple.patch | 1.68 KB | jgraham |
Comments
Comment #1
jgraham CreditAttribution: jgraham commentedog 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.
Comment #2
jgraham CreditAttribution: jgraham commentedFirst pass at a patch to update this to work with og 7.x-2.x-dev.
Comment #3
rv0 CreditAttribution: rv0 commentedWill it still work with og 1.x ?
Comment #4
pontus_nilssonJust something I saw in the patch
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 :)
Comment #5
jgraham CreditAttribution: jgraham commentedNope. The underlying architecture in og is considerably different in 2.x, and is build on top of entity reference.
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.
Comment #6
jgraham CreditAttribution: jgraham commentedUpdated patch fixes several additional issues with the migration.
Comment #7
jgraham CreditAttribution: jgraham commentedUpdated patch fixes issues with og_menu_get_menus()
Comment #8
jgraham CreditAttribution: jgraham commentedRe-rolled patch to account for the several commits from 2012/2/6
Comment #9
jgraham CreditAttribution: jgraham commentedIgnore the patch in the last comment.
Comment #10
rv0 CreditAttribution: rv0 commentedNice 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?
Comment #11
jgraham CreditAttribution: jgraham commentedUpdated 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.
Comment #12
rv0 CreditAttribution: rv0 commented@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
Comment #13
jgraham CreditAttribution: jgraham commented@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.
Comment #14
rv0 CreditAttribution: rv0 commented@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
Comment #15
bonobo CreditAttribution: bonobo commentedThe 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).
Comment #16
rv0 CreditAttribution: rv0 commentedWe 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.
Comment #17
jgraham CreditAttribution: jgraham commentedUpdated patch fixes
Comment #18
bulldozer2003@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?
Comment #19
Jackinloadup CreditAttribution: Jackinloadup commentedchanged 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
Comment #20
Jackinloadup CreditAttribution: Jackinloadup commentedMore 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.
Comment #21
zipymonkey CreditAttribution: zipymonkey commentedThanks 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).
Comment #22
Jackinloadup CreditAttribution: Jackinloadup commented#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?
Comment #23
Jackinloadup CreditAttribution: Jackinloadup commentedhere 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
Comment #24
Jackinloadup CreditAttribution: Jackinloadup commentedfixed 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.
Comment #25
jgraham CreditAttribution: jgraham commentedRe-roll of 24 removed call to og_load_multiple() in og_menu_form_menu_edit_menu_alter().
Comment #26
jgraham CreditAttribution: jgraham commentedPatches 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.
Comment #27
jgraham CreditAttribution: jgraham commentedJackinloadup 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;
Comment #28
zipymonkey CreditAttribution: zipymonkey commentedThanks @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.
Comment #29
jgraham CreditAttribution: jgraham commentedzipymonkey 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.
Comment #30
zipymonkey CreditAttribution: zipymonkey commentedTypo. I installed 27.
Comment #31
jgraham CreditAttribution: jgraham commentedRe-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?
Comment #32
zipymonkey CreditAttribution: zipymonkey commentedRe 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.
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.
Comment #33
rv0 CreditAttribution: rv0 commentedAn 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.
Comment #34
zipymonkey CreditAttribution: zipymonkey commentedI 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
Comment #35
rv0 CreditAttribution: rv0 commentedabove is wrong. you should use [LANGUAGE_NONE]
Comment #36
zipymonkey CreditAttribution: zipymonkey commentedThanks @rv0
Comment #37
jgraham CreditAttribution: jgraham commentedRe-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.
Comment #38
bulldozer2003I 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.
Comment #39
rv0 CreditAttribution: rv0 commented@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 ;)
Comment #40
jgraham CreditAttribution: jgraham commentedI 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.
Comment #41
zipymonkey CreditAttribution: zipymonkey commentedA 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).
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.
Comment #42
jgraham CreditAttribution: jgraham commentedThis 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.
Comment #43
zipymonkey CreditAttribution: zipymonkey commentedHowdy, uploading a patch to fix a couple of issues. Hopefully this is helpful.
This patch should have all the changes from #42 and is based on the newest release 2.x dev release.
Comment #44
rv0 CreditAttribution: rv0 commented@ zipymonkey
Why?
This makes sense on node creation, when using group based permissions.
Comment #45
zipymonkey CreditAttribution: zipymonkey commentedUnless 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.
Comment #46
rv0 CreditAttribution: rv0 commented@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()
Comment #47
rv0 CreditAttribution: rv0 commentedbtw, 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?
Comment #48
zipymonkey CreditAttribution: zipymonkey commentedThanks @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.
Comment #49
zipymonkey CreditAttribution: zipymonkey commented#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.
Comment #50
jgraham CreditAttribution: jgraham commentedI 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
Comment #51
jgraham CreditAttribution: jgraham commentedRe-roll of 50 that resolves the following errors:
Comment #52
pgillis CreditAttribution: pgillis commentedPatch in #51 works for me. I assume it you will want to wait for more feedback before saying it was "reviewed & tested by community"
Comment #53
jide CreditAttribution: jide commentedA 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 :)
Comment #54
zipymonkey CreditAttribution: zipymonkey commentedJust 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.
Comment #55
ditcheva CreditAttribution: ditcheva commentedI'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?
Comment #56
rv0 CreditAttribution: rv0 commentedimo, 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?
Comment #57
ditcheva CreditAttribution: ditcheva commentedRe 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?
Comment #58
rv0 CreditAttribution: rv0 commentedDev branch created.
Please use the issue queue for new issues.
Comment #59
pgillis CreditAttribution: pgillis commentedCan you add this branch to the list of available downloads so that drush can get it? Thanks.
Comment #60
rv0 CreditAttribution: rv0 commentedSnapshot release needed time.
You're lucky, it has just become available :)
Comment #61
jide CreditAttribution: jide commentedYay ! @rv0 : Sorry, didn't see your comment about my commit access.