Add user relationship to Post and Comment entities #434
Conversation
|
@yceruto, I think we don't need the relation between |
|
@voronkovich before this PR to comment on Post is required to be logged, just I keep the current behavior for now :) |
|
@yceruto, ok, let's keep it as is for now. |
| { | ||
| $this->publishedAt = $publishedAt; | ||
| return $user === $this->author; |
There was a problem hiding this comment.
I think we need an extra check here since the user variable might be null:
if (null === $user) {
return false;
}There was a problem hiding this comment.
For extra readability? when the user is null anyway it already returns false.
Edit: it was necessary before since we check $user->getEmail() ;)
There was a problem hiding this comment.
@yceruto, both variables $user and $this->author can be null at the same time.
There was a problem hiding this comment.
@voronkovich It shouldn't happen I think, as each Post is initialized with its author and is required also. I'm missing something?
There was a problem hiding this comment.
@yceruto, you're right! So, we don't need to check the user variable for null value here.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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 :)
There was a problem hiding this comment.
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!
|
Rebase with master and added more tests. |
|
This was a much needed improvement. @yceruto thanks for contributing this feature. |
…uto) This PR was merged into the master branch. Discussion ---------- Add user relationship to Post and Comment entities Actually, we are related Post/User and Comment/User indirectly throught `authorEmail` field which IMHO makes things more complicated to understand for the beginner, because really we need this relationship. Related discussion in #429. Commits ------- 52d2fa3 Add user relationship to Post and Comment entities
Actually, we are related Post/User and Comment/User indirectly throught
authorEmailfield which IMHO makes things more complicated to understand for the beginner, because really we need this relationship.Related discussion in #429.