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

Fix Largo_Related #1338

Merged
merged 4 commits into from
Oct 11, 2016
Merged

Fix Largo_Related #1338

merged 4 commits into from
Oct 11, 2016

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Oct 10, 2016

Changes

This probably would have been caught by #884 if we had tests.

I'll make a note of this PR on #1274, which removes this widget and puts it in a plugin.

@benlk benlk added type: bug priority: high Either blocks work on a priority-normal task or a solution here informs other work. status: needs review labels Oct 10, 2016
@benlk benlk added this to the 0.5.5 - Story Elements milestone Oct 10, 2016
@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2016

The current state of the plugin queries for categories and terms, and displays related posts from those taxonomy terms in that order. But categories are generally more generic than tags - should this instead display related posts from tags first, and then from categories if there aren't enough related tags?

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2016

Looks like the tests fixed in https://github.com/INN/Largo/pull/1293/files are broken by this; possibly because they're expecting the wrong order of posts or no posts? #1293 introduced the bad slugs at least.

…put, in accordance with wp_insert_post parameters
…e tax_input, in accordance with wp_insert_post parameters"

This reverts commit 0a43166.
@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2016

I'm leaving the series query alone for now, and making some notes on #1335 about that. This PR now only touches the term args, and doesn't appear to fail those tests.

@aschweigert
Copy link

Just a note that we'll also need to update the plugin version of this (for NPRDS and because that will ultimately be the replacement for this when it's removed from Largo and pulled out into a separate plugin)

@aschweigert aschweigert deleted the 1335-fix-Largo_Related-tax_query branch October 11, 2016 13:02
@aschweigert
Copy link

@aschweigert
Copy link

re: "The current state of the plugin queries for categories and terms, and displays related posts from those taxonomy terms in that order. But categories are generally more generic than tags - should this instead display related posts from tags first, and then from categories if there aren't enough related tags?"

Yes, it should use tags over categories because they're more specific (usually)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work. status: needs review type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants