Skip to content

Add user relationship to Post and Comment entities #434

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

Merged
merged 1 commit into from
Jan 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/Resources/views/admin/blog/new.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
{{ form_row(form.title) }}
{{ form_row(form.summary) }}
{{ form_row(form.content) }}
{{ form_row(form.authorEmail) }}
{{ form_row(form.publishedAt) }}

<input type="submit" value="{{ 'label.create_post'|trans }}" class="btn btn-primary" />
Expand Down
2 changes: 1 addition & 1 deletion app/Resources/views/admin/blog/show.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
</tr>
<tr>
<th scope="row">{{ 'label.author'|trans }}</th>
<td><p>{{ post.authorEmail }}</p></td>
<td><p>{{ post.author.email }}</p></td>
</tr>
<tr>
<th scope="row">{{ 'label.published_at'|trans }}</th>
Expand Down
2 changes: 1 addition & 1 deletion app/Resources/views/blog/index.xml.twig
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<link>{{ url('blog_post', {'slug': post.slug}) }}</link>
<guid>{{ url('blog_post', {'slug': post.slug}) }}</guid>
<pubDate>{{ post.publishedAt|date(format='r', timezone='GMT') }}</pubDate>
<author>{{ post.authorEmail }}</author>
<author>{{ post.author.email }}</author>
</item>
{% endfor %}
</channel>
Expand Down
4 changes: 2 additions & 2 deletions app/Resources/views/blog/post_show.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<div class="row post-comment">
<a name="comment_{{ comment.id }}"></a>
<h4 class="col-sm-3">
<strong>{{ comment.authorEmail }}</strong> {{ 'post.commented_on'|trans }}
<strong>{{ comment.author.email }}</strong> {{ 'post.commented_on'|trans }}
{# it's not mandatory to set the timezone in localizeddate(). This is done to
avoid errors when the 'intl' PHP extension is not available and the application
is forced to use the limited "intl polyfill", which only supports UTC and GMT #}
Expand All @@ -50,7 +50,7 @@
{% endblock %}

{% block sidebar %}
{% if app.user and post.isAuthor(app.user) %}
{% if post.isAuthor(app.user) %}
<div class="section">
<a class="btn btn-lg btn-block btn-success" href="{{ path('admin_post_edit', { id: post.id }) }}">
<i class="fa fa-edit" aria-hidden="true"></i> {{ 'action.edit_post'|trans }}
Expand Down
8 changes: 4 additions & 4 deletions src/AppBundle/Controller/Admin/BlogController.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class BlogController extends Controller
public function indexAction()
{
$entityManager = $this->getDoctrine()->getManager();
$posts = $entityManager->getRepository(Post::class)->findBy(['authorEmail' => $this->getUser()->getEmail()], ['publishedAt' => 'DESC']);
$posts = $entityManager->getRepository(Post::class)->findBy(['author' => $this->getUser()], ['publishedAt' => 'DESC']);

return $this->render('admin/blog/index.html.twig', ['posts' => $posts]);
}
Expand All @@ -73,7 +73,7 @@ public function indexAction()
public function newAction(Request $request)
{
$post = new Post();
$post->setAuthorEmail($this->getUser()->getEmail());
$post->setAuthor($this->getUser());

// See http://symfony.com/doc/current/book/forms.html#submitting-forms-with-multiple-buttons
$form = $this->createForm(PostType::class, $post)
Expand Down Expand Up @@ -122,7 +122,7 @@ public function showAction(Post $post)
// This security check can also be performed:
// 1. Using an annotation: @Security("post.isAuthor(user)")
// 2. Using a "voter" (see http://symfony.com/doc/current/cookbook/security/voters_data_permission.html)
if (null === $this->getUser() || !$post->isAuthor($this->getUser())) {
if (!$post->isAuthor($this->getUser())) {
throw $this->createAccessDeniedException('Posts can only be shown to their authors.');
}

Expand All @@ -139,7 +139,7 @@ public function showAction(Post $post)
*/
public function editAction(Post $post, Request $request)
{
if (null === $this->getUser() || !$post->isAuthor($this->getUser())) {
if (!$post->isAuthor($this->getUser())) {
throw $this->createAccessDeniedException('Posts can only be edited by their authors.');
}

Expand Down
2 changes: 1 addition & 1 deletion src/AppBundle/Controller/BlogController.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function commentNewAction(Request $request, Post $post)
if ($form->isSubmitted() && $form->isValid()) {
/** @var Comment $comment */
$comment = $form->getData();
$comment->setAuthorEmail($this->getUser()->getEmail());
$comment->setAuthor($this->getUser());
$comment->setPost($post);

$entityManager = $this->getDoctrine()->getManager();
Expand Down
10 changes: 6 additions & 4 deletions src/AppBundle/DataFixtures/ORM/LoadFixtures.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use AppBundle\Entity\Comment;
use AppBundle\Entity\Post;
use AppBundle\Entity\User;
use Doctrine\Common\DataFixtures\FixtureInterface;
use Doctrine\Common\DataFixtures\AbstractFixture;
use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
Expand All @@ -32,7 +32,7 @@
* @author Ryan Weaver <[email protected]>
* @author Javier Eguiluz <[email protected]>
*/
class LoadFixtures implements FixtureInterface, ContainerAwareInterface
class LoadFixtures extends AbstractFixture implements ContainerAwareInterface
{
use ContainerAwareTrait;

Expand All @@ -55,6 +55,7 @@ private function loadUsers(ObjectManager $manager)
$encodedPassword = $passwordEncoder->encodePassword($johnUser, 'kitten');
$johnUser->setPassword($encodedPassword);
$manager->persist($johnUser);
$this->addReference('john-user', $johnUser);

$annaAdmin = new User();
$annaAdmin->setUsername('anna_admin');
Expand All @@ -63,6 +64,7 @@ private function loadUsers(ObjectManager $manager)
$encodedPassword = $passwordEncoder->encodePassword($annaAdmin, 'kitten');
$annaAdmin->setPassword($encodedPassword);
$manager->persist($annaAdmin);
$this->addReference('anna-admin', $annaAdmin);

$manager->flush();
}
Expand All @@ -76,13 +78,13 @@ private function loadPosts(ObjectManager $manager)
$post->setSummary($this->getRandomPostSummary());
$post->setSlug($this->container->get('slugger')->slugify($post->getTitle()));
$post->setContent($this->getPostContent());
$post->setAuthorEmail('[email protected]');
$post->setAuthor($this->getReference('anna-admin'));
$post->setPublishedAt(new \DateTime('now - '.$i.'days'));

foreach (range(1, 5) as $j) {
$comment = new Comment();

$comment->setAuthorEmail('[email protected]');
$comment->setAuthor($this->getReference('john-user'));
$comment->setPublishedAt(new \DateTime('now + '.($i + $j).'seconds'));
$comment->setContent($this->getRandomCommentContent());
$comment->setPost($post);
Expand Down
41 changes: 22 additions & 19 deletions src/AppBundle/Entity/Comment.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ class Comment
*/
private $content;

/**
* @var string
*
* @ORM\Column(type="string")
* @Assert\Email
*/
private $authorEmail;

/**
* @var \DateTime
*
Expand All @@ -76,6 +68,14 @@ class Comment
*/
private $publishedAt;

/**
* @var User
*
* @ORM\ManyToOne(targetEntity="AppBundle\Entity\User")
* @ORM\JoinColumn(nullable=false)
*/
private $author;

public function __construct()
{
$this->publishedAt = new \DateTime();
Expand Down Expand Up @@ -109,27 +109,30 @@ public function setContent($content)
$this->content = $content;
}

public function getAuthorEmail()
public function getPublishedAt()
{
return $this->authorEmail;
return $this->publishedAt;
}

/**
* @param string $authorEmail
*/
public function setAuthorEmail($authorEmail)
public function setPublishedAt(\DateTime $publishedAt)
{
$this->authorEmail = $authorEmail;
$this->publishedAt = $publishedAt;
}

public function getPublishedAt()
/**
* @return User
*/
public function getAuthor()
{
return $this->publishedAt;
return $this->author;
}

public function setPublishedAt(\DateTime $publishedAt)
/**
* @param User $author
*/
public function setAuthor(User $author)
{
$this->publishedAt = $publishedAt;
$this->author = $author;
}

public function getPost()
Expand Down
47 changes: 25 additions & 22 deletions src/AppBundle/Entity/Post.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ class Post
*/
private $content;

/**
* @var string
*
* @ORM\Column(type="string")
* @Assert\Email
*/
private $authorEmail;

/**
* @var \DateTime
*
Expand All @@ -87,6 +79,14 @@ class Post
*/
private $publishedAt;

/**
* @var User
*
* @ORM\ManyToOne(targetEntity="AppBundle\Entity\User")
* @ORM\JoinColumn(nullable=false)
*/
private $author;

/**
* @var Comment[]|ArrayCollection
*
Expand Down Expand Up @@ -149,35 +149,38 @@ public function setContent($content)
$this->content = $content;
}

public function getAuthorEmail()
public function getPublishedAt()
{
return $this->authorEmail;
return $this->publishedAt;
}

/**
* @param string $authorEmail
*/
public function setAuthorEmail($authorEmail)
public function setPublishedAt(\DateTime $publishedAt)
{
$this->authorEmail = $authorEmail;
$this->publishedAt = $publishedAt;
}

/**
* Is the given User the author of this Post?
* @return User
*/
public function isAuthor(User $user)
public function getAuthor()
{
return $user->getEmail() === $this->getAuthorEmail();
return $this->author;
}

public function getPublishedAt()
/**
* @param User $author
*/
public function setAuthor(User $author)
{
return $this->publishedAt;
$this->author = $author;
}

public function setPublishedAt(\DateTime $publishedAt)
/**
* Is the given User the author of this Post?
*/
public function isAuthor(User $user = null)
{
$this->publishedAt = $publishedAt;
return $user === $this->author;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need an extra check here since the user variable might be null:

if (null === $user) {
    return false;
}

Copy link
Member Author

@yceruto yceruto Jan 15, 2017

Choose a reason for hiding this comment

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

For extra readability? when the user is null anyway it already returns false.

Edit: it was necessary before since we check $user->getEmail() ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

@yceruto, both variables $user and $this->author can be null at the same time.

Copy link
Member Author

@yceruto yceruto Jan 15, 2017

Choose a reason for hiding this comment

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

@voronkovich It shouldn't happen I think, as each Post is initialized with its author and is required also. I'm missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yceruto, you're right! So, we don't need to check the user variable for null value here.

Copy link
Contributor

Choose a reason for hiding this comment

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

As Oleg said, both variables $user and $this->author can be null at the same time - I just want to prevent weird bugs in the future. I think it's unsafe just rely on nullable option which could be changed.

Copy link
Member Author

@yceruto yceruto Jan 17, 2017

Choose a reason for hiding this comment

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

@bocharsky-bw Sorry for the delay, I understand your preoccupation, but apparently that case is not possible. If a post exists this have an author and this field is not nullable in the database either. Thanks for the discussion guys, but I don't think it's worth it :)

Copy link
Member

Choose a reason for hiding this comment

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

All of you have given good reasons ... so let's keep this as it is to move on ... and we'll add the extra check if needed in the future. Thanks!

}

public function getComments()
Expand Down
4 changes: 3 additions & 1 deletion src/AppBundle/EventListener/CommentNotificationListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace AppBundle\EventListener;

use AppBundle\Entity\Comment;
use Symfony\Component\EventDispatcher\GenericEvent;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Translation\TranslatorInterface;
Expand Down Expand Up @@ -63,6 +64,7 @@ public function __construct(\Swift_Mailer $mailer, UrlGeneratorInterface $urlGen
*/
public function onCommentCreated(GenericEvent $event)
{
/** @var Comment $comment */
$comment = $event->getSubject();
$post = $comment->getPost();

Expand All @@ -82,7 +84,7 @@ public function onCommentCreated(GenericEvent $event)
// See http://symfony.com/doc/current/email.html#sending-emails
$message = \Swift_Message::newInstance()
->setSubject($subject)
->setTo($post->getAuthorEmail())
->setTo($post->getAuthor()->getEmail())
->setFrom($this->sender)
->setBody($body, 'text/html')
;
Expand Down
3 changes: 0 additions & 3 deletions src/AppBundle/Form/PostType.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'attr' => ['rows' => 20],
'label' => 'label.content',
])
->add('authorEmail', null, [
'label' => 'label.author_email',
])
->add('publishedAt', DateTimePickerType::class, [
'label' => 'label.published_at',
])
Expand Down
22 changes: 22 additions & 0 deletions tests/AppBundle/Controller/Admin/BlogControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,26 @@ public function testIndex()
'The backend homepage displays all the available posts.'
);
}

/**
* @dataProvider getPostUrls
*/
public function testOnlyAuthorCanAccessToThePostInTheBackend($method, $url, $statusCode)
{
$client = static::createClient([], [
'PHP_AUTH_USER' => 'anna_admin',
'PHP_AUTH_PW' => 'kitten',
]);

$client->request($method, $url);

$this->assertEquals($statusCode, $client->getResponse()->getStatusCode());
}

public function getPostUrls()
{
yield ['GET', '/en/admin/post/1', Response::HTTP_OK];
yield ['GET', '/en/admin/post/1/edit', Response::HTTP_OK];
yield ['POST', '/en/admin/post/1/delete', Response::HTTP_FOUND];
}
}
Binary file modified var/data/blog.sqlite
Binary file not shown.