Skip to content

fix: fallback on CursorPagination ordering if unset on the view #8954

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

Merged
merged 1 commit into from
May 2, 2023

Conversation

fdomain
Copy link
Contributor

@fdomain fdomain commented Apr 19, 2023

Description

#8953

  • this commit fixes the usage of a CursorPagination combined with a view implementing an ordering filter, without a default ordering value.
  • former behavior was to fetch the ordering value from the filter, and raises an error if the value was None, preventing the fallback on the ordering set on the CursorPagination class itself.
  • we reversed the logic by getting first the value set on the class, and override it by the ordering filter if the parameter is present

* this commit fixes the usage of a CursorPagination combined with a view
  implementing an ordering filter, without a default ordering value.
* former behavior was to fetch the ordering value from the filter, and
  raises an error if the value was None, preventing the fallback on the
  ordering set on the CursorPagination class itself.
* we reversed the logic by getting first the value set on the class,
  and override it by the ordering filter if the parameter is present
@auvipy auvipy self-requested a review April 19, 2023 09:47
filter_cls=filter_cls.__name__
)
)
else:
Copy link
Member

Choose a reason for hiding this comment

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

else statement block is not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no it's not needed.

previous behavior was:

  • get the ordering from the filter if it exists, raises an error if the value is None
  • else get the ordering from the instance (self.ordering) and raise an error if None

new behavior is:

  • get the ordering from the instance (default case)
  • get the ordering from the filter if it exists, override ordering value if it's not None
  • raises an error if the ordering variable is None

@fdomain fdomain requested a review from auvipy April 24, 2023 09:03
@auvipy auvipy added this to the 3.15 milestone May 2, 2023
@auvipy auvipy added the Bug label May 2, 2023
@auvipy auvipy merged commit f1a11d4 into encode:master May 2, 2023
@fdomain fdomain deleted the fix-cursor-pagination branch May 2, 2023 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants