-
Notifications
You must be signed in to change notification settings - Fork 6
wip: fixes controller #850
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
base: main
Are you sure you want to change the base?
Conversation
72c340e
to
3104b7b
Compare
This PR will trigger no release when merged. |
type: string | ||
format: uuid | ||
suggestionId: |
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.
Here it s actually the opportunityId
constructor(dataAccess) { | ||
this.#FixEntity = dataAccess.FixEntity; | ||
|
||
this.getAllForOpportunity = this.getAllForOpportunity.bind(this); |
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.
Why do we need to bind the methods for the 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.
because they are simply referenced at https://github.com/adobe/spacecat-api-service/pull/850/files#diff-268a45f8e3c5fba5953593271f7aaed3d91812853a17dea472417fcc17c84bfdR169-R176
Alternatively, I can use arrow functions there.
lmk what you prefer
Please ensure your pull request adheres to the following guidelines:
describe here the problem you're solving.
If the PR is changing the API specification:
yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
If the PR is changing the API implementation or an entity exposed through the API:
If the PR is introducing a new audit type:
Related Issues
Thanks for contributing!