magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.55k stars 9.32k forks source link

[2.3.1] Wishlist module shouldn't have a hard dependency on Captcha module #22084

Open hostep opened 5 years ago

hostep commented 5 years ago

Preconditions (*)

  1. Magento 2.3.1
  2. PHP 7.2.15

Steps to reproduce (*)

  1. Install Magento 2.3.1 using composer
  2. Look at the file vendor/magento/module-wishlist/composer.json

Expected result (*)

  1. Not seeing magento/module-captcha in there
  2. The Wishlist module should be able to work when the captcha module isn't installed

Actual result (*)

  1. Seeing magento/module-captcha in there
  2. Not sure the wishlist module works without captcha installed, not tested yet.

Discussion

This dependency got introduced in Magento 2.3.1 in https://github.com/magento/magento2/commit/d8eacadc25cd6a98fbaf646ca39c9bb9ebb9f5de#diff-649e244f0c6d1bd88471ebeee385eb6e

We don't want this hard dependency, because we don't want to install the default Captcha module of Magento, we have our own module for that and we don't need two modules providing similar functionality.

Until now, it looks like only the sendfriend module had a hard dependency on the captcha module (this is probably incorrect as well). We don't need the sendfriend module either, so we also don't install that, which allowed us to remove the captcha module as well. But we do need the wishlist module so we can no longer remove the captcha module.

I'm not sure if this hard dependency can be removed somehow, it probably won't be that easy? Maybe the coupling between captcha & wishlist should have been done in a new module instead of adding it to the existing wishlist module?

m2-assistant[bot] commented 5 years ago

Hi @hostep. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me 2.3-develop instance - upcoming 2.3.x release

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

@hostep do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

orlangur commented 5 years ago

Totally agree on that, it could have been easily implemented as a plugin/interceptor in a separate module. Especially such unreasonable dependencies should not be introduced in patch releases. Generally amount of hard dependencies must decrease, not increase.

m2-assistant[bot] commented 5 years ago

Hi @shikhamis11. 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:

magento-engcom-team commented 5 years ago

:white_check_mark: Confirmed by @shikhamis11 Thank you for verifying the issue. Based on the provided information internal tickets MAGETWO-99067, MAGETWO-99068 were created

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

m2-assistant[bot] commented 5 years ago

Hi @geet07. 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:

m2-assistant[bot] commented 5 years ago

Hi @Surabhi-Cedcoss. 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:

m2-assistant[bot] commented 5 years ago

Hi @vishesh-webkul. 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:


m2-assistant[bot] commented 5 years ago

Hi @viniciusfabri. 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:


viniciusfabri commented 5 years ago

@magento I am working on it

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

bartoszkubicki commented 3 years ago

Is there any fix for that?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

bartoszkubicki commented 3 years ago

Still an issue

Vikaashkarthick-K commented 1 year ago

@magento I am working on this

Aditya-Prakash-Talluru commented 1 year ago

@magento give me 2.4-develop instance

Aditya-Prakash-Talluru commented 1 year ago

@magento I am working on this

Aditya-Prakash-Talluru commented 1 year ago

@hostep @viniciusfabri I tried removing the captcha module from the sequence in the Wishlist module in 2.4-develop.

image

The captcha is still getting displayed.

image
hostep commented 1 year ago

Thanks for the effort @Aditya-Prakash-Talluru, but in my opinion this is not enough. I think all references to the Captcha module should be removed from the Wishlist module and moved to new module, called Magento_WishlistCaptcha or something like that (a bit similar to Magento_PaypalCaptcha I think).

The problem is that I don't know if Adobe will allow such big changes, they might consider it backwards incompatible, but I'm not sure...

m2-assistant[bot] commented 5 months ago

Hi @engcom-Delta. 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: