- Issue created by @omkar.podey
- ๐ฎ๐ณIndia omkar.podey
register
key could have 3 values according to\Drupal\user\UserInterface
cancel_method
can have any function assigned to it but i think it can't be empty.password_reset_timeout
It is never being set just being read so I think it should just not be null.
- ๐ฎ๐ณIndia omkar.podey
cancel_method
has to be nullable according totestConfigForm
- Status changed to Needs review
about 2 months ago 10:30am 28 March 2024 - Status changed to Needs work
about 2 months ago 12:30pm 29 March 2024 - ๐ฎ๐ณIndia omkar.podey
The migration tests and
UserMailNotifyTest
isn't happy with making all the keys required because of marking user.settings asfullyValidatable
.I could add keys toUserMailNotifyTest
and remove therequiredkey: false
from most of them, doing that next. - ๐ง๐ชBelgium borisson_ Mechelen, ๐ง๐ช
I could add keys to UserMailNotifyTest and remove the requiredkey: false from most of them
I agree, this looks like the best way
- ๐ฎ๐ณIndia omkar.podey
omkar.podey โ changed the visibility of the branch 3436164-add-validation-constraints to hidden.
- ๐ฎ๐ณIndia omkar.podey
Okay so I switched to 10.3.x. and right now the only test that's failing is
Drupal\Tests\user\Kernel\UserAdminSettingsFormTest::testConfigForm
To fix this there were two options, either add config target data to
core/modules/user/src/AccountSettingsForm.php
or mark the keys that are failing the test asrequiredKey: false
inuser.schema.yml
.So I thought that the requiredKey false would be the solution but then I ran into the langcode issue again where the langcode isn't defined in
user.schema.yml
so I can't mark itrequiredKey: false
.thoughts ?
- Status changed to Needs review
about 1 month ago 1:58pm 15 April 2024 - Status changed to Needs work
about 1 month ago 2:27pm 15 April 2024 The Needs Review Queue Bot โ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- Status changed to Needs review
about 1 month ago 5:45am 16 April 2024 - Status changed to Needs work
about 1 month ago 6:43am 16 April 2024 - Status changed to Needs review
about 1 month ago 8:57am 16 April 2024 - Status changed to Needs work
about 1 month ago 3:10pm 16 April 2024 - Status changed to Needs review
about 1 month ago 8:05am 18 April 2024 - Status changed to Needs work
about 1 month ago 9:09am 18 April 2024 - Status changed to Needs review
30 days ago 11:07am 18 April 2024 - Status changed to Needs work
30 days ago 1:47pm 18 April 2024 - Status changed to Needs review
26 days ago 7:29am 22 April 2024 - Status changed to Needs work
24 days ago 3:12pm 24 April 2024 - ๐บ๐ธUnited States smustgrave
This is looking really good! Can the MR be updated for 11.x though please.
- Status changed to Needs review
19 days ago 9:30am 29 April 2024 - ๐ฎ๐ณIndia omkar.podey
I would like to get it reviewed first and then we can decide if we need the 11.x branch explicitly.
- Status changed to Needs work
19 days ago 9:44am 29 April 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
I would like to get it reviewed first and then we can decide if we need the 11.x branch explicitly.
All changes land in
11.x
first. So we need an MR that targets11.x
anyway.Still, reviewed it for you โฆ and what I wrote in #24 is still true.
If this were already targeting
11.x
, I would and could've RTBC'd ๐ - ๐ฎ๐ณIndia pradhumanjainOSL
pradhumanjain2311 โ made their first commit to this issueโs fork.
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
The addition of the
UserCancelMethod
constraint seems fine, but the API addition inuser_cancel_methods()
is somewhat questionable. Then again, I don't see any better choice available to us โฆ this is just super old code that has never been converted to more modern Drupal standards.#3135592: Cannot implement a custom user cancellation method โ and โจ Provide a Entity Handler for user cancelation Needs work are the issues to modernize that part of Drupal core. So until either of those land, this is doing the best we can. We don't need to make this the most elegant thing possible, because it isn't already anyway, and
user_cancel_methods()
will disappear when those are finished.Ah, no, the
11.x
branch has two remaining failures:Drupal\Tests\user\Kernel\UserEntityLabelTest::testLabelCallback Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for user.settings with the following errors: 0 [] 'verify_mail' is a required key., 1 [] 'notify' is a required key., 2 [] 'register' is a required key., 3 [] 'cancel_method' is a required key., 4 [] 'password_reset_timeout' is a required key., 5 [] 'password_strength' is a required key.
and
Drupal\Tests\user\Kernel\UserAdminSettingsFormTest::testConfigForm Input values: <em class="placeholder">Array ( [anonymous] => 3rj{f>&3y@ [user_mail_cancel_confirm_body] => {;D5>&vM [user_mail_cancel_confirm_subject] => @R)"o=`EfB>&T)#!\)V< [register_pending_approval_admin_body] => 6e*n>&Ob [register_pending_approval_admin_subject] => %`6JwPffFF>&9WqUZ'_4 [user_mail_password_reset_subject] => C>)w>&q: [user_mail_register_admin_created_subject] => \o'A>&nH [user_mail_register_no_approval_required_subject] => APXk>&O' [user_mail_register_pending_approval_subject] => My1A>&8{ [user_mail_status_activated_subject] => ixV4>&^{ [user_mail_status_blocked_subject] => DmL!>&,z [user_mail_status_canceled_subject] => 9K1'>&=V ) </em><br/>Validation handler errors: <em class="placeholder">'register_pending_approval' is a required key. This value should not be null. This value should not be null.</em> Failed asserting that false is true.
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Wim Leers โ changed the visibility of the branch 3436164-add-validation-constraints-user-settings to hidden.
- Status changed to Needs review
18 days ago 8:53am 30 April 2024 - Status changed to RTBC
18 days ago 9:25am 30 April 2024 - Status changed to Needs work
18 days ago 9:52am 30 April 2024 - Status changed to Needs review
15 days ago 10:13am 3 May 2024 - Status changed to RTBC
15 days ago 1:41pm 3 May 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
I think this is what @alexpott was asking for โ until #3135592: Cannot implement a custom user cancellation method โ and/or โจ Provide a Entity Handler for user cancelation Needs work happen, a truly elegant approach won't be possible anyway ๐
- Status changed to Needs work
14 days ago 12:23pm 4 May 2024 - ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
Added some comments to the MR. This needs to go in 11.x first.
- Status changed to Needs review
12 days ago 9:19am 6 May 2024 - Status changed to RTBC
10 days ago 3:26pm 8 May 2024