Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables
File : core/modules/file/src/Tests/FileFieldRSSContentTest.php
$field_settings declared 2 times. Might be it replaced by field_storage_settings.
/**
* Tests RSS enclosure formatter display for RSS feeds.
*/
function testFileFieldRSSContent() {
$node_storage = $this->container->get('entity.manager')->getStorage('node');
$field_name = strtolower($this->randomMachineName());
$type_name = 'article';
$field_settings = array(
'display_field' => '1',
'display_default' => '1',
);
$field_settings = array(
'description_field' => '1',
);
................
................
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff-2598178-6-12.txt | 959 bytes | krknth |
| #12 | 2598178-12.patch | 1013 bytes | krknth |
| #6 | 2598178-2.patch | 1.11 KB | krknth |
| #2 | 2598178-1.patch | 633 bytes | krknth |
Comments
Comment #2
krknth CreditAttribution: krknth as a volunteer and commentedAttached patch
Comment #3
krknth CreditAttribution: krknth as a volunteer and commentedComment #4
krknth CreditAttribution: krknth as a volunteer and commentedComment #5
krknth CreditAttribution: krknth as a volunteer and commentedThese is $field_storage_settings field not $field_settings field. I guess
Comment #6
krknth CreditAttribution: krknth as a volunteer and commentedNew patch attached
Replaced $field_settings by $field_storage_settings
Comment #7
krknth CreditAttribution: krknth as a volunteer and commentedComment #8
krknth CreditAttribution: krknth as a volunteer and commentedComment #9
krknth CreditAttribution: krknth as a volunteer and commentedComment #10
dawehnerChecked in
core/modules/file/config/schema/file.schema.yml:61that these settings are on the storage levelComment #11
xjmIndeed, what's in HEAD is clearly a mistake. Attached screenshot shows these as storage settings in the UI.
The bug where the field settings get overwritten and used for both parameters was introduced after we renamed "field" to "field storage" and "instance" to "field" (which still confuses me to this day), in #2312093: Rename FieldInstanceConfig to FieldConfig. Just a small oops in that patch.
However, I found myself wondering why we were overriding the default settings here if it had no effect. The test was originally added in #666412: Regression: RSS feed enclosure support lost from core, and it appears that it sets display settings just from being copied from
FileFieldDisplayTest. Looking at FileFieldTestBase::createFileField(), the last three arguments are optional.So I think the best fix for this test is actually to completely remove the field, field storage, and widget settings, and just do
$this->createFileField($field_name, 'node', $type_name). That will truly be less code. :)Comment #12
krknth CreditAttribution: krknth as a volunteer and commentedUpdated as per @xjm
Comment #13
swentel CreditAttribution: swentel commentedLess code it is!
Comment #14
alexpottCommitted e01619a and pushed to 8.0.x and 8.1.x. Thanks!