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.
Comment | File | Size | Author |
---|---|---|---|
#18 | fix-update6037-188498-18.patch | 1.24 KB | pwolanin |
#15 | profile_account_reserved_4.patch | 1.83 KB | RobLoach |
#12 | profile_account_reserved_3.patch | 1.84 KB | RobLoach |
#10 | profile_account_reserved.patch | 818 bytes | RobLoach |
#5 | menu.inc_.patch | 2.44 KB | Jadelrab |
Comments
Comment #1
RobLoachI'm not encountering this warning. How did you reproduce this? Start from a fresh Drupal install....
Comment #2
catchComment #3
deekayen CreditAttribution: deekayen commenteddrupal message says
error box says:
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.
Comment #4
RobLoachAh, got it.... Interesting...
Comment #5
Jadelrab CreditAttribution: Jadelrab commentedThis 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!
Comment #6
RobLoachShouldn't it also warn the user that they can't use that category name?
Comment #7
deekayen CreditAttribution: deekayen commentedI'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.
Comment #8
chx CreditAttribution: chx commentedFix profile module to lowercase categories.
Comment #9
chx CreditAttribution: chx commentedIt should not store account and Account separately.
Comment #10
RobLoachAccount 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.
Comment #11
chx CreditAttribution: chx commentedYour 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.Comment #12
RobLoachHere it is with the update path.
Comment #13
catchThis 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.
Comment #14
Gábor HojtsyAlthough 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.Comment #15
RobLoachHere's the patch with strtolower instead of strcasecmp.
Comment #16
Gábor HojtsyThanks, committed.
Comment #17
pwolanin CreditAttribution: pwolanin commentedthis 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:
need to be this:
Comment #18
pwolanin CreditAttribution: pwolanin commentedpatch attached alters quotes and removes DSM in favor of a message shown with the update queries.
Comment #19
pwolanin CreditAttribution: pwolanin commentedtested on postgres 8.2 - and got hunmonk's concurrence on IRC on the quote changes and moving the message to $ret.
Comment #20
pwolanin CreditAttribution: pwolanin commentedComment #21
RobLoachAhh, thanks.
Comment #22
Gábor HojtsyOh, 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.
Comment #23
Gábor HojtsySeeing 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.
Comment #24
pwolanin CreditAttribution: pwolanin commented@Gabor - an SQL-like message is already used is these updates:
system_update_6017()
book_update_6000()
Comment #25
Gábor HojtsyHm, I see. I am thinking about whether we have any rule on when to use what. The current practice seems to be mixed.
Comment #26
RobLoachThink 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.Comment #27
Gábor HojtsyOK, 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.
Comment #28
pwolanin CreditAttribution: pwolanin commented@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
Comment #29
RobLoachhttp://drupal.org/node/199050