Problem/Motivation

This issue is created as part of Part of meta issue #2072251: [meta] Modernize forms to use FormBase
Above issue was part of refactoring forms to use the FormBase class.

Now that #2059245: Add a FormBase class containing useful methods is also in in, we should look at old forms that have empty validateForm() methods, or still use Drupal::service() or t().
Fix any discrepancies if there are any.

The title looks good as it is similar to all the other issues created in meta issue #2072251: [meta] Modernize forms to use FormBase

Proposed resolution

Convert existing FormInterface forms to extend FormBase
Remove un-necessary validate functions.
Fix phpstan issues if any.

Quick search yields only a few test classes.

Remaining tasks

Needs review

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#5 modernize_file_module-2077977-5.patch4.54 KBnielsonm
#4 drupal8-file-form-2077977-4.patch4.04 KBpushpinderchauhan
#1 file_form-2077977-1.patch4.11 KBplopesc

Issue fork drupal-2077977

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new file_form-2077977-1.patch4.11 KB

First round...

tim.plunkett queued 1: file_form-2077977-1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: file_form-2077977-1.patch, failed testing.

pushpinderchauhan’s picture

pushpinderchauhan commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new drupal8-file-form-2077977-4.patch4.04 KB

Unable to apply above patch. Please review updated patch.

nielsonm’s picture

nielsonm commented
StatusFileSize
new modernize_file_module-2077977-5.patch4.54 KB

Rerolled patch

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.0.x-dev » 8.2.x-dev
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work

Needs a reroll.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mohit_aghera made their first commit to this issue’s fork.

mohit_aghera opened merge request !12239

mohit_aghera’s picture

mohit_aghera commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs title update

Can we update the title about what's being accomplished here.

mohit_aghera’s picture

mohit_aghera commented
Issue summary: View changes
Status: Needs work » Needs review
mohit_aghera’s picture

mohit_aghera commented
Issue tags: -Needs title update
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Thanks but think the actual title needs to updated.

looking in the file module there appear to be other test forms if we are just adding typehints?

mohit_aghera’s picture

mohit_aghera commented
Title: Modernize file.module forms » Use FormBase class for the test module's form in file.module
Status: Needs work » Needs review
Issue tags: +Bug Smash Initiative

I've updated the title.
I did a search and didn't found additional instances except one kernel test which we can't update

grep -r 'implements FormInterface' core/modules/file/tests

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Should be fine then

xjm’s picture

xjm
she/her
Primary language English
commented

Great cleanup! Saving credits.

There is something strange going on with the issue summary. I usually use the link to view the original issue summary to see if the original issue report was helpful, but it's missing. The original revision is also missing from the revision log and it appears the first revision is from @plopesc.

However, I'm going to err on the side of assuming the best and grant credit. (If it turns out to be a mistake we can fix it later.)

It appears that this will be @alexanansi's first issue credit in the crediting system. So, I did some git archaeology:

[ayrton:drupal | Sat 07:11:08] $ git log --grep="alexanansi"
commit 0cf2996a30bf52025a0b06e395dbcb87cb56cb67
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Tue Jul 15 10:22:56 2014 +0100

    Issue #2078015 by er.pushpinderrana, RoSk0 | alexanansi: Modernize views_test_data.module forms.

commit 683223968a44fc83e94cd406c81805b4f627c89a
Author: webchick <webchick@24967.no-reply.drupal.org>
Date:   Fri Mar 21 10:36:33 2014 -0700

    Issue #2078001 by RoSk0, JayeshSolanki | alexanansi: Modernize responsive_image.module forms.

Looks like @alexanansi helped with these form cleanups back during D8 development, and this poor issue just happened to slip the net into the 2020s. :) Thanks @alexanansi, and thanks @mohit_aghera for reviving it!

xjm’s picture

xjm
she/her
Primary language English
commented

Oops, missed one.

  • xjm committed 957b56a6 on 11.x 
    Issue #2077977 by mohit_aghera, plopesc, er.pushpinderrana, nielsonm,...

  • xjm committed 43d31f84 on 11.2.x 
    Issue #2077977 by mohit_aghera, plopesc, er.pushpinderrana, nielsonm,...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed to 11.x and 11.2.x, running PHPStan locally both times. I was tempted to backport it to 10.6.x and 10.5.x as a test cleanup, but maybe we should not be making baseline changes on the maintenance minor anymore. So marking fixed for now.

Thanks everyone!

xjm closed merge request !12239

fjgarlin’s picture

fjgarlin commented

Disregard this comment. I’m trying to help debug an issue with credits.

xjm’s picture

xjm
she/her
Primary language English
commented

Trying again to save @alexanansi's credit, this time in a different browser.

xjm’s picture

xjm
she/her
Primary language English
commented

Now testing with JS disabled.

xjm credited alexanansi.

xjm’s picture

xjm
she/her
Primary language English
commented

This time manually entering alexanansi in the "Credit others" field.

xjm’s picture

xjm
she/her
Primary language English
commented

Crediting @fjgarlin for helping me debug crediting. :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.