Remove deprecated code in File module

Created on 22 March 2024, 3 months ago
Updated 13 May 2024, about 1 month ago

Problem/Motivation

Cleanup File core module from deprecated functions

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ“Œ Task
Status

Fixed

Version

11.0 ๐Ÿ”ฅ

Component
File moduleย  โ†’

Last updated about 7 hours ago

Created by

๐Ÿ‡ณ๐Ÿ‡จNew Caledonia satane

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @satane
  • Pipeline finished with Failed
    3 months ago
    Total: 204s
    #125789
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡ณ๐Ÿ‡จNew Caledonia satane

    Cleanup all deprecated calls, except those in *Test.php files.

    Not sure if classes related to "Tests legacy deprecated functions in file.module." should just be totally deleted or left there?

    • FileSaveUploadTest.php
    • FileUploadHandlerTest.php
    • LegacyFileModuleTest.php
    • LegacyFileThemeTest.php
    • LegacyValidateTest.php
    • LegacyValidatorTest.php
  • Pipeline finished with Failed
    3 months ago
    Total: 178s
    #125798
  • Pipeline finished with Failed
    3 months ago
    Total: 205s
    #125801
  • Pipeline finished with Failed
    3 months ago
    Total: 490s
    #125809
  • Pipeline finished with Failed
    3 months ago
    Total: 620s
    #125817
  • Pipeline finished with Failed
    3 months ago
    Total: 191s
    #125825
  • Pipeline finished with Failed
    3 months ago
    Total: 581s
    #125827
  • ๐Ÿ‡ณ๐Ÿ‡จNew Caledonia satane

    Merging pipeline made obvious we had to clean Test classes and even entirely delete Legacy ones, so there.

    That said, there are still some unit testing errors which I can't figure out entirely.

    There's still a call to a function isValid() which I removed as it was marked as deprecated.
    See core/modules/file/src/Upload/FileUploadHandler.php:195

    But now unit testing shows plenty of errors.
    I'm stopping here for now, feel free to jump in.

  • Status changed to Needs work 3 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Possible the deprecation was tested and those tests would need to be removed too.

    But if removing this function breaks things we may need to re-look at if some replacements were missed when it was first deprecated

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    kim.pepper โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    3 months ago
    Total: 186s
    #129138
  • Pipeline finished with Canceled
    3 months ago
    Total: 6s
    #129139
  • Pipeline finished with Failed
    3 months ago
    Total: 179s
    #129140
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    I've cleaned up some the left over deprecation code. However, I think I've hit a bug with the Validatable config job.

    Unable to install modules: module 'help_topics' is obsolete.

  • Pipeline finished with Failed
    3 months ago
    Total: 9355s
    #129148
  • First commit to issue fork.
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    Added fix to see if test will pass, probably it needs separate issue (follow-up) after ๐Ÿ“Œ Remove deprecated help_topics module Needs review

    but still not clear why nobody reported this earlier

  • Pipeline finished with Failed
    3 months ago
    Total: 1735s
    #129216
  • Pipeline finished with Success
    3 months ago
    Total: 500s
    #129230
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    #9 created a separate issue ๐Ÿ› Validatable config should skip obsolete modules Needs review

  • Pipeline finished with Success
    3 months ago
    Total: 652s
    #131102
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia
  • Status changed to RTBC 3 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Removal appears complete.

    Applied the MR and searched core/module/file folder for deprecated and 11.0 and all instances have been removed.

  • Status changed to Needs work 3 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom longwave UK

    Thanks for working on this - looking good, just a couple of missed spots.

  • Pipeline finished with Success
    3 months ago
    Total: 496s
    #133987
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    Addressed feedback.

    Coming back to this after a long while, I am wondering why we didn't deprecate hook_file_validate() at the time?

  • Pipeline finished with Success
    3 months ago
    Total: 568s
    #133993
  • Status changed to RTBC 3 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Appears feedback from @longwave has been addressed.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    Created ๐Ÿ“Œ Deprecate hook_file_validate() Active

  • Status changed to Needs work 3 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    This should remove hook_file_validation from file.api.php .... it is deprecated we do

    $errors = array_merge($errors, $this->moduleHandler->invokeAllDeprecated('Use file validation events instead. See https://www.drupal.org/node/3363700', 'file_validate', [$file]));
    

    And this change is removing that line so the hook is no longer trigger so it should remove it from file.api.php.

  • Pipeline finished with Canceled
    3 months ago
    Total: 483s
    #135002
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    Removed hook_file_validate() and doc references to calling file_validate()

  • Pipeline finished with Success
    3 months ago
    #135006
  • Status changed to RTBC 3 months ago
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    back to RTBC

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Committed 08c0efd and pushed to 11.x. Thanks!

  • Status changed to Fixed 3 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    We need to revert this as the changes made in ๐Ÿ“Œ Create an UploadedFile validator and deprecate error checking methods on UploadedFileInterface RTBC need a rework prior to 10.3.0 being released.

  • Status changed to Postponed 3 months ago
  • Status changed to Active 3 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch
  • Pipeline finished with Failed
    2 months ago
    Total: 989s
    #153904
  • Pipeline finished with Canceled
    2 months ago
    Total: 140s
    #153920
  • Pipeline finished with Canceled
    2 months ago
    Total: 276s
    #153921
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    Cleaned up after a few merge conflicts.

  • Pipeline finished with Success
    2 months ago
    Total: 956s
    #153926
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone New Zealand
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom longwave UK

    Missed one, otherwise this looks good to me:

    core/modules/file/src/Plugin/rest/resource/FileUploadResource.php:    @\trigger_error(__METHOD__ . ' is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use \Drupal\file\Upload\FileUploadLocationTrait::getUploadLocation() instead. See https://www.drupal.org/node/3406099', E_USER_DEPRECATED);
    
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia pradhumanjainOSL

    pradhumanjain2311 โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Success
    2 months ago
    Total: 1076s
    #154204
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone New Zealand
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    except nitpick I find it RTBC

  • Pipeline finished with Success
    2 months ago
    Total: 988s
    #154493
  • Status changed to RTBC 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Was about to do the same beat me to it! Tests all green and deprecation appears to be removed here.

    • catch โ†’ committed d0184496 on 11.x
      Issue #3432882 by kim.pepper, Satane, andypost, alexpott, longwave:...
  • Status changed to Fixed 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    Committed/pushed to 11.x, thanks!

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    MR needs to be closed.

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

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

    We missed a few methods on \Drupal\file\Upload\InputStreamUploadedFile

    Created ๐Ÿ“Œ [11.x] Remove deprecated methods in InputStreamUploadedFile Active

Production build 0.69.0 2024