Problem/Motivation
Views' DisplayPluginBase::isBaseTableTranslatable()
examines all entity types defined and if the first translatable one matches the base table of the view, only then would it return TRUE. With multiple entity types translatable (such as enabling block content and node modules), this method will not return correct information.
Proposed resolution
DisplayPluginBase::isBaseTableTranslatable()
should check the entity type that the base table of the view belongs to instead.
Remaining tasks
Tests.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#21 | 2450205-21.patch | 5.28 KB | plach |
#18 | 2450205-18.patch | 5.43 KB | mrjmd |
#17 | interdiff.txt | 746 bytes | mrjmd |
#17 | 2450205-17.patch | 8.72 KB | mrjmd |
#11 | interdiff.txt | 1.55 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyComment #2
Gábor HojtsyThe fix seems to make sense to me. Let's send this for testing for a start.
Comment #3
dawehnerPlach made a good point, we actually don't have to test the tables but just rely on
$entity_type->isTranslatable()
Comment #4
plachYep, checking entity type translatability should be enough, given that's what will trigger the creation of data tables.
Comment #5
Gábor HojtsyDoing that then :)
Comment #6
Gábor HojtsyWas looking into providing test coverage, but could not reproduce the bug and not sure why. I installed Standard, enabled Language module and added another language. I cannot see the language option in the file view but can see in node and user views. I also created a view of custom blocks, can see the language option there too.
I can see how the code does not make sense, but cannot reproduce a scenario where it would fail... Also, the isTranslatable() is a property of the entity type (does not require content translation module to set any translatability). Not sure if we want to tie this to ACTUAL translatability (ie. configured to be translatable) or theoretical translatability (can be configured to be translatable). It is currently the later. If its not currently configured to be translatable, then it should not have translations normally.
@dawehner or @plach: Can you help me with steps to reproduce?
Comment #7
Gábor HojtsySo I could not reproduce because I was using beta7. Using HEAD I can reproduce. Found that this is due to http://cgit.drupalcode.org/drupal/commit/core/modules/views/src/Plugin/v... via #2429447: Use data table as views base table, if available..
Working on tests.
Comment #8
plachPatch looks good, setting to NW for tests...
Comment #9
Gábor Hojtsy1. Made the popup handler use the same condition (multilingual & translatable) that the display overview used.
2. Fixed the information message that shows for the case when the feature is not supported. It depends on the view itself not if generally the user has translatable entity types or not.
3. Added tests for various stages of enabling multilingual and how that changes (or not) whether the view supports it.
Note that the test-only patch has the textual fix so it does not fail on that :)
Comment #10
Gábor HojtsyComment #11
Gábor HojtsyassertTranslationSetting() => checkTranslationSetting() to make it easier to debug. Suggestion from @dawehner on IRC.
Comment #12
dawehnerThank you!
Comment #16
mrjmd CreditAttribution: mrjmd commentedI'm going to have a look at this right now.
Comment #17
mrjmd CreditAttribution: mrjmd commentedThis should do it.
Comment #18
mrjmd CreditAttribution: mrjmd commentedOops, that interdiff was correct but the actual patch wasn't :/. One more try.
Comment #19
Gábor HojtsyTrivial fix compared to #12, so should be back to RTBC. Thanks!
Comment #20
alexpottComment #21
plachRerolled
Comment #22
plachComment #23
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a805ce2 and pushed to 8.0.x. Thanks!
Comment #25
Gábor HojtsyYay, thanks all!