Magento2: Remove dependency of Customer module on Review module

[MidnightDesign]

Preconditions

1. Magento 2.3.2 Community

Steps to reproduce (*)

1. Run bin/mage module:disable Magento_Review in a standard Magento installation

Actual result (*)

1. The module can't be disabled because Magento_Customer depends on it.

Expected result (*)

The module Magento_Review is disabled.

Is it really necessary that the Customer module depends on the Review module? It's the only reason why I can't disable the Review module and a quick search for "MagentoReview" in the Customer module's codebase reveals that only a single file references a class in Review: \Magento\Customer\Block\Adminhtml\Edit\Tab\Reviews. I'm sure this can be refactored to remove the dependency, right?

[veloraven]

@MidnightDesign thank you for your feedback.
The GitHub issue tracker is intended for technical issues only.
If you think this to be an issue please format description according to the Issue reporting guidelines: with steps to reproduce, actual result and expected result. Please, also identify which version of Magento you are running.
However it looks like more as an improvement and Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).
However if you just want to ask a question and get some discussion please refer to the Community Forums or the Magento Stack Exchange.

[MidnightDesign]

I do think this is an issue, and a technical one at that. But I will gladly post this as a feature request if you think GitHub is the wrong place for this.

[veloraven]

@MidnightDesign this issue can be left here but please format the description according to template (see the link above). It should look like this:

Steps:
1.
2.
...
Actual Result:
...
Expected Result:
...

It is easier to understand what it is about when description is structured

[MidnightDesign]

@veloraven I've updated my original post.

[mswaidan]

I'm also experiencing this issue.

[Sergeacc]

Certain modules have dependencies on other modules, in which case you might not be able to enable or disable a module because it has dependencies on other modules.
Read Magento devdocs - http://devdocs.magento.com/guides/v2.0/install-gde/install/cli/install-cli-subcommands-enable.html#instgde-cli-subcommands-enable-modules

[MidnightDesign]

@Sergeacc Did you even read my post? It's a proposal to _remove_ the dependency because it's not really necessary.

[Sergeacc]

Thank you for your submission. We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues. Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

[MidnightDesign]

I created a feature request here:
https://community.magento.com/t5/Magento-2-Feature-Requests-and/Remove-dependency-of-Customer-module-on-Review-module/idi-p/69271

[joshuaadickerson]

This is incredibly simple to fix. Just move that block to Magento_Review with the code and config. That should be all that's necessary.

This is not a feature request. This is poor design of the module. Not being able to disable a high-level feature is a bug.

[joshuaadickerson]

This is preventing me from disabling the Review module. The Review module adds HTML to the extension attributes element of the product_data_storage localStorage item. That is filling up localStorage for browsers and causing other issues. So, I’d like to just disable the module altogether since we don’t use reviews.

[hostep]

Request makes sense, let's re-open this.

There is also a mentioning of MAGETWO-71228 in the forum thread, so maybe somebody with access to Magento's Jira can leave an update here to tell us what the status is...

[m2-assistant[bot]]

Hi @engcom-Charlie. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

• [ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  Details

  If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

• [ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

• [ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• [ ] 4. Verify that the issue is reproducible on 2.3-develop branch

  Details

  - Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and _stop verification process here_!
  

• [ ] 5. Verify that the issue is reproducible on 2.2-develop branch.

  Details

  - Add the comment @magento give me 2.2-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x

• [ ] 6. Add label Issue: Confirmed once verification is complete.

• [ ] 7. Make sure that automatic system confirms that report has been added to the backlog.

[m2-assistant[bot]]

Hi @sdzhepa. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

• [ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  Details

  If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

• [ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

• [ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• [ ] 4. Verify that the issue is reproducible on 2.3-develop branch

  Details

  - Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and _stop verification process here_!
  

• [ ] 5. Verify that the issue is reproducible on 2.2-develop branch.

  Details

  - Add the comment @magento give me 2.2-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x

• [ ] 6. Add label Issue: Confirmed once verification is complete.

• [ ] 7. Make sure that automatic system confirms that report has been added to the backlog.

[joshuaadickerson]

It is reproducible in 2.2 and 2.3. The other component it affects is the Review component.

[magento-engcom-team]

@sdzhepa Thank you for verifying the issue.

Unfortunately, not enough information was provided to acknowledge ticket. Please consider adding the following:

• [ ] Update issue content to follow format required by Issue Reporting Guidelines

Once all required information is added, please add label "Issue: Confirmed" again.
Thanks!

[sdzhepa]

Hello @joshuaadickerson @hostep

Thank you for raising this issue again!

I have investigated this issue and all related tickets in the internal Jira.
Unfortunately MAGETWO-71228 this ticket still in Open stage.
I discussed it with Product Managers and Architects related to these modules and everyone agreed that it should be fixed.

By fact, it is backward incompatible changes but based on multiple feedbacks and complaints such changes have a very big chance to be approved to be delivered even in case it will be PR proposed from the community(at least we have "pre-approval", the final decision will be made based on proposed code-changes)

[magento-engcom-team]

@sdzhepa Thank you for verifying the issue.

Unfortunately, not enough information was provided to acknowledge ticket. Please consider adding the following:

• [ ] Update issue content to follow format required by Issue Reporting Guidelines

Once all required information is added, please add label "Issue: Confirmed" again.
Thanks!

[magento-engcom-team]

:white_check_mark: Confirmed by @sdzhepa
Thank you for verifying the issue. Based on the provided information internal tickets MC-18314 were created

Issue Available: @sdzhepa, _You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself._

[joshuaadickerson]

How is this backwards incompatible? It removes the Review dependency for Customer. Removing dependencies should not be backwarda incompatible. Then Review is already dependent on Customer. Adding dependencies should be a BC change but this already exists. This could be done in a patch. Considering everyone using Magento is using the Customer module and thus must be using the Review module, nobody has it disabled. This should be non-breaking.

Hmm.... unless someone is targeting that block with it being in customer? Ah, that would be breaking.

[artyomyangolenko]

@magento I am working on it

[m2-assistant[bot]]

Hi @artyomyangolenko! :wave:
Thank you for joining. Please accept team invitation :point_right: here :point_left: and self-assign the issue.

[m2-assistant[bot]]

Hi @artyomyangolenko. Thank you for working on this issue.
Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

• [ ] 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• [ ] 2. Verify that the issue is reproducible on 2.3-develop branch

  Details

  - Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and _stop verification process here_!

• [ ] 3. If the issue is not relevant or is not reproducible any more, feel free to close it.

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[drpayyne]

Closing issue since fix has already been submitted: https://github.com/magento/magento2/commit/91a4ba542cdfb99da5501468126744d23841d26d.

CC: @krzksz