Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clone media metadata (Fixes #1280) #1337

Merged
merged 4 commits into from Aug 29, 2018
Merged

Clone media metadata (Fixes #1280) #1337

merged 4 commits into from Aug 29, 2018

Conversation

dac514
Copy link
Contributor

@dac514 dac514 commented Aug 29, 2018

Fix is different than what #1280 recommends. Reasons:

  • If a book is public then wp/v2/media exposes all media in the library. No concept of public/private per image.
  • A user could upload 100 images but their book only uses 1 image.
  • _wp_attachment_metadata can be fake or incompatible. It's more accurate to side load the file and have WordPress recreate this kind of info.

Fix keeps the current approach of looking for media actually being used in book, then copies PB metadata when found.

@greatislander greatislander self-assigned this Aug 29, 2018
@greatislander greatislander added this to the 5.5.0 milestone Aug 29, 2018
@codecov
Copy link

codecov bot commented Aug 29, 2018

Codecov Report

Merging #1337 into dev will increase coverage by 0.01%.
The diff coverage is 90.47%.

@@             Coverage Diff              @@
##                dev    #1337      +/-   ##
============================================
+ Coverage     61.87%   61.88%   +0.01%     
- Complexity     4174     4179       +5     
============================================
  Files           112      112              
  Lines         18212    18230      +18     
============================================
+ Hits          11268    11282      +14     
- Misses         6944     6948       +4

@greatislander greatislander merged commit 05b1839 into dev Aug 29, 2018
@greatislander greatislander deleted the clone-media-meta branch September 12, 2018 18:31
@bessfernandez bessfernandez mentioned this pull request Dec 4, 2018
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants