- Issue created by @heddn
- First commit to issue fork.
- Merge request !5505Issue #3400522 by Boobaa: TimestampFormatter / time_diff missing config schema β (Open) created by boobaa
- Status changed to Needs review
about 1 year ago 12:58pm 22 November 2023 - Status changed to Needs work
about 1 year ago 2:04pm 22 November 2023 - πΊπΈUnited States smustgrave
Can the rest of the issue summary be filled in please.
Also change caused test failures.
- heddn Nicaragua
Obviously we need tests added or this wouldn't have passed any if there were any. Tagging.
- πΊπΈUnited States agentrickard Georgia (US)
Here's a simple view that shows the issue -- let me see if I can work up an MR.
- Status changed to Needs review
11 months ago 5:55pm 10 February 2024 - πΊπΈUnited States agentrickard Georgia (US)
OK, here's what I did.
- Added that view as
core/modules/node/tests/modules/node_test_views/test_views/views.view.test_field_created.yml
- Reverted the schema commit
If I read the code from
core/modules/node/tests/src/Functional/Views/NodeTestBase.php
, the setup() method should try to import the new view and then auto-fail schema checks. - Added that view as
- Status changed to Needs work
11 months ago 6:23pm 10 February 2024 - πΊπΈUnited States smustgrave
There's a test-only feature so don't need to push separately.
- πΊπΈUnited States agentrickard Georgia (US)
Well, I suspect this minor issue may be covering up a more major problem (which we should file separately).
This is the YML from the test file:
time_diff: enabled: false future_format: '@interval hence' past_format: '@interval ago' granularity: 2 refresh: 60 description: ''
This is the YML from core views.view_content
time_diff: enabled: false future_format: '@interval hence' past_format: '@interval ago' granularity: 2 refresh: 60
When the schema change was made in core, the affected core views were not updated. The following core views are potentially affected:
- views.view.media.yml
- views.view.test_content_ajax.yml
- views.view.test_field_created.yml
- views.view.glossary.yml
- views.view.content.yml
- views.view.media_library.yml
- views.view.media.yml
- views.view.files.yml
- views.view.moderated_content.yml
- views.view.comment.yml
- views.view.block_content.yml
- core.entity_view_display.media.type_five.default.yml
- core.entity_view_display.media.type_two.default.yml
- core.entity_view_display.media.type_three.default.yml
- core.entity_view_display.media.type_one.default.yml
What does this potentially mean?
- Core is not updating its views consistently -- for both distribution and tests -- this seems like a process issue
- Views only updates yml elements when the plugin usage is added (not edited, so far as I can tell)
If the affected core views had been updated with the original change was made, tests would have caught the issue.
- Status changed to Needs review
11 months ago 4:50pm 11 February 2024 - πΊπΈUnited States agentrickard Georgia (US)
Here's an updated MR that does the following:
* Removes the new test file
* Adds the `description` element to `views.view.content.yml` - πΊπΈUnited States agentrickard Georgia (US)
Update:
* This merge shows the issue (and the test fails) https://git.drupalcode.org/project/drupal/-/merge_requests/5505/diffs?co...
* Re-added the original patch / change
* Updated all affected views - Status changed to Needs work
10 months ago 3:44pm 15 February 2024 - πΊπΈUnited States smustgrave
Seems to have failures
But if we are updating all views will need an upgrade path for existing sites
Also not sure the title reflects what the MR is doing so tagged for that.
- π©πͺGermany Anybody Porta Westfalica
Confirming this issue, we just ran into it. Resaving the view adds the empty string.
- First commit to issue fork.
I will update the title and confirm the steps in the updated summary by EOD Monday 2024-07-29.
- Status changed to Needs review
5 months ago 5:57am 30 July 2024 - π·πΊRussia sorlov
Added hook_post_update_NAME() to update existing views with description for time_diff for timestamp formatter
need review
- Status changed to Needs work
5 months ago 2:20pm 30 July 2024 - πΊπΈUnited States smustgrave
See upgrade path but still needs upgrade test.
- π«π·France andypost
The test is incomplete but please combine it with checking that field is propagated after upgrade, so we can get rid of "Needs test" tag
1) Drupal\Tests\views\Functional\Update\TimestampFormatterUpdateTest::testViewsFieldPluginConversion Exception: Deprecated function: mb_strtolower(): Passing null to parameter #1 ($string) of type string is deprecated Drupal\Core\Config\Entity\Query\Condition->compile()() (Line: 39)
- π΅π±Poland Graber
Maybe I'm missing something but:
Drupal\Core\Field\Plugin\Field\FieldFormatter::settingsForm()
$form['time_diff']['description'] = [ '#type' => 'item', '#title' => $this->t('Fallback configuration'), '#description' => $this->t('The configuration below is used as a fallback when JavaScript is not available on the page.'), '#states' => $states, ];
Description is just a user information element, it doesn't need a schema, how come it got to form state values and exported config in the first place?
- π΅π±Poland Graber
Changing this to:
$form['time_diff']['description'] = [ '#markup' => 'The configuration below is used as a fallback when JavaScript is not available on the page.', ];
fixes the config after save in views. So the issue is that "item" form element value gets to form state values as an empty string instead of not being passed there at all. On the other hand when saving on entity display settings form, description is not in the config (maybe there's a empty value filter).
And we need to get rid of those 'description' config values in time_diff everywhere. - π΅π±Poland Graber
I'd do this. It's a very deep core form API change so let's see if it doesn't break anything.
- π΅π±Poland Graber
Created π Item (display only) element results in an empty string in form_state values Active that addresses the root cause. Unfortunately, many tests fail so it's not going to be that easy.
- Status changed to Needs review
4 months ago 7:50am 19 August 2024 - π·πΊRussia sorlov
Opened separate MR https://git.drupalcode.org/project/drupal/-/merge_requests/9250 for another solution
Issue was fixed by adding
'#input' => FALSE,
to$form['time_diff']['description']
So now we won't have empty
description
key in formatter config - π΅π±Poland Graber
Nice one, but shouldn't we fix it in a more global way, adding it to item element defaults instead?
- Status changed to Needs work
4 months ago 1:36pm 19 August 2024 - πΊπΈUnited States smustgrave
If a new solution is used will need to update the issue summary, maybe? the title also.
Would need to check the criteria to see if this change is small enough to warrant not needing test coverage
- π«π·France andypost
New workaround point to problem with
item
form element inside of forms, git research points to #1825044: Turn contact form submissions into full-blown Contact Message entities, without storage βSo I think if no other option to replace the element with
#markup
for example the !5505 looks nice idea but surely it needs follow-up to prevent export of this element to config at all, maybe event remove it at form state level via π Item (display only) element results in an empty string in form_state values Active - π«π·France andypost
Issue could be considered a duplicate if hack with input will be moved to contact module in π Item (display only) element results in an empty string in form_state values Active
- π·πΊRussia sorlov
I'm not sure if this is global issue, related to any item element, cause for example, we have a lot of item elements in FileSystemForm form, but they are not exposed to config values