-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix(specs): add missing attributes for CT Source update #2792
Conversation
✅ Deploy Preview for api-clients-automation ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
fallbackIsInStockValue: | ||
type: boolean | ||
default: true | ||
description: > | ||
Determines the value that will be stored in the Algolia record if there's no inventory information on the product. |
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.
This means during an update, it will default to true...
Are you sure this is the wanted behavior?
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.
With the dashboard, the value should always be sent, but you're right, let's remove the default value on update
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.
Then you should add it to the required array
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
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.
(and removed, see below)
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.
This means during an update, it will default to true... Are you sure this is the wanted behavior?
We don't (and shouldn't) use default values in the API client, if the API defaults to true it's normal to mention it here
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.
ah it's an update nvm
customFields: | ||
$ref: '#/CommercetoolsCustomFields' | ||
required: |
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.
the goal of the update is not to have required properties
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.
Removed, please, I just want to close my PR about the custom fields...
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.
wonderful
🧭 What and Why
🎟 JIRA Ticket: DI-1788
Changes included:
url
andfallbackIsInStockValue
for CT Source Update