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

Move dashboard API filter fields to model queryset filter fields #11330

Merged
merged 2 commits into from
May 21, 2024

Conversation

agjohnson
Copy link
Contributor

This moves a lot of dashboard JS/API logic into our standard FilterSet
instances:

  • Project listing filter field
  • Version listing filter field
  • Build list version listing filter field

To accomplish this, a few helper classes were added for making the
fields easier to work with and display.

Template and JS were updated in

This moves a lot of dashboard JS/API logic into our standard FilterSet
instances:

- Project listing filter field
- Version listing filter field
- Build list version listing filter field

To accomplish this, a few helper classes were added for making the
fields easier to work with and display.

Template and JS were updated in

- readthedocs/ext-theme#351
@agjohnson agjohnson requested a review from a team as a code owner May 17, 2024 19:46
@agjohnson agjohnson requested a review from stsewd May 17, 2024 19:46
@agjohnson agjohnson requested review from humitos and removed request for stsewd May 17, 2024 19:47
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Looks good. I'm not sure to understand this 100% because of django filter deepness, but I didn't see anything that called my attention 👍🏼

@@ -36,7 +37,13 @@ class BuildListFilter(FilterSet):
)

# Attribute filter fields
version = CharFilter(field_name="version__slug", widget=HiddenInput)
version__slug = FilteredModelChoiceFilter(
Copy link
Member

Choose a reason for hiding this comment

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

I like this pattern using model__field 👍🏼

Comment on lines +106 to +108
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

@humitos humitos merged commit 8e2bcb9 into main May 21, 2024
7 checks passed
@humitos humitos deleted the agj/filter-form-ux branch May 21, 2024 09:29
Copy link

sentry-io bot commented May 21, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: ProjectVersionListFilterSet.get_visibility() takes 2 positional arguments but 4 were given /projects/{project_slug}/ View Issue

Did you find this useful? React with a 👍 or 👎

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.

None yet

2 participants