Skip to content
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

chore: added mocked adapters #25

Open
wants to merge 10 commits into
base: development
Choose a base branch
from
Open

Conversation

tolerants
Copy link
Contributor

No description provided.

@tolerants tolerants requested a review from a team as a code owner June 10, 2024 15:02
Alexander Kovalenko added 3 commits June 11, 2024 18:35
# Conflicts:
#	apps/storefront/dependencies.sh
#	apps/storybook/dependencies.sh
@@ -0,0 +1,18 @@
// import {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why commented?

export * from './default-checkout.adapter';
export * from './normalizers';
export * from './serializers';
export * from './spryker-glue';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Spryker Glue is the default here? Shouldn't that be the mocks?

@@ -0,0 +1,308 @@
import { PlaceOrderData } from '@spryker-oryx/checkout';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment at the top on how to use these files.
Smth like "to be replaced with a backend system API"

@@ -1,2 +1,2 @@
export * from './default-product-relations-list.adapter';
export * from './glue-product-relations-list.adapter';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should that be spryker-glue?

DefaultSuggestionRendererService,
FacetColorsMapping,
FacetListService,
productSuggestionRenderer,
GlueSuggestionAdapter,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these plugins be SprykerGlue...?

@@ -3,7 +3,7 @@ import { createInjector, destroyInjector, getInjector } from '@spryker-oryx/di';
import { MessageType, postMessage } from '@spryker-oryx/experience';
import { RouteType } from '@spryker-oryx/router';
import { of } from 'rxjs';
import { SuggestionField } from '../adapter';
import { SuggestionField } from '../adapter/spryker-glue';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to define the path for all of these imparts? all at once?
Typically you will have one set of adapters used everywhere. Cases where I need to specify a separate import directory when multiple vendors involved are limited, and then it can be a little more complex to do. But per default it should be a one-time switcher.

export const StoreNormalizer = 'oryx.StoreNormalizer*';

export function storeAttributesNormalizer(data: DeserializedStores): Store[] {
// TODO: drop this when the backend is healthy again; current dynamic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An issue please.

Copy link
Contributor

@PhilinTv PhilinTv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of work, thanks @tolerants ! 👏

We need to sill settle on the naming and keep it consistent.
Also we need to check if we can make a step forward to new developers, avoiding per component mock/custom definition (of possible). Additionally please merge master and fix CI.

@PhilinTv PhilinTv added enhancement New feature or request Minor change labels Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Minor change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants