Skip to content

Use server side search for user fields - #2677

Draft
hansegucker wants to merge 11 commits into
e-valuation:mainfrom
hansegucker:server-side-user-search
Draft

Use server side search for user fields#2677
hansegucker wants to merge 11 commits into
e-valuation:mainfrom
hansegucker:server-side-user-search

Conversation

@hansegucker

Copy link
Copy Markdown
Collaborator

Close #1681

@richardebeling richardebeling left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks. My biggest concern is that we need to find some descriptive terminology around this (and tests), otherwise just nits. Marking as draft until we have tests.

Comment thread evap/evaluation/templates/base.html Outdated
Comment thread evap/evaluation/forms.py Outdated
Comment thread evap/evaluation/forms.py
Comment thread evap/evaluation/forms.py Outdated
Comment thread evap/staff/forms.py Outdated
Comment on lines +636 to +639
self.fields["contributor"].queryset = self.get_contributor_queryset() | UserProfile.objects.filter(
pk=self.instance.contributor.pk
)
self.fields["contributor"].widget.search_url = reverse("staff:fetch_contributor_user_profiles")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this mean that if you accidentally remove someone from the selection, you'll not be able to add them back?

How does TomSelect handle a mixture of HTML-supplied and server-supplied options? Does merge the server-options with the HTML options on an update, or will the HTML options be overwritten? If the former, maybe we can remove a large part of the specialization logic we have for "keep allowing existing participants" etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The HTML options don't get overriden.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this work then?

maybe we can remove a large part of the specialization logic we have for "keep allowing existing participants" etc.

I would imagine that if we just provide all current participants as in-HTML options, we can simplify the view logic drastically, leading to cacheability etc

Comment thread evap/staff/forms.py Outdated
Comment thread evap/staff/views.py Outdated
Comment thread evap/evaluation/templates/base.html Outdated
Comment thread evap/contributor/forms.py
@richardebeling
richardebeling marked this pull request as draft April 5, 2026 16:40
@niklasmohrin

Copy link
Copy Markdown
Member

I think this is very cool! I didn't have time to look at it closely yet, but now I will let you settle Richard's comments first :)

Comment thread evap/evaluation/models.py Outdated
Comment thread evap/evaluation/views.py
Comment thread evap/contributor/forms.py
def get_participants_queryset(cls, evaluation: Evaluation | None) -> QuerySet[UserProfile]:
queryset = UserProfile.objects.exclude(is_active=False)
if evaluation is not None and evaluation.pk is not None:
queryset = (queryset | evaluation.participants.all()).distinct()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I had hoped we would not need this anymore (by just passing the initial options in HTML) -- why is this still needed here?

Comment thread evap/evaluation/templates/base.html Outdated
Comment thread evap/staff/views.py Outdated
@hansegucker
hansegucker force-pushed the server-side-user-search branch from d69cc52 to a6a4a08 Compare May 4, 2026 19:21
@hansegucker
hansegucker force-pushed the server-side-user-search branch from a6a4a08 to 10719c9 Compare May 18, 2026 17:16
@hansegucker
hansegucker force-pushed the server-side-user-search branch from 10719c9 to 23734a6 Compare June 1, 2026 18:41

@niklasmohrin niklasmohrin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

("reviewing" to get this out of the queue, feel free to re-request)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

User selection field update

3 participants