Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add image uploads #76

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,7 @@ app/config/parameters.yml

composer.phar
behat.yml
.idea
web/uploads/*
!web/uploads/.gitkeep
!web/uploads/media/.gitkeep
113 changes: 113 additions & 0 deletions src/Event/EventBundle/Controller/Backend/MediaController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php

namespace Event\EventBundle\Controller\Backend;

use Symfony\Component\Config\Definition\Exception\Exception;
use Symfony\Component\HttpFoundation\Request;
use Event\EventBundle\Controller\Controller;
use Event\EventBundle\Entity\Media;
use Event\EventBundle\Form\Type\MediaType;

class MediaController extends Controller
{

public function getUploadPath()
{
return $this->get('kernel')->getRootDir() . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR .
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be taken from the paramters.yml

'web' . DIRECTORY_SEPARATOR . 'uploads' . DIRECTORY_SEPARATOR . 'media' . DIRECTORY_SEPARATOR;
}

public function getUploadUrl(){
return '/uploads/media/';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be taken from the paramters.yml

}

public function indexAction()
{
return $this->render('EventEventBundle:Backend/Media:index.html.twig', array(
'media' => $this->getRepository('EventEventBundle:Media')->findAll(),
'uploadPath' => $this->getUploadPath(),
'uploadUrl' => $this->getUploadUrl()
));
}

public function manageAction(Request $request, $id = null)
{
$oldFileName = '';
if ($id === null) {
$entity = new Media();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$entity is not yet defined, as well use user-friendly naming, not entity but $media

} else {
$entity = $this->findOr404('EventEventBundle:Media', $id);
$oldFileName = $entity->getFilename();
}

$form = $this->createForm(MediaType::class, $entity);

if ($request->getMethod() === 'POST') {
$form->handleRequest($request);

if ($form->isValid()) {
if(is_null($entity->getId())){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check CS (code style) for all files what you added

if ( should be with the space, use code sniffer from sensiolab

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this condition in general is excess here

$entity->setCreatedDate(new \DateTime());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set it in entity constructor

}

if($oldFileName !== $entity->getFilename()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this to the proxy method

$url = $request->request->get('file_url');
$path = $this->getUploadPath();
$entity->setFilename( time() . '_' . $entity->getFilename());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setFilename(time() no space, CS

$file = $this->getFile($url);
file_put_contents($path . $entity->getFilename(), $file);
chmod($path . $entity->getFilename(), 0777);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(is_file($path . $oldFileName)) {
unlink($path . $oldFileName);
}
}

$entity->setUpdatedDate(new \DateTime());
$this->getManager()->persist($entity);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$this->getManager()->flush();

$successFlashText = sprintf('Media %s updated.', $entity->getTitle());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

messages should be translatable

if (!$id) {
$successFlashText = sprintf('Media %s added.', $entity->getTitle());
}
$this->setSuccessFlash($successFlashText);

return $this->redirectToRoute('backend_media');
}
}

return $this->render('EventEventBundle:Backend/Media:manage.html.twig', [
'media' => $entity,
'form' => $form->createView(),
// 'configLocales' => $this->container->getParameter('media.locales'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove any code comments what not used

]);
}

protected function getFile($url){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be on a new line

$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
$file = curl_exec($ch);
curl_close($ch);
return $file;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS, add nl above the return

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe better to use Guzzle or similar http client for requests

}

public function deleteAction($id)
{
$this->isGrantedAdmin();

$entity = $this->findOr404('EventEventBundle:Media', $id);
$file = $this->getUploadPath() . $entity->getFilename();
$this->getManager()->remove($entity);
$this->getManager()->flush();
if(is_file($file)) {
unlink($file);
}

$this->setSuccessFlash('Media deleted.');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translation

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be translatable


return $this->redirectToRoute('backend_media');
}


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra spaces

}
269 changes: 269 additions & 0 deletions src/Event/EventBundle/Entity/Media.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
<?php

namespace Event\EventBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\ArrayCollection;
use Event\EventBundle\Entity\Translation\Translation;

/**
* Media
*
* @ORM\Table(name="ev_media")
* @ORM\Entity(repositoryClass="Event\EventBundle\Entity\Repository\MediaRepository")
*/
class Media
{

/**
* @var integer
*
* @ORM\Column(name="id", type="integer")
* @ORM\Id
* @ORM\GeneratedValue(strategy="AUTO")
*/
private $id;

/**
* @var string
*
* @ORM\Column(name="title", type="string", length=255)
*/
private $title;

/**
* @var string
*
* @ORM\Column(name="filename", type="string", length=255)
*/
private $filename;

/**
* @var string
*
* @ORM\Column(name="description", type="text", nullable=true)
*/
private $description;

/**
* @var string
*
* @ORM\Column(name="copyrightInfo", type="text", nullable=true)
*/
private $copyrightInfo;

/**
* @var string
*
* @ORM\Column(name="mediaCredits", type="string", nullable=true)
*/
private $mediaCredits;

/**
* @var \DateTime
*
* @ORM\Column(name="created_date", type="datetime")
*/
private $createdDate;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave just created


/**
* @var \DateTime
*
* @ORM\Column(name="updated_date", type="datetime")
*/
private $updatedDate;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave just updated


public function getUploadImg(){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS, { should be on the new line (nl)

return '/uploads/media/' . $this->filename;
}

public function __construct()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pilot why?
here should be

$this->created = new \DateTime();
$this->updated = new \DateTime();

{

}

/**
* Get id
*
* @return integer
*/
public function getId()
{
return $this->id;
}

/**
* Set title
*
* @param string $title
* @return Media
*/
public function setTitle($title)
{
$this->title = $title;

return $this;
}

/**
* Get title
*
* @return string
*/
public function getTitle()
{
return $this->title;
}

/**
* Set filename
*
* @param string $filename
* @return Media
*/
public function setFilename($filename)
{
$this->filename = $filename;

return $this;
}

/**
* Get filename
*
* @return string
*/
public function getFilename()
{
return $this->filename;
}

/**
* Set description
*
* @param string $description
* @return Media
*/
public function setDescription($description)
{
$this->description = $description;

return $this;
}

/**
* Get description
*
* @return string
*/
public function getDescription()
{
return $this->description;
}

/**
* Set copyrightInfo
*
* @param string $copyrightInfo
* @return Media
*/
public function setCopyrightInfo($copyrightInfo)
{
$this->copyrightInfo = $copyrightInfo;

return $this;
}

/**
* Get copyrightInfo
*
* @return string
*/
public function getCopyrightInfo()
{
return $this->copyrightInfo;
}

/**
* Set mediaCredits
*
* @param string $mediaCredits
* @return Media
*/
public function setMediaCredits($mediaCredits)
{
$this->mediaCredits = $mediaCredits;

return $this;
}

/**
* Get mediaCredits
*
* @return string
*/
public function getMediaCredits()
{
return $this->mediaCredits;
}

/**
* Set createdDate
*
* @param \DateTime $createdDate
* @return Media
*/
public function setCreatedDate($createdDate)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add typehint

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would replace setCreated and setUpdated with Gedmo Timestampable

{
if (!$createdDate instanceOf \DateTime) {
$createdDate = \DateTime::createFromFormat('m/d/Y', $createdDate);
}

$this->createdDate = $createdDate;

return $this;
}

/**
* Get createdDate
*
* @return \DateTime
*/
public function getCreatedDate()
{
return $this->createdDate;
}

/**
* Set updatedDate
*
* @param \DateTime $updatedDate
* @return Media
*/
public function setUpdatedDate($updatedDate)
{
if (!$updatedDate instanceOf \DateTime) {
$updatedDate = \DateTime::createFromFormat('m/d/Y', $updatedDate);
}

$this->updatedDate = $updatedDate;

return $this;
}

/**
* Get updatedDate
*
* @return \DateTime
*/
public function getUpdatedDate()
{
return $this->updatedDate;
}

public function __toString()
{
return $this->title;
}
}
Loading