-
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
Migrate ProductProjection model #738
Conversation
🦋 Changeset detectedLatest commit: bfa97b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 47 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
buildFields: ['obj'], | ||
replaceFields: ({ fields }) => { | ||
return { | ||
...fields, | ||
obj: | ||
(fields.obj as TExpandedReferenceObject) || omit(fields, ['typeId']), | ||
}; | ||
}, |
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 is so we can forward to the obj
object all the properties we potentially have received so, in case the model was populated with a reference object, the model instance has its properties.
|
||
it( | ||
...createBuilderSpec<TProductProjection, TProductProjectionRest>( | ||
...createBuilderSpec<TProductProjectionRest, TProductProjectionRest>( |
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 the default type test as we don't have it anymore.
@@ -0,0 +1,25 @@ | |||
import { createSpecializedBuilder } from '@commercetools-test-data/core'; |
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.
Part of the migration of this model to use the new implementation patterns.
@@ -0,0 +1,131 @@ | |||
import { LocalizedString, Reference } from '@commercetools-test-data/commons'; |
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.
Part of the migration of this model to use the new implementation patterns.
@@ -1,4 +1,14 @@ | |||
export { default as random } from './builder'; | |||
export { default as presets } from './presets'; | |||
import * as modelBuilders from './builders'; |
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.
Since this model was not used yet, I'm not including the compat builder.
@@ -0,0 +1,52 @@ | |||
import { LocalizedString, Reference } from '@commercetools-test-data/commons'; |
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.
Part of the migration of this model to use the new implementation patterns.
import type { TBuilder } from '@commercetools-test-data/core'; | ||
import { TState } from '@commercetools-test-data/state'; | ||
import { TTaxCategory } from '@commercetools-test-data/tax-category'; | ||
import type { TCtpProductProjection } from '@commercetools-test-data/graphql-types'; |
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.
Simplified the types by leveraging the generated GraphQL ones.
@@ -45,6 +45,7 @@ const transformers = { | |||
(category) => ({ | |||
id: category.id, | |||
typeId: 'category', | |||
obj: category, |
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.
Consumers (MC in the case I detected) were expecting the obj
to be populated, which makes sense.
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, looks good to me 👍
We need to add the changeset, right?
Thanks for the reminder, Kacper 🙇 Changesets added here: bfa97b0 |
The
ProductProjection
model was one of the first ones we created when we started the initiative to remove local test data models from the MC frontend monorepo but we never got to actually use it.Now that I was reviewing the pending work we have, I noticed we had this PR that was trying to address some issues related to this model but that was something we tried before the introduction of the new implementation patterns so I took the opportunity to start cleaning up the model and migrate it to the new patterns.
The main compatibility issue for this model to be usable was it had an incorrect preset name.