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

Scheduled posts not publishing #116

Closed
bcampeau opened this issue Mar 29, 2016 · 24 comments
Closed

Scheduled posts not publishing #116

bcampeau opened this issue Mar 29, 2016 · 24 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@bcampeau
Copy link
Member

Had a couple reports, worth investigating: https://wordpress.org/support/topic/schedule-articles-not-publishing-to-apple-news?replies=2

@bcampeau bcampeau added the bug Something isn't working label Mar 29, 2016
@agk4444
Copy link

agk4444 commented Mar 31, 2016

This is also happening when posts are auto-published from a source feed. This could be corn job issue.

@bcampeau
Copy link
Member Author

bcampeau commented Apr 5, 2016

Appears that it could be related to the publish capability not being valid while publishing via cron

@agk4444
Copy link

agk4444 commented Apr 5, 2016

is there a way to ignore that check if it's a cron like feed-pull ?

@bcampeau
Copy link
Member Author

bcampeau commented Apr 5, 2016

We'll evaluate the best solution for an upcoming release of the plugin. You're welcome to submit a pull request of your own as well.

@kevinlisota
Copy link

We are seeing issues like this as well, though I'm not entirely sure if it is just scheduled posts.

Specifically, certain posts will appear in the Apple News dashboard as "last updated at Never" and "Sync Status" not published. Hitting publish pushes them to Apple News.

In addition, we are seeing posts that "needs to be updated" under sync status after post edits are made, but the updates aren't being pushed. This should happen automatically when an update to a post is made.

This may be relevant. Our site uses alternative (real) cron, rather than WP Cron, since we don't want cron running on every pageload. Our cron runs every minute.

@bcampeau bcampeau modified the milestones: Version 1.1.1, Version 1.1.2, Version 1.1.3 May 6, 2016
@cpabon
Copy link

cpabon commented Jun 9, 2016

Hello, thank you for the work on the plugin. Any update on this bug?

@bcampeau
Copy link
Member Author

bcampeau commented Jun 9, 2016

We're currently reevaluating priorities. I'll update soon hopefully.

@bcampeau bcampeau modified the milestones: Version 1.1.5, Version 1.1.8 Jul 26, 2016
@smerriman
Copy link

Should just be a case of disabling the check for cron:

        if ( 'publish' != $post->post_status
            || ! in_array( $post->post_type, $this->settings->get( 'post_types' ) )
            || ! current_user_can( apply_filters( 'apple_news_publish_capability', 'manage_options' ) ) && ! ( defined( 'DOING_CRON' ) && DOING_CRON ) ) {
            return;
        }

@bcampeau bcampeau self-assigned this Aug 19, 2016
@bcampeau
Copy link
Member Author

We'll take a look at this next week and sort out the root of why this was here in the first place, but it seems like a reasonable suggestion.

@bcampeau
Copy link
Member Author

Where did you pull those lines of code from? That's not actually in the source anywhere. What's actually in the source does not include a cron check:

https://github.com/alleyinteractive/apple-news/blob/master/admin/class-admin-apple-post-sync.php#L53-L57

I'm thinking this is more of a permissions issue, to be honest.

@smerriman
Copy link

The above code was the suggested replacement for the code you linked to.

current_user_can will always return false during a cron run (deliberately), so it needs the code altered to bypass the check.

@kevinlisota
Copy link

@bcampeau @smerriman I just updated this plugin today, and auto-publishing is still not working on our site. Our site does not use wp_cron. DISABLE_WP_CRON is true, and we run a real cron job every minute for performance reasons. (High traffic sites often use real cron jobs instead of wp_cron)

I'm guessing that our use of real cron jobs may be the issue here. I haven't had a chance to troubleshoot more yet, but will do so and report back.

@bcampeau
Copy link
Member Author

I would suspect that you're possibly getting caught in the permission check because the DOING_CRON constant doesn't exist. I'd start testing with alterations to this logic and report back:

https://github.com/alleyinteractive/apple-news/blob/master/admin/class-admin-apple-post-sync.php#L53-L58

Thanks.

@bcampeau bcampeau reopened this Aug 29, 2016
@bcampeau bcampeau reopened this Aug 31, 2016
@kevinlisota
Copy link

@bcampeau Is the plugin purposely meant to only publish to Apple News for administrators? I think we're getting caught in two things here. The first is that we have editors and authors who publish articles, who will not have the default "manage_options" capability that the plugin is looking for.

I see the apple_news_publish_capability filter that I can use. However, seems an odd choice to restrict to admins only. Seems better to tie it to when folks are able to publish a post.

Without a different default setting or a UI, I'd think that many sites are going to fail to push articles based on this restriction, and the UI and documentation isn't clear about it.

@bcampeau
Copy link
Member Author

bcampeau commented Sep 1, 2016

Our team took over development of the plugin about a year ago from another agency. In the original version of the plugin (version 0.2.0) they chose to make manage_options the default permission for this.

In version 1.0.0, we added the apple_news_publish_capability filter to allow this to be overridden. The exact reason I added this filter is that I agree with you insofar that it is very arguable that manage_options was the right choice for this capability.

However, as with any open source software, once something is established in code and put out into the world, you can't just change it. For every site like yours that this is a problem for, there are other sites who may be depending on this being the established default.

I'd accept the documentation could be clearer about it, but at this point, there's no changing it easily. It's very simple to override in your theme as needed via the filter.

I do agree this could use further consideration for an update in a future version of the plugin and I've opened issue #223 accordingly.

@agk4444
Copy link

agk4444 commented Sep 6, 2016

I found a new issue with latest update. Its is publishing even the scheduled drafts into apple news. we used a feed pull as and save them as a draft. After updating the plugin the saved draft is being published on apple news automatically. Loos like the logic needs to be rechecked. Looks like it is bypassing if ( 'publish' != $post->post_status) check.

@bcampeau
Copy link
Member Author

bcampeau commented Sep 6, 2016

@agk4444 can you please reference the specific line of code you believe is causing problems? There is no such thing as a 'scheduled draft' in WordPress, so maybe your terminology is just confusing here.

@agk4444
Copy link

agk4444 commented Sep 6, 2016

I am sorry for the confusion. I meant to say we are using a cron to create a draft post. When the draft is created it publishes to apple news with being published.

if ( ( 'publish' != $post->post_status
|| ! in_array( $post->post_type, $this->settings->get( 'post_types' ) )
|| ! current_user_can( apply_filters( 'apple_news_publish_capability', 'manage_options' ) ) )
&& ! ( defined( 'DOING_CRON' ) && DOING_CRON ) ) {
return;
}

@bcampeau
Copy link
Member Author

bcampeau commented Sep 6, 2016

Your use case is non-standard, but it definitely introduces a problem with this logic. We'll look at pushing a hotfix this week but you can hook your own custom function into the filter apple_news_skip_push in the meantime to prevent those from publishing: https://github.com/alleyinteractive/apple-news/blob/master/admin/apple-actions/index/class-push.php#L125

@bcampeau bcampeau reopened this Sep 6, 2016
@agk4444
Copy link

agk4444 commented Sep 6, 2016

Thanks, I already commented out that part of the code for now. Thanks for looking into this.

@smerriman
Copy link

Looks like an extra set of brackets was added in the committed version vs my suggestion above (where the cron check only cancels out the permission check, not the whole statement).

@agk4444
Copy link

agk4444 commented Sep 6, 2016

I thought the same thing but did not have time to test it. So I commented out both permission and cron check which was working for some time for me.

@bcampeau bcampeau closed this as completed Sep 6, 2016
@bcampeau bcampeau reopened this Sep 6, 2016
@bcampeau
Copy link
Member Author

bcampeau commented Sep 6, 2016

We'll look at it soon.

@bcampeau
Copy link
Member Author

bcampeau commented Sep 6, 2016

The fix for this is in master, releasing to .org now as well. Thanks for the catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants