Problem/Motivation
See #3110954: Paragraphs field translatability checked by default when reusing fields.
I think this is a bug/inconsistency in Field UI.
New fields added in the UI default to not-translatable, per #2143291: Clarify handling of field translatability that is done so explicitly, with a comment.
However, when re-using an existing storage then that line is missing and those fields are then translatable by default.
This is especially awkward when translation is not enabled for a bundle, as then the checkbox is disabled and it can't be changed.
This was last touched by #1963340: Change field UI so that adding a field is a separate task, didn't check if that introduced this problem or just moved things around.
This has been like this for a long time, but I think this is a bug that we should fix.
Proposed resolution
Also set translatable => FALSE when creating a field for an existing field storage.
Remaining tasks
Tests
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | 3111550-19.patch | 2.56 KB | smustgrave |
| #19 | interdiff-18-19.txt | 892 bytes | smustgrave |
| #18 | field-translatable-3111550-18.patch | 2.56 KB | berdir |
| #16 | field-translatable-3111550-16.patch | 3.2 KB | berdir |
| #16 | field-translatable-3111550-16-testonly.patch | 2.55 KB | berdir |
Comments
Comment #2
berdirComment #3
primsi commentedAdding tests.
Comment #4
berdirthe message on the assertTrue/False is confusing, lets add a description for what we expect/why.
Comment #5
primsi commented... copycat leftover. Fixed now, but I am not sure if the storage one that I came up with makes that much more sense.
Comment #6
plachLooks good and works as advertised, RTBC +1
it would be good to post a test-only version of the patch.
Comment #7
primsi commentedSeems like I wasn't testing that correctly after all - adding an existing field to a content type with the same field. Hopefully addressed that now.
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
berdirConflicted with [##2717319] but I think it's still needed.
Comment #17
smustgrave commentedAgreed this would be nice
Seems to have build errors
Comment #18
berdirYeah, old tests.
Also, conflicted again already in #3346682: Improve re-use an existing field user experience and I think this became kind of a duplicate as the refactoring there seems to have fixed this too.
I assume that was more an unintended side effect even though I didn't look at that other issue, so I doubt tests were added for this. Changing it to a task to add test coverage.
Comment #19
smustgrave commentedTests look good and pass locally. Just fixing up a super minor issue.
Comment #20
quietone commentedComment #22
lauriiiCommitted 357b600 and pushed to 10.1.x. Thanks!