Added additional random parameters #877

Merged
merged 2 commits into from Aug 8, 2014

Projects

None yet

5 participants

@iannuttall
Contributor

Added random_cat_id and random_post_type parameters to match the functionality of the original Random Redirects plugin.

@iannuttall iannuttall Added additional random parameters
Added random_cat_id and random_post_type parameters to match the functionality of the original Random Redirects plugin.
a9f1f4f
@georgestephanis

cc: @enejb @blobaugh -- could one / both of you read this through and vet it?

@iannuttall -- should be fine to pull request, and we can test it more easily there.

Should line 23 be an or?

This looks cleaner than it was before to me.

FYI the LIMIT in the sql is specific to MySQL and will break on other sql servers if they do not have good parsing and translation of the sql calls.

On line 50 we should prepend the table name for clarity. post_type => p.post_type.

Other suggestion would be to use is_post_type_archive() instead of ! is_front_page()
Use function post_type_exists() to check if the post type being passed via the url $_GET['random'] or $_GET['random_post_type']
is actually a valid post type instead of line 40.

Another suggestion would be to be able to use the existing url structure to get the query parameters. This way we don't reinvent the wheel. The idea is that you take any archive url and add ?random and are redirected to one a single post based that would have been part of that archive list.
Since the function is being added to template redirect. We have already fetched the posts once. If we are don't have any pagination we can just use php to pick a random number and redirect the user there.

This would also allow us to have have more flexibility not just be restricted to categories and post types but any type of custom taxonomy, authors, dates.

@blobaugh blobaugh added this to the 3.2 milestone Aug 5, 2014
Contributor
blobaugh commented Aug 5, 2014

@iannuttall could you please check over the feedback and update the PR?

Contributor

@blobaugh yeah, will take a look this weekend and see what I can do :)

@iannuttall iannuttall Added additional random parameters Fixes #867
Made changes as per @enejb's comments.
5319095
Contributor

@blobaugh:

Should line 23 be an or?

It was && in the original version. I tried with an or, but it doesn't fire without &&.

FYI the LIMIT in the sql is specific to MySQL and will break on other sql servers if they do not have
good parsing and translation of the sql calls.

I'm not sure what to change if anything for this. The query was already in the original file. :/

@enejb:

On line 50 we should prepend the table name for clarity. post_type => p.post_type.

Done!

Other suggestion would be to use is_post_type_archive() instead of ! is_front_page()
Use function post_type_exists() to check if the post type being passed via the url $_GET['random'] or $_GET['random_post_type']
is actually a valid post type instead of line 40.

Instead of setting the default to be 'post' I changed it to be get_post_type(); instead. In the event of the $_GET['random_post_type'] I changed it to use post_type_exists().

Another suggestion would be to be able to use the existing url structure to get the query parameters. This way we don't reinvent the wheel. The idea is that you take any archive url and add ?random and are redirected to one a single post based that would have been part of that archive list.

You should be able to use &random on any category or author archive pages now without having to specify the category ID.

Since the function is being added to template redirect. We have already fetched the posts once. If we are don't have any pagination we can just use php to pick a random number and redirect the user there.

I wasn't really sure what you meant here, so I didn't action anything.

This would also allow us to have have more flexibility not just be restricted to categories and post types but any type of custom taxonomy, authors, dates.

This would be awesome to add, I'm just not 100% sure how to go about covering all the different types of archives programmatically.

@blobaugh blobaugh merged commit 212a24c into Automattic:master Aug 8, 2014

2 checks passed

ci/scrutinizer Scrutinizer: No new issues — Tests: passed
Details
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment