Create / edit menu form

This had been committed to the main branch (7.x-1.x). We use feature branches, and the code needs to have tests and be reviewed before it gets merged into 7.x-1.x. This is roughly the procedure:

# Create a new branch off the latest main branch using the issue number.
$ git checkout 7.x-1.x
$ git pull
$ git checkout -b 1936370

# Code and make as many commits as necessary.
$ git commit -m "Issue #1936370: Created awesome feature."
...

# Push the branch so it can be reviewed.
$ git push origin 1936370

I will move the code to a feature branch and revert the commit in 7.x-1.x for now.

3dfe022 Menu create functionality added. Visit /paddle_menu_manager/js/main_menu_overview to test

From a first quick look I see this still needs work on the coding standards-side.

The dependency on ctools is missing in the .info file.

A PHP parse error occurs on line 159 of paddle_menu_manager.module.

Tests are still missing. At least a test is required which submits the form with some random values, then checks if the menu was created correctly.

    $menu = array(
      'manu_name' => '',
      'title' => '',
      'description' => '',
    );

'manu_name' needs to be 'menu_name".

When using the ctools nojs fallback directly by navigating to the path 'paddle_create_menu/nojs/add' a PHP notice shows up:

Notice: Undefined index: menu in paddle_menu_manager_create_menu_form() (line 97 ...)

When submitting that same form, there is no feedback displayed, just the form again, which could make users think the menu wasn't saved. The user should also be redirected back to the overview page.

Preferably everything related to menu items should be removed again from this branch, so we can safely ignore that part for now and first get in the forms for menus themselves.

I've done a lot more cleanup of the code, merged in 7.x-1.x to use the menu translation sets feature.

Paths you can use already for testing manually:

admin/structure/menu_manager/nojs/add
admin/structure/menu_manager/nojs/[menu_name]/edit

Now updating the tests.

Added tests for checking functionality without JS.

Writing tests for the JS part actually appears to be very difficult, without having an underlaying test infrastructure for ctools modal itself. DrupalWebTestCase::drupalPostAJAX() does not support the JS commands added by ctools, it even doesn't have full support for Drupal core JS commands. I'm afraid we're crossing the boundaries of SimpleTest here.

So for now I added a test module which can be used for manual testing. Enable paddle_menu_manager_test (it's hidden, so use Drush) and navigate to paddle_menu_manager_test/menu_form.

After looking at #1937100: Use TSID for menu calling, I think there is still a menu_rebuild() missing in the menu create/edit form.

Reviewing.

Review:

Code duplication

+function paddle_menu_manager_menu_form_submit($form, &$form_state) {
...
+    // Generate a uuid as machine name for new menu's.
+    // We need to replace the '-' in the uuid because uuid is 36 chars long
+    // (32 alphanumerical and 4 '-') and the menu_name can be up to 32 in the
+    // database.
+    module_load_include('inc', 'uuid', 'uuid');
+    $menu['menu_name'] = str_replace('-', '', uuid_generate());
...

This is also used when menus are cloned. We can factor this out in a function paddle_menu_manager_uuid_generate().

Form redirection

+function paddle_menu_manager_menu_form_submit($form, &$form_state) {
...
+  // @todo Define which page to redirect to.
+  $form_state['redirect'] = '';

Discussed this with Cyberwolf, in the case that there is no javascript we can redirect to the menu overview form of the created / edited menu. In case of ajax dialogs, the dialog closes and the same page should be refreshed.

Test module

A test module has been added that provides some clickable links. This should be removed in the future when these links are included in the module. I created a followup issue for this: #1941776: Remove test module.

While implementing the redirect I noticed that indeed the call to menu_rebuild() is missing from the form submit as noted in #9.

I addressed the remarks from #11 and #12.

Here are the commits I made, if these can get a quick review this is ready for merging into 7.x-1.x:

475c3cd Issue #1936370: Redirect to the overview form after creating or editing a menu with no js enabled.
4d1bc63 Issue #1936370: Rebuild the menus after creating or editing.
60c3678 Return the tsids that are created during cloning.
c5eb500 Refactored the 32-character UUID generation in a separate function.

Just one small remark:

 * @return string
 *   An UUID, made up of 32 hex digits and 0 hyphens.

Should be: "A UUID, ..."

I fixed that and merged to 7.x-1.x, finally! :)