3
\$\begingroup\$

In my web-app, it is possible to delete an Item when visiting any of 4 different pages:

  • the Show Item page
  • the Edit Item page
  • the List Items page
  • the List Items in Room page

If the visitor uses a delete button on either of the 2 List pages, they should remain on their current page when the deletion is complete. But if the visitor is on the Show or Edit pages, they should be redirected to the List Items in Room page when deletion is complete.

I have the following method in a Symfony3 controller, which uses the HTTP referer header:

/**
 * @Route("/delete/{id}", name="ci_item_delete", requirements={"id"="\d+"}, options={"expose"=true})
 * @Security("has_role('ROLE_USER') && is_granted('delete', item)")
 * @param Request $request
 * @param Item $item
 * @return RedirectResponse
 */
public function deleteAction(Request $request, Item $item)
{
    $referer = $request
        ->headers
        ->get('referer');
    $rpath = parse_url($referer, PHP_URL_PATH );
    $badreferrers = array(
        $this->generateUrl('ci_item_show', ['id' => $item->getId()]),
        $this->generateUrl('ci_item_edit', ['id' => $item->getId()])
    );

    if(in_array($rpath, $badreferrers) ){ //coming from a page that won't exist once the Item is deleted
        $redirect = $this->generateUrl('ci_item_list_format_room', ['format_id'=> $item->getFormat()->getId(), 'room_id'=> $item->getRoom()->getId()]);
    } else {
        $redirect = $referer;
    }

    $em = $this->getDoctrine()->getManager();
    $em->remove($item);
    $em->flush();
    $this->addFlash('success', 'msg.delete.item.success');

    return $this->redirect($redirect);
}

Would it be better to have different routes for the different use cases? Something like this?

/**
 * @Route("/delete/{id}", name="ci_item_delete", requirements={"id"="\d+"}, options={"expose"=true})
 * @Route("/deletefromlist/{id}", name="ci_item_delete_from_list", requirements={"id"="\d+"}, options={"expose"=true})
 * @Security("has_role('ROLE_USER') && is_granted('delete', item)")
 * @param Request $request
 * @param Item $item
 * @return RedirectResponse
 */
public function deleteAction(Request $request, Item $item)
{
    $referer = $request
        ->headers
        ->get('referer');

    if($request->get('_route') == 'ci_item_delete' ){ //coming from a page that won't exist once the Item is deleted
        $redirect = $this->generateUrl('ci_item_list_format_room', ['format_id'=> $item->getFormat()->getId(), 'room_id'=> $item->getRoom()->getId()]);
    } else {
        $redirect = $referer;
    }

    $em = $this->getDoctrine()->getManager();
    $em->remove($item);
    $em->flush();
    $this->addFlash('success', 'msg.delete.item.success');

    return $this->redirect($redirect);
}

Or is there another better way altogether?

\$\endgroup\$
2
  • \$\begingroup\$ Please provide more context. Could you clarify the relationship between items and rooms? \$\endgroup\$ – 200_success Feb 24 '17 at 8:49
  • \$\begingroup\$ @200_success, item is many-to-one with room with item on the owning side. \$\endgroup\$ – dnagirl Feb 24 '17 at 14:36
1
\$\begingroup\$

There really is not much tangible difference between the two. I think I would not want to introduce a second route that does precisely the same thing as another route. I think it is sufficient to differentiate the redirect behavior based on the request properties.

In first, code example I think $badreferrers is a bad variable name. There is nothing bad about the referrer right? Those refererr values lead to a different redirect.

\$\endgroup\$

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.