1

I am rewriting some old code according to the latest reference, although the page description is coming out correctly in the old code.

But when rewritten into new code, the page is redirecting to 404 as page not found.

Old Code

function loadProduct($vid){
    $db = JFactory::getDbo();

    $sql = "Select *, 
            (select prod_name from #__usedtoy_products where id=v.v_prod_id)
            as prod_name  from #__usedtoy_variants AS v
            Where v.state='1' and v.id = " . $db->quote($vid);
    $db->setQuery($sql);

    if ($rows = $db->loadObjectlist()) {
        return $rows[0];
    } else {
        if ($db->getErrorNum()) {
            JError::raiseError(500, "Something went horribly wrong, the query returned the error ". $db->getErrorMsg());
        } else {
            JError::raiseError(404, "404, Page does not Exists ". $db->getErrorMsg());
        }
    }
}

to

Improved code - it's working

function loadProduct($vid){
    $mainframe =JFactory::getApplication();
    $option = JRequest::getCmd('option');
    $db =JFactory::getDBO();
    global $Itemid;
    $query = $db->getQuery(true);

    $query->select('#__usedtoy_variants.id');
    $query->from($db->quoteName('#__usedtoy_variants'));
    $query->where($db->quoteName('#__usedtoy_variants.id')." = ".$db->quote($vid), 'AND');
    $query->where($db->quoteName('#__usedtoy_variants.v_prod_id')." = ".$db->quote($pid));
    $query = "Select *, 
              (select prod_name from #__usedtoy_products where id=v.v_prod_id)
              as prod_name  from #__usedtoy_variants AS v
              Where v.state='1' and v.id = " . $db->quote($vid);
    $db->setQuery($query);

    if ($rows = $db->loadObjectlist()) {
        return $rows[0];
    } else {
        if ($db->getErrorNum()) {
            JError::raiseError(500, "Something went horribly wrong, the query returned the error ". $db->getErrorMsg());
        } else {
            JError::raiseError(404, "404, Page does not Exists ". $db->getErrorMsg());
        }
    }
}

Final New Code - it's not working

function loadProduct($vid){
    $mainframe =JFactory::getApplication();
    $option = JRequest::getCmd('option');
    $db =JFactory::getDBO();
    global $Itemid;
    $query = $db->getQuery(true);

    $query->select('#__usedtoy_variants.id');
    $query->from($db->quoteName('#__usedtoy_variants'));
    $query->where($db->quoteName('#__usedtoy_variants.id')." = ".$db->quote($vid), 'AND');
    $query->where($db->quoteName('#__usedtoy_variants.v_prod_id')." = ".$db->quote($pid));

    $query->select(array('v.*', 'p.prod_name'))
          ->from($db->quoteName('#__usedtoy_variants', 'v'))
          ->join('LEFT', $db->quoteName('#__usedtoy_products', 'p') . ' ON (' . $db->quoteName('p.id') . ' = ' . $db->quoteName('v.v_prod_id') . ')')
          ->where($db->quoteName('v.state')." = ".$db->quote(1), 'AND')
          ->where($db->quoteName('v.id')." = ".$db->quote($vid));

    $db->setQuery($query);

    if ($rows = $db->loadObjectlist()) {
        return $rows[0];
    } else {
        if ($db->getErrorNum()) {
            JError::raiseError(500, "Something went horribly wrong, the query returned the error ". $db->getErrorMsg());
        } else {
            JError::raiseError(404, "404, Page does not Exists ". $db->getErrorMsg());
        }
    }
}

Reference - https://docs.joomla.org/Selecting_data_using_JDatabase#Selecting_Records_from_Multiple_Tables

Can some one point out what the issue is?

2

The short answer is because you have two queries that sort of mush together. In your 'improved code' version you build the query object and then replace the object with a string query.

I have removed all the code that has no role like this:

function loadProduct($vid){
    $db = JFactory::getDBO();
    $query = $db->getQuery(true);
    $query->select('v.*, p.prod_name')
      ->from($db->qn('#__usedtoy_variants', 'v'))
      ->join('LEFT', $db->qn('#__usedtoy_products', 'p') . ' ON (' . $db->qn('p.id') . ' = ' . $db->qn('v.v_prod_id') . ')')
      ->where($db->qn('v.state')." = 1 AND " . $db->qn('v.id')." = ".$db->q($vid));
    $db->setQuery($query);  

    if ($result = $db->loadObject()) {
        return $result;
    } else {
        if ($db->getErrorNum()) {
            JError::raiseError(500, "Something went horribly wrong, the query returned the error ". $db->getErrorMsg());
        } else {
            JError::raiseError(404, "404, Page does not Exists ". $db->getErrorMsg());
        }
    }
}

Note: qn and q are handy short names for quoteName and quote. If you only want a max of 1 record then user $db->loadObject() instead of objectList.

| improve this answer | |
0

For the record, I find @jamesgarrett's answer to be perfectly adequate. I just want to offer some alternative syntax for researchers to consider.

$db = JFactory::getDBO();
try {
    $query = $db->getQuery(true)
                ->select("A.*, B.prod_name")
                ->from("#__usedtoy_variants A")
                ->leftJoin("#__usedtoy_products B ON A.id = B.v_prod_id")
                ->where("A.state = 1 AND A.id = " . (int)$vid);
    $db->setQuery($query);
    if (!$result = $db->loadObject()) {  // one-dimensional, not multi-dimensional
        JFactory::getApplication()->enqueueMessage('Potential Logic Failure - No Rows Returned', 'notice');
        return false;
    }
    return $result;
} catch (Exception $e) {
    JFactory::getApplication()->enqueueMessage('Syntax Failure', 'error');  // use these to debug: $query->dump() && $e->getMessage()
    return false;
}
  • I prefertry {} catch {} blocks because I feel they make error handling simpler/cleaner.
  • Method chaining can be done from getQuery(). Doing so removes the need to repeat $query for each query method call.
  • I have removed every non-essential qn() and q() to reduce code bloat and have cast $vid as an integer for security reasons assuming that vid stands for a table id (integer). If $vid is not an integer, use q() on it.
  • leftJoin() is a dedicated join method.
  • I am checking for an empty resultset with ! and issuing a JFactory::getApplication()->enqueueMessage() notice then returning false when there are no rows. Depending on your project design, this outcome may not be a "Logic Failure".
  • I am offering a vague "Syntax Failure" response when the query breaks as a matter of best practice. If you want to fill the enqueueMessage() text with something more informative when you are debugging, use the dump() and/or getMessage() calls which I have left as inline comments. When finished debugging, be sure to remove the specific query/error details from view.
  • As an un-associated suggestion, I'll urge you to avoid using global in your function call. Many developers consider this to be "sloppy" and unnecessarily adds variables to the global scope. If you can accomplish the same effect by passing variables as arguments in the function -- do that.
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.