User account default setting for cancel confirm is not used

There is a user setting named notify.cancel_confirm, which should be used a default value for "Account cancellation confirmation".

I am not sure about this. Because if I set notify.cancel_confirm to true, a confirmation email will be sent, when a user (not admin - just a user with permission to cancel his account) wants to cancel his account. If the value is false, the email won't be send. Tried this a few times.

So if notify.cancel_confirm is set to false I suggest: cancel the account directly. Because no email will be send. What do you think? Have you tried to set notify.cancel_confirm to true or false and cancel a user account on your end?

I confirm pminf's problem (maybe it has to be refined),

* notify.cancel_confirm is set to false by administrator
* User cancels his account from /user/UID/edit
* redirects to /user/UID/cancel
* he has the options 'cancel account' and 'cancel'
* (The naming seems to be a bit confusing but this is not in the scope of this issue)
* He clicks 'cancel account'
* redirects to /user/UID
- the message "A confirmation request to cancel your account has been sent to your email address."
- but the notification mail is NOT send - as defined by the settings
- The account remains active - there is no posibility to realy cancel the account as the confirmation link can not be retrieved.

Thanks for confirming @mmbk and @pminf. So I suggest that we cancel the account immediately, if "notify.cancel_confirm" is set to false. Without sending a confirmation email. Also we should make this setting editable from admin backend via webbrowser. Do you agree? If so, we could start coding a patch here.

Thanks for the patch with the form edit. No idea why Drupal\Tests\dblog\Functional\DbLogTest is failing here.

I have rerolled the patch and added changes. So the user account will be immediately canceled, if the cancel notify option has been disabled.

Also I have noticed, that the cancel notify option has not been working, if you have selected it as admin. Means that you can select a notify option, if you want to cancel a user account from the page at /admin/people. I have also made this option working and tried it a few times.

Please review. If the tests are failing: please assist in tests fixing.

Now that the setting is being respected does that mean a default setting of true is being obeyed in the test?

Do the tests now need that setting asserted to make sure that drupal isn't waiting on a confirmation email to be sent.

$this->config('user.settings')->set('notify.cancel_confirm', FALSE)->save();

And we may perhaps need another failure test to check if the cancel_confirm is set to true that the user account isn't deleted automatically?

I have rerolled the patch added in 11, but added the test assertion mentioned above into the tests
testUserCancelByAdmin() and testUserWithoutEmailCancelByAdmin()

Missed a test, obviously

Extraneous dot!

Last attempt at getting the tests working before someone else will need to have a look.

Slightly different attempt this time, rather than setting the default value using:
$this->config('user.settings')->set('notify.cancel_confirm', FALSE)->save();

I'm setting the cancel form field to FALSE.

If this doesn't work, hopefully someone else can take a look.

OK, so I found some legitimate issues with the multipleCancelConfirm submit function and the issue doesn't look like it was just tests.

That being said I have updated the tests a bit as well, in case the default settings weren't set properly in the first place.

On cancel submission it is possible that if you give "Cancel own user account" to authenticated users, they could remove themselves and be redirected to 'collection', but that would be 404 for a deleted user, so I have added a redirect in there for those users too.

This will be my last attempt however.

Slightly changes for 8.9.x to avoid Hunk succeeed as tests changes.

Rerolled patch #28.

Handled one of the failures.

Fixed the test case. Remove a assertions that is not working with current scenario. Added some assertions also in DbLogTest.

We are using the #32 one, because the last one, the #35 one include code that prevents the user from deleting it's own account.

+ // Prevent the account deletion for the current logged-in user.
+ if ($this->entity->id() != $this->currentUser()->id()) {
+ user_cancel($form_state->getValues(), $this->entity->id(), $form_state->getValue('user_cancel_method'));
$form_state->setRedirectUrl($this->entity->toUrl('collection'));

Here is a ptach compatible with 10.2.7

This came up as daily target for the #bugsmash initative.

Looking at the summary appears to be missing some sections, specifically what are the steps. Recommend using the standard issue template.

Also will need some kind of test coverage if a bug.

Also all fixes should be in MRs.

I think I remember that the current tests are falsely passing (without any patch). As I recall, it was a while ago, I think I found that there were tests scenarios that were never activated and as such the tests always just passed, but I could be wrong.

I think with this new setting I tried to update the tests, to check all the scenarios but found I couldn't get to the point where they could pass.

Hopefully #32 or #35 have improved that.

That latest patch doesn't include any test changes at all, I would ignore that and can only assume it is here to get the functionality added to a site without tests.

I will try to take another look.

As this issue was raised as a follow up to #2854884: Not able to disable account cancel confirm e-mail, which looks like it was fixed in #2854252: User forms broken for admin without 'administer users'. Hopefully the original tests were fixed there too.

This issue provides a form that allows the default setting to be changed and used if it is set.

We will need to check how the changes to the original issue has affected the tests and if they need to be updated to use the default settings.

Added a new test, but need to fixup old tests too.

I think there is some work required in core/modules/user/src/Form/UserMultipleCancelConfirm.php as well.

Left comments on the MR. Seems to contain a number of out of scope changes. If they're all needed to get the tests to just pass then something is probably missing.

But new config key will need a schema + upgrade path.

Issue summary still needs some attention.

I added thread to the usercancelTest.php. There is a function that makes PHPSTAN fail as it doesnt exist.

Took another look on a new branch. Should be good now.

@mdranove You may have had a particular reason? but normally it's best to stick to one branch per issue. Anyway checking your latest changes.

Still appears to be missing default config, schema, and post update.
Update

Also with the fix being in user module that’s probably where test coverage should be.

@mdranove Just ran the test-only test (you have to run it manually). It is passing but it should fail. What that normally means is the test coverage is passing before and after the code fixes in the MR are applied. That is not happening. Changing to 'needs work'.

@mdranove I see the test-only test is now failing and only one other test is failing. Worth re-running. For #63 hopefully the test code that seems to work can be moved/ refactored into the user module.

Re: #63 There are two tests in the user module.
Re: #66 The thread has been resolved. Test coverage seems also ready for review. Back to 'Needs review.'

Also summary is still incomplete.

Also think we need to think this one through a little now that I'm looking more closely

'When enabled, the user must confirm the account cancellation via email.'),

While this does set the checkbox to TRUE the person can still uncheck it.