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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

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

Status: Active » Needs review
fago’s picture

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

Issue tags: +Entity Field API
fago’s picture

Title: file_field_prepare_view() deletes items » file_field_prepare_view() should not delete items
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

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

Dave Reid’s picture

Status: Fixed » Needs work

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

Dave Reid’s picture

Priority: Normal » Critical
fago’s picture

Title: file_field_prepare_view() should not delete items » Change notice: file_field_prepare_view() should not delete items
aspilicious’s picture

Category: bug » task

Making this a task :)

scor’s picture

Issue tags: +Needs change record

update tags (normalize to "Needs change notification")

yched’s picture

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

Priority: Critical » Normal

resetting priority

yched’s picture

Title: file_field_prepare_view() should not delete items » Followup: file_field_prepare_view() should not delete items
Status: Fixed » Needs review
FileSize
2.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

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

Bump :-)

yched’s picture

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

Issue summary: View changes

added issue link