Download & Extend

Menu admin page should say something when you save

Project:Drupal core
Version:7.x-dev
Component:menu system
Category:bug report
Priority:minor
Assigned:sivaji
Status:closed (fixed)
Issue tags:Novice

Issue Summary

If you go to admin/structure/menu/manage/management, for example, and click the save button, you get no feedback that anything happened. A drupal_set_message() should display something generic like "Your configuration has been saved." Check what other places in core are doing for a hint.

Comments

#1

#2

Status:active» needs review

Attached patch provides appropriate status message when you add or edit a menu or menu item.

AttachmentSizeStatusTest resultOperations
609108_menu_admin_page_feedback.patch1.68 KBIdlePassed on all environments.View details

#3

Can we simplify the message and make it consistent with #620592: Taxonomy admin page ... need feedback when saving?

#4

Assigned to:Anonymous» sivaji
Status:needs review» needs work

That sounds good. I had the same thought when i am rolling this patch, but other status messages in the menu module is using something similar to "The new menu %name has been created". Anyways i will roll it again.

#5

Attached patch simplifies the status message and makes it consistent with #620592: Taxonomy admin page ... need feedback when saving.

AttachmentSizeStatusTest resultOperations
609108_menu_admin_page_feedback.patch2.51 KBIdleFailed on MySQL 5.0 ISAM, with: 15,327 pass(es), 5 fail(s), and 0 exception(es).View details

#6

Status:needs work» needs review

#7

Status:needs review» needs work

The last submitted patch failed testing.

#8

Status:needs work» needs review

Attached patch should work

AttachmentSizeStatusTest resultOperations
609108_menu_admin_page_feedback_1.patch4.06 KBIdlePassed on all environments.View details

#9

Status:needs review» needs work

The last submitted patch failed testing.

#10

Status:needs work» needs review

sivaji requested that failed test be re-tested.

#11

I applied the patch in #8 and created a link from admin/structure/menu/manage/management and got a feedback message 'Created new menu link Test Link.', so it seems to work . I have not reviewed the PHP code. I also noticed that in the menu.test file that some of the UI text strings were inconsistent - some with periods ('.'), some without. Not sure if this is a concern or not, or deserving of its own issue.

#12

I applied the patch in #8 and it was grumpy.

flickerfly$ patch -p0 < 609108_menu_admin_page_feedback_1_0.patch
patching file modules/menu/menu.admin.inc
Hunk #1 succeeded at 161 (offset -11 lines).
Hunk #2 FAILED at 381.
Hunk #3 FAILED at 534.
Hunk #4 FAILED at 582.
Hunk #5 succeeded at 552 (offset -51 lines).
Hunk #6 FAILED at 585.
4 out of 6 hunks FAILED -- saving rejects to file modules/menu/menu.admin.inc.rej
can't find file to patch at input line 74
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: modules/menu/menu.test
|===================================================================
|RCS file: /cvs/drupal/drupal/modules/menu/menu.test,v
|retrieving revision 1.27
|diff -u -p -r1.27 menu.test
|--- modules/menu/menu.test 2 Dec 2009 19:26:22 -0000 1.27
|+++ modules/menu/menu.test 4 Dec 2009 09:43:12 -0000
--------------------------
File to patch: q 
q: No such file or directory
Skip this patch? [y] y
Skipping patch.
2 out of 2 hunks ignored

flickerfly$ cat modules/menu/menu.admin.inc.rej
*************** function menu_edit_item_submit($form, &$
*** 380,385 ****
    if (!menu_link_save($item)) {
      drupal_set_message(t('There was an error saving the menu link.'), 'error');
    }
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
 
--- 381,396 ----
    if (!menu_link_save($item)) {
      drupal_set_message(t('There was an error saving the menu link.'), 'error');
    }
+
+   // display status message
+   $op = $form_state['build_info']['args'][0];
+   if ($op === 'add') {
+     drupal_set_message(t('Created new menu link %name.', array('%name' => $item['link_title'])));
+   }
+   elseif ($op === 'edit') {
+     drupal_set_message(t('Updated menu link %name.', array('%name' => $item['link_title'])));
+   }
+
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
 
*************** function menu_delete_menu_confirm_submit
*** 523,529 ****
    menu_delete($menu);
 
    $t_args = array('%title' => $menu['title']);
-   drupal_set_message(t('The custom menu %title has been deleted.', $t_args));
    watchdog('menu', 'Deleted custom menu %title and all its menu links.', $t_args, WATCHDOG_NOTICE);
  }
 
--- 534,540 ----
    menu_delete($menu);
 
    $t_args = array('%title' => $menu['title']);
+   drupal_set_message(t('Deleted custom menu %title and all its menu links.', $t_args));
    watchdog('menu', 'Deleted custom menu %title and all its menu links.', $t_args, WATCHDOG_NOTICE);
  }
 
*************** function menu_edit_menu_submit($form, &$
*** 571,576 ****
 
      menu_link_save($link);
      menu_save($menu);
    }
    else {
      menu_save($menu);
--- 582,588 ----
 
      menu_link_save($link);
      menu_save($menu);
+     drupal_set_message(t('Created new menu %name.', array('%name' => $menu['title'])));
    }
    else {
      menu_save($menu);
*************** function menu_item_delete_form_submit($f
*** 572,578 ****
    $item = $form['#item'];
    menu_link_delete($item['mlid']);
    $t_args = array('%title' => $item['link_title']);
-   drupal_set_message(t('The menu link %title has been deleted.', $t_args));
    watchdog('menu', 'Deleted menu link %title.', $t_args, WATCHDOG_NOTICE);
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }
--- 585,591 ----
    $item = $form['#item'];
    menu_link_delete($item['mlid']);
    $t_args = array('%title' => $item['link_title']);
+   drupal_set_message(t('Deleted menu link %title.', $t_args));
    watchdog('menu', 'Deleted menu link %title.', $t_args, WATCHDOG_NOTICE);
    $form_state['redirect'] = 'admin/structure/menu/manage/' . $item['menu_name'];
  }

#13

Attached patch should work.

AttachmentSizeStatusTest resultOperations
609108_menu_admin_page_feedback_2.patch4.86 KBIdlePASSED: [[SimpleTest]]: [MySQL] 17,375 pass(es).View details

#14

Status:needs review» reviewed & tested by the community

#13 applied properly and provided a message saying that menu items had changed when saved. It checks out nicely for me.

#15

Version:7.x-dev» 8.x-dev
Component:menu system» user interface text

Hey, thanks for this. Unfortunately, we're not making any more non-critical changes to the user interface text this release. :( Bumping to Drupal 8.

#16

Version:8.x-dev» 7.x-dev
Component:user interface text» menu system
Status:reviewed & tested by the community» needs review

I don't think it is good to leave this bug open for UI text freeze. I agree that #13 makes unwanted checks and UI text changes. Attached patch will fix this bug just by adding three drupal_set_message(t('Your configuration has been saved.')); in menu_foo_bar_submit(). I am sure its worth adding this now.

For Testing
1. Go to List menu links page eg. admin/structure/menu/manage/user-menu
2. Go to menu item edit page eg. admin/structure/menu/item/28/edit
3. Go to menu description edit page eg. admin/structure/menu/manage/user-menu/edit

Submitting menu form in any of the above pages should display drupal_set_message(t('Your configuration has been saved.')); message.

AttachmentSizeStatusTest resultOperations
609108_menu_admin_page_16.patch1.09 KBIdlePASSED: [[SimpleTest]]: [MySQL] 20,141 pass(es).View details

#17

Patch file from #16 applies cleanly to CVS HEAD for me and has the desired effect. Those different edit pages all show a message when the configuration changes are saved where they did not before.

#18

Status:needs review» reviewed & tested by the community

Same here.

RTBC :)

#19

I did notice this lack of feedback on certain admin pages, too. Definitely worth fixing for D7.

#20

@yoroy That's true, One of the admin pages i recently noticed is reported here #787684: Role admin page should say something when you save hope someone here will have time to put some effort.

#21

Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks.

#22

Status:fixed» closed (fixed)

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