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

google news smart max #79

Closed
selul opened this issue Oct 21, 2017 · 7 comments
Closed

google news smart max #79

selul opened this issue Oct 21, 2017 · 7 comments
Assignees
Labels
enhancement Request to improve or optimize an existing feature or functionality in the project

Comments

@selul
Copy link
Contributor

selul commented Oct 21, 2017

when using feeds from google news, and the user is using the max attribute within the shortcode which is greater than the num query argument of the news feed, the feed is still limited to the num query arg,

E.g [feedzy-rss feeds="http://news.google.com/news?hl=en&gl=us&q=Tourette&num=20&um=1&ie=UTF-8&output=rss" max="30" target="_blank" refresh="1_hours" title="120" meta="yes" summary="yes" summarylength="275" size="160"]

because num=20 the max=30 is ignored as we dont have more than 20 items in the feed.

We should do a smart feed convert, which alter the num query arg if the google news feed is used.

@selul selul added the enhancement Request to improve or optimize an existing feature or functionality in the project label Feb 26, 2018
contactashish13 added a commit to contactashish13/feedzy-rss-feeds that referenced this issue Mar 2, 2018
@contactashish13
Copy link
Contributor

@selul PR: #111

@selul
Copy link
Contributor Author

selul commented Mar 5, 2018

@contactashish13 i'm getting this -> http://prntscr.com/in18mj

Also, please make a php unit test which should cover this.

@contactashish13
Copy link
Contributor

@selul have fixed that and also added a test case. PR: #114

The build is partially failing. I guess this is because google is restricting the hits on its feed. Can you please check?

@selul
Copy link
Contributor Author

selul commented Mar 6, 2018

I don't think is about this, I have restarted the build and it seems I get the same error. Also, I've checked the error -> https://askubuntu.com/questions/770407/curl-56-gnutls-recv-error-9-a-tls-packet-with-unexpected-length-was-recei/966040 and it seems there is nothing related to restrictions.

Also, I think the logic is a bit overcomplicated, that unparse URL is not needed and also nether breaking the URL into parts. You just need to apply -> https://developer.wordpress.org/reference/functions/add_query_arg/ with num=> $attributes['max'] and this will overwrite the initial value.

Can you refactor and use this?

@contactashish13
Copy link
Contributor

@selul thanks, you are right about that logic.

the test cases are still failing but they pass at my end.

@selul
Copy link
Contributor Author

selul commented Mar 7, 2018

@contactashish13 i have removed the unparse function as it redundant now.

@selul
Copy link
Contributor Author

selul commented Mar 7, 2018

@contactashish13 i've added this check

$check_travis = getenv( 'TRAVIS' );
if ( boolval( $check_travis ) ) {
	return;
}

To remove the test on Travis but keep using it on the local environment. It seems, indeed a Travis issue.

@selul selul closed this as completed Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request to improve or optimize an existing feature or functionality in the project
Projects
None yet
Development

No branches or pull requests

2 participants