Skip to content

Allow filtering patches by committer - #124

Open
jonasrochasilva-prog wants to merge 2 commits into
postgres:mainfrom
jonasrochasilva-prog:feat/sort-filter-by-committer
Open

jonasrochasilva-prog wants to merge 2 commits into
postgres:mainfrom
jonasrochasilva-prog:feat/sort-filter-by-committer

Conversation

@jonasrochasilva-prog

Copy link
Copy Markdown
Contributor

This builds on #123. Until that is merged, this PR also shows its
commit. Only the last commit, "Allow filtering patches by committer",
is new here.

This adds a Committer filter to the page of a commitfest. It lists the
committers of the patches in that commitfest, including committers that
are no longer active. Since that list is short, it's a plain dropdown
instead of a searchable one. It's sorted by first name, like the other
user dropdowns, because that's how the names are shown.

The filter uses the current committer of a patch. Values that are not a
number, or that are not one of the committers in the dropdown, are
ignored.

The dashboard and global search use the same filter form, but they
don't get this filter: they don't show it and ignore the committer
parameter.

Notes

  • Allow sorting by committer #112 only asks for sorting, so this goes a bit further. Together with
    the Committed status filter and the sorting from Allow sorting patches by committer #123, it lets you
    list everything one committer committed in a commitfest. If you'd
    rather not have it, Allow sorting patches by committer #123 works on its own.
  • The filter is enabled by an explicit include_committer_filter flag
    on CommitFestFilterForm and patchlist(), which defaults to
    False. I didn't use the presence of a commitfest as the switch,
    because the dashboard passes one too.
  • The filter uses Patch.committer, so it doesn't know about earlier
    committers of a patch.

🤖 Generated with Claude Code

On the page of a commitfest, the Committer column header can now be
clicked to sort the patch list by committer, just like the other
sortable columns. Combined with the Committed status filter, this makes
it easy to see who committed what in a commitfest.

Committers are sorted by last name, then first name and username, which
is the same order that the Committer model uses. Patches without a
committer are always shown last, and the patch ID is used as a
tiebreaker. Like with the other sort orders, open patches are still
shown before closed ones.

The dashboard is left unchanged, since this was only requested for the
commitfest page.

Fixes postgres#112
This adds a Committer filter to the page of a commitfest. It lists the
committers of the patches in that commitfest, including committers that
are no longer active. Since that list is short, it's a plain dropdown
instead of a searchable one. It's sorted by first name, like the other
user dropdowns, because that's how the names are shown.

The filter uses the current committer of a patch. Values that are not a
number, or that are not one of the committers in the dropdown, are
ignored.

The dashboard and global search use the same filter form, but they
don't get this filter: they don't show it and ignore the committer
parameter.
tag = forms.MultipleChoiceField(required=False, label="Tag (type to search)")
author = forms.ChoiceField(required=False, label="Author (type to search)")
reviewer = forms.ChoiceField(required=False, label="Reviewer (type to search)")
committer = forms.ChoiceField(required=False, label="Committer")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The filter bar is already pretty wide. I think we might need a redesign before we can add more filter options.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback, JelteF.

I took another look at the current filter form. Since "filter_form.html" renders all fields in a single table row, I think we might be able to address the width issue with a smaller, responsive layout change.

My initial preference would be to let the existing filters wrap into multiple rows, keeping the current Search/filter behavior and avoiding another collapsible section.

Would that be an acceptable direction, or do you think the filtering UI needs a broader redesign first?

I can prepare a small prototype before making changes to this PR.file_000000006228820ea56d3bbdfe294650.png

This branch has not been deployed

No deployments
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