Hi, I can't get drupal_goto() to work in my custom module. I've tried using header('location: foo') with success but drupal_goto just breaks and returns a white screen..

I have triple checked my code but here it is anyway:

<?php
#return users usernode to theme
function getUsernode(){
    global
$user;
   
$uid = $user->uid;
   
$node = db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = $uid");
   
$usernode = db_fetch_object($node);
    if (
$usernode) return $usernode;
}
#take user to usernode or to edit usernode when URL is '/profile' or '/user/XXX'
if (arg(0) == "profile" && !arg(1) || arg(0) == "user" && !arg(1)){
   
$usernode = getUsernode();
    if (
$usernode){
        if (
$usernode->created != $usernode->changed){#has usernode
           
drupal_goto("node/$usernode->nid");
    } else {
           
drupal_goto("node/$usernode->nid/edit");
        }
    }
}
?>

Any idea's what this may be?

Many thanks,
Marc

Comments

because $usernode is an object, you can't just call it's properties between ""

instead of drupal_goto("node/$usernode->nid"); u have to use drupal_goto("node/{$usernode->nid}");
same with drupal_goto("node/$usernode->nid/edit"); -> don't forget {}

btw:
db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = $uid")
db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = %d", $uid) -> safer (prevents mysql injection)

thankyou filster :]

well it should be mentionned that SELECT * FROM is a bad practice, return only what you need... and about argument substitution, i don't understand how only doing that without using mysql_real_escape_string function would provide security

db_query sanitizes the arguments before inserting them into the query. (Well, really, _db_query_callback is where the arguments are sanitized, but you get the idea.)

db_query does it when using substitutions, not when doing it like he has (as far as I know). It should be written like this:

db_query("SELECT column1, colum2 FROM {node} WHERE type = 'usernode' AND uid = %d", $user->uid);

In this manner the code will be sanitized. Although in this situation, it's a system variable, not user inputted data, so it should be ok.

Jaypan
Our newest Drupal site: PacificAikido.com (Drupal showcase)

Right. Filster suggested the correct code in his first comment, but thanks for clarifying that. People reading the thread later might not notice. db_query only sanitizes variables that are passed via substitution for %s, %d, etc.

Ahh, I missed that Filster had put the correct code in there. My bad, but good to reinforce good coding practices.

Jaypan
Our newest Drupal site: PacificAikido.com (Drupal showcase)

Actually drupal_goto("node/$usernode->nid") should work since it unambiguous.

This code though looks questionable

#take user to usernode or to edit usernode when URL is '/profile' or '/user/XXX'
if (arg(0) == "profile" && !arg(1) || arg(0) == "user" && !arg(1)){
    $usernode = getUsernode();
    if ($usernode){
        if ($usernode->created != $usernode->changed){#has usernode
            drupal_goto("node/$usernode->nid");
    } else {
            drupal_goto("node/$usernode->nid/edit");
        }
    }
}

as it is outside any function and likely interferes with the normal operation of Drupal.

thanks for your input! I'm learning a lot :]

nevets,

I can't see any problems with this code as It's only redirecting users to usernodes instead of account profiles. I think it's quiet common practice and seems to work well. I never did find out why I had trouble with drupal_goto() but I created a new custom module and pasted my code in there success so all's well..

Best,
Marc