Update Core helpers to support the new proposed way of writing test data models #688

CarlosCortizasCT · 2024-10-03T16:08:08Z

Based on the agreements achieved in this RFC, we're now implementing (step by step) the changes required in the repository in order to support them.

We already have this PR when we add documentation about this new patterns to follow when writing test data models.

In this PR we're actually updating the core helpers so they can support the current versions of the test data models as well as the new one and also a compatibility mode where some data models need to support both at the same time.

changeset-bot · 2024-10-03T16:08:12Z

🦋 Changeset detected

Latest commit: 76f492e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 46 packages

Name	Type
@commercetools-test-data/standalone-price	Minor
@commercetools-test-data/customer-group	Minor
@commercetools-test-data/product-type	Minor
@commercetools-test-data/channel	Minor
@commercetools-test-data/commons	Minor
@commercetools-test-data/core	Minor
@commercetools-test-data/cart-discount	Minor
@commercetools-test-data/customer	Minor
@commercetools-test-data/order	Minor
@commercetools-test-data/quote-request	Minor
@commercetools-test-data/quote	Minor
@commercetools-test-data/staged-quote	Minor
@commercetools-test-data/product-discount	Minor
@commercetools-test-data/product-projection	Minor
@commercetools-test-data/product	Minor
@commercetools-test-data/cart	Minor
@commercetools-test-data/inventory-entry	Minor
@commercetools-test-data/store	Minor
@commercetools-test-data/associate-role	Minor
@commercetools-test-data/attribute-group	Minor
@commercetools-test-data/business-unit	Minor
@commercetools-test-data/category	Minor
@commercetools-test-data/custom-application	Minor
@commercetools-test-data/custom-object	Minor
@commercetools-test-data/custom-view	Minor
@commercetools-test-data/customers-search-list-my-view	Minor
@commercetools-test-data/discount-code	Minor
@commercetools-test-data/discounts-custom-view	Minor
@commercetools-test-data/organization	Minor
@commercetools-test-data/payment	Minor
@commercetools-test-data/platform-limits	Minor
@commercetools-test-data/product-selection	Minor
@commercetools-test-data/review	Minor
@commercetools-test-data/shipping-method	Minor
@commercetools-test-data/shopping-list	Minor
@commercetools-test-data/state	Minor
@commercetools-test-data/tax-category	Minor
@commercetools-test-data/type	Minor
@commercetools-test-data/user	Minor
@commercetools-test-data/zone	Minor
@commercetools-test-data/filter-values	Minor
@commercetools-test-data/organization-extension	Minor
@commercetools-test-data/project-extension	Minor
@commercetools-test-data/project	Minor
@commercetools-test-data/graphql-types	Minor
@commercetools-test-data/utils	Minor

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

CarlosCortizasCT · 2024-10-03T16:11:31Z

core/src/builder.ts

@@ -34,8 +30,12 @@ const createState = <Model>({

  return {
    get: () => state,
-    merge: (update: Partial<Model>) => {
-      state = { ...state, ...update };
+    merge: (update: Partial<Model>, overwrite = true) => {


Changes in this object are required so we can initialize the state with the generator result after the builder is actually built.

We need a state in the builder so chained calls to its properties can be registered but we cannot set the initial state at builder instantiation time as we don't know yet which version of the builder we're going to use at that point (this is because the compatibility mode).

This solution allows to update the state with the generator result later during the actual build call.

CarlosCortizasCT · 2024-10-03T16:12:11Z

core/src/builder.ts

@@ -77,13 +86,27 @@ function PropertyBuilder<Model>(initialProps?: Partial<Model>) {
 function Builder<Model>({
  generator,
  transformers,
+  type,


In the new pattern, we will be receiving which representation this builder is for: rest | graphql

CarlosCortizasCT · 2024-10-03T16:13:02Z

core/src/builder.ts

@@ -77,13 +86,27 @@ function PropertyBuilder<Model>(initialProps?: Partial<Model>) {
 function Builder<Model>({
  generator,
  transformers,
+  type,
+  postBuild,


In the new pattern, this is a callback used to manipulate the output of the build before it's returned to the consumer. This allows to resolve dependent properties of the data model.

CarlosCortizasCT · 2024-10-03T16:13:16Z

core/src/builder.ts

@@ -77,13 +86,27 @@ function PropertyBuilder<Model>(initialProps?: Partial<Model>) {
 function Builder<Model>({
  generator,
  transformers,
+  type,
+  postBuild,
+  name = 'Unknown Builder',


This helps debugging a lot.

CarlosCortizasCT · 2024-10-03T16:13:52Z

core/src/builder.ts

+  type,
+  postBuild,
+  name = 'Unknown Builder',
+  compatConfig,


This is for the compatibility mode of the new pattern where a single builder needs to support both representations.

CarlosCortizasCT · 2024-10-03T16:15:29Z

core/src/helpers.ts

+  return modelBuilder as TBuilder<TModel>;
+};
+
+const createCompatibilityBuilder = <TModel>(params: {


Helper to create the compatibility builder so current data models can be migrated to the new patterns but still (temporary) expose the same external API (backwards compatibility).

CarlosCortizasCT · 2024-10-03T16:16:18Z

core/src/transformer.ts

@@ -36,6 +43,21 @@ function Transformer<Model, TransformedModel>(
          };
        }
      });
+    } else if (fieldsToBuild !== false) {


We now build all buildable data model properties by default so we don't need to specify an array of those ones at the data model level.

Nit: Would it make sense to "invert" the logic? If some fields are NOT supposed to be built, we would need to explicitly define them.

If so, we can replace the property fieldsToBuild with something like omitFieldsToBuild.

Thanks for the suggestion 👍

I understand having this type dual is not very conventional but I found it to be the simplest solution that fulfills the requirement.
Places where we need this you would need to declare a new property with all models fields so that's why I think the current proposal is a good compromise.

Anyway, I see all the changes implemented here to be temporary. Once we migrate all the models to use the new patterns I believe we will be able to simplify things a lot. For instance, fieldsToBuild is only needed for the (now) legacy data models; new ones does not use it.

CarlosCortizasCT · 2024-10-03T16:17:58Z

core/src/types.ts

    args?: TFieldBuilderArgs<OriginalModel>
  ): TransformedModel;
-  buildGraphql<TransformedModel>(
+  buildGraphql<TransformedModel = OriginalModel>(


This allows for the function to return the expected type by default which works very handy with the new patterns.

CarlosCortizasCT · 2024-10-03T16:18:33Z

models/channel/src/builder.ts

@@ -6,6 +6,7 @@ import type { TCreateChannelBuilder } from './types';

 const Model: TCreateChannelBuilder = () =>
  Builder<Channel>({
+    name: 'Channel',


There are a bunch of models where I added the name in order to help debugging.

I think is better to keep them.

CarlosCortizasCT · 2024-10-03T16:20:21Z

models/commons/src/client-logging/transformers.ts

@@ -12,6 +12,7 @@ const transformers = {
    buildFields: ['customer'],
  }),
  graphql: Transformer<TClientLogging, TClientLoggingGraphql>('graphql', {
+    buildFields: false,


Since previously we needed to provide the buildFields array in order to specify which data models properties should be automatically built, omitting that property meant build no property at all.

Now that this field is not even needed, for those current cases where we don't actually want to build any property we need to explicitly return false.

CarlosCortizasCT · 2024-10-03T16:26:25Z

core/src/builder.spec.ts

@@ -287,71 +277,6 @@ describe('building', () => {
      );
    });

-    it('should build all build upon properties with callbacks', () => {


We had a feature that allowed properties updaters to provide a value based on the previous one.
Keeping this feature with the new proposal is quite hard so, since I haven't found any model in this repo using this feature, I'd like to remove it by now.

If we find any MC applications tests actually using this feature, then I'll get back to it if necessary but I would like to forget about it by now if possible.

emmenko

Thanks again for tackling this beast! 🙌

I think I was able to follow along with the core changes but I trust you know what you are doing 😉

Overall it looks good to me 🤗

emmenko · 2024-10-04T09:48:03Z

core/src/types.ts

@@ -49,12 +51,17 @@ export type TTransformerOptions<Model, TransformedModel> = {
  addFields?: (args: { fields: Model }) => Partial<TransformedModel>;
  removeFields?: (keyof Model)[];
  replaceFields?: (args: { fields: Model }) => TransformedModel;
-  buildFields?: (keyof Model)[];
+  buildFields?: (keyof Model)[] | false;


Nit: let's add a comment to explain what false refers to and when it's supposed to be used.

Added here: af759da

emmenko · 2024-10-04T09:51:59Z

core/src/transformer.ts

@@ -36,6 +43,21 @@ function Transformer<Model, TransformedModel>(
          };
        }
      });
+    } else if (fieldsToBuild !== false) {


Nit: Would it make sense to "invert" the logic? If some fields are NOT supposed to be built, we would need to explicitly define them.

If so, we can replace the property fieldsToBuild with something like omitFieldsToBuild.

CarlosCortizasCT · 2024-10-04T11:59:00Z

.changeset/blue-sloths-repair.md

+
+The main change is about the `core` package where we are introducing support for writing test data models using new implementation patterns which makes the process simpler. Also, the resulting code will be more maintainable.
+
+You can head over [here]() for updated documentation about those new patterns.


I'll update this link once the docs PR is merged.

Sarah4VT

This is not my area of expertise, but I didn't notice anything I would obviously change.

CarlosCortizasCT added 3 commits October 3, 2024 15:33

refactor(core): support new test data models pattern

44ba2e5

refactor(models/commons): add meta info

06c3086

fix(core): transformers execution

01ed88a

CarlosCortizasCT added 🚧 Status: WIP fe-chapter-rotation Tasks coming from frontend chapter work labels Oct 3, 2024

CarlosCortizasCT self-assigned this Oct 3, 2024

refactor(core): refine code

ff1c024

CarlosCortizasCT commented Oct 3, 2024

View reviewed changes

refactor(core): remove unsupported tests

b2c3057

CarlosCortizasCT commented Oct 3, 2024

View reviewed changes

CarlosCortizasCT marked this pull request as ready for review October 3, 2024 16:29

CarlosCortizasCT requested review from a team as code owners October 3, 2024 16:29

CarlosCortizasCT requested review from emmenko, tdeekens and a team October 3, 2024 16:29

CarlosCortizasCT removed the 🚧 Status: WIP label Oct 3, 2024

emmenko reviewed Oct 4, 2024

View reviewed changes

CarlosCortizasCT added 2 commits October 4, 2024 13:53

chore(core): add code comment

af759da

chore: add changeset

76f492e

CarlosCortizasCT commented Oct 4, 2024

View reviewed changes

tdeekens approved these changes Oct 4, 2024

View reviewed changes

Sarah4VT approved these changes Oct 7, 2024

View reviewed changes

leventekobor approved these changes Oct 8, 2024

View reviewed changes

CarlosCortizasCT merged commit 6bdcbe6 into main Oct 8, 2024
4 checks passed

CarlosCortizasCT deleted the cm-fec-91 branch October 8, 2024 13:43

ct-changesets bot mentioned this pull request Oct 8, 2024

Version Packages #692

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Core helpers to support the new proposed way of writing test data models #688

Update Core helpers to support the new proposed way of writing test data models #688

CarlosCortizasCT commented Oct 3, 2024

changeset-bot bot commented Oct 3, 2024 •

edited

Loading

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

emmenko Oct 4, 2024

CarlosCortizasCT Oct 4, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

CarlosCortizasCT Oct 3, 2024

emmenko left a comment

emmenko Oct 4, 2024

CarlosCortizasCT Oct 4, 2024

emmenko Oct 4, 2024

CarlosCortizasCT Oct 4, 2024

Sarah4VT left a comment


		The main change is about the `core` package where we are introducing support for writing test data models using new implementation patterns which makes the process simpler. Also, the resulting code will be more maintainable.

		You can head over [here]() for updated documentation about those new patterns.

Update Core helpers to support the new proposed way of writing test data models #688

Update Core helpers to support the new proposed way of writing test data models #688

Conversation

CarlosCortizasCT commented Oct 3, 2024

changeset-bot bot commented Oct 3, 2024 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sarah4VT left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Oct 3, 2024 •

edited

Loading