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.
EntityNG uses Language however doesn't declare thus causing certain circumstances to error.
Comment | File | Size | Author |
---|---|---|---|
#1 | 1894436-entityng-declare-language.patch | 458 bytes | nick_schuch |
Comments
Comment #1
nick_schuch CreditAttribution: nick_schuch commentedComment #2
Berdirmakes sense but probably needs a test to verify. When does this happen?
We could also say that we don't care much as it will get merged back into Entity and postpone this issue...
Comment #3
larowlanLooking at the code I think an entity create without a langcode then attempt to access the language would throw an error, but it would be a fatal. Not sure how we'd test that.
Comment #4
aspilicious CreditAttribution: aspilicious commentedyou can test fatals with a try catch.
Comment #5
larowlanIs that new? I thought fatals couldn't be caught.
Happy to be wrong on that.
Comment #6
BerdirNo, they can't.
But a fatal error in a test is a test fail, so that's fine.
To get the fix in, we need test coverage. However, I'm not sure if we should add more NG specific tests as we want to merge all *NG files back into * anyway and I assume that we're going to stumble over this sooner or later when doing the real conversions.
Comment #7
larowlanSo should we just add the fix at #1 for now to save someone getting a wtf during conversions and add a postponed follow up for tests if it is still applicable after the *NG merge into *?
Comment #8
BerdirYeah. I didn't expect that it would happen that fast, but #293318: Convert Aggregator feeds into entities did run into that problem and I fixed it over there. That means we also have test coverage for it, so closing this issue as a duplicate.