Problem/Motivation

Currently, only a few tests exist, which don't cover all the complex functionality of this module.

To be sure, everything works as expected, especially

  • allowed patterns
  • reject patterns

with combinations, single words, phrases etc.

Furthermore install and uninstall without errors should be checked and we should ensure the schema is correct, e.g. using config_inspector.

This is something the community (us!) can do, if the maintainer doesn't have the time.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork protected_forms-3340780

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
altagrade’s picture

altagrade commented
Status: Active » Needs work

Unfortunately, no time for writing tests. Mind becoming a co-maintainer?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank you, @AltaGrade, well if you'd like to, we can help. In that case also please add @Grevil as Co-Maintainer and I'll add these task to our companies TODO-list. Nice and helpful module, thanks :)

altagrade’s picture

altagrade commented

Thanks for agreeing to co-maintain this module. I've just given you all maintainer privileges, including the "Administer maintainers", so please go ahead and add @Grevil or whomever you deem necessary to add.

We have developed and do keep maintained this module mainly for our Backdrop customer project. Because of Backdrop and Drupal 7 similarities the changes made on Backdrop are easy to quickly pass to Drupal 7.

However, unfortunately Drupal 9/10 version is many features behind. So we do hope you can move all the new features from either Backdrop or Drupal 7 to Drupal 9/10.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank you, I just created a separate issue for that: #3341697: Implement Backdrop / Drupal 7 features for 2.x

I don't think we'll have the time short-term, but I'll add this to our list.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Needs tests for all functionality » Needs schema check and tests for all functionality
Priority: Normal » Major
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

@Grevil will have a look when we have the time! :)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Needs schema check and tests for all functionality » Needs tests for all functionality
Related issues: +#3436411: Fix schema and add generic functional test

Moved schema fixes to #3436411: Fix schema and add generic functional test

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: grevil » Unassigned
Priority: Major » Normal

https://www.drupal.org/project/crowdsec is an alternative now.

lrwebks’s picture

lrwebks commented
Assigned: Unassigned » lrwebks

What tests are left to do after the merge of #3348032: Colon identified as a spam character:

  • Language Script validation
  • Reject message changes
  • Logging rejected submissions

Regarding the other things mentioned in the issue summary: Install / uninstall tests are already covered by ProtectedFormsGenericTest.php, but I will take a look at the schema manually via Config Inspector.

lrwebks opened merge request !29

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: lrwebks » grevil
Status: Needs work » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Category: Feature request » Task
lrwebks’s picture

lrwebks commented

All three tests are added now, however the last one isn't fully working yet since I struggle to find the correct permission name for a user to access the DBLog page. @anybody are you in the know?

lrwebks’s picture

lrwebks commented
Status: Needs review » Needs work
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

It's simply not a good idea to give anonymous users all these permissions. Would be better to use an authenticated user.
But let's stop here, enough time went in. I think this is a good one for @Grevil to review and teach you a bit in testing, if he finds possible improvements.

Nice work :)

lrwebks’s picture

lrwebks commented
Assigned: grevil » Unassigned
Status: Needs work » Needs review

Fixed the test, everything should be working now.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil
grevil’s picture

grevil commented
Assigned: grevil » lrwebks
Status: Needs review » Needs work

Added a few comments. I understand that much of the code I commented was simply copied and pasted from the existing test, please also adjust it there. Furthermore, many of my critics given also apply to the other tests.

The core of the tests are great! Well done! :)

lrwebks’s picture

lrwebks commented
Status: Needs work » Needs review

The tests are alright now! I have added “whitelisted” to the CSpell project words, since it is part of a reject pattern and also used a lot in the tests (I think we don't need to open a separate issue just for that and the pipeline is entirely green now).

lrwebks’s picture

lrwebks commented
Status: Needs review » Needs work

Need to remove "whitelisted" from project words again since apparently Drupal Core flags it directly via cspell...

lrwebks’s picture

lrwebks commented
Status: Needs work » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: lrwebks » Unassigned
Status: Needs review » Reviewed & tested by the community

Nice work @lrwebks thank you!

  • anybody committed 3bb01d0e on 2.0.x authored by lrwebks 
    Issue #3340780 by lrwebks, anybody, grevil: Needs tests for all...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.