Contrib.social

Add missing update hook for adjusting viewsreference field settings

Open on Drupal.org →
Created on 3 May 2024, about 2 months ago
Updated 23 June 2024, 3 days ago

Problem/Motivation

This is a follow-up issue to my comment in 🐛 Update viewsreference_update_8103 fails if table name is too long and shortened by Drupal Fixed and 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed .

From 🐛 Update viewsreference_update_8103 fails if table name is too long and shortened by Drupal Fixed :

We forgot to include removing all unchecked viewsreference "plugin_types", "preselect_views" and "enabled_settings" field settings.

Before this patch, we had no schema definition for "plugin_types", "preselect_views" and "enabled_settings" AND no fieldSettingsFormValidate() method, which simply removes all unchecked setting values before form submission. So the module used to save both unchecked and checked field settings in config. If the value was checked, it saved the name of the value as a string (e.g. page:page) or if it was uncheckd as "0" (e.g. page: 0).

After this patch, the empty values are not getting saved anymore, but they still exist in config if not manually resaved. And since "viewsreference.plugin_types" and "preselect_views" (also "enabled_settings" now through 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed ), are defined as sequences of strings, but the old implicit schema definition for an unchecked value was integer, the following error will appear using config_inspector:

variable type is integer but applied schema class is Drupal\Core\TypedData\Plugin\DataType\StringData

Here is an exported config as an example, showing the old unchecked values still saved:

  plugin_types:
    default: default
    page: page
    block: block
    attachment: attachment
    feed: feed
    entity_browser: 0
  preselect_views:
    admin_header_slides: 0
    block_content: 0
    content: 0
    content_recent: 0
    files: 0
    frontpage: 0
    glossary: 0
    media_library: 0
    redirect: 0
    redirect_404: 0
    scheduler_scheduled_content: 0
    taxonomy_term: 0
    user_admin_people: 0
    user_profile_display: 0
    watchdog: 0
    webks_inhalt_unterseiten: 0
    who_s_new: 0
    who_s_online: 0
  enabled_settings:
    argument: argument
    offset: 0
    limit: 0
    pager: 0
    title: title

Furthermore, we missed the opportunity to create an update hook in 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed . Both issues can be easily combined to create one hook implementation to automatically adjust the field settings values for already existing site installations.

Steps to reproduce

1. Clone repo, checkout commit 3d1d66cc902624511175f713ade2b34e65bc2b0c, enable module.
2. Add view reference field to entity (e.g Article), ensure at least one setting is unchecked in field config.
3. Confirm an unchecked setting e.g settings.enabled_settings.limit in field config uses 0 value when unchecked:

settings:
...
  enabled_settings:
    title: title
    argument: 0
    limit: 0
    offset: 0
...

4. Checkout commit d444b603d8880e5944fbf294e847e1cd18715b8c from 🐛 Update viewsreference_update_8103 fails if table name is too long and shortened by Drupal Fixed (don't re-save field config after this point)
5. Confirm field config fails validation for the unchecked setting in config checker, via /admin/reports/config-inspector: variable type is integer but applied schema class is Drupal\Core\TypedData\Plugin\DataType\StringData`

Proposed resolution

Add the missing update hook for adjusting viewsreference field settings

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

🇩🇪Germany Grevil

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

Merge Requests

Comments & Activities

  • Issue created by @Grevil
  • Merge request !50Issue #3445081 by Grevil: Add missing update hook for adjusting viewsreference field settings → (Open) created by Grevil
  • Comment about 2 months ago →
    🇩🇪Germany Grevil

    Alright, this does the trick! Please review!

    Here is the drush cex --diff output after running the update hook:

    diff --git a/tmp/drush_tmp_1714738545_6634d571e4fb1/field.field.node.article.field_viewsreference.yml b/tmp/drush_tmp_1714738545_6634d571ec102/field.field.node.article.field_viewsreference.yml
index 7191901..feadc2c 100644
--- a/tmp/drush_tmp_1714738545_6634d571e4fb1/field.field.node.article.field_viewsreference.yml
+++ b/tmp/drush_tmp_1714738545_6634d571ec102/field.field.node.article.field_viewsreference.yml
@@ -26,24 +26,14 @@ settings:
     default: default
     page: page
     block: block
-    feed: '0'
   preselect_views:
     block_content: block_content
     comment: comment
     comments_recent: comments_recent
     content: content
     content_recent: content_recent
-    files: '0'
-    frontpage: '0'
-    taxonomy_term: '0'
-    user_admin_people: '0'
-    watchdog: '0'
-    who_s_new: '0'
-    who_s_online: '0'
   enabled_settings:
-    limit: '1'
-    offset: '1'
-    title: '1'
-    argument: '0'
-    pager: '0'
+    limit: limit
+    offset: offset
+    title: title
 field_type: viewsreference

    Please review!

  • Status changed to Needs review about 2 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 2 months ago
    1 fail
  • Comment about 2 months ago →
    🇨🇦Canada Sseto

    Hi Grevil,

    I updated the module to beta7 and my view block disappeared. I applied patch 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed and my view is back BUT only intermittently. Very strange that it disappears and reappears.

    Do you think applying your update hook will fix my issue?

    Thanks!

  • Comment about 2 months ago →
    🇩🇪Germany Grevil

    No, I don't think this patch will help you in this regard. Have you tried using the current dev version of this module?

    There are currently six issues merged in dev, since 8.x-2.0-beta7: https://git.drupalcode.org/project/viewsreference/-/commits/8.x-2.x?ref_...
    Maybe one of them solves your issue.

  • Status changed to RTBC 26 days ago
  • Comment 26 days ago →
    🇩🇪Germany harkonn

    The update-hook fixed the problem for our systems and is already running in our sites.

    Side-node: We copied the code into our own update-hook into our profile so we don't run into problems with update-hook-versions in our projects.

    Good work! :)

  • Comment 22 days ago →
    🇳🇱Netherlands ekes

    Isn't it the schema that is wrong, or rather missing? Those look like they should be integers not strings. I see there is for example viewsreference.enabled_settings.limit which is set for integer. It will be that it also needs one for when it's used *view:?

  • Comment 15 days ago →
    🇩🇪Germany lmoeni

    The update hook fixed the problem for us. Thanks!

  • First commit to issue fork.
  • Comment 11 days ago →
    🇬🇧United Kingdom scott_euser

    Thanks for the work on this! Could someone update the issue summary with steps to reproduce please? I believe I should check out a particular version of the 2x branch, set up a Views Reference Field with a particular set of settings, then update to latest 2x branch and see the incorrect configuration saved.

  • Comment 11 days ago →
    🇬🇧United Kingdom scott_euser

    For now I pulled all the latest changes from 2x into this merge request so tests start running as well on it.

  • Comment 3 days ago →
    🇦🇺Australia jnlar Sydney, Australia

    hi @scott_euser, I've updated the IS with the reproducible steps. config_inspector module was used to produce the failed validation error.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024