Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48164 closed enhancement (fixed)

media_sideload_image should store original URL and optionally check for dupes

Reported by: dshanske's profile dshanske Owned by: antpb's profile antpb
Milestone: 5.4 Priority: low
Severity: trivial Version:
Component: Media Keywords: has-patch has-dev-note
Focuses: Cc:

Description

When sideloading a file from a URL, using media_sideload_image, the original URL should be stored as metadata automatically.

The reason for this to be done in Core, as opposed to a plugin, is that the reason why individuals use media_sideload_image is to store a local copy of a file. It should, for copyright and fairness reasons, store where it was originally hosted.

The secondary advantage is the sideload can query existing attachments and not sideload the same file twice.

Attachments (1)

48164.diff (870 bytes) - added by dshanske 5 years ago.

Download all attachments as: .zip

Change History (12)

@dshanske
5 years ago

#1 @dshanske
5 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.4
  • Priority changed from normal to low
  • Severity changed from normal to trivial

This patch needs no unit testing I can think of. It is merely storing information so that we can be sure to attribute images that are passed in by URL. The only question is whether the meta key name is the best one for this.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

#3 @antpb
5 years ago

  • Keywords needs-dev-note added
  • Owner set to antpb
  • Status changed from new to assigned

Hi @dshanske ! Thank you so much for submitting this patch. It was discussed in the recent media meeting and I am going to work to get this merged before the 5.4 beta. One thing I’ll make an adjustment on the media_sideload_image() function to give some indication of the new meta field being added. @joemcgill mentioned something like Since 5.4, the original URL of the attachment is stored in post meta Feel free to do this if you don’t see my patch here yet. I’ll likely get to this early next week.

Here’s that function for handy reference: https://developer.wordpress.org/reference/functions/media_sideload_image/

#4 @dshanske
5 years ago

I am not sure if I can get to this this week, but I'm glad it made sense to the media team. There's really no way to do this with a hook and I really want to properly attribute my cached copies of images.

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


5 years ago

#6 @davidbaumwald
5 years ago

  • Milestone changed from 5.4 to Future Release

This ticket still needs a final patch, and with 5.4 Beta 1 landing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#7 @SergeyBiryukov
5 years ago

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

In 47251:

Media: In media_sideload_image(), store the original attachment URL in the _source_url post meta value.

Props dshanske, joemcgill, antpb.
Fixes #48164.

#8 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 5.4

#9 @audrasjb
5 years ago

  • Keywords has-dev-note added; needs-dev-note removed

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


5 years ago

This ticket was mentioned in PR #190 on WordPress/wordpress-develop by killua99.


5 years ago
#11

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

-->

Since #48164(https://core.trac.wordpress.org/ticket/48164) when an image is sideloaded we store the original attachment URL in _source_url. There should be test coverage across this feature that validates that the meta is being properly stored.

@pbiron mentioned in the Media meeting where this was discussed that there are currently no existing tests around the media_sideload_image() function. This would be a great start to getting that covered.

@joemcgill also mentioned that "You may want to consider writing the test in a way that sideloads an image that is already available on the test site."

Trac ticket: https://core.trac.wordpress.org/ticket/49631

Note: See TracTickets for help on using tickets.