Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Trying out the new Menu Links support described here http://drupal.org/node/633804 I was unable to get the Menu Links to import/revert. Currently working with a fresh copy of Drupal 6.17 and the current Features dev.
I tried it two ways:
1. Made an sql dump, built the menu, exported the feature, imported the sql dump, turned on the feature, tried to revert Menu Links listed as OVERRIDDEN.
2. Built the menu, exported the feature, removed some menu items that were exported, enabled the feature, tried to revert Menu Links listed as OVERRIDDEN.
Comment | File | Size | Author |
---|---|---|---|
#44 | features-860974-menu-links-not-reverting-44.patch | 2.83 KB | goldenboy |
#43 | features-860974-menu-links-not-reverting-43.patch | 2.33 KB | goldenboy |
#40 | features-860974-menu-links-not-reverting-40.patch | 4.35 KB | q0rban |
#11 | features.860974.patch | 871 bytes | jhedstrom |
#7 | aaml_primary_links.tar_.gz | 2.13 KB | lobo235 |
Comments
Comment #1
yhahn CreditAttribution: yhahn commentedWhoops, can you see if this commit fixes things? http://drupal.org/cvs?commit=396186
Comment #2
yhahn CreditAttribution: yhahn commentedI believe this is fixed now, please open a new ticket if it reoccurs.
Comment #3
cabbiepete CreditAttribution: cabbiepete commentedStill occuring for me. I find I can only get the menu imported at all when enabling the menu.
Have issues with the items it is importing also but there are other bugs covering those so will update there.
Comment #4
cabbiepete CreditAttribution: cabbiepete commentedAttached the menu custom and menu links file out of the feature export that I am testing with.
Comment #5
yhahn CreditAttribution: yhahn commentedHm, using the files you've provided I get a menu called "M&G Administration Menu" with a handful of menu items as well.
Any more information you can provide as to why this might not be working for you?
Comment #6
hefox CreditAttribution: hefox commentedNot sure if this will fix current issues, but it fixes mine
Link get saved correctly, but soon as a menu rebuild comes along, get reverted back, cause not set to 'custimized'.
In the original hook, the actually menu hook was altered so customized = 0 was good, but now since the actual menu router items are not changed, need to mark it as custmized or menu rebuild can wipe changes sometimes.
I'm also suspicious of this line,
Particularly the $link, $menu_links, should likely be
But haven't gotten around to testing that, so just posting the first for now.
Comment #7
lobo235 CreditAttribution: lobo235 commentedI have a feature that contains our primary links menu and the feature when enabled/reverted does not restore the menu links correctly. I have attached the feature tarball. I am using Features version 6.x-1.0 and Drupal core 6.19.
Comment #8
jgraham CreditAttribution: jgraham commentedI was experiencing a similar issue. The patch provided in #6 applies cleanly to 1.0 and resolves the issue.
Without having dug into this too deeply I think in our case the issue was triggered for any link that already had an entry registered in menu_router. We have a few features with additional navigation options exposed in additional menus. This issue seemed to trigger consistently for items that were essentially duplicate navigational entries and not necessarily unique.
Comment #9
hefox CreditAttribution: hefox commentedThat is what was happening with also, jgraham; if it existed by default in another menu via hook_menu, it'd change to that menu, leading to duplicate menu links in the other menu >.O The patch just tells menu router that the links are custimized, so no touchie.
Comment #10
scottrigby@hefox ty - #6 sorts this issue for us. Also, prior to this, drush cc would reset the menu items back to the module that already inserted them initially via hook_menu - and drush fr would place them back in our custom menu - causing a duplicate each time, but also requiring constantly reverting the feature. lovely. I'm mark RTBC, hopefully not prematurely ;)
Comment #11
jhedstromWhen exporting menu links with a parent item that was not defined by a feature (in this case, it was defined by a view), the fix in #6 didn't fix the issue for me. The attached patch gets this working, and still contains the change from #6.
Comment #12
hefox CreditAttribution: hefox commentedjhedstrom: #910604: parent_path set to path not in hook_menu_default_menu_links causes menu link to not revert. Patch does the same TMK, but changes the first if statement instead making a new one.
Also, #920888: Issues when adding a non-customized menu link may be relavent.
(I'm a bit ornery at times about making a new issue for different bugs, or didn't really associate this with the other two.)
Comment #13
archnode CreditAttribution: archnode commentedI did a little bit of testing on the issue because of a problem that might be related:
I tried to add a custom menu item as a parent item of a menu item that was provided by views. Both menu items have the same path. When I add the new menu item it marks the feature that provides the view with the menu item as overridden.
I can't export the custom menu item in a feature, it doesn't get displayed in the feature creation screen.
Neither one of the patches provided worked for me.
Comment #14
hefox CreditAttribution: hefox commentedKeeping track of different issues in one issue is too complicated, so going to use this to post to other issues.
#927576: Menu links not set as customized, revert when menu rebuilt -- (#6 patch) Items not set as customized (see issue for why I broke it off, despite being reviewed).
#927566: Add link title to menu link identifiers to make them more unique. -- (#13) Archnode's issue (I had given it some thought a few weeks ago, but it's more complicated then it sounds initially).
#910604: parent_path set to path not in hook_menu_default_menu_links causes menu link to not revert -- (sorta #11) parent path not handled by features issue.
#920888: Issues when adding a non-customized menu link -- Export having values that for some reason the rebuild didn't like.
(jhedstrom, could you please review 910604?)
Comment #15
greg.harveyI am getting a fifth, more generic case ... we have menus already on a stage site, when we change those menu items and re-export our feature, copy it up to stage and revert all features our menu items do not update. The only way to revert them to the feature's captured state in code is to physically delete each affected menu item, which could be hundreds.
Is anyone else seeing this? Is that what the patch here is supposed to address?
Comment #16
DamienMcKennaAm seeing some oddity here too, will report what I find.
Comment #17
ezra-g CreditAttribution: ezra-g commentedI marked #927576: Menu links not set as customized, revert when menu rebuilt as a duplicate of this patch.
Comment #18
hefox CreditAttribution: hefox commentedAs mentioned in 14, keeping track of the various reasons why menu links don't import/revert correctly was getting messy in this issue, so created the other issue for that specific bug.
As being mentioned in #744450: Why would a feature not revert?, there's some many ways for features/feature components to not import/revert/etc., so these general issues turn fugly.
I believe I asked yhahn's opinion about how best to handle the issues before splitting it up, but it was a while ago.
Would it be objectional to undo the status change? (and mark this active, as I likely should have done earlier.).
Comment #19
ezra-g CreditAttribution: ezra-g commentedI'm running the conglomeration of patches here. I think we need an additional change which I'll describe before I reverse the other patches and re-roll this one ;):
When we do
$existing = features_menu_link_load($identifier);
, the subsequent conditional should only respect existing links if they are not customized, since one expects a customized menu link to be reverted to what's defined in the feature, even if the exported one is itself a custom link.So, we should do
if ((!$existing && !$link['customized']) || in_array($link, $menu_links)) {
.Comment #20
ezra-g CreditAttribution: ezra-g commentedMy apologies: I incorrectly marked this patch as "needs review" due to a code change that was in the scope of the patch at http://drupal.org/node/968826#comment-3804498, which I have re-rolled. Got confused with all these Features menu patches :).
In my experience this patch resolved the issue of menu links not being reverted and allowed me to successfully export hierarchical (parent and children) menu links.
It seems this patch fixes the issues described by folks in the issue, and seems unlikely that without being affected by these issues firsthand and being familiar with the menu system, that folks will take the time to review this issue.
Marking it RTBC in the hopes it gets committed and the benefit of more widespread testing in dev.
Comment #21
ezra-g CreditAttribution: ezra-g commentedTo be clear, I'm referring to #11.
Comment #22
wizonesolutionsLooking good! Let's get it committed!
...and, once again, I'm willing to help commit to 6.x. I noticed the 6.x -dev version isn't currently published...
Comment #23
kndr#11 works for me. Thanks!
Comment #24
kasperg CreditAttribution: kasperg commented#11 worked for me as well.
Comment #25
justafishAny fix the 7.x branch? This appears to be broken for me in the latest beta.
Comment #26
scottrigby@ezra-g - nice one. #11 also works for 6.x-1.0
Comment #27
Lc5 CreditAttribution: Lc5 commented#11 works for me to.
Comment #28
KentBye CreditAttribution: KentBye commentedWe're shooting a features video for Drupalize.me and we ran into this issue with 7.x-1.0-beta2
We created a menu item through a view and reverting wasn't removing it. Clearing the cache did help either.
We did find a brute force method of removing the menu item it by turning off the feature, deleting the View, and then turning the feature back on.
Comment #29
daveparrish CreditAttribution: daveparrish commentedFor version 6.x-1.0 I had the problem where views menu settings would take precedence over menu link settings. I had a menu item created by views and hidden in by the menu link settings. When I would revert the menu link settings the menu link settings would take effect but if I cleared the cache then the view settings would take effect.
My fix was to make sure all the settings were in the views for the weights and I simple removed menu items from views that were supposed to be hidden in menu link settings.
It seems to me that menu link settings should take precedence over the views settings since it seems that is the way it works in the Drupal by default.
Comment #30
bsnodgrass CreditAttribution: bsnodgrass commentedI made changes to the menu link in a dev site. The menu link is coming from a view and the view is in another feature. I have the menu link also exported in another feature. Code looks as expected:
When I apply the features to staging, each feature shows as default but the existing menu 'link_title' in the database is actually overriding the feature.
Comment #31
hefox CreditAttribution: hefox commentedAll the patches are covered by other issues, this is being used more for a tracking issue.
Most people's main issue seems to be the customized issue #927576, which needs to decide whether to just commit it to fix the major bugs and deal with the fallout, or try and find some other way to handle it.
Comment #32
ice5nake CreditAttribution: ice5nake commentedThe inability to revert menu links has been a hindrance in our adoption of features.
Here's my drush features-diff output
Comment #33
Grayside CreditAttribution: Grayside commented@ice5nake menu_links being a poorly implemented system, you might need to manage them via update hooks and leave them out of your Feature export.
Comment #34
ice5nake CreditAttribution: ice5nake commented@Grayside, Any chance you can point me in a direction to the specifics of using update hooks?
Comment #35
ice5nake CreditAttribution: ice5nake commentedHere's another thought, would this Feature request help solve this issue? http://drupal.org/node/968826
Would having uuid numbers for the menu links allow for proper reverting of the menu links. Conceptually it seems like it should.
Comment #36
mpotter CreditAttribution: mpotter commentedClosing this since there are other issues about adding unique ids to menu items and there is a chance that menu links may be dropped from Features entirely.
Comment #37
ezra-g CreditAttribution: ezra-g commentedI spoke to mpotter in IRC and we agreed to keep this issue open. The D6 patch is essentially RTBC but needs to be re-rolled and tested against D7 (though there's a chance that the feature will be removed entirely for D7).
Comment #38
hefox CreditAttribution: hefox commentedThis issue was closed as it was a broad issue that covered various smaller bugs, and the indivudal bugs had/have their own issues, e.g. http://drupal.org/node/910604
Comment #39
mpotter CreditAttribution: mpotter commentedI closed #910604: parent_path set to path not in hook_menu_default_menu_links causes menu link to not revert for inactivity, but if somebody can clarify a patch between that issue and this one that applies to D7 then I'll look at it.
Comment #40
q0rban CreditAttribution: q0rban commentedWow, this is a really confusing set of issues. Trying to grok everything that's going on across all the issues, but it's not easy. Can someone who understands the larger picture please update the summary? As far as I can tell there's really just this issue, about existing menu items not getting reverted, and #927576: Menu links not set as customized, revert when menu rebuilt, where the issue of links not being set as customized is being worked on.
The code itself is confusing as well, and I'm seeing a couple of red flags regarding this issue:
Attached patch addresses the above. The big departure here from the previous version of the patch is that every menu link that is being passed gets saved, instead of only items that did not exist. In addition, menu items that are not passed, are not saved. Because of that, one whole level of indentation was removed, as the if statement becomes unnecessary. It makes the patch look bigger than it is, but it's actually a pretty simple change, just doing an array intersection between the ordered array and the array of actual menu links to be reverted.
Comment #41
q0rban CreditAttribution: q0rban commentedComment #42
tchopshop CreditAttribution: tchopshop commentedI'm trying to save a highly customized Management menu via Features. This is a case where none of the menu links are nodes -- just system paths. I use this for my own responsive Admin menu. I cannot get the feature to revert, which means I have to redo the Management menu each time. Very frustrating. The patch in #40 doesn't work. I've tried all the other patches also.
Comment #43
goldenboy CreditAttribution: goldenboy commentedI changed the version of the bug to 7.x-2.x-dev because the last patch submitted refers to that version.
Hi, I think that the real problem is related to
features_menu_link_load
function. More precisely, where the link identified by parameter$identifier
is compared to the ones extracted by the queries.Consider the following use case:
A site that exposes 2 organisations in his main menu. The main menu will have following structure:
When we export the links in a feature we'll get the following identifiers:
Now, we have the feature and we want to import the menu in out production site. This is what happens.
When we revert the feature,
features_menu_link_load
is called for each exported link bymenu_link_features_rebuild_ordered
.Inside
features_menu_link_load
, after executing the queries, we cycle over the result and, if the identifier isn't the same, we look at the title.So, in the use case described, the second link with title
about
is matched and is returned.It's not what you expect because the second
about
belongs to another subtree.Instead, this behaviour could be accepted in case there are 2 links with the same title which are siblings.
So, IMHO, I think we need to consider the parent to solve this bug.
A solution is to add a parameter to the
features_menu_link_load
function to pass in the parent identifier. Then, inside the function we can load the parent, if exists, and modify the conditional statements to consider the parent when we compare the titles.Attached there is a patch.
Comment #44
goldenboy CreditAttribution: goldenboy commentedThe last patch broke the export workflow. When features executes
menu_links_features_export
the title-match-condition doesn't work anymore.Attached a patch that solve this problem too.
Comment #45
drupov CreditAttribution: drupov commentedJust reporting that the patch from #44 (against 7.x-2.x-dev from 2014-Mar-28) does not solve the issue for me: the feature part containing the menu link on the target site does not get reverted.
Comment #46
dagomar CreditAttribution: dagomar commentedAgreed, this patch doesn't revert a menu link.
Comment #47
hefox CreditAttribution: hefox commentedLet's go back to creating separate issues for the identified issues that are clear on what the condition is. As can seen above, the issue is gaining different patches and reviews for different issues encountered and thus confusing to evaluate each patch
Comment #48
Rafal LukawieckiIs there a newer version of any of these patches—they fail to apply to the current dev branch? I am unable to revert menu links with both the current 7.x-2.11 and 7.x-2.x-dev versions of Features. 2.11 does not think that a menu needs reverting. 2.x-dev agrees they need reverting, but it fails to revert. Any suggestions much appreciated.