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

sales.xsd #953

Closed
fooman opened this issue Jan 12, 2015 · 8 comments
Closed

sales.xsd #953

fooman opened this issue Jan 12, 2015 · 8 comments
Assignees

Comments

@fooman
Copy link
Contributor

fooman commented Jan 12, 2015

The new format for the sales.xml files moves both the instance and the sort order into the item node. Previously in Mage1 it was possible to change both the instance / sort order via an xml directive that got merged, for example:

<quote>
    <totals>
        <subtotal>
            <class>mySalesExtension/quote_address_total_subtotal</class>

The equivalent would not work in Magento2. Not sure if this currently an intended outcome to disallow this or an unintended consequence.

Subnodes are allowed in Mage2 to supply a different renderer like the Tax module currently does.

@antonkril antonkril added the CS label Jan 12, 2015
@antonkril
Copy link
Contributor

Item name is an identifier attribute, so you should be able to modify both instance type and sort order.

@antonkril antonkril self-assigned this Jan 12, 2015
@fooman
Copy link
Contributor Author

fooman commented Jan 12, 2015

Doesn't seem to work for me. If I use

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../Magento/Sales/etc/sales.xsd">
    <section name="quote">
        <group name="totals">
            <item name="test" instance="Fooman\Example\Model\Subtotal" sort_order="100"/>
        </group>
    </section>
</config>

I get

There has been an error processing your request

Class Fooman\Example\Model\Subtotal does not exist

which is the expected outcome.

However if I use if with the existing subtotal

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../Magento/Sales/etc/sales.xsd">
    <section name="quote">
        <group name="totals">
            <item name="subtotal" instance="Fooman\Example\Model\Subtotal" sort_order="100"/>
        </group>
    </section>
</config>

I would expect to see the same error message but it processes the cart normally.

It would also be great to have this consistent with the renderer (ie choose 1 approach)

@antonkril
Copy link
Contributor

This can happen because original config overrides your because your module is loaded later.

@fooman
Copy link
Contributor Author

fooman commented Jan 12, 2015

Indeed when I move it to the end of the module declaration in config.php it works.

I did have

    <sequence>
        <module name="Magento_Sales"/>
    </sequence>

so it appears that the xml file loader for sales.xml files does not get sorted by sortBySequence the same as module.xml files do.

@antonkril
Copy link
Contributor

Sales.xml reader uses standard configuration reader so it should be sorted in the same way as other files do.

app/etc/config.php is created by install script taking into account module dependencies so the list is sorted in specific way. And editing it manually to turn off/on modules might lead to problems that you encountered.

@fooman
Copy link
Contributor Author

fooman commented Feb 10, 2015

@antonkril the following statement is probably more correct: only the loading of module.xml files gets sorted by sequence see https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Module/ModuleList/Loader.php#L61 all others are sorted the same way (order in config.php).

My expectation would have been that the sequence defined in module.xml is adhered to when loading sales.xml (or any other xml) which is not the case.

app/etc/config.php is created by install script taking into account module dependencies so the list is sorted in specific way. And editing it manually to turn off/on modules might lead to problems that you encountered.

Manually editing config.php is currently the only way to extend Magento so as long as there is no alternative for it it will be used.

@antonkril
Copy link
Contributor

Sorting happens only once for module.xml, and sort order is then used for all configuration files. So sales.xml is loaded/merged in same order. See https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Module/Dir/Reader.php#L78

You can use
php -f setup/index.php <module-enable|module-disable> --modules=Magento_Foo,Magento_Bar... [--force]
to enable/disable modules

@fooman
Copy link
Contributor Author

fooman commented Feb 15, 2015

@antonkril that's great to hear that this command is now available. Hopefully this gets a bit more exposure in the docs so that everyone knows how to use this going forward.

@fooman fooman closed this as completed Feb 15, 2015
magento-team pushed a commit that referenced this issue Mar 23, 2017
magento-engcom-team added a commit that referenced this issue Sep 25, 2019
 - Merge Pull Request magento/graphql-ce#953 from pmclain/graphql-ce:issue/881
 - Merged commits:
   1. 17ea087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants