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

Temporarily empty out $wp_query->posts in the homepage bottom widget area #1354

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Oct 21, 2016

Changes

  • in the Homepage Bottom Widget Area homepage bottom partial, temporarily empty $wp_query->posts
  • add comments explaining why we do this
  • reference a commit and pull request that explains that a little more, because otherwise we'll be scratching our heads when we debug this the next time.

Why

Because the first recent posts widget in homepage bottom widget area skips 10 posts when "do not duplicate" is enabled.

From Asana:

Ben: In largo-dev/inc/widgets/largo-recent-posts.php, line 63-69, if the checkbox to ignore shown posts is checked, the widget not only ignores $shown_ids but adds the posts from the main query on the page to the list of ids to be ignored in the query. It doesn't add those posts to $shown_ids, but just joins them to the array of post ids to be ignored.

Adam: i would want to look first at why we did that in the first place. It seems like probably not a thing we'd want to do...unless it is? for some unknown reason.

Ben: Largo commit 4443b30 has the reason: Adding the main river of posts to $shown_ids messes up the LMP button that gets rendered at the end of the river. So grabbing the posts from $wp_query->posts fixes that.
Homepage bottom widget area, partials/home-bottom-widget-area.php, would be the place to nix the global $wp_query->posts array and then put it back, if we wanted to do that.

Adam: yeah i guess that's what we'd need to do (until we blow up LMP or figure out a better way to handle that)

This PR doesn't have a corresponding issue because of the Dyn DNS DDOS of 2016-10-21. 😞

@benlk benlk added type: bug priority: normal Must be completed before release of this version of plugin. labels Oct 21, 2016
@benlk benlk added this to the 0.5.5 - Story Elements milestone Oct 21, 2016
@rclations
Copy link

@benlk looks good, just want to confirm this should be applied against master

@benlk
Copy link
Collaborator Author

benlk commented Oct 24, 2016

Yep, this should be in master, because this is a hotfix for production.

The failing Travis is due to

Either https://www.facebook.com/abcdefghijklmnopqrstuvwxyz12 is user that exists and allows follows, or the Facebook follow button iframe HTML structure has changed and largo_fb_url_to_username no longer operates predictably.

@benlk benlk modified the milestones: hotfix, 0.5.5 - Story Elements Oct 24, 2016
@aschweigert aschweigert merged commit 6b53cd1 into master Oct 24, 2016
@aschweigert
Copy link

need to merge master -> develop to make sure we catch this there as well, @benlk can you take care of that?

@aschweigert aschweigert deleted the benlk-fix-missing-posts-in-widget-bottom branch October 24, 2016 14:18
@benlk
Copy link
Collaborator Author

benlk commented Oct 24, 2016

Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: normal Must be completed before release of this version of plugin. type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants