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

videodb: fix smart playlists not honoring "Show empty TV shows" setting. #8824

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@ghost
Copy link
Contributor

commented Jan 10, 2016

Looks like moving the setting check from GetTvShowsByWhere to GetTvShowsNav in
PR 5752 broke smart playlists as they were previously honoring the advanced
setting (hideemptyseries) and then the GUI one.
Previous behavior is restored by adding additional logic in GetItems.

@Montellese Didn't feel like making filter non-const considering this is only needed in one case out of 16, are you ok with this?

{
Filter filter2(filter);
if (!CSettings::GetInstance().GetBool(CSettings::SETTING_VIDEOLIBRARY_SHOWEMPTYTVSHOWS))
filter2.AppendWhere("totalCount IS NOT NULL AND totalCount > 0");

This comment has been minimized.

Copy link
@Montellese

Montellese Jan 16, 2016

Member

Maybe it would be better to introduce a URL option like showempty or hideempty which could be appended to the base URL and then the actual SQL implementation could be hidden in CVideoDatabase::GetFilter() otherwise we end up with the same SQL query hacks in different code locations.

videodb: fix smart playlists not honoring "Show empty TV shows" setting.
Looks like moving the setting check from GetTvShowsByWhere to GetTvShowsNav in
PR 5752 broke smart playlists as they were previously honoring the advanced
setting (hideemptyseries) and then the GUI one.
Previous behavior is restored by adding additional logic in GetItems.

@anaconda anaconda force-pushed the menakite:5752-fix-smartplaylist branch from ef513ec to b9ae916 May 7, 2016

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jun 12, 2016

Ping @anaconda

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Oct 7, 2016

What is the plan for this PR - I've had it in my builds for months but will drop it if there are no immediate plans to merge this now.

@Montellese

This comment has been minimized.

Copy link
Member

commented Oct 7, 2016

I'd prefer it if my comment was considered. But apart from that I'm all for fixing this.

@da-anda

This comment has been minimized.

Copy link
Member

commented Jan 22, 2017

@anaconda if you have some time, mind finishing this PR? Time to get our list of open PRs down a bit :)

@Razzeee

This comment has been minimized.

Copy link
Member

commented Apr 11, 2017

@anaconda
I'll close this as you don't seem to be around
If you want to revisit this, just reopen the PR! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.