- Issue created by @scott_euser
- Merge request !8683Views content language field default configuration should use field_language plugin β (Closed) created by scott_euser
- Status changed to Needs review
6 months ago 5:15am 6 July 2024 - π¬π§United Kingdom scott_euser
Not sure if there is a way for the schema of views.field.field_language to just inherit everything from views.field.field? It would make sense to inherit if we can since the 'Language' field plugin extends 'Field' field plugin with nothing other than an access control method (so that the field does not show up in a monolingual site).
- π¬π§United Kingdom scott_euser
I wonder if its also worth having an update hook to update the views config of existing sites if its unchanged?
- π¬π§United Kingdom catch
I wonder if its also worth having an update hook to update the views config of existing sites if its unchanged?
We didn't do that with the original issue (which means a relatively small number of sites will be affected by the fact the shipped config wasn't updated in the original issue) because there's always risk of breaking customisations if the detection goes wrong etc. I think it's fine without. Should probably add a new change record encouraging people to update their admin/content views to get the new behaviour though.
- π¬π§United Kingdom scott_euser
Sounds good, here is a draft change record: https://www.drupal.org/node/3460053 β
- Status changed to RTBC
6 months ago 2:49pm 10 July 2024 - πΊπΈUnited States smustgrave
Seems straight forward.
Applied the MR before doing an install and filter appears to be working.
Imagine this is one of those views it would be more disruptive to post_update existing sites then to just fix for new ones.
- Status changed to Needs work
6 months ago 2:58pm 10 July 2024 - π¬π§United Kingdom catch
I think we still need feedback on #3 here, adding all that config schema seems like a lot.
- Status changed to Needs review
6 months ago 3:58pm 10 July 2024 - π¬π§United Kingdom scott_euser
Okay @alexpott helped solve that extending here https://drupal.slack.com/archives/C1BMUQ9U6/p1720626481941179 (thank you!).
I tested via π Views handler loading should respect configuration Needs review running core/modules/views/tests/src/Functional/Entity/BaseFieldAccessTest.php
- Without any `views.field.field_language` schema: test fails
- With it in place and `type: views.field.field` extending: test passes
- π¬π§United Kingdom scott_euser
Okay added back in field_api_classes - confirmed that the test still passes in π Views handler loading should respect configuration Needs review with this.
- Status changed to RTBC
5 months ago 5:48am 19 July 2024 - Status changed to Needs work
5 months ago 1:18am 1 August 2024 - π³πΏNew Zealand quietone
I read the IS, comments and MR. There are no unanswered questions here. The proposed resolution should explain what the agreed solution is.
I read the change record and I think it needs updating. I think it should stated that all views using the 'Translation language' field need to be updated and that there is an addition to the views field schema. It is not clear to me why the re-save is not needed on multi-lingual sites using the field in a view. Setting to NW for updating the change record. Oh, and it needs the branch/version added as well.
I do want to remind everyone that discussions in Slack should be summarized in the issue. That is the only way to ensure we have a record of the conversation.
- Status changed to Needs review
5 months ago 4:13am 1 August 2024 - π¬π§United Kingdom scott_euser
I read the change record and I think it needs updating. I think it should stated that all views using the 'Translation language' field need to be updated
Hmmm I am not sure if that is the case. If you create a new view with the Translation Language field you get the correct field_language plugin (just re-checked for sanity). The problem is that the config install in core/modules/node/config/optional/views.view.content.yml was outdated. If I understand the situation right, it stems from the underlying problem that there is no validation or update hooks covering anything in config/install/* or config/optional/* anything there (including in Core) slowly get out of date. This is just a symptom of that wider problem, but will become an issue if π Views handler loading should respect configuration Needs review gets in. So I do think it is really isolated to this specific view.
So if I understand it right (maybe @catch you can confirm), once π Views handler loading should respect configuration Needs review lands it is only monolingual sites that have a problem and only on the /admin/content View as the field plugin would then show the Translation Language column whereas it currently automatically hides that because Views does not respect the configuration of the View.
So we are currently letting people install a problem, and the goal here is to at least stop new sites from installing that same problem: the outdated views.view.content.yml.
I do want to remind everyone that discussions in Slack should be summarized in the issue. That is the only way to ensure we have a record of the conversation.
Apologies for that. Actually this came from MR comments https://git.drupalcode.org/project/drupal/-/merge_requests/8588#note_336534 where @catch suggested:
This might be worth splitting out to its own issue since it seems like a straight bugfix of the previous change to add this filter. Could backport to 10.4 then potentially.
The Slack conversation was not about the proposed resolution, just because I did not know how to answer my own question from #3 on whether there was a better way to code this sorry
Not sure if there is a way for the schema of views.field.field_language to just inherit everything from views.field.field?
And @alexpott answered
type: other_field
to point me in the right direction. Still learning config schema stuff, all a bit of a black box to me still.
Setting to NW for updating the change record. Oh, and it needs the branch/version added as well.
Can someone help me with that please? I do not know the policy there; is that 11.x for where it ideally originally gets committed, or would that be 10.4.x where the proposed back-port is from @catch's comment? For now I just make a guess at 11.x / 11.1.0 looking at other change records, in order to be able to move this issue back to Needs Review, but I have zero confidence in this.
For now I updated the second paragraph text to be this:
Existing monolingual sites should resave the 'Translation language' field in the Admin > Content View (found at /admin/structure/views/view/content when Views UI is enabled) so that in the future the 'Translation language' does not start appearing when only one language is available on the site.
Should I be referencing an unsolved issue π Views handler loading should respect configuration Needs review in there rather than just saying 'in the future'? Sorry for my ignorance here, change record writing also new to me obviously - still learning Core contributions.
Also added a third paragraph for clarity:
Multilingual sites can re-save this as well, but it is not necessary since the 'Language' plugin's only change is to hide itself on monolingual sites.
The proposed resolution should explain what the agreed solution is.
Sorry about that! I updated the issue summary to be me clear on the code change. Would welcome any further feedback there if I should add more details.
- πΊπΈUnited States smustgrave
Thanks for the details. Is there any blocker for this?
- π¬π§United Kingdom scott_euser
I don't think so, I believe everything from #12 is addressed.
- Status changed to RTBC
4 months ago 8:00pm 22 August 2024 - πΊπΈUnited States smustgrave
Going to go on a limb and say so too. Thanks!
- π¬π§United Kingdom catch
Yep this all looks good to me.
Committed/pushed to 11.x and cherry-picked to 11.0.x, 10.4.x and 10.3.x, thanks!
- Status changed to Fixed
4 months ago 12:40am 30 August 2024 Automatically closed - issue fixed for 2 weeks with no activity.