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

Filter out doesn't work anymore #151

Closed
Preovaleo opened this issue Oct 18, 2015 · 20 comments
Closed

Filter out doesn't work anymore #151

Preovaleo opened this issue Oct 18, 2015 · 20 comments

Comments

@Preovaleo
Copy link

Hi,

i don't know since when but the "filter out" option doesn't work anymore. I have all the results.

thank you

@bastimeyer
Copy link
Member

Yeah, I've noticed this as well and I've already investigated this.

It's Twitch which is incorrectly returning all items when requesting the stream list. The GUI is working properly though and is using the right query parameters.
The weird thing is, that when the broadcaster_language parameters get ignored, the set of response headers is differing from the working requests. So I can just guess that there's a server misconfiguration on their end. Can't do anything about it 😞

To fix this, just refresh the page by clicking the button at the top or by pressing F5 or CTRL+R...

@Preovaleo
Copy link
Author

ok thank you

@bastimeyer
Copy link
Member

Reopening this...

@bastimeyer bastimeyer reopened this Oct 29, 2015
@Preovaleo
Copy link
Author

Hello, i finally understand why it doesn't work anymore
the new url is:
http://streams.twitch.tv/kraken/streams?limit=60&offset=79&broadcaster_language=en&on_site=1

yep they change api.twitch.tv to streams.twitch.tv

@Preovaleo
Copy link
Author

i think they change because streams isn't in secure http

@bastimeyer
Copy link
Member

Thanks, I'll see what I can do when I'll get the time.
Fixing this issue could also already justify a new release (v0.11.1)...

yep they change api.twitch.tv to streams.twitch.tv

I doubt that they are changing their API endpoints. For me this looks like an alternative that is still working, whereas the main one is not anymore.

i think they change because streams isn't in secure http

If you remove the on_site parameter, you'll be forced to use https.

@bastimeyer
Copy link
Member

So I found the time to track down the source of this issue:

streams.twitch.tv and api.twitch.tv both work exactly the same way, so a hostname switch isn't needed and won't fix the issue...

In a post above I said that some HTTP requests will return a correct and some will return an incorrect list of streams and that this must have something to do with their servers. This was the case a couple of days ago when you were still able to receive correct results, but now I'm unable to receive those anymore. So I guess they've disabled this feature completely.

Now to the issue... Compare these three URLs:

The first one with a single broadcaster_language value does work, the other ones with multiple values don't. %5B%5D is the encoded string [], which is used to define a parameter array.

See LanguageFilterMixin and ChannelsRoute

The GUI will either ignore the broadcaster_language parameter or will use an array of language code strings. Reducing this to only one language and a single string would be a stupid idea, if this would be the only way to "fix" this issue. I would rather remove/disable this feature again then...

@vitteloil
Copy link

Hi, I am using version 0.11.2 on Windows 7. When I select 'American' + 'French' in the language settings and select the 'filter out' option, the top channels still contain channels with other languages. Same if I restart the application or if I force a refresh of the 'page'.

Because the 'extra' channels have a correct flag as well as a correct tooltip (ex: 'This channel is in chinese'), I believe it's possible to filter them out since my settings don't include 'chinese' in this example ?

@bastimeyer
Copy link
Member

See my posts above, this feature is still broken and it has also been included in the changelogs of the last couple of releases.

Twitch has removed the support for filtering multiple languages from their API, there is nothing I can do about to fix this issue. Some language specifications have also been removed, like en-us and en-gb, so choosing those languages is pretty much useless.

I believe it's possible to filter them out

No. See #113 (comment) and #133

@Bluscream
Copy link

Why dont you filter them out locally? Like

for(JSON.stream in streams){
    if (this[language] == settings.filterlanguage) { showInTheGUI(); } 
}
? :D

@vitteloil
Copy link

Yeah that's what I thought also. But it seems it's maybe not possible ?
(Which blew my mind since the flags are displayed in the UI)
Le 13 févr. 2016 03:20, "Bluscream" notifications@github.com a écrit :

Why dont you filter them out locally? Like

for(JSON.stream in streams){
if (this[language] != settings.filterlanguage) { showInTheGUI(); }
}? :D


Reply to this email directly or view it on GitHub
#151 (comment)
.

@bastimeyer
Copy link
Member

Why dont you filter them out locally?

Removing items on the client side is a very stupid idea.
Please see the post I've already linked in my post above...
#113 (comment)

Let me explain this again in a more detailed way:
Let's say you have selected only one language and you are filtering everything else out on the client side. When requesting a set of streams from the API, your goal is of course filling the entire space (calculated by the InfiniteScrollMixin) with a single API request, which means you have an expected minimum amount of returned items. This does work well for unfiltered or server side filtered lists.
But now, since you are requesting all of the streams (no server side filtering) from the API and you are removing those which don't match your criteria afterwards, you may need to make multiple requests, so you can fill the available space on the screen.
Imagine there's only a single broadcaster online who has set your selected language and he also doesn't have any viewers. Since streams are always sorted by the number of viewers, he will most likely appear at the bottom of the entire list.
Let's say you are navigating to the "top channels" menu and overall there are 10000 broadcasters online. What's going to happen under these circumstances now? Since the maximum amount of requested streams can only be 100, you would need to make 100 consecutive API request, just to find out, that there's only a single streamer online who has set this language.
All of those requests can only be made after the previous one has successfully finished, which means you are chaining requests and the loading time will increase by a huge amount.
This is of course the worst case szenario, but even if the expected minimum amount of items of a single request can not be fulfilled, the client side filtering approach fails.

Do you still think filtering on the client side is a good idea?

Filtering has to be done on the server side by adding parameters to the API request, otherwise it's not going to work.

@Preovaleo
Copy link
Author

Hello,

Let me re-up that.
So yes it is impossible to filter out more than one language.
But we can filter out one language.

how can we do that :

  • when i check filter out :
    • disable languages check box
    • enable a dropdown with all the language

With this technique we can have only one language selected so we can re-enable the filter out option

Are you agree?

@bastimeyer
Copy link
Member

So yes it is impossible to filter out more than one language

No, Twitch has disabled this feature in their API.

But we can filter out one language.

No, see my comment above #151 (comment)
Edit: yes

This would require a change here, so that a single string is being returned instead of an array containing one string if the user has selected just a single language.

As I've already said, filtering a single language is a terrible idea. I don't want to encourage users to decide between stream languages. It would work for monolingual people, but for multilingual people it would not. And selecting multiple languages would still be broken and therefore confusing.

@bastimeyer
Copy link
Member

Added single language filtering support and a message to the settings describing the limitation.

@Preovaleo
Copy link
Author

thank you

@vitteloil
Copy link

great ! thank you !

@Bluscream
Copy link

Thanks m8 👍

@bastimeyer
Copy link
Member

Closing this now...

bastimeyer added a commit that referenced this issue Oct 1, 2016
Using a comma separated broadcaster_language list does now work.
Good job, Twitch 👍
@bastimeyer
Copy link
Member

Selecting multiple languages is now supported again...
b7209df

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants