arambalakjian / DataObject-as-Page

A SilverStripe module for displaying DataObjects as Pages
53 stars 27 forks source link

Controller::redirect on show/ instead of 404? #34

Closed aledbrown closed 11 years ago

aledbrown commented 11 years ago

File: DataObjectAsPageHolder.php

DOAP uses a show/item URL scheme.

At present the controller returns a 404.

Is there anything wrong with it automatically redirecting the user to the parent link which is usually the Holder/Listing page anyway instead of a 404?

Modified SS3 code below.

/*
 * Renders the detail page for the current item passed into the URLs ID 
 * 
 * Uses DataObjectAsPageViewer_show.ss by default
 */
public function show()
{
    if($item = $this->getCurrentItem())
    {
        if ($item->canView())
        {
            $data = array(
                'Item' => $item,
                'Breadcrumbs' => $item->Breadcrumbs(),
                'MetaTags' => $item->MetaTags(),
                'BackLink' => base64_decode($this->request->getVar('backlink'))
            );

            return $this->customise(new ArrayData($data));
        }
        else
        {
            return Security::permissionFailure($this);
        }
    }
    else
    {
        return Controller::redirect(parent::Link()); // better than a 404?
        //return $this->httpError(404);
    }
}
arambalakjian commented 11 years ago

I think it is important to let the user know that the URL they have tried to access is not a real page, rather than just redirect them and make them think that it was right, so I think I will leave this for now.

Thanks for the though though :)

aledbrown commented 11 years ago

Because you have to have the /show/ part added to any url with a DOAP area I think it's a weird convention to show a 404. It should try to redirect to the correct parent "Holder" page instead of having a 404.

Dumping a user is not really an option.

Is there no better way? Wish there was a way to completely hide the /show/ bit but I know that's how SS is picking up the route to load the DOs.

Cheers for looking at it.

Aled

On 25 Sep 2013, at 14:20, Aram Balakjian notifications@github.com wrote:

I think it is important to let the user know that the URL they have tried to access is not a real page, rather than just redirect them and make them think that it was right, so I think I will leave this for now.

Thanks for the though though :)

— Reply to this email directly or view it on GitHub.

arambalakjian commented 11 years ago

Do you mean when the url is just "mypage/show"? That is a little odd to show a 404, but when it's mypage/show/item, I don't think it's strange to show a 404 if 'item' doesn't exist?

I can look at redirecting to the listing page if there is no URL segmenet specified (i.e. it's just .../show/)?

Aram

aledbrown commented 11 years ago

Hi Aram

Do you mean when the url is just "mypage/show"?

That is a little odd to show a 404,

Yes, that's right.

but when it's mypage/show/item, I don't think it's strange to show a 404 if 'item' doesn't exist?

That's correct. I agree entirely.

I can look at redirecting to the listing page if there is no URL segmenet specified (i.e. it's just .../show/)

I think that's what I did manage to achieve on something I was prototyping for a client.

I'm actually going back to that project next week and looking forward to using SS3 again.

Yes, that's exactly what I did, just had a look.

Cheers Aled