Fix 24666 disabled modules still included less files

[mrtuvn]

Description (*)

This PR bring the patch for fix issue modules disabled still include less files. It's should not included in output final style files. Previously styles still included in output final css styles

Use case 1: Disable module example Company_CustomCatalog in app/etc/config.php. But assume accidental add module styles in theme folder in
app/design/frontend/theme/theme_name/Company_CustomCatalog/web/css/source/_module.less
Actually after run deploy output css magento still included Company_CustomCatalog styles in file styles-m.css in pub folder

Related Pull Requests

Keep continue works by @Echron after long inactivity in PR 25183

Fixed Issues (if relevant)

1. Fixes Static content is deploying for disabled modules #24666 : Static content is deploying for disabled modules

Manual testing scenarios (*)

1. Magento 2.3.4+
2. blank or luma theme
3. Disable a couple module use less styles: Ex: Magento_Review, Magento_LoginAsCustomerFrontendUi
4. Run deploy static content and clear cache

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @mrtuvn. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[hostep]

@mrtuvn: thanks for picking this up!
Have you also tried this much simpler solution already: #25183 (comment) ?

It was determined to be a better solution during the contribution day at Magento Live Europe last year. The original pull request was determined to not being the best possible solution. And the core Magento developers who were there said it would not be accepted.

Sorry for not having picked this up myself yet, I simply don't have adequate time at the moment to start figuring out how to make extra tests for that simpler solution.

[mrtuvn]

sure i will take a try on that approach

[hostep]

The failing integration test looks like a problem which I fixed before in scope of this issue you are trying to fix, but seems like part of that fix got reverted again by MC-31752 which is slightly annoying. I'll take a quick look to see if we can fix that again ...

[hostep]

Ok done, PR #27892 fixes the failing integration tests in this PR.

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7466 has been created to process this Pull Request
✳️ @ihor-sviziev, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[ihor-sviziev]

Test failures looks not related to changes from this PR

[ihor-sviziev]

Hi @mrtuvn,
I just double checked and found that you changed is named "../Decorator/ModuleOutput.php", so it should filter by module output, not by enabled module or not.

I think we can't use accept it as is, instead we need to create similar class in the same namespace, but name it as "../Decorator/ModuleEnabled.php" and use it as decorator in di.xml.

Could you update your PR? Sorry for confusing you.

[mrtuvn]

Added Decorator Filter modules Enabled with test coverage too

[lenaorobei]

Hi @mrtuvn! Thanks for this great improvement. I'm adding 2.5 milestone since this change may affect customers who are using styles from disabled modules. That's definitely not recommended way of doing things, but potentially can cause some issues.

I also added High risk evaluation since it is a framework change.

Anyway, thanks again! We look forward to delivering this improvement ASAP.

[ihor-sviziev]

Hi @mrtuvn
As approval process might take some time - I think you could rebase your changes when someone request it :)

[mrtuvn]

Dunno why this PR don't have higher priority, Obviously it's issue still there. Milestone 2.5 seem too lates

[ihor-sviziev]

@mrtuvn unfortunately some themes might depends on styles from disabled modules and doing such big change in patch release will be too much. That's why this PR will be targeted to 2.5

[mrtuvn]

@magento run Database Compare, Functional Tests B2B, Unit Tests

[mrtuvn]

@magento run Functional Tests B2B
Should i move this to 2.5-develop branch ?

[ihor-sviziev]

@mrtuvn yes, please re-target your PR to 2.5-develop branch

[mrtuvn]

Done! I have created new one here #31593

[ihor-sviziev]

@mrtuvn no, I meant that you could just change the target branch for this PR, not creating a new one. But anyway, we can close this one if you wish

[mrtuvn]

Oh i did know that ! Btw let close this

[m2-assistant]

Hi @mrtuvn, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[hostep]

Hi @mrtuvn: could you add a link to this PR in your new PR? So people can find the entire discussion which happened here? Thanks! :)