- Issue created by @kim.pepper
- Merge request !7069#3431203 Deprecate user_validate_name() and replace with service β (Closed) created by kim.pepper
- Status changed to Needs review
6 months ago 6:03am 18 March 2024 - π¬π§United Kingdom longwave UK
UserPasswordForm has a copy-paste of user_validate_name() that could also be replaced here:
// First, see if the input is possibly valid as a username. $name = trim($form_state->getValue('name')); $definition = BaseFieldDefinition::create('string') ->addConstraint('UserName', []); $data = $this->typedDataManager->create($definition); $data->setValue($name); $violations = $data->validate(); // Usernames have a maximum length shorter than email addresses. Only print // this error if the input is not valid as a username or email address. if ($violations->count() > 0 && !$this->emailValidator->isValid($name)) {
- Status changed to Needs work
6 months ago 2:02pm 18 March 2024 - πΊπΈUnited States smustgrave
Have not reviewed
But appears to have several test failures.
- Status changed to Needs review
6 months ago 9:45pm 18 March 2024 - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Thanks for the reviews. I feel like this could be simpler if we didn't need to use typed data as we are not validating an entity, but
\Drupal\user\Plugin\Validation\Constraint\UserNameConstraintValidator
assumes it is. π€·Also, since we are only checking the name, I think we can rename it to UserNameValidator.
- Status changed to RTBC
5 months ago 4:02pm 28 March 2024 - πΊπΈUnited States smustgrave
Searched for user_validate_name() and appears all instances have been replaced.
Based on the small number in core assume that D11 should be fine for removal vs D12.
Test coverage can be seen here https://git.drupalcode.org/issue/drupal-3431203/-/jobs/1099143
- Status changed to Needs work
5 months ago 12:11pm 1 April 2024 - π¬π§United Kingdom alexpott πͺπΊπ
I've added some review comments to address - thanks!
- Status changed to Needs review
5 months ago 2:23am 2 April 2024 - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Addressed feedback.
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Had a look for docs that say we require a
@return
annotation and couldn't find one. Only docs for describing the format of the@return
annotation. https://www.drupal.org/docs/develop/standards/php/api-documentation-and-... β - Status changed to Needs work
5 months ago 10:35pm 3 April 2024 - πΊπΈUnited States smustgrave
For the @return statement. If phpcs doesn't complain about it being included think it can't hurt.
- Status changed to Needs review
5 months ago 12:50am 5 April 2024 - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Addressed feedback.
- Status changed to RTBC
5 months ago 12:59am 5 April 2024 - Status changed to Needs work
5 months ago 8:41am 5 April 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Given where we are in the release cycle and the number of usages I think we should push the deprecation of user_validate_name out to 12. See https://git.drupalcode.org/search?group_id=2&repository_ref=&scope=blobs...
Also I think we should update the code to call the new service for consistency.
- Status changed to RTBC
5 months ago 8:46am 5 April 2024 - Status changed to Fixed
5 months ago 9:15am 5 April 2024 - π¬π§United Kingdom alexpott πͺπΊπ
-
alexpott β
committed 3194f768 on 10.3.x
Issue #3431203 by kim.pepper, alexpott, smustgrave, longwave: Deprecate...
-
alexpott β
committed 3194f768 on 10.3.x
-
alexpott β
committed 8e7d3e0f on 11.x
Issue #3431203 by kim.pepper, alexpott, smustgrave, longwave: Deprecate...
-
alexpott β
committed 8e7d3e0f on 11.x
Automatically closed - issue fixed for 2 weeks with no activity.