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

Wrong response count in lists causes showing "Next page" #1663

Merged

Conversation

Falke-Design
Copy link
Contributor

Check if this PR fulfills these requirements:

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Feature change (non-breaking change which changes behaviour of an existing functionality)
  • Improvement (non-breaking change which improves functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (non-breaking performance or readability improvements)

Description

Fixes: #1659

"Lists of all kinds" / "TV show genres" have the "Next page" folder but there are no entries anymore.

This is caused because the response_count is higher then the requested size.

if response_count < response_size:
# There are no other elements to request
break

After parsing in VideoListSorted only two entries are existing, so response_count is counted wrong.

The counting happens here:

def jgrapgh_len(data_dict):
"""
Count the values in a JSON Graph dict list of dicts (break counting at first 'atom')
e.g.
data_dict = {"0": {"$type": "ref"}, "1": {"$type": "atom"}}
"""
count = 0
for value in data_dict:
if data_dict[value].get('$type') == 'atom':
break
count += 1
return count

The response from the api looks like this:
grafik

The issue is now that we have here nested entries with the property reference which contains then the type. But the count logic checks if the type is a direct child of the entry.
That causes that nested entries without reference are counted too.

Solution: remove the nesting and then check if a reference is existing

Screenshots (if appropriate):

Before:
grafik

After:
grafik

Copy link
Owner

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you for your help

@CastagnaIT CastagnaIT merged commit 663376a into CastagnaIT:master Dec 31, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List has "Next page" folder but there are no entries anymore
2 participants