A good start. Most of these are formatting comments, can you apply these comments to your other changes too.

Can you add a blank line between these.

For this and any other class attributes that are newly added, can you add a Version Added to its docstring, for example:

#: Keyword copy of username for ES ``terms`` aggregations.
#:
#: Version Added:
#:     8.0

For our class variable docstrings, we like to have a one-sentence summary, then a newline, then a description if needed:
#: Summary.
#:
#: Optional description if things need more explanation.


Can you reformat this and the rest of the docstrings.

Was the previous description for this supposed to be deleted, does it no longer apply?

We still like to keep arguments on one line per argument, e.g:
    def prepare_groups(
        self,
        user: RBUser,
    ) -> str:


Can you revert this change to go back to that format, here and below.

It seems like this is meant to use user.username_display? Is username_display even needed or can we just use username?

For lists that are expected to be immutable, where we're just reading from them and not modifying, we use Sequence[int] instead of list.

We already import django.contrib.auth.models.User at the top of the test file. Let's use that instead of re-importing here. Update this here and below.

This description got moved here from the UserIndex.

For every new method that was added, let's add a
Version Changed:
    8.0


to the docstring. It comes before the Args section.

Can you add a blank line before and after the if block. Same below.

Same comment about using Sequence and everywhere else where we return list.

Do these need to be imported here for some reason? If not we can import them at the top of the file.

Instead of hardcoding mimetypes here, which can lead to some getting forgotten (for example pdfs can also have an application/x-pdf type), lets build these programmatically using our Review UIs. For example:

from reviewboard.attachments.mimetypes import TextMimetype

code_mimetypes = set(TextMimetype.supported_mimetypes)
...
elif any(mime.startswith(p) for p in code_mimetypes)



Also you don't have access to this codebase, but we recently fleshed out document review in the new RB release, so instead of a pdf type can you switch it to document, and then for the mimetypes you can do this import:
from rbpowerpack.docreview import (OFFICE_DOC_SUPPORTED_MIMETYPES,
                                   PDF_SUPPORTED_MIMETYPES)


And then do a set union (the two variables are already sets).

I'm gonna give this and your other changes a deeper review of the implementation over the next couple days, I need to learn a bit more about search and Elasticsearch. For now can you go over your other changes and apply all the formatting feedback that I gave here.

Omg sorry I had a brain fart... we just release Review Board 8, all the "Version Added"s should say Review Board 9.0.

What are all the files added in reviewboard/templates/search/indexes/, what are they used for.

These points can be removed since the changes are specific to the implementation of the index_queryset and prepare_groups methods. If you had added, removed, or deprecated these methods then it would be appropriate to list it under the Version Changed here, since you're modifying the API of the UserIndex. 
This information about the changes to the methods themselves can just be displayed in the Version Changed for that method.

We should reconsider this filter name. We have a concept called "User Roles" that was recently added as a Power Pack feature. You can assign named roles to users and the role can appear next to the user's name throughout the UI. Eventually it would be nice to filter search by those user roles too, so we should reserve that filter name for it.

Isn't this the same thing as has_open_review_requests.

We don't need "version changed" sections for test methods. You can remove it here and for the rest of the test methods.

Test methods also don't need "Version Added" sections.

Why does this need to be SequenceType instead of Sequence?

Same comment about removing these.

Does this need to exist? Why can't we use the existing last_updated field instead.

Can you add a blank line between these (always have a blank line before comments)

Let's make an _IMAGE_MIMETYPES that uses the supported mimetypes ImageMimetype.

We don't need the + here, the strings become combined just by listing them in the ().

We don't need the + here, the strings become combined just by listing them in the (), same below.