Fix: The update to convert "numeric" arguments to "entity_target_id" is deprecated

Created on 1 June 2024, 12 months ago
Updated 27 August 2024, 9 months ago

Testing against "Next Major" Drupal 10.3 shows the following in every PHPunit test:

The update to convert "numeric" arguments to "entity_target_id" for entity reference fields for view "scheduler_scheduled_content" is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0.
Profile, module and theme provided configuration should be updated.
See https://www.drupal.org/node/3441945

The linked Change Record is https://www.drupal.org/node/3441945

The original issue is 🐛 Label token replacement for views entity reference arguments not working Needs work

📌 Task
Status

Fixed

Version

2.0

Component

Code

Created by

🇬🇧United Kingdom jonathan1055

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

Merge Requests

Comments & Activities

  • Issue created by @jonathan1055
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    1 pass, 64 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    1 pass, 64 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    1 pass, 64 fail
  • 🇬🇧United Kingdom jonathan1055

    Using a 10.3.x site and the config_update module the differences report showed that for the Node and Media views, the user_page::display_options::arguments::uid::plugin_id has been changed from numeric to entity_target_id and there is a new key target_entity_type_id with a value of user

    --- a/config/install/views.view.scheduler_scheduled_content.yml
    +++ b/config/install/views.view.scheduler_scheduled_content.yml
    @@ -1106,7 +1106,8 @@ display:
               not: false
               entity_type: node
               entity_field: uid
    -          plugin_id: numeric
    +          plugin_id: entity_target_id
    +          target_entity_type_id: user
    

    The tests now run OK at 10.3 (both locally and in pipelines), but every test fails at 10.2 with

    Drupal\Core\Config\Schema\SchemaIncompleteException:
    Schema errors for views.view.scheduler_scheduled_content with the following errors:
        views.view.scheduler_scheduled_content:display.user_page.display_options.arguments.uid.not
        missing schema,
        views.view.scheduler_scheduled_content:display.user_page.display_options.arguments.uid.target_entity_type_id
        missing schema
    
  • 🇺🇸United States dww

    Quite confused. Nothing was backported to 10.2.X. I don’t see how this could possibly be causing any test failures. But it’s late and I’m not of my right mind, so I’ll try to look again when I’m sober and have had some sleep. 😂

  • 🇺🇸United States dww

    Oh, I misread. Yeah, if you’re running against 10.3.x, you need to update any default views to use the new handler. Maybe we need another CR to clarify?

  • 🇬🇧United Kingdom jonathan1055

    Thanks for the reply. I have updated the views .yml files so that the tests run and pass at 10.3 without the deprecation. But it has caused the tests to fail at 10.2

    Nothing was backported to 10.2.X.

    That might be the problem. I want the module to be compatible with 10.2 and 10.3. Is that possible?

  • 🇬🇧United Kingdom catch

    That exception should only be thrown when strictConfigSchema is set to TRUE. Could you set it to FALSE, and then set it to TRUE again when 10.2 support is dropped (i.e. in about six months). Or... possibly conditionally set it based on Drupal core version?

  • 🇬🇧United Kingdom jonathan1055

    Thank you @catch, yes setting strictConfigSchema to FALSE has solved it, running locally the tests pass at 10.2 now. I will do a conditional to set it based on core version, as I presume we want it TRUE for all versions 10.3+

  • 🇺🇸United States dww

    When I get a moment when I’m not on my phone, I’ll try to update the CR for some additional clarity. But a few points here now that I’ve slept:

    1. The change to fix the bug was to add new argument handler plugins that properly deal with entity references instead of using entity IDs as numbers. These made it into 10.3.x and up, but not 10.2.x and down.
    2. If you ship a view still using numeric it’ll keep “working” as well as it always has.
    3. You can’t change your default view to use the new plugin, strict config schema or otherwise, in 10.2.x since the new handlers do not exist there.
    4. But what you can (and probably should) do is to ignore the deprecation warning in 10.3.x about it until you can safely drop support for 10.2.x and require at least 10.3.x.
    5. This deprecation is about the fact that the automatic conversion to the new/shiny handlers will be removed in 12.0.x. So you’ve got plenty of time and things will be auto-converted for you.

    So my recommendation for right now is to leave your views using numeric, tell GitLab CI to ignore the resulting depreciation notices until you’re ready to ship a release that requires at least 10.3.x, then you can convert your shipped views and reconfigure your pipelines to fail on depreciations (if that’s what you actually want). 😅

    Thanks / apologies,
    -Derek

  • 🇺🇸United States dww

    Mentioned at the core issue, but maybe we want a quick follow up to silence this deprecation in 10.3.x so it only starts getting hit in 10.4.x when 10.2.x is no longer supported?

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    Build Successful
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 12 months ago
    Build Successful
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    Build Successful
  • Pipeline finished with Failed
    11 months ago
    Total: 483s
    #191387
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    Build Successful
  • Pipeline finished with Failed
    11 months ago
    Total: 614s
    #191423
  • 🇬🇧United Kingdom jonathan1055

    Thanks dww for the info in #9. Sounds like I should remove the strictConfigSchema=FALSE and also revert the change to the views.

    I understand when you say that the new plugins were not backported to 10.2, but I am not seeing any error, and the views appear to work. What problem would I see? I'd like to expand the test coverage on the views we provide so that modifiying the view in this way would fail a phpunit test. But I can't see anything wrong in the UI, when using the view in 10.2 with the changed plugin.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    1 pass, 64 fail
  • 🇬🇧United Kingdom jonathan1055

    Adding the core issue that introduced this change.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    1 pass, 64 fail
  • First commit to issue fork.
  • Pipeline finished with Failed
    10 months ago
    Total: 3179s
    #232769
  • 🇮🇳India vishalkhode

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

  • Pipeline finished with Failed
    10 months ago
    Total: 620s
    #233902
  • Pipeline finished with Success
    10 months ago
    Total: 580s
    #233927
  • Status changed to Needs review 10 months ago
  • 🇮🇳India vishalkhode

    @jonathan1055: The CI is green now. Please review the changes. To address the issue, I created a test module named scheduler_config_legacy_test. This module updates the views configuration at runtime and reverts it to the previous value before saving, preventing schema exception errors for Previous Minor/Major Drupal Core versions. The reason for creating a separate module is to ensure it can be installed and enabled only for Previous Minor/Major Drupal core versions, thereby avoiding unnecessary configuration save events for the current and Next Major Drupal core versions. Thanks.

  • Pipeline finished with Success
    10 months ago
    Total: 3834s
    #233946
  • 🇮🇳India chandu7929 Pune

    I have verified the views configuration changes on 10.2 and I don't see any functional impact, I think we should proceed further by adding $strictConfigSchema = FALSE; in test, considering the comment from other issue 🐛 Label token replacement for views entity reference arguments not working Needs work

    -          plugin_id: numeric
    +          plugin_id: entity_target_id
    +          target_entity_type_id: user
    
  • Status changed to Needs work 10 months ago
  • 🇺🇸United States dww

    Got really slammed with many things. Finally circling back to this. Upon some of my testing, I was greatly confused as to why there weren't fatal errors if you updated your views config to point to plugins that don't exist. The answer is there's a bug in core 🐛 Views handler loading should respect configuration Active . 😂 Views doesn't actually honor the plugin IDs in the exported config, and instead uses the whole views data API to determine exactly what plugin to use for every specific field on every specific table.

    So, due to #3458099, I was totally wrong above. 😅 Until 3458099 is committed, it's actually completely harmless to "fix" your config and backport it, even to branches where the new plugin doesn't exist. So nearly all the complications in this MR are unnecessary and all you really have to do for now is update your views config. Nothing will actually use that config to determine what plugin is used until 3458099 is fixed. All that actually matters is the views data API implementations in each branch, and those will always find the correct plugin based on what exists.

  • 🇬🇧United Kingdom jonathan1055

    Thank you dww! I was not happy with all the work-around in this MR, and I was going to see if we could ignore the deprecation error until later. Your observed behavior at 10.2 matches mine in that the views still appear to work. However, I did report that the tests fail at 10.2, so we may still need to set strictConfigSchema = FALSE conditionally when running versions prior to 10.3

  • 🇬🇧United Kingdom jonathan1055

    Using the usual conditional version_compare to set $strictConfigSchema = FALSE in the test setUp() functions is too late to prevent the schema validation. Therefore we can patch the test files to set this value as if it was defined from the start.

    https://git.drupalcode.org/project/scheduler/-/pipelines/240327 shows this working for Previous Major functional test (not inlcuded Kernel and Javascript in the patch yet).

  • Status changed to Needs review 9 months ago
  • 🇬🇧United Kingdom jonathan1055

    This is ready for review. All phpunit tests now pass an 'Next Major' but I have used Gitlab Templates MR240 from #3414505: Allow test dependencies to also be compatible with next_major to help in testing.

  • 🇬🇧United Kingdom jonathan1055

    jonathan1055 changed the visibility of the branch 3451750-numeric-to-entity-target-id to hidden.

  • Pipeline finished with Skipped
    9 months ago
    #245509
  • Status changed to Fixed 9 months ago
  • 🇬🇧United Kingdom jonathan1055

    Fixed using my alternative (simpler) method. But also adding credit to @vishalkhode and @dww for their contribiutions on this issue. Thank you.

  • 🇬🇧United Kingdom jonathan1055

    Backported to 8.x-1.x

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

Production build 0.71.5 2024