Skip to content

Conversation

@Gawuww
Copy link
Collaborator

@Gawuww Gawuww commented Dec 16, 2025

MjHead and others added 2 commits December 2, 2025 14:55
ADD: Info that password comparison works correctly with DEFAULT.PROCESS event
FIX: Extract user_id directly from ID property value
@github-actions
Copy link

🤖 AI PR Review

Risk level: low

Review

The change in user-compare-password-property.php primarily adjusts how the user ID is retrieved via the modifier; replacing calls to get_value() with get() and then accessing the value attribute is consistent with expected data encapsulation improvements. This is a good practice if it aligns with the Modifier API, improving clarity and reducing ambiguity.

The addition of the get_help() method to provide guidance on usage of the password comparison is a useful enhancement for developers.

I verified that the user retrieval via get_user_by() remains the same, and appropriate exception throwing remains in place for invalid user data.

Minor point: the false === $id && false === $user_id check might not be necessary if $user_id is always assigned directly from $id->value now. Consider verifying that $id is always an object or properly handled when not present.

No security concerns identified since this relates to user ID fetching and the exception flow remains.

No performance issues or backward compatibility breaks observed.

No unit or integration tests have been added or pointed out in the diff. Since this impacts user ID retrieval logic and password comparison, consider adding or updating tests to cover these changes to prevent regressions.

Overall, this is a minor, low-risk improvement aligning with the JetFormBuilder coding standards and architecture guidelines.

Suggested changelog entry

- IMPROVE: update user ID retrieval method and add helper text in user-compare-password property

@Gawuww Gawuww merged commit 06c2649 into release/3.5.6 Dec 17, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants