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

require aggregate changed to have non empty ar id #87

Closed
wants to merge 1 commit into from
Closed

Conversation

prolic
Copy link
Member

@prolic prolic commented Nov 2, 2018

resolves #83

ping @enumag - does that work for you?

@prolic prolic added the bug label Nov 2, 2018
@prolic prolic self-assigned this Nov 2, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e367150 on ar_id into ce3e2ca on master.

@enumag
Copy link
Member

enumag commented Nov 2, 2018

Not really because my events often override the aggregateId method to return one of the properties. In the case I reported it was a wrong property. This wouldn't have helped with that particular case.

@prolic
Copy link
Member Author

prolic commented Nov 3, 2018 via email

@enumag
Copy link
Member

enumag commented Nov 3, 2018

A bug in the getter was what caused the event to be commited without aggregateId. Anyway it's not too important. I think you can just close it.

@prolic
Copy link
Member Author

prolic commented Nov 3, 2018

Okay, thanks for your input.

@prolic prolic closed this Nov 3, 2018
@prolic prolic deleted the ar_id branch November 3, 2018 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing aggregate ID
3 participants