Welcome Guest, Not a member yet? Register   Sign In
Custom, database-driven 404 page -- can this be improved?
#1

[eluser]arlodesign[/eluser]
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:

Code:
show_404($segments[0]);

became

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

Here's my page controller:
Code:
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:

Code:
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.
#2

[eluser]wiredesignz[/eluser]
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.
#3

[eluser]Colin Williams[/eluser]
You can't just serve up the content from the Page controller with a 404 header? I only briefly checked.

Instead of:

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

do:

Code:
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
#4

[eluser]nirbhab[/eluser]
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!
#5

[eluser]arlodesign[/eluser]
[quote author="Colin Williams" date="1216067066"]
Code:
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[/quote]

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:

Code:
class Portfolio extends Page

Then maybe it would be a matter of:

Code:
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.
#6

[eluser]Colin Williams[/eluser]
Quote:but I can’t figure out how to then actually use the function from the Page controller in the Portfolio controller.

Code:
$this->method_to_call()
#7

[eluser]arlodesign[/eluser]
[quote author="wiredesignz" date="1216064565"]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.[/quote]

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.
#8

[eluser]Colin Williams[/eluser]
Quote: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.
#9

[eluser]arlodesign[/eluser]
Hey, Colin. I took your advice and created a library:
Code:
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:
Code:
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:
Code:
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.




Theme © iAndrew 2016 - Forum software by © MyBB