Make ban dependency optional and alternatively allow advban

Is there any plan to support advban module?
Thanks

I totally agree. Now that ban is also contrib, there should be only one ban module combining both. That would also fix the logical issues... guess it would make sense to discuss in the ban and advban module to join forces now!

Nothing good we can do about here, I think.

Okay let's do it. Anyone willing to implement a MR?

https://www.drupal.org/project/crowdsec did similar and already supports both: #3553801: Remove dependency on ban module

I guess the implementation there can be used as template!

I updated the new Ban service with check Protected IP addresses for AdvBan module

Thanks @jiskra That doesn't look bad, please see my comment.

Could we add some information to the README.md of the module to let users know one of both modules is needed?

Furthermore we should add a hook_requirements() check to ensure one of both dependencies is present?

Any updates on this since 11.3 is out and marks core Ban module as deprecated?

Fixed your two comments and added the README section, @anybody. Unfortunately, I cannot resolve the threads as I am not maintainer of this module.

I left a comment. Please test accordingly with only one of the modules present.

I have tested the ban functionality with all combinations of modules:

1. Ban: Works as expected
2. AdvBan: Works as expected
3. Both modules: Works as expected (prioritizes AdvBan over Ban)
4. No modules: Breaks the site because of the exception thrown in getBanIpManager(), which is why I have surrounded all calls to it with a try block

So, it should be in a fully working condition now!

I agree that both modules should be optional

My point is: The end-user sees the exception, not the admin... that's the problem.

Then we need a setting to opt-in, which we could set to enabled by default using an update hook, but FALSE for new installations. On hook_install() we could then detect the setting.

@anybody and I have come to a conclusion about how we want to handle this going forward. We will employ the use of hook_requirements to both prevent the module from installing if neither the Ban nor AdvBan modules are present, as well as showing a site status error when the Ban/AdvBan module is uninstalled later.

Implemented the hook and works as expected now. I will attach screenshots of the installation and status messages, respectively. The messages themselves are still a bit crudely formulated, though. If anyone has a better suggestion for the wording, go ahead!

The rest of the code (including the try blocks) is still necessary, of course, to cover the “uninstall AdvBan while keeping login security” case and not throw any errors in that situation but rather do nothing.

@LRwebks: Some final points.

Re #21 and #23: Correct. Previously ban was a hard dependency and we'll keep the requirement of a ban module. Without that it makes things a lot more complicated and it's the core functionality. Not worth further edge-cases. If anyone REALLY needs it optional, a follow-up can be made by the person. But I don't see really good reasons.

Thanks @lrwebks let's do it!

Please note that RequirementSeverity was introduced in Drupal Core 11.2 so I had to revert that part of the change. See https://www.drupal.org/node/3410939

Hi Could this be a solution for site-installs ?