-
Notifications
You must be signed in to change notification settings - Fork 40
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 new watermill handlers that get or refresh entities by properties and call another handler #4545
Conversation
6252c1f
to
965442a
Compare
d5b3a6f
to
a93a3f4
Compare
I split the code into multiple sub-packages with smaller files so it's easier to digest |
ba5a06d
to
ceaa2f8
Compare
There are no functional changes in the latest push, just beefing up the test coverage. Almost there! |
// HandleEntityAndDoMessage is a message that is sent to the entity handler to refresh an entity and perform an action. | ||
type HandleEntityAndDoMessage struct { | ||
Entity TypedProps `json:"entity"` | ||
Owner TypedProps `json:"owner"` |
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.
Owner can be a little confusing (at first I thought we were conflating GitHub terms again). Can we rename this to originator or begetter?
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.
done!
… and call another handler These will be used in webhook handlers instead of calling the property service directly. The handlers follow the same base logic, just with "pluggable" ways of retrieving the entity, converting the entity to properties and which handler to call next. Related: mindersec#4327
ceaa2f8
to
238f52f
Compare
Summary
These will be used in webhook handlers instead of calling the property
service directly. The handlers follow the same base logic, just with
"pluggable" ways of retrieving the entity, converting the entity to
properties and which handler to call next.
The test coverage is not great yet, but I did test the handlers manually
outside the tests I have in another branch and they seemed to work well.
I will be adding more tests as this PR is reviewed.
Related: #4327
Change Type
Testing
some unit tests are added in this PR, but I'll add more. Most of the testing has been manual as part of another branch.
Review Checklist: