Part of the EllisLab Network
   
 
Custom, database-driven 404 page—can this be improved?
Posted: 13 July 2008 08:42 PM   [ Ignore ]  
Summer Student
Total Posts:  4
Joined  03-04-2007

Using the code from this forum post, I was able to pull off something I really wanted from my CodeIgniter application: if the first segment of the URI doesn’t have a matching controller, send the request to a page controller and check for content in the page database. I did it by extending the Router.php library, as described in that post, and changing _validate_requests() as so:

show_404($segments[0]);

became

return array('page', 'index', $segments[0]);

Here’s my page controller:

class Page extends Controller {
    
    
function __construct()
    
{
        parent
::Controller();
        
$this->load->model('Page_model');
    
}
    
    
function index($link)
    
{
        $data[
'page'] = $this->Page_model->get_page($link);
        if (!
$data['page']) show_404($this->uri->uri_string());
        
$data['section'] = array(
            
'title' => $data['page']['page_title'],
            
'class' => $data['page']['page_link']
        
);
        
$data['page']['page_primary'] = markdown(smartypants($data['page']['page_primary']));
        
$data['page']['page_secondary'] = markdown(smartypants($data['page']['page_secondary']));
        
// print_r($data);
        
$this->load->view('global/head_view', $data);
        
$this->load->view('global/nav_view');
        
$this->load->view('page_view');
        
$this->load->view('global/foot_view');
    
}
    
}

Here’s where it gets interesting and I turn to you: I also wanted the content of my 404 page stored in the same pages database table so it could be edited by a user in an admin panel. So I created MY_Exceptions.php:

class MY_Exceptions extends CI_Exceptions {

    
function MY_Exceptions()
    
{
        parent
::CI_Exceptions();
    
}

    
function show_404($page = '')
    
{
        
// Allows me to use information from database to make 404 page that matches site
        
        
$CI =& get_instance();
        
        
$CI->load->model('Page_model');
        
$data['page'] = $CI->Page_model->get_page('404');
        
$CI->output->set_header("HTTP/1.1 404 Not Found");
        
$data['section'] = array(
            
'title' => $data['page']['page_title'],
            
'class' => $data['page']['page_link']
        
);
        
$data['page']['page_primary'] = markdown(smartypants($data['page']['page_primary']));
        
$data['page']['page_secondary'] = markdown(smartypants($data['page']['page_secondary']));
        
$CI->load->view('global/head_view', $data);
        
$CI->load->view('global/nav_view');
        
$CI->load->view('page_view');
        
$CI->load->view('global/foot_view');
        echo
$CI->output->get_output();

        
log_message('error', '404 Page Not Found --> '.$page);
        exit;
    
}

My question to the always brilliant CodeIgniter community: My solution to this 404 thing feels like a total hack. I had to manually invoke the output class to get the views to appear and, worst of all, I had to reuse an entire block of code which I would have to change in two places should I need to make adjustments.

Any ideas on how I could improve on this idea? Can I actually call the page controller from the library item, or is that a terrible idea?

UPDATE: It should also be noted that, while I’m not a total n00b, my programming skills are somewhere between novice and intermediate. I apologize now if I follow up your kind answer with a bunch of seemingly stupid questions. Thanks. wink

ANOTHER UPDATE: Fixed link to forum post in first paragraph.

Profile
 
 
Posted: 14 July 2008 09:42 AM   [ Ignore ]   [ # 1 ]  
Sr. Research Associate
RankRankRankRankRank
Total Posts:  2634
Joined  06-10-2007

Actually your code is ok. Extending the CI core is always a hack in most cases, but so long as it works for you and it doesn’t interfere with other CI libraries it’s all good.

 Signature 

URI Language Identifier | Modular Extensions - PHP5 | Modular Separation - PHP5 | Widget plugin | Access Control library

Profile
 
 
Posted: 14 July 2008 10:24 AM   [ Ignore ]   [ # 2 ]  
Sr. Research Associate
Avatar
RankRankRankRankRank
Total Posts:  2916
Joined  07-27-2006

You can’t just serve up the content from the Page controller with a 404 header? I only briefly checked.

Instead of:

if (!$data['page']) show_404($this->uri->uri_string());

do:

if (!$data['page'])
{
   $data[
'page'] = $this->Page_model->get_page('404');
   
$this->output->set_header("HTTP/1.1 404 Not Found");
}

You’ll probably have to do more, for sure, but no reason to call show_404

 Signature 

Check out the Template Library
Oh yeah, I tweet, too (regarding CodeIgniter on occassion).

Profile
 
 
Posted: 14 July 2008 11:49 AM   [ Ignore ]   [ # 3 ]  
Lab Assistant
Avatar
RankRank
Total Posts:  169
Joined  10-11-2007

Guys,
As i was developing my website, i thought of something like this,
but only difference or mistake i did was that i hacked or modified the core CI classes.
http://www.nirbhab.com/controller-404error.html

Please review once, did i performed the task without security issue, or whatever….is it fine on my website.
thanx in advance!

 Signature 

http://www.nirbhab.com/  |  Follow Me On Twitter

Profile
 
 
Posted: 14 July 2008 11:53 AM   [ Ignore ]   [ # 4 ]  
Summer Student
Total Posts:  4
Joined  03-04-2007
Colin Williams - 14 July 2008 10:24 AM
if (!$data['page'])
{
   $data[
'page'] = $CI->Page_model->get_page('404');
   
$CI->output->set_header("HTTP/1.1 404 Not Found");
}

You’ll probably have to do more, for sure, but no reason to call show_404

Thanks so much for the response. I tried that first, and it worked great for pages that didn’t exist. Where I run into trouble would be if someone tries to request a function of another controller. For example, I have a portfolio controller with two functions: the index and item. If someone requests the URI /portfolio/something-else/ or requests /portfolio/item/item-that-does-not-exist, then I would have to redirect to /page/index/404, which would then change the URL in the browser. Or, I’d have to copy that page controller code into every new controller. Instead, I chose to copy it to my own show_404 function so I only had to copy it once.

But my issue is that I shouldn’t have to copy that code at all. Write once and reuse all over the place, right?

So here’s a thought: Could I make the page controller a front controller and then extend all of my other controllers from it? Something like:

class Portfolio extends Page

Then maybe it would be a matter of:

if (!data['portfolio_item'])
{
     $this
->output->set_header("HTTP/1.1 404 Not Found");
     
// Request the function from the Page controller
     
exit;
}

Could this work? I hunted through the forums and found this thread, but I can’t figure out how to then actually use the function from the Page controller in the Portfolio controller.

Profile
 
 
Posted: 14 July 2008 11:56 AM   [ Ignore ]   [ # 5 ]  
Sr. Research Associate
Avatar
RankRankRankRankRank
Total Posts:  2916
Joined  07-27-2006

but I can’t figure out how to then actually use the function from the Page controller in the Portfolio controller.

$this->method_to_call()
 Signature 

Check out the Template Library
Oh yeah, I tweet, too (regarding CodeIgniter on occassion).

Profile
 
 
Posted: 14 July 2008 12:01 PM   [ Ignore ]   [ # 6 ]  
Summer Student
Total Posts:  4
Joined  03-04-2007
wiredesignz - 14 July 2008 09:42 AM

Actually your code is ok. Extending the CI core is always a hack in most cases, but so long as it works for you and it doesn’t interfere with other CI libraries it’s all good.

Thanks for assuaging my guilt. smile I still know it can be done better, but at least I know I haven’t done anything asinine.

Profile
 
 
Posted: 14 July 2008 12:09 PM   [ Ignore ]   [ # 7 ]  
Sr. Research Associate
Avatar
RankRankRankRankRank
Total Posts:  2916
Joined  07-27-2006

For example, I have a portfolio controller with two functions: the index and item. If someone requests the URI /portfolio/something-else/ or requests /portfolio/item/item-that-does-not-exist, then I would have to redirect to /page/index/404, which would then change the URL in the browser. Or, I’d have to copy that page controller code into every new controller

Hrm.. I’d make the 404 stuff a library, then. Even though it performs something very similar to $page->index(), your other controllers are still handling the request and serving the 404, just via your library.

 Signature 

Check out the Template Library
Oh yeah, I tweet, too (regarding CodeIgniter on occassion).

Profile
 
 
Posted: 14 July 2008 04:51 PM   [ Ignore ]   [ # 8 ]  
Summer Student
Total Posts:  4
Joined  03-04-2007

Hey, Colin. I took your advice and created a library:

class MYPage {
    
    
function load_page($link)
    
{
        $CI
=& get_instance();
        
$CI->load->model('Page_model');
        
$data['page'] = $CI->Page_model->get_page($link);
        if(
$data['page'])
        
{
            $data[
'section'] = array(
                
'title' => smartypants($data['page']['page_title']),
                
'class' => $data['page']['page_link']
            
);
            
$data['page']['page_title'] = smartypants($data['page']['page_title']);
            
$data['page']['page_primary'] = markdown(smartypants($data['page']['page_primary']));
            
$data['page']['page_secondary'] = markdown(smartypants($data['page']['page_secondary']));
            return
$data;
        
}
        
else return false;
    
}
    
    
function display_page($data)
    
{
        $CI
=& get_instance();
        
$CI->load->view('global/head_view', $data);
        
$CI->load->view('global/nav_view');
        
$CI->load->view('page_view');
        
$CI->load->view('global/foot_view');
    
}

So now my Page controller simply contains:

function index($link)
    
{
        $this
->load->library('MYPage');
        
$data = $this->mypage->load_page($link);
        if (!
$data['page'])show_404($this->uri->uri_string());
        
$this->mypage->display_page($data);
    
}

and my show_404() function is now:

function show_404($page = '')
    
{
        
// Allows me to use information from database to make 404 page that matches site
        
        
$CI =& get_instance();
        
$CI->load->library('MYPage');
        
$data = $CI->mypage->load_page('404');
        
$CI->mypage->display_page($data);
        echo
$CI->output->get_output();
        
log_message('error', '404 Page Not Found --> '.$page);
        exit;
    
}

And there are other controllers where I’ll want to use the load_page() function, so this solution makes perfect sense. Thanks so much for the tip.

Perhaps this thread will help others, and I’m still open to suggestions if there is a still sleeker way to do this.

Profile
 
 
   
 
 
‹‹ Using models within models      CI & extjs ››
Post Marker Legend
New Topic New posts Hot Topic Hot Topic with new posts New Poll New Poll Moved Topic Moved Topic Sticky Topic Sticky topic
Old Topic No new posts Hot Old Topic Hot Topic with no new posts Old Poll Old Poll Closed Topic Closed Topic Announcement Announcements
Theme
Change Theme
Visitor Statistics
The most visitors ever was 819, on March 11, 2010 11:15 AM
Total Registered Members: 119988 Total Logged-in Users: 61
Total Topics: 126117 Total Anonymous Users: 4
Total Replies: 663424 Total Guests: 475
Total Posts: 789541    
Members ( View Memberlist )
Newest Members:  Allen22GurindergilesbShinkenLorBackL1progmaslinda326C.A.FabWebGifts5A. Stice