menucopy

Hi,

This project needs much work. (please check the Drupal coding standards)

try to resolve erros and warnings.
http://ventral.org/pareview/httpgitdrupalorgsandboxkris-o31853420git

in your .module file :
you can remove blank lines in lines : 2 , 3 , 11 , 40
in lines 13 , 14 , 15

/**
 * Function menucopy_form_menu_edit_menu_alter, adds select field to form.
 */

and also in lines 42 , 43 , 44

/**
 * Function menucopy_edit_menu_submit, copies items to menu, hierarchy intact.
 */

you wrote a comment for explaining about your functions but please read http://drupal.org/coding-standards/docs#hooks it's better that you use a standard way to explain about yout functions

http://drupal.org/project/menu_clone

This sounds like a feature that should live in the existing menu_clone project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the menu_clone issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

menu_clone began as a very simple module that would clone an existing menu into a new one. It was abandoned by it's original author (Garrett Albright) so I took over and rewrote the whole thing from scratch fixing all of the bugs in D6. Some non-reproducible issues have been reported for D6.

The ability to edit the menu before creating the actual clone just integrated the whole process: instead of cloning then editing, this is now done in one go.

Someone approached me to do a D7 port, which I gladly welcomed: I always appreciate collaboration. Unfortunately this was a really bad port omitting all the important stuff about the tricky Drupal menu handling (let's hope the menu system finally gets a full rewrite in D8). All of these issues have been addressed in the version I made in July of this year.

The core of the cloning process in menu_clone uses Drupal Core code (all documented in the module) that up until now works like a charm.
The most critical code, that effectively fixed 90% of the problems with the original menu_clone module resides in menu_clone_clone_form_submit() in the menu_clone.admin.inc file. In the bad D7 port this code was completely omitted causing a lot of problems and bug reports. Also note the big blob of comment there.

The open issues in the queue have so far not been reproducible and appear to be single occurances.

Menucopy looks more like the first D6 version of menu clone also mentioned in the links you post: http://drupal.org/node/252200#comment-1568986 . That's how it starded and it ended where it is now, due to feature requests etc.

I have no problem with criticism whatsoever, but I do think menucopy should be able to handle all kinds of menu's (even the very complex admin menu) if it aims to be a fully supported and mature contrib module that covers all it's bases making me think that, by community requests and support, it will end up very close to what menu_clone is now.

Again, I always appreciate collaboration and am happy to make anyone a co-maitainer to help advance menu_clone.

@kris-o3: there are only 5 open bug reports against the 7.x version of menu_clone: http://drupal.org/project/issues/search/menu_clone?status[]=Open&version...

Only one of them is critical and could not be reproduced by the maintainer, so I do not see any serious bugs. I also don't see any bug reports submitted by you in that queue, so it looks like you did not try the module at all.

malc0mn has also stated that he is open about collaboration, so I can only repeat myself: Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the menu_clone issue queue to discuss what you need. You could even get co-maintainership, just post your plans to that queue and ask.

If that fails for whatever reason please get back to us and set this back to "needs review".

I see two fundamental things in your code (just reading it, not actually testing it):

1. Any menu should have an entry in the menu_custom table. Menus created by users are prefixed with menu-, the others are system and/or 'special' menus, not created using the interface. Your code does not seem to update that table.

2. The big blob of comment I was referring to is:

    // When dealing with saving menu items, the order in which these items are
    // saved is critical. If a changed child item is saved before its parent,
    // the child item could be saved with an invalid path past its immediate
    // parent. To prevent this, save items in the form in the same order they
    // are sent by $_POST, ensuring parents are saved first, then their children.
    // See http://drupal.org/node/181126#comment-632270

My module has a form from which the menu data is taken (same as core) and is then ordered and saved properly (again as Drupal core does). You simply save the items as you receive them from the database.

I'm curious to know if you have tested this code with big and complex menu's (may not at all be the scope of your usecase)? The bad D7 port I mentioned before appeared to work like a charm to the person that ported it (especially for the usecases he needed it for), but when let loose in the community, cloning any kind of more complex menu with hierarchy and what not, it went hopelessly wrong because of the way the menu items were saved to the database.

The advantage of using menu_link_save() is that it handles all sort of Drupal Core stuff for you, such as:
- calling the right hooks so that other modules can act on the menu links that you are creating (e.g. hook_menu_link_alter but also in our case hook_menu_link_insert)
- handling the menu caching
- handling the paths for the menu link
- ...

There are tons and tons of ways to do things in Drupal, also by completely bypassing the Drupal API in various ways (often making it seem easier to code), but when creating a Drupal contrib module, it is important to integrate properly with the Core so that any module can be easily extended and maintained. When using the API and available core functions you cover most (if not all) of that. It would also allow users to attach themselves to your module without having to modify the module's code.

Take the hook_menu_link_alter. You could implement that in any module and thus alter the links that menu_clone creates. In your module that would not be possible and could easily come up as a feature request by users, causing you to implement your own invocation, rewriting the code to use menu_link_save() or dismissing the issue since it is not in your scope.

Ofcourse, you always need to keep your target ahead. Bypassing the API is sometimes necessary to avoid huge performance issues that could be caused by hook invocations.

In the end: your module may work perfectly fine for your particular use case, but when you want to release it in contrib, all sorts of stuff starts to happen :-D

If the original submit handler is being called still, then that would explain perfectly why you do not need the cache clear. But the hooks that are fired there will not allow users to modify the menu items that you insert into the database directly...

As said before, you can implement your own invocations of existing hooks. But that requires more maintenance on your side if, for whatever reason, new hooks are introduced in Core. You would have to add them in the right spot as well.

now, a fair question, would you necessarily Want a hook triggered for every new *item* that is copied?

Fair question indeed, but this is directly aimed at the Drupal Core: this is just the way Core does it and, in my personal opinion, a conrtib module should follow core behaviour unless there is a true valid reasoning to step away from this, other then 'I don't like the Core way of doing things' (lol).
The reason for these specific hooks being in core really do serve a purpose; I'm convinced of that even if I cannot come up with examples of use cases right now. And that's just it: the community has so many use cases to throw at you (have you seen this one [still can't find a use case for it myself] or this one requested by a client, and there was me just wanting to clone menu's :-D).

Bottom line stays the same: if your module serves what you aim at, then it it is just fine. But when released in the community you will be confronted with feature requests and other silly things that will require you to eventually kill your darlings to keep the module usefull for all.

Thanks Kris! I'm sure any of your input will be highly appreciated for menu_clone.

I'm closing this project application, feel free to open a new application whenever you want to promote a new project.

You know where to find me :-)