Possibility to set words as reject pattern

Thanks for the work done. However, I'd love to hear we hear from other users of the module on this. Let's keep this open for now.

We should review and add tests for this in the context of ✨ Needs schema check and tests for all functionality Needs work

Conflicts need to be resolved.

Working on it

Tests are failing

Hello. I did some cleanup on this fork. Took a lot of the previous commits, but also got cli tests passing for eslint, phpcs, and took some of the phpunit tests we had and cleaned that up so that we have some basic functionality, login, and install/uninstall tests. Even though I had a fight with phpstan, I think it's in a decent spot.

I had a few site accidentally updated to the release version of this module. My clients had included "spac" as a reject pattern. (Not really sure why or what that means, but they added it) And I had a lot of forms rejected for using the word "spacing". I've found this module tremendously helpful, but it's hard for me to imagine using it without the option to check whole words.

There must be use cases for checking within strings instead of whole words, but I haven't encountered that in my experience.

This fork also logs rejections to it's own table rather than the main watchdog table. (that was added a while back) I've had way too many instances where I have a rejected form submission but it was already removed from the watchdog list.

So whole word checking and logging to a separate table are essential to my workflow. I'd love to see this become part of the primary module if possible. And I'm happy to help in any way I can. Let me know if anyone has any thoughts or problems.

Hi all, I replicated the issue and the issue persists. Upon adding any pattern in the Reject patterns field, the user is unable to submit the form due the any word containing the pattern added in the reject patterns field. I also applied the provided MR as a patch and it applied cleanly without any errors. However, after applying the patch getting below issue:

The website encountered an unexpected error. Try again later.

TypeError: Drupal\protected_forms\Form\ProtectedFormsForm::__construct(): Argument #1 ($module_handler) must be of type Drupal\Core\Extension\ModuleHandlerInterface, Drupal\Core\Config\ConfigFactory given, called in /app/web/core/lib/Drupal/Core/Form/ConfigFormBase.php on line 57 in Drupal\protected_forms\Form\ProtectedFormsForm->__construct() (line 28 of modules/custom/protected_forms/src/Form/ProtectedFormsForm.php).

Now on visiting the module configuration page getting the issue and also on submitting the form on which the protected form is enabled getting the issue shown in the screenshot. The provided MR still need works hence moving it to Needs work.

Thanks!

Looks like the MR reverts previous expected changes. Instead it should fix the issues without reverting the improvements. Especially I don't like the schema to be reverted.

Apologies. I did fix the constructor error. After another fight with phpstan, I have the changes committed with all tests passing to Drupal standards. Green checks everywhere. Though certainly more tests could be added (and welcomed).

But again. Apologies if I have taken this in the wrong direction, that was not my intention. I was only hoping to help and contribute because that is the "Drupal" thing to do.

For me, pattern matching within strings was never going to be a solution to any problem I had. For all my sites, and I presumed others, it needed to check whole word instances. However, I've felt a bit of push back on this the entire time. I did like how this module was set up overall, so it just made sense to add that feature to it while keeping your initial string check. Which is what I tried to do. I wanted to contribute.

My last merge request was an attempt to:
- maintain whole word instance checking as an option
- add cli tests for eslint, phpcs, and phpstan
- add/refine phpunit tests based on what existed, what was suggested, and what I thought was logical
- log rejections in it's own table rather than watchdog (which could perhaps be a setting) because on high traffic sites, I was often asked what was rejected only to find it was too late

Looks like the MR reverts previous expected changes. Instead it should fix the issues without reverting the improvements. Especially I don't like the schema to be reverted.

My intention was not to revert any improvements. I tried to carry over most of the previous commits. If "improvements" were reverted, I'm open to discuss.

It's been a few months, but I believe my changes to the schema were to address code quality and test issues that arose. I certainly wouldn't change the schema for a fun time. I have this MR running on many high profile, high traffic sites. I saw no issues with my changes on those sites and in my tests. Please let me know if they caused any other issues though.

I have this module in a really good place for my needs, and I realize that's "my" needs. This issue/thread began what is going on 3 years now. Perhaps I should take this version of the module I have now and make it a private custom module from this point forward. I'm honestly not trying to step on any toes or cause any issues for anyone. So again, apologies if it feels that way.

Hi @anybody @xpersonas, I've applied the latest changes in the MR as a patch and it applied cleanly without any error. On applying the patch, the issue mentioned in the comment #31 ✨ Possibility to set words as reject pattern Needs review has been resolved. However, when the module is installed and then the patch has been applied, then on visiting the module configuration page i.e., /admin/config/content/protected_forms getting the below error:

The website encountered an unexpected error. Try again later.

InvalidArgumentException: "0" is an invalid render array key. Value should be an array but got a string. in Drupal\Core\Render\Element::children() (line 97 of core/lib/Drupal/Core/Render/Element.php).

But after reinstalling the module with the applied patch then the above issue has not been encountered.

After applying the patch, a new field Pattern matching has been added with options Check whole word instances and Check within strings. The newly added field provides the option to match the added pattern to match the whole or match only the pattern within the words. Both the provided field options are working fine with the forms and blocking the form submission as per the selected option in the field. I've attached the video clip for your reference and screenshots of before and after fixes as well.

As getting the mentioned error after applying the patch, hence keeping the issue to Need works.

Thanks!

@tirupati_singh: Thank you for testing things out so thoroughly and adding a video. I see the issue now. I pushed up a fix. If you try to run a new patch with the current diff, that error should go away. At least it did for me. So let me know if you have any issue with it.

@xpersonas I think @anybody accidentally looked at a different MR. This:

Especially I don't like the schema to be reverted.

Doesn't make much sense, as you were the only one working on the current MR AND there were no schema changes in the original commit in https://git.drupalcode.org/project/protected_forms/-/commit/5180c2349098....

@grevil sadly not.
https://git.drupalcode.org/project/protected_forms/-/commit/40f8328270ff... has been reverted here and that's not what we want.

@anybody, do you not want me contributing to this module? I'm not getting the Drupal community love vibes here. Lol

Regarding that schema change. I agree. Sadly, that schema change did not make sense to me either. That's why I worked to revert that change already.

https://git.drupalcode.org/project/protected_forms/-/merge_requests/8/diffs

That change to the schema was added to the this MR at some point, but didn't really make sense to me.

https://git.drupalcode.org/project/protected_forms/-/merge_requests/8/di...

That's why I reverted it. There were quite a few changes, some that made sense, some that did not, and some that were going in a good direction. Which is why I consolidated all that into the current state of the MR.

I would ask that you review the current state of this MR vs only looking at the commit history.

If this MR stands no chance of ever being approved, that's ok too. Not a big deal. Just let me know so I can take the appropriate action with all my client sites.