-
Notifications
You must be signed in to change notification settings - Fork 4
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 featured snippet DA interfaces for the builtin fieldType #207
Conversation
Pull Request Test Coverage Report for Build 2862788583
💛 - Coveralls |
* | ||
* @public | ||
*/ | ||
export interface RichTextFeaturedSnippetDirectAnswer extends Omit<BaseFeaturedSnippetDirectAnswer<string>, 'value'> { |
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.
are you sure you don't want to put RichTextFeaturedSnippetDirectAnswer, MultiLineTextFeaturedSnippetDirectAnswer, and BaseFeaturedSnippetDirectAnswer into FeaturedSnippetDirectAnswer.ts ? There's a lot of jumping around when I try to navigate the code, and each file is less than 20 lines (with ~4 of those lines being import statements) and I don't see them getting much bigger in the future
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.
It would only add about ~30 lines to FeaturedSnippetDirectAnswer.ts, I feel like that's totally reasonable
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.
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.
sure, I can combine them
src/models/searchservice/response/directanswer/FeaturedSnippetDirectAnswer.ts
Outdated
Show resolved
Hide resolved
src/models/searchservice/response/directanswer/MultiLineTextFeaturedSnippetDirectAnswer.ts
Outdated
Show resolved
Hide resolved
src/models/searchservice/response/directanswer/MultiLineTextFeaturedSnippetDirectAnswer.ts
Outdated
Show resolved
Hide resolved
src/models/searchservice/response/directanswer/BaseFeaturedSnippetDirectAnswer.ts
Outdated
Show resolved
Hide resolved
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.
lgtm!
## Version 2.0.0 ### Changes - `additionalQueryParameter` is now a public field in `SearchConfig` models (#217) ### Breaking Changes - Updated default and sandbox URL endpoints from `../answers/..` to `../search/..` as part of rebranding process (#196 ) - Restructured StaticFilters models for better developer experience and enforce proper restriction in the kind of combinations of filters supported by the backend - Now, static filters can be either a field value filter or a nested object that is composed by combining field value filters. (#201 ) - `ConjunctionStaticFilter` and `DisjunctionStaticFilter` models were created to reflect such limitation. For example, ANDs of ORs combination is allowed, ORs of ANDs combination is not allowed. (#204 ) - Updated `FieldValueDirectAnswer` model to properly handle different `value` types. Previously, FieldValueDirectAnswer interface enforces that the `value` field will always be of type `string`. Now, `FieldValueDirectAnswer` is a union type of predefined interfaces with known `value` type and `UnknownFieldValueDirectAnswer` with a generic `value` type for other `fieldType` outside of `BuiltInFieldType`. (#200 #202 ) - The newly added built-in interfaces can be found in the document page [**here**](https://github.com/yext/search-core/blob/develop/docs/search-core.fieldvaluedirectanswer.md) (#206 #208 #210 #211 #205 #209 #203 #220 ) - Narrow down `FeaturedSnippetDirectAnswer` TypeScript model to be a union type of `MultiLineTextSnippetDirectAnswer` and `RichTextSnippetDirectAnswer` as a featured snippet direct answer can only be of fieldType `multi_line_text` or `rich_text`. (#207 #212 ) - All exports marked as `@deprecated` in previous version(s) as part of the rebranding process is now removed in V2 (#216 ) - For more details, the deprecated identifiers are listed in [v1.8 release notes](https://github.com/yext/search-core/releases/tag/v1.8.0)
MultiLineTextFeaturedSnippetDirectAnswer
andRichTextFeaturedSnippetDirectAnswer
for the two types that fieldType of featured snippet direct answer can be.multi_line_text
for feature snippet DA have non optionalvalue
field of type stringrich_text
for feature snippet DA does not havevalue
field so that is omitted in the interfaceFeaturedSnippetDirectAnswer
is now the base interfaceBaseFeaturedSnippetDirectAnswer
that the specific interfaces extend from. The newFeaturedSnippetDirectAnswer
is the union type.J=SLAP-2330
TEST=auto&manual
oshi confirmed the structure of multi_line_text and rich_text through examples listed in the comments under this item where rich_text does NOT provide
value
field andmulti_line_text
does.tested locally that:
![Screen Shot 2022-08-15 at 1 19 51 PM](https://user-images.githubusercontent.com/36055303/184686077-a6ac524a-43f6-427f-a777-c0d044f542b3.png)
- 'value' field is present, of type string, and NOT optional in IDE for the switch case 'multi_line_text'