Skip to content

Commit

Permalink
Add regex search option for text searches
Browse files Browse the repository at this point in the history
Allow user to select to use regex search for student form contents
and teacher response. This is done by checking a checkbox by the field.
Implement filter, field and widget that allow selecting between default
search method and a regex search.
Implement simple validation and error handling (also for exercise identifier,
which didn't validate regex) for regex.
Change form styling to use flex, so the checkbox and label can also be
positioned well.

Fix apluslms#87
  • Loading branch information
etanttila committed Jun 4, 2024
1 parent f7b360f commit b14475c
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 9 deletions.
104 changes: 96 additions & 8 deletions feedback/filters.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import datetime
import re

from django.db import models
import django_filters
from django import forms
from django.core.exceptions import ValidationError
from django.utils.translation import gettext_lazy as _

from django_colortag.filters import ColortagIEAndOrFilter
Expand Down Expand Up @@ -32,6 +34,17 @@ def is_empty_value(value):
)


def validate_regex(value: str):
"""Check if the value is a valid regex pattern."""
try:
re.compile(value)
except re.error as exc:
raise ValidationError(
_("Not a valid regex pattern"),
code="invalid"
) from exc


class SplitDateTimeRangeWidget(forms.MultiWidget):
"""Add support to get widgets as parameter"""
# NOTE: 'wants to be' a reimplementation of django_filters.widgets.DateRangeWidget
Expand Down Expand Up @@ -156,6 +169,76 @@ def filter(self, qs, value):
return qs.filter_flags(*value)


class ComboTextSearchWidget(forms.MultiWidget):
"""Widget for displaying a text search field with a checkbox
that allows using another search method.
The checkbox label and helptext need to be defined through widget
attributes 'bool_label' and 'bool_helptext'.
"""
template_name = "feedback/widgets/combo_textsearch_widget.html"

def __init__(self, attrs=None):
widgets = {
'text': forms.TextInput(attrs=attrs),
'regex': forms.CheckboxInput(attrs=attrs),
}
super().__init__(widgets, attrs)

def decompress(self, value) -> tuple[str, bool]:
if value is None:
return [None, None]
return value


class ComboTextSearchField(forms.MultiValueField):
widget = ComboTextSearchWidget

def __init__(self, *args, **kwargs):
fields = (
forms.CharField(required=False),
forms.BooleanField(required=False),
)
kwargs.setdefault('require_all_fields', False)
super().__init__(fields, *args, **kwargs)

def compress(self, data_list: tuple[str, bool]) -> tuple[str, bool]:
return data_list

def validate(self, value: tuple[str, bool]):
"""Validate field value, especially when regex search is used.
If an another search method is used as alternative instead of
regex or iregex, this method should be overwritten.
"""
super().validate(value)
# if regex search is selected, check that string is valid regex
if value[1]:
validate_regex(value[0])


class ComboTextSearchFilter(django_filters.CharFilter):
"""Text search filter that supports two different search methods.
By default, the filter uses the filter_text method of the queryset.
However, if the checkbox is selected, uses alternative method for
searching (which is by default case-insensitive regex search).
"""
field_class = ComboTextSearchField

def __init__(self, *args, lookup_expr=None, **kwargs):
if lookup_expr is None:
lookup_expr = 'iregex'
super().__init__(*args, lookup_expr=lookup_expr, **kwargs)

def filter(self, qs: FeedbackQuerySet, value: tuple[str, bool]) -> FeedbackQuerySet:
text_val, cb_val = value
if text_val:
if cb_val: # checkbox selected, use alternative search method
lookup = "%s__%s" % (self.field_name, self.lookup_expr)
return qs.filter(**{lookup: text_val})
# use our default method
return qs.filter_text(self.field_name, text_val)
return qs


class ContainsTextFilter(django_filters.BooleanFilter):
field_class = forms.BooleanField

Expand Down Expand Up @@ -241,15 +324,15 @@ class FeedbackFilter(django_filters.FilterSet):
path_key = django_filters.CharFilter(
lookup_expr='iregex',
label=_("Exercise identifier"),
validators=[validate_regex],
help_text=_(
"Filter based on the exercise path key (typically of the "
"format 'modulekey_chapterkey_exercisekey'). "
"The filter uses case-insensitive regular expression match."
)
)
student_text = django_filters.CharFilter(
student_text = ComboTextSearchFilter(
field_name='form_data',
method='filter_text',
label=_("Student content"),
help_text=_(
"Filter conversations based on content of the student "
Expand All @@ -259,16 +342,25 @@ class FeedbackFilter(django_filters.FilterSet):
"Otherwise the search is case-insensitive."
),
)
teacher_text = django_filters.CharFilter(
student_text.field.widget.attrs.update({
'bool_label': _('Use regex search'),
'bool_helptext': _('Override default search method and use case-insensitive regex search instead.'),
'class': 'combosearch',
})
teacher_text = ComboTextSearchFilter(
field_name='response_msg',
method='filter_text',
label=_("Teacher content"),
help_text=_(
"Filter conversations based on text in the teacher responses. "
"The operators 'AND', 'OR' and 'NOT' (case-sensitive) are supported. "
"Otherwise the search is case-insensitive."
),
)
teacher_text.field.widget.attrs.update({
'bool_label': _('Use regex search'),
'bool_helptext': _('Override default search method and use case-insensitive regex search instead.'),
'class': 'combosearch',
})
contains_text = ContainsTextFilter(
label=_("Display only feedback with text content"),
help_text=_(
Expand Down Expand Up @@ -330,7 +422,3 @@ def form(self):
form.fields['student_tags'].set_queryset(studenttags)
form.fields['paginate_by'].initial = self.data.get('paginate_by', None)
return form

@staticmethod
def filter_text(queryset, name, value):
return queryset.filter_text(name, value)
15 changes: 15 additions & 0 deletions feedback/locale/fi/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ msgstr ""
"Plural-Forms: nplurals=2; plural=(n != 1);\n"
"X-Generator: Lokalize 2.0\n"

#: feedback/filters.py
msgid "Not a valid regex pattern"
msgstr "Ei ole säännöllinen lauseke"

#: feedback/filters.py
msgid "Oldest first"
msgstr "Vanhemmat ensin"
Expand Down Expand Up @@ -81,6 +85,17 @@ msgstr ""
"'AND', 'OR' ja 'NOT' käyttöä. (Operaattorit ovat aakkoskoosta riippuvia, "
"mutta muutoin haku on aakkoskoosta riippumaton.)"

#: feedback/filters.py
msgid "Use regex search"
msgstr "Käytä regex-hakua"

#: feedback/filters.py
msgid ""
"Override default search method and use case-insensitive regex search instead."
msgstr
"Etsi tavallisen hakumenetelmän sijaan säännöllisellä lausekkeella "
"(case-insensitive regex)."

#: feedback/filters.py
msgid "Teacher content"
msgstr "Opettajan vastaus"
Expand Down
53 changes: 52 additions & 1 deletion feedback/static/feedback.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ body {
margin-top: 0.7em;
margin-bottom: 0.7em;
align-items: center;
display: flex;
}
.feedback-filter > label {
margin-bottom: 0;
Expand All @@ -27,9 +28,12 @@ body {
.fb-filter-field {
display: inline;
}
.fb-filter-field.input-flex {
width: max(min(65%, 350px), 200px) !important;
}
.input-flex > div,
.input-flex > input {
width: max(min(65%, 350px), 200px) !important;
width: max(100%, 200px) !important;
}
#filter-panel-bottom {
display: flex;
Expand All @@ -43,6 +47,19 @@ body {
float: right;
}

/* combo textsearch filter */
label:has(~ .fb-filter-field > .combosearch) {
align-self: start;
margin-top: 0.5em;
}
input[type="checkbox"].combosearch {
margin-right: 0.3em;
margin-left: 0.3em;
}
input[type="checkbox"].combosearch + label {
margin-bottom: 0;
}

/* timestamp filter */
#extra-filter-pane > div:first-child {
display: flex;
Expand All @@ -53,6 +70,30 @@ body {
width: calc(100% - min(30%, 150px) - 0.5em)
}

/* tag filters */
.tag-filter-pane > .feedback-filter {
display: inline;
}

/* flag filters */
.fb-filter-field:has(> #id_feedbackfilter_flags) {
width: 100%;
}

/* Displaying errors */
.feedback-filter:has(> .errorlist) {
flex-wrap: wrap;
border: 1px #ebccd1;
background-color: #f2dede;
border-radius: 4px;
}
.feedback-filter > .errorlist {
width: 100%;
margin-bottom: 0;
}

/* responsive for different widths */

@media (min-width: 750px) {
#filter-container {
display: grid;
Expand Down Expand Up @@ -133,10 +174,20 @@ body {
}

@media (max-width: 450px) {
.feedback-filter:has(> label.right-align),
.feedback-filter:has( #id_feedbackfilter_response_grade) {
flex-direction: column;
}
.feedback-filter label.right-align {
width: 100%;
text-align: left;
}
.feedback-filter:has( #id_feedbackfilter_response_grade) {
align-items: start;
}
.fb-filter-field.input-flex {
width: 100% !important;
}
.input-flex > div,
.input-flex > input {
width: 100% !important;
Expand Down
18 changes: 18 additions & 0 deletions feedback/templates/feedback/widgets/combo_textsearch_widget.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{% spaceless %}
{% for widget in widget.subwidgets %}
{% if not forloop.last %}
{% include widget.template_name %}
{% else %}
<div>
{% include widget.template_name %}
<label{% if widget.attrs.id %} for="{{ widget.attrs.id }}"{% endif %}
{% if widget.attrs.bool_helptext %}
data-toggle="tooltip" data-placement="right"
data-original-title="{{ widget.attrs.bool_helptext }}"
{% endif %}
>
{{ widget.attrs.bool_label }}
</label>
</div>
{% endif %}
{% endfor %}{% endspaceless %}

0 comments on commit b14475c

Please sign in to comment.