why?
because then you can add two menu items to your menu(s),
first one is log in the second one is log out
--> and the user does only see the one which is important for him

without this patch the users sees log in the whole time...

imho this is more consistent than the old approach, which looks a little bit like a hack...

btw. the patch is against drupal 4.7 but the changes are small so it should apply...

CommentFileSizeAuthor
#14 user_login.patch_drupal5.patch.txt2.35 KBTobias Maier
#9 user_login.patch_1.txt2.03 KBTobias Maier
#5 user.patch_9.txt1.71 KBTobias Maier
#1 user_login.patch_0.txt1.25 KBTobias Maier
user_login.patch.txt1.61 KBTobias Maier

Comments

Tobias Maier’s picture

Tobias Maier commented
StatusFileSize
new user_login.patch_0.txt1.25 KB

*argh*

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

What is the second hunk needed for? I think the form's #action should be fine as is.

Tobias Maier’s picture

Tobias Maier commented
Status: Needs work » Needs review

argh - this was a left over, too
such a small patch and such a lot of stupidness (new word ;) ) in my brain...

Index: modules/user.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/user.module,v
retrieving revision 1.612.2.10
diff -u -F^f -r1.612.2.10 user.module
--- modules/user.module	23 May 2006 09:39:47 -0000	1.612.2.10
+++ modules/user.module	20 Jun 2006 20:58:29 -0000
@@ -707,7 +711,7 @@ function user_menu($may_cache) {
 
     // Registration and login pages.
     $items[] = array('path' => 'user/login', 'title' => t('log in'),
-      'callback' => 'user_login', 'type' => MENU_DEFAULT_LOCAL_TASK);
+      'callback' => 'user_login', 'type' => MENU_DEFAULT_LOCAL_TASK, 'access' => $user->uid == 0);
     $items[] = array('path' => 'user/register', 'title' => t('register'),
       'callback' => 'user_register', 'access' => $user->uid == 0 && variable_get('user_register', 1), 'type' => MENU_LOCAL_TASK);
     $items[] = array('path' => 'user/password', 'title' => t('request new password'),
moshe weitzman’s picture

moshe weitzman commented

could be even briefer. 'access' => $user->uid is enough though perhaps not as semantically correct?

Tobias Maier’s picture

Tobias Maier commented
StatusFileSize
new user.patch_9.txt1.71 KB

true, but I read $user->uid == 0 somewhere in the code of the user module so I copy-pasted it...

I replaced the other one with the right one...
shoud work the same way

halkeye’s picture

halkeye commented

'access'=> $user->uid would be true for all logged in users, false for logged out
isn't that the reverse of the desiered behavior?

Tobias Maier’s picture

Tobias Maier commented

read again

'access' => !$user->uid

if you look closely at the sourcecode you can see the ! in front of $user
this means if $user->uid == true, means everything else than 0 then give back the opposite
this means it gives back false...

halkeye’s picture

halkeye commented

sorry, i should have explicitly said so
i was talking about moshe's comment

Tobias Maier’s picture

Tobias Maier commented
StatusFileSize
new user_login.patch_1.txt2.03 KB

I was confused

$form['#action'] = url('user');

is needed, because without this drupal tries to go back to /user/login after you hitted login
but this path isn't accessible anymore
so we send him to /user and he sees the same as before, his profile

dries’s picture

dries commented
Status: Needs review » Needs work

AFAIK, you shouldn't use $user in the $may_cache-section of user_menu().

Also, if it is an invalid menu callback, it is better not to register it. The menu system isn't the fastest thing on earth.

Steven’s picture

Steven commented
Status: Needs work » Needs review

Dries: Sure you can use $user... how else would user_access() calls work in hook_menu? The menu cache is done per user:

// menu.inc

$cid = "menu:$user->uid:$locale";
...
cache_set($cid, serialize($_menu), time() + (60 * 60 * 24));

I don't particularly mind this patch, though I'm not sure I see the benefit. If I point my browser to drupal.org/user/login, I'm just redirected to my own user page.

Tobias Maier’s picture

Tobias Maier commented

steven:
add a login menu entry and a logout menu entry to your menu, for example the primary menu
then log out, you see the just the log in entry, the log out entry is hidden
then log in. you see both entries, but for what do I need a menu entry called "log in" at this state?
so it needs to be hidden

dries’s picture

dries commented

Steven: I think they want the 'login' menu item to automagically disappear from their menus.

Tobias Maier’s picture

Tobias Maier commented
Version: x.y.z » 4.7.3
Assigned: Unassigned » Tobias Maier
Category: feature » bug
StatusFileSize
new user_login.patch_drupal5.patch.txt2.35 KB

a few months later i ask you to review this patch again.

You can call it a bug, too.
Because it is a menu-definied path which hasn't 'access' set properly.

here comes the drupal 5 version of this patch
not really tested (i got my notebook back from ASUS service yesterday and have to reinstall everything.. - this means I dont have a webserver running on my machine atm)

Tobias Maier’s picture

Tobias Maier commented
Title: user/login shouldn't be accesible for logged in users » user/login shouldn't be accessible for logged in users
Tobias Maier’s picture

Tobias Maier commented

Patch for Drupal 5.0 tested - works as expected

killes@www.drop.org’s picture

killes@www.drop.org commented
Version: 4.7.3 » x.y.z

moving to HEAD to get more reviews.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)