-
Notifications
You must be signed in to change notification settings - Fork 2
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
WEBDEV-7246: Extract item metadata #63
base: main
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
=========================================
+ Coverage 0 97.16% +97.16%
=========================================
Files 0 22 +22
Lines 0 2754 +2754
Branches 0 217 +217
=========================================
+ Hits 0 2676 +2676
- Misses 0 66 +66
- Partials 0 12 +12 ☔ View full report in Codecov by Sentry. |
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, nice consolidation & purge 👍
src/models/hit-types/item-hit.ts
Outdated
* In bytes; computed during document construction, for collection items only. | ||
* Optional. | ||
*/ | ||
@Memoize() get collection_size(): typeof Metadata.prototype.collection_size { | ||
@Memoize() | ||
get collection_size(): typeof SearchMetadata.prototype.collection_size { | ||
return this.rawMetadata?.fields?.collection_size != null | ||
? new ByteField(this.rawMetadata.fields.collection_size) |
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.
Looking through these, it occurs to me that some of these fields are properties of search documents and not necessarily of actual item metadata. The distinction is probably not super meaningful given the way these are used, but figured it might be worth mentioning in case there's a desire to more clearly document the difference where these properties exist in the new item-metadata repo. (num_favorites
is perhaps another example)
src/models/search-metadata.ts
Outdated
export class SearchMetadata extends Metadata { | ||
/** | ||
* some extra fields for "favorited_search" hits | ||
*/ |
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.
Thanks for splitting these out!
Now that we have the iaux-item-metadata package as the authoritative source of metadata, this removed the models from this repo and uses the models from that package.