ConfirmFormInterface::getFormName() serves no purpose in ConfirmFormBase

Code archaeology seems to show this is cruft from this commit 19 years ago, and how confirm forms originally worked: https://git.drupalcode.org/project/drupal/-/blob/198ec98f756673da8c899bb0236a91808ed010ec/includes/theme.inc#L898

Working on it.

Looks like a good start.

We also need to remove the condition check from UserMultipleCancelConfirm.

And I don't know what BC handling we need.

ConfirmFormInterface is what I call an inward-facing interface: 3rd parties are meant to implement it, not call it. So I don't think we need to deprecate getFormName() before removing it. But we might want to notify implementors that they can remove getFormName() from their class.

Hi @joachim,
Yes, I think we can remove the if ($form_state->getValue('confirm')) condition from UserMultipleCancelConfirm::submitForm() as the confirm field is no longer relevant. Since ConfirmFormInterface is inward-facing, I agree that getFormName() can be removed without deprecation.

I've done the changes pls check.

Thanks
Arun

Removing the if() in core/modules/user/src/Form/UserMultipleCancelConfirm.php means that the indentation needs to be changed too!

Corrected the indentation.

Nearly there -- needs a few coding style fixes.

@arunsahijpal It may be helpful for you to make sure the MR build passes before setting to Needs Review. The build for the latest changes still fails.

See https://git.drupalcode.org/project/drupal/-/merge_requests/10785. You can review the build job log https://git.drupalcode.org/issue/drupal-3497021/-/jobs/3926956 to see why it is failing.

Looks like you've run it through a code formatter that's not configured for Drupal, as it's put all the { on new lines.

I've fixed the indentation issue but now a phpunit functional is failing
I think it is due to this Line of code as it should be this if ($form_state->getValue('confirm')) { because Form submissions in Drupal are processed through $form_state.

Should i change it?

When making the change, you'd need to make sure the functionality is not affected. If it isn't, then the test needs to be updated as well. There are two test failures, so you'd also need to make sure that the necessary changes can be made and are applied for both.

@joachim, IDK why it is not finding ''confirm" key in SearchConfigSettingsFormTest could you pls check it, and the second SearchMultilingualEntityTest error is not due to the change made in getFormName() function.

Yup, looks like we need to remove

> if ($form['confirm']

in ReindexConfirm.

Which is really weird code anyway -- that's ALWAYS going to be in the form array!

niharika.s → made their first commit to this issue’s fork.

@niharika.s You seem to have made a new branch and a new MR, neither of which are necessary. New branches are only needed when there's a completely different approach to fixing an issue.

If you want to work on this issue, you can make a commit to the existing branch.

Your change isn't quite right though, as it also needs the indentation fixing.

@joachim,
I've updated the ReindexForm and the pipeline is also green now, pls check.

There's stray whitespace being added in core/modules/search/tests/src/Functional/SearchMultilingualEntityTest.php.

Other than that, it's RTBC.

Removed the whitespace.

I'm not sure you've removed the right line -- the diff in the MR for core/modules/search/tests/src/Functional/SearchMultilingualEntityTest.php now shows one removed and one added.

It's a good idea to do a `git diff 11.x` locally before you commit and push.

@joachim , could you pls check now.

joachim → changed the visibility of the branch 3497021- to hidden.

Looks good. Thanks!