Media: send numerical post id when uploading image #57388
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing ids comprising "theme + // + template name".
ramonjd
added
[Type] Bug
This was referenced Dec 27, 2023
|
Added @danielbachhuber, who might know off the top of his head whether the postId still serves the original purpose1 of linking an uploaded media attachment to a post id. Otherwise I can debug later. Footnotes
|
|
Size Change: +195 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 0078019. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7335658008
|
Yes, here we go. Right in front my eyes. The |
There was a problem hiding this comment.
Thanks for all the effort debugging this one and coming up with a fix @ramonjd 👍
This PR tests as advertised for me.
✅ Can upload an image while editing a template
✅ Can upload image in post editor and post id is included in call
I think I'm still missing something though as to why we have to fall back to passing 0 as the post value. My understanding is that prior to #55970 no post value was passed in the site editor.
The change to pass a template's actual numerical ID makes sense though.
Omitting the post value from additionalData entirely if no numerical ID is found seems to work still for me locally, but as I said, I must be missing something.
|
Thanks for testing @aaronrobertshaw As far as I can tell, Core doesn't care whether a post id is sent, though the attachments rest api controller has one in the schema. Through its use of I guess the least we can say about this PR is that it's making the return value consistent 🤷🏻 Still, the question of whether this is the right approach is open. Keen to hear what @fullofcaffeine thinks. |
…tes) doesn't exist.
ramonjd
changed the title
|
Thanks for helping test this @aaronrobertshaw If you're fine with update in f44f8fe, I'll merge and continue to troubleshoot |
|
I have a Jetpack PR with a similar approach. Hopefully it addresses issues there too: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Follow up to:
When uploading media, send a numerical post ID where possible.
Why?
The apparent purpose of the post id here is so that WordPress can assign media to a parent post.
See:
editorMediaUpload, which includespostcontext for uploads #6231Templates and template parts have string ids comprising "theme + // + template name". Their numerical IDs — the unique ids in the database — are stored in
wp_idproperty. So let's send that.How?
Checking for a numerical
idandwp_id. If there is none, then don't send additional data.Testing Instructions
As with #57040