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 reopen todo #59

Merged
merged 6 commits into from
Feb 17, 2016
Merged

add reopen todo #59

merged 6 commits into from
Feb 17, 2016

Conversation

basz
Copy link
Member

@basz basz commented Feb 16, 2016

exersize #2

* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*
* Date: 5/2/15 - 5:50 PM
Copy link
Member

Choose a reason for hiding this comment

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

you did this last year? :)

@prolic
Copy link
Member

prolic commented Feb 16, 2016

Also some php-cs issues, see travis build.

*/
public function onTodoWasReopened(TodoWasReopened $event)
{
$user = $this->userFinder->findUserOfTodo($event->todoId()->toString());
Copy link
Member

Choose a reason for hiding this comment

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

@basz I don't think that this is needed.

/cc @codeliner thougts?

Copy link
Member

Choose a reason for hiding this comment

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

all methods of the UserProjector look like the new one added by @basz
So I'd say it is ok for the exercise but we should think about refactoring it.

Unfortunately, I cannot remember why assignee_id is not part of the events. However, this would be a better approach as an event should contain all data required to populate the read model. If we add the assignee_id to the events we can remove the UserFinder dependency and the exception if user cannot be found. It is task of the domain to protect invariants, so the user must exist if referenced in an event.

Copy link
Member Author

Choose a reason for hiding this comment

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

most things were a copy of the TodoWasMarkedAsDone behaviour, that's my excuse here... ;-p

It is task of the domain to protect invariants

sounds sane

@codeliner
Copy link
Member

excellent work @basz

codeliner added a commit that referenced this pull request Feb 17, 2016
@codeliner codeliner merged commit 927cd96 into prooph:master Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants