1. I found it mighty confusing that \Drupal\jsonapi\Plugin\FileDownloadUrl lives in the src/Plugin directory. I didn't understand why it's listed as a plugin, even though it doesn't even have an annotation. Turns out my confusion is correct: it's not a plugin. It's just a class.
  2. If anything, this belongs in src/Field (or just in src).
  3. jsonapi_entity_base_field_info() describes this as a work-around until #2517030: Add a URL formatter for the image field lands. But that issue landed, and it didn't fix it yet — it's #2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field that will fix it. So we should update this.
CommentFileSizeAuthor
#4 2831127--interdiff--2-4.txt515 bytese0ipso
#4 2831127--file-download-rename--4.patch1.87 KBe0ipso
#2 2831127-2.patch1.51 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
2831127-2.patch1.51 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2831127-2.patch, failed testing.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso commented
Status: Needs work » Needs review
FileSize
2831127--file-download-rename--4.patch1.87 KB
2831127--interdiff--2-4.txt515 bytes

Minor fix added.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

RTBC, except that the test still lives in the wrong place, it should be moved. But that can happen on commit, or just at a later time.

  • e0ipso committed 50b97e2 on 8.x-1.x authored by Wim Leers 
    refactor(Normalization) \Drupal\jsonapi\Plugin\FileDownloadUrl is not a...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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