Found when debugging some fails in #1969728: Implement Field API "field types" as TypedData Plugins.

When an uploaded file is configured not to be displayed, file_field_prepare_view() simply deletes it from the list of values.
This affects the entity for the rest of the request.

That's not how it should work, items that are supposed to remain hidden should stay out of the way, but they cannot be simply removed.

CommentFileSizeAuthor
#14 file_prepare_view-2020677-followup-14.patch2.41 KByched
#1 file_prepare_view-2020677-1.patch6.29 KByched

Comments

yched’s picture

yched commented
Issue tags: +entity API, +Field API NG blocker
StatusFileSize
new file_prepare_view-2020677-1.patch6.29 KB

Patch.

#2020677: file_field_prepare_view() should not delete items currently needs to include some of those fixes, so in a way it is blocked by this.
Thus, adding the corresponding tags...

yched’s picture

yched commented
Status: Active » Needs review
fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Reviewed & tested by the community

That's not how it should work, items that are supposed to remain hidden should stay out of the way, but they cannot be simply removed.

Totally. I agree that's a reasonable improvement.

Patch looks good to me -> RTBC.

fago’s picture

fago
Primary language German
Location Vienna
commented
Issue tags: +Entity Field API
fago’s picture

fago
Primary language German
Location Vienna
commented
Title: file_field_prepare_view() deletes items » file_field_prepare_view() should not delete items
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 2635b66 and pushed to 8.x. Thanks!

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Fixed » Needs work

This will need a change notice for anyone that has written file or image formatters.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Priority: Normal » Critical
fago’s picture

fago
Primary language German
Location Vienna
commented
Title: file_field_prepare_view() should not delete items » Change notice: file_field_prepare_view() should not delete items
aspilicious’s picture

aspilicious commented
Category: bug » task

Making this a task :)

scor’s picture

scor commented
Issue tags: +Needs change record

update tags (normalize to "Needs change notification")

yched’s picture

yched commented
Title: Change notice: file_field_prepare_view() should not delete items » file_field_prepare_view() should not delete items
Status: Needs work » Fixed
Issue tags: -Needs change record

Change notice added at https://drupal.org/node/2047939

yched’s picture

yched commented
Priority: Critical » Normal

resetting priority

yched’s picture

yched commented
Title: file_field_prepare_view() should not delete items » Followup: file_field_prepare_view() should not delete items
Status: Fixed » Needs review
StatusFileSize
new file_prepare_view-2020677-followup-14.patch2.41 KB

Followup: formatters should use the helper FileFormatterBase::isDisplayed($item) method that was added in the late iterations of #1969728: Implement Field API "field types" as TypedData Plugins.
(the code example provided in the change notice linked above already does so)

Once #2015697: Convert field type to typed data plugin for file and image modules is done, this isDisplayed() method would be better off moved to the FileItem class, but for now it has to stay in FileFormatterBase.

Status: Needs review » Needs work
Issue tags: -entity API, -Entity Field API, -Field API NG blocker

The last submitted patch, file_prepare_view-2020677-followup-14.patch, failed testing.

yched’s picture

yched commented
Status: Needs work » Needs review
Issue tags: +entity API, +Entity Field API, +Field API NG blocker

#14: file_prepare_view-2020677-followup-14.patch queued for re-testing.

yched’s picture

yched commented

Bump :-)

yched’s picture

yched commented
Title: Followup: file_field_prepare_view() should not delete items » file_field_prepare_view() should not delete items
Status: Needs review » Fixed

Moved #14 to a separate issue at #2090619: Move file visibility check to the FileItem class.
Putting this one back to fixed.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

added issue link