Problem/Motivation

When the user add a new policy and tries to configure the constraint setting, if the minimum length is already set and the user tries to set a maximum length that is lower than the minimum length it allows to save.

The same occurs when the user tries to set another minimum length constraint, and the opposite situation.

Steps to reproduce

Install the Password Character Length Policy extension;
Goes to the configuration page;
Add a new policy;
Click on "Configure Constraint Settings" to add a new password character length;
Add the constraints:
- Number of characters: 8, Opertion: minimum length
- Number of characters: 4, Opertion: minimum length
- Number of characters: 2, Opertion: maximum length

Proposed resolution

Creates a validation to verify if already exists another password character length constraint set.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 3308607-8.patch1.99 KBchaitanyadessai
#4 after-3.png21.93 KBlucassc
#4 after-2.png22.25 KBlucassc
#4 after-1.png21.44 KBlucassc
#4 before-2.png27.64 KBlucassc
#4 before-1.png52.36 KBlucassc
#2 3308607-2.patch1.97 KBsophiavs
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sophiavs created an issue. See original summary.

sophiavs’s picture

sophiavs CreditAttribution: sophiavs at CI&T commented
Assigned: sophiavs » Unassigned
Status: Active » Needs review
FileSize
3308607-2.patch1.97 KB

I created the validation when the user are trying to add a new constraint.

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
CreditAttribution: lucassc at CI&T commented
Assigned: Unassigned » lucassc

Hi!

I'll review.

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
CreditAttribution: lucassc at CI&T commented
Assigned: lucassc » Unassigned
FileSize
before-1.png52.36 KB
before-2.png27.64 KB
after-1.png21.44 KB
after-2.png22.25 KB
after-3.png21.93 KB

Good job, @sophiavs!

I applied patch in #2 for Drupal 9.4.5 and works fine. Looks really good to me, +1 for this patch.

Steps (part 1):

For the previous behavior reported in the IS, please check before-1.png.

After:
constraint 1) Number of characters = 8 and operation = minimum length:
Works as expected.

2) Number of characters = 4 and operation = minimum length:
Error message: The selected operation is already defined. (check after-1.png)

3) Number of characters = 2 and operation = maximum length:
Error message: The selected length is lower than the minimum length defined. (check after-2.png)

constraint 1 deleted.

Steps (part 2):

For the previous behavior please check before-2.png.

After:
4) Number of characters = 2 and operation = maximum length:
Works as expected.

5) Number of characters = 4 and operation = minimum length:
Error message: The selected length is higher than the maximum length defined. (check after-3.png)

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
CreditAttribution: lucassc at CI&T commented
Status: Needs review » Reviewed & tested by the community

I think it can be marked as RTBC If no one else disagrees.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital for govCMS (Australian Government Department of Finance) commented
Assigned: Unassigned » Kristen Pol

Assigning to myself as I'm reviewing/merging ready RTBC fixes/updates over the next few days.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital for govCMS (Australian Government Department of Finance) commented
Assigned: Kristen Pol » Unassigned
Status: Reviewed & tested by the community » Needs work

This is working as expected. Note that if you already had constraints added, it doesn't check those when saving. This is only relevant for new policies.

Thanks for the work on this issue. This is a nice little UX improvement.

Reviewing the code:

1. This is misspelled: $constaint

2. This shouldn't be inside the loop since it doesn't change:

+        $new_operation = $form_state->getValue('character_operation');
+        $new_length = $form_state->getValue('character_length');

3. And, since there are other changes, I think these two variable names should change:

* Change $other_operation to $constraint_operation
* Change $other_length to $constraint_length

chaitanyadessai’s picture

chaitanyadessai CreditAttribution: chaitanyadessai at Specbee commented
Status: Needs work » Needs review
FileSize
3308607-8.patch1.99 KB

Added patch please review.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital for govCMS (Australian Government Department of Finance) commented
Version: 8.x-3.x-dev » 4.0.0
Assigned: Unassigned » Kristen Pol

Assigning to myself to review, and changing this to 4.0.0.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital for govCMS (Australian Government Department of Finance) commented
Assigned: Kristen Pol » Unassigned
Status: Needs review » Fixed

Thanks everyone. I improved the messages a bit. Merged and will be in the next release.

Status: Fixed » Closed (fixed)

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

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital for govCMS (Australian Government Department of Finance) commented
Version: 4.0.0 » 4.0.x-dev

This is part of the new 4.0.1 release.