WordPress.org

Make WordPress Core

#44750 closed defect (bug) (fixed)

Attributes unexpectedly lost in attachment links

Reported by: TimothyBlynJacobs Owned by: danielbachhuber
Milestone: 5.0 Priority: normal
Severity: major Version: 4.9.8
Component: REST API Keywords: has-patch has-unit-tests fixed-5.0
Focuses: rest-api Cc:
PR Number:

Description

#44287 changed the attachments controller to use the links generated from the original response by the posts controller. However, the add_links() method used has a different format than get_links() returned causing the original attributes to be nested under an “attributes” key instead of being inline with the href attribute.

I’ve attached a patch to fix this that adds tests to check the attributes for the links as well, not just that the relations exist.

Reported by @soean in Slack, https://wordpress.slack.com/archives/C02RQC26G/p1533552138000130.

Attachments (2)

44570.patch (1.6 KB) - added by TimothyBlynJacobs 18 months ago.
44750.1.patch (1.6 KB) - added by TimothyBlynJacobs 18 months ago.

Download all attachments as: .zip

Change History (12)

This ticket was mentioned in Slack in #core by pbiron. View the logs.


18 months ago

#2 @SergeyBiryukov
18 months ago

  • Milestone changed from Awaiting Review to 4.9.9

#3 @danielbachhuber
16 months ago

  • Keywords commit added
  • Milestone changed from 4.9.9 to 5.0

#4 @danielbachhuber
16 months ago

  • Summary changed from #44287 caused attachment link attributes to be lost to Attributes unexpectedly lost in attachment links

#5 @danielbachhuber
16 months ago

  • Keywords commit removed

@TimothyBlynJacobs When I ran phpunit --group restapi locally, I ended up with this unintended side-effect:

svn diff tests/qunit/fixtures/wp-api-generated.js
Index: tests/qunit/fixtures/wp-api-generated.js
===================================================================
--- tests/qunit/fixtures/wp-api-generated.js	(revision 43679)
+++ tests/qunit/fixtures/wp-api-generated.js	(working copy)
@@ -3943,27 +3943,22 @@
         "_links": {
             "self": [
                 {
-                    "attributes": [],
                     "href": "http://example.org/index.php?rest_route=/wp/v2/media/8"
                 }
             ],
             "collection": [
                 {
-                    "attributes": [],
                     "href": "http://example.org/index.php?rest_route=/wp/v2/media"
                 }
             ],
             "about": [
                 {
-                    "attributes": [],
                     "href": "http://example.org/index.php?rest_route=/wp/v2/types/attachment"
                 }
             ],
             "replies": [
                 {
-                    "attributes": {
-                        "embeddable": true
-                    },
+                    "embeddable": true,
                     "href": "http://example.org/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=8"
                 }
             ]

Do you know what the cause might be?

#6 follow-up: @TimothyBlynJacobs
16 months ago

I think that might be the byproduct of the fix. When I used get_links() it had the attributes nested in that format, but fixing it to do it iteratively means we don't have the attributes listed for links with no extra attributes. Looking at other mocked API responses in that file, it looks to be the intended format.

#7 in reply to: ↑ 6 @danielbachhuber
16 months ago

Replying to TimothyBlynJacobs:

Looking at other mocked API responses in that file, it looks to be the intended format.

Ah, I should've done this :) You're a smarter person than I.

#8 @danielbachhuber
16 months ago

  • Owner set to danielbachhuber
  • Resolution set to fixed
  • Status changed from new to closed

In 43681:

REST API: Persist attributes for attachment links

In [43437], the link definition implementation caused attachment links to be unexpectedly nested under an attributes key. This changeset restores the prior behavior.

Props TimothyBlynJacobs.
Fixes #44750.

#9 @SergeyBiryukov
16 months ago

  • Keywords fixed-5.0 added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging to trunk.

#10 @jorbin
14 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43973:

REST API: Persist attributes for attachment links

In [43437], the link definition implementation caused attachment links to be unexpectedly nested under an attributes key. This changeset restores the prior behavior.

Merges [43681] to trunk.

Props TimothyBlynJacobs, danielbachhuber.
Fixes #44750.

Note: See TracTickets for help on using tickets.