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
Course creator use case #15
Course creator use case #15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good work @mangelsnc!
Some feedback provided. Take special attention to the comment with the
|
||
codely.api.controller.course.post: | ||
class: CodelyTv\Api\Controller\Course\CoursePostController | ||
parent: codely.api.controller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip!
@@ -2,3 +2,4 @@ imports: | |||
|
|||
- { resource: status.yml } | |||
- { resource: video.yml } | |||
- { resource: course.yml } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏👏👏
use CodelyTv\Types\ValueObject\Uuid; | ||
use Symfony\Component\HttpFoundation\Request; | ||
|
||
final class CoursePostController extends ApiController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏👏👏
|
||
public function create(CourseId $id, CourseTitle $title, CourseDescription $description) | ||
{ | ||
$course = new Course($id, $title, $description); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use a named constructor as in https://github.com/CodelyTV/cqrs-ddd-php-example/blob/d3da9074b765d0612e4c467e54d0cac94ba7d7c9/src/Context/Video/Module/Video/Application/Create/VideoCreator.php#L26
Why: https://codely.tv/screencasts/constructores-semanticos/ + record events inside the named constructor leaving the default constructor for reconstructing Video
instances in a certain state without recording these events.
{ | ||
parent::__construct($messageId); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary 2 blank lines 🙂
|
||
final class CourseCreatedDomainEvent extends DomainEvent | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary blank line 🙂
|
||
final class CourseRepositoryMySql extends Repository implements CourseRepository | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary blank line
final class CourseRepositoryMySql extends Repository implements CourseRepository | ||
{ | ||
|
||
public function save(Course $course) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add return type (: void
) in order to make the side effect more explicit.
|
||
$this->repository->save($course); | ||
|
||
$this->publisher->publish(...$course->pullDomainEvents()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
$this->publisher = $publisher; | ||
} | ||
|
||
public function create(CourseId $id, CourseTitle $title, CourseDescription $description) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add return type (: void
) in order to make the side effect more explicit.
@mangelsnc I merged it with a minor changes. Also I created an issue (#24) in order to add some missing things 😋 If you want to solve that issue feel free!! 😬 |
Also, @mangelsnc you're our first external contributor (https://github.com/CodelyTV/cqrs-ddd-php-example/graphs/contributors)!! 🎉🎉🎊🎊 |
Cool! I will take a look at the issue! 😬 |
No description provided.