Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

wip: fixes controller #850

wants to merge 3 commits into from

Conversation

davidaurelio
Copy link
Member

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description. Or if there's no issue created, make sure you
    describe here the problem you're solving.
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

If the PR is changing the API specification:

  • make sure you add a "Not implemented yet" note the endpoint description, if the implementation is not ready
    yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
  • make sure you add at least one example of the request and response.

If the PR is changing the API implementation or an entity exposed through the API:

  • make sure you update the API specification and the examples to reflect the changes.

If the PR is introducing a new audit type:

  • make sure you update the API specification with the type, schema of the audit result and an example

Related Issues

Thanks for contributing!

@davidaurelio davidaurelio force-pushed the aurelio/fixes-controller branch from 72c340e to 3104b7b Compare May 14, 2025 14:15
Copy link

This PR will trigger no release when merged.

type: string
format: uuid
suggestionId:
Copy link
Contributor

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);
Copy link
Contributor

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 ?

Copy link
Member Author

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

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.

2 participants