I seem to be able to reproduce this warning regardless of the type of field I create. I know there have been lots of new menu system bugs, but I haven't been able to keep up with them, so maybe this is dup (I don't think this is a dup of http://drupal.org/node/154283).

user warning: Duplicate entry 'user/%/edit/Account' for key 1 query: INSERT INTO menu_router (path, load_functions, to_arg_functions, access_callback, access_arguments, page_callback, page_arguments, fit, number_parts, tab_parent, tab_root, title, title_callback, title_arguments, type, block_callback, description, position, weight, file) VALUES ('user/%/edit/Account', 'a:1:{i:1;s:9:\"user_load\";}', '', 'user_edit_access', 'a:1:{i:0;i:1;}', 'user_edit', 'a:2:{i:0;i:1;i:1;i:3;}', 11, 4, 'user/%/edit', 'user/%', '', 'check_plain', 'a:1:{i:0;s:7:\"Account\";}', 128, '', '', '', 3, 'modules/user/user.pages.inc') in /Users/davidnorman/Sites/drupal6/includes/menu.inc on line 2139.

user warning: Duplicate entry 'user/%/edit/Account' for key 1 query: INSERT INTO menu_router (path, load_functions, to_arg_functions, access_callback, access_arguments, page_callback, page_arguments, fit, number_parts, tab_parent, tab_root, title, title_callback, title_arguments, type, block_callback, description, position, weight, file) VALUES ('user/%/edit/Account', 'a:1:{i:1;s:9:\"user_load\";}', '', 'user_edit_access', 'a:1:{i:0;i:1;}', 'user_edit', 'a:2:{i:0;i:1;i:1;i:3;}', 11, 4, 'user/%/edit', 'user/%', '', 'check_plain', 'a:1:{i:0;s:7:\"Account\";}', 128, '', '', '', 3, 'modules/user/user.pages.inc') in /Users/davidnorman/Sites/drupal6/includes/menu.inc on line 2139.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobLoach’s picture

I'm not encountering this warning. How did you reproduce this? Start from a fresh Drupal install....

catch’s picture

Status: Active » Postponed (maintainer needs more info)
deekayen’s picture

Status: Postponed (maintainer needs more info) » Active
  1. install drupal in english - no table prefixes
  2. load main page
  3. click administer
  4. click modules
  5. check profile
  6. save configuration
  7. click user management
  8. click profiles
  9. click URL
  10. category = Account
  11. title = Favorite website
  12. form name = profile_favorite_website
  13. save field

drupal message says

The field has been created.

error box says:

user warning: Duplicate entry 'user/%/edit/Account' for key 'PRIMARY' query: INSERT INTO menu_router (path, load_functions, to_arg_functions, access_callback, access_arguments, page_callback, page_arguments, fit, number_parts, tab_parent, tab_root, title, title_callback, title_arguments, type, block_callback, description, position, weight, file) VALUES ('user/%/edit/Account', 'a:1:{i:1;s:9:\"user_load\";}', '', 'user_edit_access', 'a:1:{i:0;i:1;}', 'user_edit', 'a:2:{i:0;i:1;i:1;i:3;}', 11, 4, 'user/%/edit', 'user/%', '', 'check_plain', 'a:1:{i:0;s:7:\"Account\";}', 128, '', '', '', 3, 'modules/user/user.pages.inc') in /Users/davidnorman/Sites/drupal6/includes/menu.inc on line 2139.

Fresh update of CVS HEAD as of 5 mins ago, MacOS 10.5, PHP 5.2.5RC3-dev, MySQL 5.1.22 and 5.0.45.

RobLoach’s picture

Ah, got it.... Interesting...

user warning: Duplicate entry 'user/%/edit/Account' for key 1 query: INSERT INTO drupal_menu_router (path, load_functions, to_arg_functions, access_callback, access_arguments, page_callback, page_arguments, fit, number_parts, tab_parent, tab_root, title, title_callback, title_arguments, type, block_callback, description, position, weight, file) VALUES ('user/%/edit/Account', 'a:1:{i:1;s:9:\"user_load\";}', '', 'user_edit_access', 'a:1:{i:0;i:1;}', 'user_edit', 'a:2:{i:0;i:1;i:1;i:3;}', 11, 4, 'user/%/edit', 'user/%', '', 'check_plain', 'a:1:{i:0;s:7:\"Account\";}', 128, '', '', '', 3, 'modules/user/user.pages.inc') in includes\menu.inc on line 2139.

Jadelrab’s picture

Status: Active » Needs review
FileSize
2.44 KB

This warning because the path "user/%/edit/account" is already exist in the menu_router table

the patch attached just do a check to see if the path exist or not .. if it is the query of insertion will be ignored if not it will be executed!

RobLoach’s picture

Shouldn't it also warn the user that they can't use that category name?

deekayen’s picture

Status: Needs review » Needs work

I'm not convinced #5 is the best solution. Account could be considered a reserved category and denied strictly on checking that string as opposed to adding another query. Besides, if I create two profile fields with the same category, the second one doesn't throw an error like it does if you try to use Account.

chx’s picture

Fix profile module to lowercase categories.

chx’s picture

Title: Warning when adding a new profile field » Profile does not lowercase categories
Component: menu system » profile.module

It should not store account and Account separately.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
818 bytes

Account is a reserved category name and the check is already done for us, it's just that it's a case sensitive check. Doing a case insensitive string comparison fixes the problem.

chx’s picture

Your strcasecmp patch is elegant and in itself is RTBC but the upgrade case needs to be covered. I advise renaming it to profile_account "UPDATE profile_fields SET category = 'profile_account' WHERE LOWER(category) = 'account'" and please do not forget to set a message if db_affected_rows . Thanks for fixing this.

RobLoach’s picture

Here it is with the update path.

catch’s picture

Status: Needs review » Reviewed & tested by the community

This fixes the issue. As far as I'm concerned, renaming "Account" to "Account settings" is completely fine since the admin is warned. Even if someone has a profile category called 'Account', the worst that might happen is someone somewhere has made a hard coded link to user/uid/edit/Account - but that's a tiny tiny edge case, and it'd have to be not only hardcoded, but also have php in it. So RTBC.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

Although we seem to use strcasecmp() three times in Drupal 6 already, I would say we use strtolower(...) == 'account' type of comparisons instead. In this case we don't care about UTF-8 conformant comparisons (like at most other places), so this should be fine.

RobLoach’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.83 KB

Here's the patch with strtolower instead of strcasecmp.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

pwolanin’s picture

Priority: Normal » Critical
Status: Fixed » Needs work

this patch has improper use of quote chars in the SQL! It breaks on Postgres (see: http://drupal.org/node/164532#comment-646472)

Inside a query you can only use single quote chars.

this line:

$ret[] = update_sql('UPDATE {profile_fields} SET category = "Account settings" WHERE LOWER(category) = "account"');

need to be this:

$ret[] = update_sql("UPDATE {profile_fields} SET category = 'Account settings' WHERE LOWER(category) = 'account'");
pwolanin’s picture

Status: Needs work » Needs review
FileSize
1.24 KB

patch attached alters quotes and removes DSM in favor of a message shown with the update queries.

pwolanin’s picture

Status: Needs review » Reviewed & tested by the community

tested on postgres 8.2 - and got hunmonk's concurrence on IRC on the quote changes and moving the message to $ret.

pwolanin’s picture

Priority: Critical » Normal
RobLoach’s picture

Ahh, thanks.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review

Oh, doh. I am not sure that moving the DSM to an SQL look-alike return value is a "Drupal standard" through. Looking at the system updates, we have two DSM-s in this cycle, one about welcome email bodies moved, one about update module being available.

Gábor Hojtsy’s picture

Seeing this blocks testing http://drupal.org/node/164532, I committed the SQL fix part of this patch. So only the DSM is up for discussion.

pwolanin’s picture

@Gabor - an SQL-like message is already used is these updates:

system_update_6017()
book_update_6000()

Gábor Hojtsy’s picture

Hm, I see. I am thinking about whether we have any rule on when to use what. The current practice seems to be mixed.

RobLoach’s picture

Think we should we create a new issue to move all the drupal_set_message's into $ret[] = array('success' => TRUE, 'query' => $message);? There are about three other instances of it.

Gábor Hojtsy’s picture

OK, let's open an issue about that and discuss it there. (Then mark this one closed). I am not sure about that is goes without discussion.

pwolanin’s picture

@Gabor - it may make sense to use DSM in some cases - for example if there is an important setting that needs to be checked at the end of the update. For more routine messages, the 'query' method seems appropriate

RobLoach’s picture

Status: Needs review » Closed (fixed)