Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
FileFormatterBase::prepareView has code to check that the file exists for a given target when a target_id is set, but it does not consider that the target_id could be for an unsaved entity - and in doing so wipes out any unsaved entities.
Proposed resolution
Make it check for unsaved items first.
Remaining tasks
Decide if this is worth adding a test
User interface changes
None
API changes
None
Postponed on #2403793: EntityReferenceItem uses a static, but it was most likely supposed to be a constant because that is required before you can actually use the constant.
Beta phase evaluation
Issue category | Bug because data-loss |
---|---|
Issue priority | Major because data loss |
Disruption | None |
Comment | File | Size | Author |
---|---|---|---|
#12 | file-formatter-base-2403873.pass_.patch | 2.16 KB | larowlan |
#12 | file-formatter.fail_.patch | 1.24 KB | larowlan |
#9 | file-formatter-base-2403873.3.patch | 938 bytes | larowlan |
#5 | file-formatter-base-2403873.2.patch | 3.97 KB | larowlan |
#5 | interdiff.txt | 1.19 KB | larowlan |
Comments
Comment #1
larowlanComment #2
larowlanintroduced in #2232477: Fatal when adding new fields with NOT NULL constraints in a base table that contains existing entities
Comment #3
larowlanBlocker is in.
Comment #4
plachThere's a convenient
$item->hasNewEntity()
method for that :)Comment #5
larowlanThanks
Comment #7
yched CreditAttribution: yched commentedThe logic provided by FileFormatterBase is now implemented, in a more solid way, by EntityReferenceFormatterBase.
I think we should get rid of FileFormatterBase, and have the existing file / image formatters extend EntityReferenceFormatterBase.
EntityReferenceFormatterBase is currently provided by entity_reference.module, but I think that's wrong, it should be in Core (like the ER formatters and widgets themselves, BTW)
Comment #8
yched CreditAttribution: yched commentedOpened #2404021: entity_reference formatters should be in Core
Comment #9
larowlanreroll
Comment #10
larowlanComment #11
larowlanTrying to think of how to test this, @alexpott said needs a test
Comment #12
larowlanComment #14
larowlanCan we move forward here without waiting for the other issue?
Simple patch with a test here.
Comment #15
yched CreditAttribution: yched commentedAgreed, let's do this.
Opened #2405469: FileFormatterBase should extend EntityReferenceFormatterBase, postponed on #2404021: entity_reference formatters should be in Core
Comment #16
larowlanThank you
Comment #17
webchickCommitted and pushed to 8.0.x. Thanks!
Comment #19
yched CreditAttribution: yched commentedUnpostponed #2405469: FileFormatterBase should extend EntityReferenceFormatterBase and posted an initial patch.