Remove unused file_file_video_presave()

Created on 30 June 2025, 2 days ago

Problem/Motivation

✨ Video media needs playsinline option Needs work introduced a file_file_video_presave() hook, but as @berdir pointed out:

but that's not a thing at all, because there's no such thing as a file_video entity type. It's also a legacy hook function.

I guess this should be a entity_view_display_presave() hook that checks for something using that formatter, but that's a lot of complexity for something as banal as fixing up a missing key in default config.

We have file_post_update_add_playsinline() post-update hook, so this seems to be dead code. Let's remove it.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component

file.module

Created by

πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

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

Merge Requests

Comments & Activities

  • Issue created by @kim.pepper
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia
  • Pipeline finished with Failed
    2 days ago
    Total: 182s
    #535599
  • Pipeline finished with Success
    2 days ago
    Total: 634s
    #535602
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Seems straight forward.

  • πŸ‡¬πŸ‡§United Kingdom catch

    This isn't dead code as such, the upgrade path in that issue wasn't implemented correctly.

    When we're updating configuration, we need a post update that updates configuration in the site database, but also a presave hook that updates config that is shipped with modules/install profiles/recipes and triggers deprecations.

    See for example views_post_update_table_css_class()

    And

      /** 
       * Implements hook_ENTITY_TYPE_presave().
       */ 
      #[Hook('view_presave')]
      public function viewPresave(ViewEntityInterface $view): void {
        /** @var \Drupal\views\ViewsConfigUpdater $config_updater */
        $config_updater = \Drupal::classResolver(ViewsConfigUpdater::class);
        $config_updater->updateAll($view);
      }
    

    πŸ“Œ Add generic interface + base class for upgrade paths that require config changes Active would make it harder to get this wrong but for now we should fix this specific update to follow that pattern.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Another possibility here is to ensure that the config is always valid with or without this config key. This is the current approach taken in πŸ“Œ Make menu trail behaviour in SystemMenuBlock optional Active (see the config schema in the MR) which avoids having any update path at all for existing config - that issue isn't in yet but it might be viable for changes like this that are adding something brand new that defaults to off anyway.

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    So it's just adding `requiredKey: false` under the playsinline block in the schema?

  • Pipeline finished with Success
    about 7 hours ago
    Total: 626s
    #537626
Production build 0.71.5 2024