A place where Magento architectural discussions happen

Problem

Given an implementation where Magento acts as a headless server, there are cases where Magento should accept redirect URLs from the client. These URLs should be validated in some way to prevent an open redirect vulnerability.

Solution

Introduce a new configuration where a list of trusted redirect domains can be specified. Implement a class top validate redirect URLs against this whitelist.

Requested Reviewers

@paliarush @joni-jones @AlexMaxHorkun

Status (1/4/19): This proposal is still in an active state, but is currently held up on some other priorities. Before this can be closed out and worked, a new library with a comparable feature set needs to be decided on.

Rendered Proposal

I'd like to hear from Partners, SIs, and anyone else working on front-end for Magento stores on a day-to-day basis. I'd imagine there are some common slider/gallery libs in use - would love to hear suggestions.

The document describes changes to module registration and management implemented in scope of "Lighter application" POC for Service Isolation project.

Clarification: the proposal does not discuss the removal of app/code folder or ability to keep project-specific code in the project folder.

Problem

CSP not implemented in Magento

Solution

Implement CSP in Magento in a backward compatible way and allow extension developers to configure it

Requested Reviewers

@joni-jones @paliarush @melnikovi

Problem

PWA-2110: [GraphQL] check_inventory parameter and redundant collectQuoteTotals() call (2.4.4) Need to port changes from PWA-2098 to 2.4.4 and add automated tests and proper doc comments for the new optional parameter.

Solution

Add optional parameter "check_inventory" to GraphQL resolvers to override "Check inventory on cart load" admin setting

Notes

There are changes in this pull request that may not seem directly related to the check_inventory parameter. These changes are to provide more complete type definitions that had not been included in the architecture repo. In several cases, entirely new schema files were also added for this reason.

Problem

The is no document that describes Magento Coding Standard versioning.

Solution

The proposal describes rules that need to be followed when making new Magento Coding Standard release.

Requested Reviewers

@buskamuza @paliarush @melnikovi

Problem

The current GraphQL customer orders API has limited functionality and is inconsistent with the Magento Luma theme.

Solution

Introduce changes in customer orders API with better customizability and extensibility while supporting the same set of data provided for Luma theme.

Requested Reviewers

@paliarush @akaplya

Requriements As a developer, I want to create a flexible login as a customer schema that can be added to the graph so a merchant or merchant admin can log into an end customer's account to assist them with their account.

Additional Changes

• Improved file structure of GraphQL-related proposals.
• Updated repository's README.md

Investigate a possibility to integrate Depenabot with Magento infrastructure.

• Check PR creation to update composer dependencies
• POC Dependabot notifications into the slack channel

This (if approved) impacts an in-flight PR: https://github.com/magento/magento2/pull/28210

These changes are based on discussions I've had with @prabhuram93 and @nrkapoor.

There were a few concerns I wanted to address:

1. We should be using the ID type for IDs, because we don't want clients using them for anything but lookups. Mentioned in a separate design document

2. The document mentioned that id_v2 was a temporary field until we start using UUIDs. If we use the ID type, though, we shouldn't need a temporary field. If a client deploys a new version of the GraphQL API (after we switch from base64 >> uuid) and a client has cache entries for the old IDs, those old entries just won't be looked up in the client-side cache anymore. No loss/corruption should happen

   I believe this matches the vision for every entity to have a globally unique ID, but with less downstream changes for clients. Please correct me if I'm wrong!

3. @nrkapoor and I discussed various names for the field. I've gone with uid in the PR, but happy to change if we can reach consensus on a different name. Here's the names we discussed:

   • uuid: Bad because UUID is a specific encoding algorithm, and a client should not care about that
   • gid: "Global Identifier"
   • uid: "Unique Identifier"
   • _id: Just kind of ugly
   • identifier: Verbose, but accurate

Note: It's worth knowing that popular GraphQL client libraries (Apollo/urql) have default cache configurations that look for the field name id or _id on Object Types. Whatever new field we choose, we should be consistent going forward, as each headless solution will now need to customize their client to use this new cache key. See https://www.apollographql.com/docs/react/caching/cache-configuration/#custom-identifiers

Problem

Whenever we add requisition list items to cart if the cart has items we have an option to replace or merge the requisition list items with the existing cart items. We don't have the option in the addRequisitionListItemToCart mutation.

Solution

Added a seperate parameter ( is_replace ) to the addRequisitionListItemToCart mutation.

Requested Reviewers

@nrkapoor @paliarush

Updated purchase order graphql schema

• Split into separate purchase-order and purchase-order-rule schemas per module
• Added created_at and updated_at fileds to PurchaseOrder
• Added addPurchaseOrderItemsToCart mutation:q

Problem

Recently, we introduced parameterized fixture for integration and api-functional test that accepts parameters directly from @magentoDataFixture annotation.

We enhanced @magentoDataFixture annotation format to support additional information that contains the parameters and the alias of a fixture. And we introduced a new annotation @magentoDataFixtureDataProvider for advanced fixture configuration.

However, with the current implementation, fixture parameters have to be passed in JSON format which is hard to read and maintain. PHP 8.0 introduced a new feature (Attributes) that we can utilize to generate fixtures instead of DocBlock annotations which will enable us to use PHP build in syntax and increase the code readability.

Example

class AddSimpleProductToCartSingleMutationTest extends GraphQlAbstract
{
    /**
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product1
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product2
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product3
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\GuestCart as:cart
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\AddProductToCart as:cartItem1
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\AddProductToCart as:cartItem2
     * @magentoDataFixtureDataProvider {"cartItem1":{"cart_id":"$cart.id$","product_id":"$product1.id$","qty":1}}
     * @magentoDataFixtureDataProvider {"cartItem2":{"cart_id":"$cart.id$","product_id":"$product2.id$","qty":1}}
     */
    public function testAddMultipleProductsToNotEmptyCart(): void
    {
        $product1 = $this->fixtures->get('product1');
        $product2 = $this->fixtures->get('product2');
        $product3 = $this->fixtures->get('product3');
        $cart = $this->fixtures->get('cart');
        //...
    }
}

Solution

PHP 8.0 introduced a new feature (Attributes) that we can utilize to generate fixtures instead of DocBlock annotations.

Implementation

Create PHP attribute equivalent for all existing DocBlock annotations used in integration and functional tests

• @magentoAppIsolation -> AppIsolation
• @magentoDbIsolation -> DbIsolation
• @magentoDataFixtureBeforeTransaction ->DataFixtureBeforeTransaction
• @magentoDataFixture -> DataFixture (1)
• @magentoApiDataFixture -> DataFixture (1)
• @magentoIndexerDimensionMode-> IndexerDimensionMode
• @magentoComponentsDir -> ComponentsDir
• @magentoAppArea -> AppArea
• @magentoCache -> Cache
• @magentoAdminConfigFixture -> Config (2)
• @magentoConfigFixture -> Config (2)

(1) Both magentoDataFixture and magentoApiDataFixture can be replaced with DataFixture attribute. Currently we use magentoDataFixture in integration tests and magentoApiDataFixture in functional api tests (WebAPI). Both executes data fixtures except that magentoApiDataFixture does not trigger db transaction (db isolation) by default (without explicitly enabling db isolation with @magentoDbIsolation) whereas magentoDataFixture does.

All it takes to make this work is to replace \Magento\TestFramework\Annotation\DataFixture with \Magento\TestFramework\Annotation\ApiDataFixture in WebapiDocBlock and override \Magento\TestFramework\Annotation\AbstractDataFixture::getDbIsolationState in \Magento\TestFramework\Annotation\ApiDataFixture (ApiDataFixture extends DataFixture which also extends AbstractDataFixture) to return [disabled] instead of NULL in case the db isolation is not explicitly defined. This will prevent the transaction from auto starting in \Magento\TestFramework\Annotation\DataFixture::startTestTransactionRequest unless explicitly enabled with @magentoDbIsolation.

(2) Both magentoAdminConfigFixture and magentoConfigFixture can be replaced with Config attribute.

Example:

namespace Magento\TestFramework\Fixture;

use Attribute;

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
class DataFixture
{
    /**
     * @param string $type
     * @param array $data
     * @param string|null $as
     */
    public function __construct(
        public string $type,
        public array $data = [],
        public ?string $as = null
    ) {
    }
}

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
class Config
{
    /**
     * @param string $path
     * @param mixed $value
     * @param string $scopeType
     * @param string|null $scopeValue
     */
    public function __construct(
        public string $path,
        public mixed $value,
        public string $scopeType = ScopeConfigInterface::SCOPE_TYPE_DEFAULT,
        public ?string $scopeValue = null
    ) {
    }
}

class AddSimpleProductToCartSingleMutationTest extends GraphQlAbstract
{
    #[
        Config(Configuration::XML_PATH_SHOW_OUT_OF_STOCK, 1, ScopeInterface::SCOPE_STORE, 'default'),
        DataFixture(ProductFixture::class, as: 'product1'),
        DataFixture(ProductFixture::class, as: 'product2'),
        DataFixture(ProductFixture::class, as: 'product3'),
        DataFixture(GuestCartFixture::class, as: 'cart'),
        DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$product1.id$']),
        DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$product2.id$']),
    ]
    public function testAddMultipleProductsToNotEmptyCart(): void
    {
        $product1 = $this->fixtures->get('product1');
        $product2 = $this->fixtures->get('product2');
        $product3 = $this->fixtures->get('product3');
        $cart = $this->fixtures->get('cart');
        //...
    }
}

Pros

• Best time to make such changes as Parameterized Fixture is not released yet
• Fixture classes can be imported and referenced using constant (Fixture::class)
• PHP Native syntax
• Easy to maintain and extend

Cons

• Require PHP >= 8.0 to run tests

Thumbs up 👍 if you like this proposal

Problem

we have various integration so need different module to add functionality and we have some functionality inbuilt so Need to create single module to add all possible integration domain wise like Reviews, Product Management , Carriers, Payment, Order, etc/

Example: Customer need some third party review services from external source like google or trustpilot so need custom module to integrate & API Adapter to compatible with existing review system in-built.

• Existing system doesn't support for DDD by use-case. As every customer have different use-case for same features/functionality

Solution

Main focus is to change module structure to ports architecture so module is more testable by use-case & domain oriented so we can use single module to add all possible integration for payment OR shipping in plug-n-play manner.

• testable by use-case( can be test like unit test )
• DDD (domain driven development)
• Adapter test
• Loosely couple

Requested Reviewers

Problem

ACP2E-267: GraphQl - Issues with add Configurable product to Wishlist

[2022-01-27T01:13:07.037742+00:00] report.ERROR: Call to a member function getProduct() on null

GraphQL (20:13)
19:           ... on ConfigurableWishlistItem  {
20:             child_sku
                ^
21:             customizable_options {

Solution

Deprecate ConfigurableWishlistItem.child_sku and add a new field ConfigurableWishlistItem.configured_variant that returns child product details (ProductInterface) or NULL if the child product is not configured

Requested Reviewers

@cpartica

I have developed monolith for 10 years, the architecture becomes very hard to understand, no matther how many effort we put into the arhitecture design, I am wondering when and whether Magento will be microservice based? I see a picture Architectural diagrams, it looks like the magento is alreay microservices, apparently it is not. I am very loooking forward to this architecture, so I asked a question here.