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

Make community_list endpoint support required functionality #863

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

anttimaki
Copy link
Collaborator

Moving from placeholder implementation to the real deal on the client side has revealed some shortcomings in the functionality and the returned data of this endpoint.

  • Add support for varying page size via page_size query param
  • Add support for filtering based on name and description fields via search query param
  • Add support for ordering by name and datetime_created fields
  • Add datetime_created and icon_url to serializer
  • Fields that can return null due to having Null=True in model definition are marked as optional in the serializer
  • Add naive implementations for Community's total_package_count and total_downloads_count properties
  • Add less naive implementation for icon_url property

Refs TS-1815

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04% 🎉

Comparison is base (1e88847) 92.00% compared to head (03b4275) 92.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #863      +/-   ##
==========================================
+ Coverage   92.00%   92.04%   +0.04%     
==========================================
  Files         252      252              
  Lines        7289     7307      +18     
  Branches      702      707       +5     
==========================================
+ Hits         6706     6726      +20     
+ Misses        486      485       -1     
+ Partials       97       96       -1     
Files Changed Coverage Δ
...understore/api/cyberstorm/serializers/community.py 100.00% <100.00%> (ø)
...hunderstore/api/cyberstorm/views/community_list.py 100.00% <100.00%> (ø)
django/thunderstore/community/models/community.py 92.72% <100.00%> (+2.10%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

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

Approved with comments, feel free to merge once addressed

django/thunderstore/api/cyberstorm/views/community_list.py Outdated Show resolved Hide resolved
django/thunderstore/community/models/community.py Outdated Show resolved Hide resolved
@anttimaki anttimaki force-pushed the cyberstorm-communitylist branch 2 times, most recently from 2a9641c to 7e1f1ac Compare September 19, 2023 08:03
Moving from placeholder implementation to the real deal on the client
side has revealed some shortcomings in the functionality and the
returned data of this endpoint.

- Add support for varying page size via page_size query param
- Add support for filtering based on name and description fields via
  search query param
- Add support for ordering by name and datetime_created fields
- Add datetime_created and icon_url to serializer
- Fields that can return null due to having Null=True in model
  definition are marked as optional in the serializer
- Add naive implementations for Community's total_package_count and
  total_downloads_count properties
- Add less naive implementation for icon_url property

Refs TS-1815
@anttimaki
Copy link
Collaborator Author

  • removed page_size_query_param and max_page_size
  • changed page_size to 150 to circumvent the lack of pagination on client side, for now
  • added the debug check as requested and updated/augmented unit tests

@anttimaki anttimaki merged commit 0399518 into master Sep 19, 2023
19 checks passed
@anttimaki anttimaki deleted the cyberstorm-communitylist branch September 19, 2023 08:47
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.

2 participants