Skip to content
This repository has been archived by the owner on Mar 26, 2021. It is now read-only.

modified class/NPRAPIWordpress.php #4

Closed
wants to merge 1 commit into from

Conversation

rellison959
Copy link

To get $image_metas (Caption, Credit, Agency) to update the Post Meta the variable $id on line 331 needed to be $post_id.

line 209 I deleted a trailing comma. Does not affect functionality.

Lines 327-328 I move lines of code into alignment with others, though it does not appear that way below. Does not affect functionality.

@CrookedNumber
Copy link
Contributor

Thanks, rellison959.
I couldn't just merge the pull request as-is, but this was extremely helpful.

  • The variable name should indeed by $post_id
  • The update_post_meta() call should be inside the check to see if the image==primary, or else you will just get the image metadata for the LAST image evaluated (which may or may not be the primary, but likely isn't)
  • Please note: this won't work retroactively by clicking Update Story -- unless the story has actually changed in the API.
    -FWIW, trailing commas are recommended by WP. http://make.wordpress.org/core/handbook/coding-standards/php/#indentation

@CrookedNumber
Copy link
Contributor

I've pushed this fix. If you get the latest version of master, this should work now.

@rellison959
Copy link
Author

David,

Is there not a way to save the $image_meta to the actual fields for the image? That way if an image were to be reused or needed to be referenced on it’s own that info would be present and attached to the image instead of just the post?

Just a though.

Thanks,
Ryan

On May 9, 2014, at 12:59 PM, David Moore notifications@github.com wrote:

Thanks, rellison959.
I couldn't just merge the pull request as-is, but this was extremely helpful.

The variable name should indeed by $post_id
The update_post_meta() call should be inside the check to see if the image==primary, or else you will just get the image metadata for the LAST image evaluated (which may or may not be the primary, but likely isn't)
Please note: this won't work retroactively by clicking Update Story -- unless the story has actually changed in the API. -FWIW, trailing commas are recommended by WP. http://make.wordpress.org/core/handbook/coding-standards/php/#indentation

Reply to this email directly or view it on GitHub.

@CrookedNumber
Copy link
Contributor

Thanks. I absolutely agree.

Though, my short-term goal here, in this ticket, was a surgical strike: fix the bug so that the plug-in honors the expected behavior laid out in the release notes.

Want to add a separate ticket for this (something like "attach NPR metadata to each image")?

@rellison959
Copy link
Author

Is there any support or planned support for Wordpress based station sites to get Audio into NPR One app like what has been made possible for Core Pub station sites?

Did I miss anything on this?

Thanks,
Ryan Ellison
WMFE

@dan-simplisafe
Copy link

Hi Ryan,
It's in the works. We were almost ready to release it last week when we discovered a problem with the API (once you put a story into NPR One you couldn't remove it). The API team is looking into that and we'll release an update to the plugin as soon as they have it fixed.

-dan sundell
Directory of Application Development
NPR Digital Services

@rellison959
Copy link
Author

I have actually gone back and forth on how this should work for me, and have now found that WP is not allowing the captions to show on the story/image if they are not put in at the same time as the image is uploaded and saved to the story. NPR captions and credits always work since they are custom meta.

In our case, if the image caption was added after the image was initially saved to the story, the caption will not show up. I am likely seeing this due to Amazon S3 integration, but regardless, having a caption that relates to the story is more useful that hooking it to the image forever. We reuse images for commentaries and such quite often, so I am toying with the idea of having the caption as a custom meta box and have it live strictly with the story ID.

I also have a question about how I am handling audio within our posts, and whether that will work with NPR API PUSH. Currently I have a custom metabox that is for a .mp3 url of the audio file. That gets added when/if there is an associated audio file. Will API pick up a audio link if it’s coming from a custom meta?

Thanks,
Ryan

On Sep 2, 2014, at 12:19 PM, Dan Sundell notifications@github.com wrote:

Hi Ryan,
It's in the works. We were almost ready to release it last week when we discovered a problem with the API (once you put a story into NPR One you couldn't remove it). The API team is looking into that and we'll release an update to the plugin as soon as they have it fixed.

-dan sundell
Directory of Application Development
NPR Digital Services


Reply to this email directly or view it on GitHub.

@rellison959
Copy link
Author

This issue was addressed in past Plugin updates, last year sometime I think, but when we have a post set to “sticky” and the NPR API plugin set to pull and set the stories to DRAFT, what actually happens is the STICKY story actually gets hijacked and a new revision is created of that post for ever single api story that is pulled. You can look back through the revisions of the sticky post and see content for every pulled api story from the last pull. The final story in the pull will actually then be displayed as the new content for the sticky post.

Like a mentioned, this issue was addressed (by Kevin Moylan?) in the last version of the plugin and we had no issue with this until this most recent update. The only different on my end is I have the PUSH url set because we are testing API PUSH right now.

Please advise

Thank you,
Ryan Ellison

[unknown.jpg]
Our ORION story on left, NPR content from pull on right. ALL revisions after this are also NPR content from PULL as well.

@CrookedNumber
Copy link
Contributor

Hi Ryan,

Thanks. I hope to look into this today. Could I ask you to open a separate issue for this, to keep things clean? Thanks.

--Dave

@rellison959
Copy link
Author

Any ideas on this? Have I missed an update on the issue?

Thanks
Ryan

Begin forwarded message:

This issue was addressed in past Plugin updates, last year sometime I think, but when we have a post set to “sticky” and the NPR API plugin set to pull and set the stories to DRAFT, what actually happens is the STICKY story actually gets hijacked and a new revision is created of that post for ever single api story that is pulled. You can look back through the revisions of the sticky post and see content for every pulled api story from the last pull. The final story in the pull will actually then be displayed as the new content for the sticky post.

Like a mentioned, this issue was addressed (by Kevin Moylan?) in the last version of the plugin and we had no issue with this until this most recent update. The only different on my end is I have the PUSH url set because we are testing API PUSH right now.

Please advise

Thank you,
Ryan Ellison

[cid:30804ECE-D6E6-43E3-AE8F-CAB68495BA39]
Our ORION story on left, NPR content from pull on right. ALL revisions after this are also NPR content from PULL as well.

@CrookedNumber
Copy link
Contributor

Still looking. Could I ask you to open a separate issue for this, to keep things clean? Thanks.

@ghost
Copy link

ghost commented Dec 10, 2014

David, Ryan,

If I'm matching up the same problem & solution this was the updated code on 11/18/13 to solve the problem you are experiencing (sounds like it never got included):

Hi Ryan, Kevin,

I spent a little time looking at this, this morning.
It looks as though adding 'post_status' => 'any' instead of the default (publish only) to the query did indeed kick the sticky posts to the front, thus overwriting as content is ingested.

I didn't spend a lot of time testing this, but it appears that adding 'ignore_sticky_posts' => true moves sticky posts back into the natural order.

So the full query would now be:
$exists = new WP_Query( array( 'meta_key' => NPR_STORY_ID_META_KEY,
'meta_value' => $story->id,
'post_type' => $pull_post_type,
post_status' => 'any',
'ignore_sticky_posts' => true
));

Again, I only had about 20 minutes to test, but hopefully this will help?
Thank you,

Ryan Tainter
Interactive Content Producer
214-740-9249


From: David Moore notifications@github.com
Sent: Wednesday, December 10, 2014 3:04 PM
To: npr/WP-DS-NPR-API
Subject: Re: [WP-DS-NPR-API] modified class/NPRAPIWordpress.php (#4)

Still looking. Could I ask you to open a separate issue for this, to keep things clean? Thanks.

Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-66523739.

@rellison959
Copy link
Author

I believe that may have in fact been the resolution.

master/classes/NPRAPIWordpress.php @ line 113 is where the query is found.

I will test and report back if still an issue.

Thanks,
Ryan

On Dec 10, 2014, at 4:07 PM, KeraDigital notifications@github.com wrote:

David, Ryan,

If I'm matching up the same problem & solution this was the updated code on 11/18/13 to solve the problem you are experiencing (sounds like it never got included):

Hi Ryan, Kevin,

I spent a little time looking at this, this morning.
It looks as though adding 'post_status' => 'any' instead of the default (publish only) to the query did indeed kick the sticky posts to the front, thus overwriting as content is ingested.

I didn't spend a lot of time testing this, but it appears that adding 'ignore_sticky_posts' => true moves sticky posts back into the natural order.

So the full query would now be:
$exists = new WP_Query( array( 'meta_key' => NPR_STORY_ID_META_KEY,
'meta_value' => $story->id,
'post_type' => $pull_post_type,
post_status' => 'any',
'ignore_sticky_posts' => true
));

Again, I only had about 20 minutes to test, but hopefully this will help?
Thank you,

Ryan Tainter
Interactive Content Producer
214-740-9249


From: David Moore notifications@github.com
Sent: Wednesday, December 10, 2014 3:04 PM
To: npr/WP-DS-NPR-API
Subject: Re: [WP-DS-NPR-API] modified class/NPRAPIWordpress.php (#4)

Still looking. Could I ask you to open a separate issue for this, to keep things clean? Thanks.

Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-66523739.

Reply to this email directly or view it on GitHub #4 (comment).

aschweigert added a commit that referenced this pull request Jun 27, 2016
delete ReleaseNotes.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants