Skip to content

[GraphQL] Remove dynamic attributes from ProductInterface #429

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

Conversation

akaplya
Copy link
Contributor

@akaplya akaplya commented Aug 31, 2020

Problem

At the moment, Magento exposes Product Interface as an interface with the dynamic structure. As a result, the Product Interface in Magento has a state. This creates unnecessary complexity on the GraphQL tier. (it is challenging to cache, read schema, aggregate stores behind the same gateway so on).

Solution

Remove the state from the interface.

@akaplya akaplya added the GraphQL label Sep 1, 2020
}
```

## System EAV attributes that should be deprecated at the storefront
Copy link
Member

Choose a reason for hiding this comment

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

How do you decide which attributes to move to a new container? Why not move for example new_from_date, new_to_date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each eav attribute has a property is_system. If the attribute is system this is no way to remove it and very limited capabilities to manage it. that why is safe to assume that this attribute is not dynamic and can be represented as top-level fied.

* Regular product interface overloaded with fields and almost unreadable.
* It is quite often practice to create product programmatically, so the number of attributes could be really excessive with tens thousands of attributes.
* Two Magento instances do not have the same GraphQL schema. As a result, it is near impossible to build a simple client/SDK, which could be easily transferred from one instance to another.
* It is impossible to aggregate the several Magento instances behind the common [BFF](https://docs.microsoft.com/en-us/azure/architecture/patterns/backends-for-frontends).
Copy link
Member

Choose a reason for hiding this comment

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

Extensions can add new attributes on the top level too. Why do we need to hide several Magento instances behind common BFF?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for instance, if we need to add a common gateway with query cache and want to make this gateway saas solution.

* `special_price: Float` These fields should not be used a the source of the discount since the discount also could be caused by group price or rule price, which are not reflected in this field.
The same is true for `special_from_date: String` and `special_to_date`.

* `options_container: String` - this field just an exposure of `catalog_product_entity` table field, information about product options could be explicitly retrieved from the corresponding field.
Copy link
Member

Choose a reason for hiding this comment

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

Agree, makes sense to deprecate it.

@melnikovi melnikovi self-requested a review September 9, 2020 19:28
@melnikovi melnikovi merged commit 4688d4b into magento:master Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants