Skip to content

Commit 577be69

Browse files
[ENG-11866][ENG-11524][ENG-11871] Refactor notification campaign filters to use a dynamic query builder + Bug fixes (#11842)
* Refactor notification campaign filters to use a dynamic query builder and streamline filter handling in the UI * Refactor campaign recipient filters to use Q objects and dynamic query building * Enhance notification campaign creation UI with improved group structure and styling * Remove copy * Refactor initial state handling in filter mode to improve readability * Add manual filter validation and improve filter display in notification campaigns
1 parent de20f5b commit 577be69

6 files changed

Lines changed: 293 additions & 147 deletions

File tree

admin/notifications/views.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@
1919
from mako.parsetree import ControlLine
2020
from string import Formatter
2121
from osf.email import _render_email_html
22-
from osf.email.notification_campaign import FILTER_PRESETS, counter_subquery
22+
from osf.email.notification_campaign import FILTER_PRESETS, counter_subquery, build_query
2323
from website import settings
24+
from urllib.parse import urlencode
2425

2526

2627
def delete_selected_notifications(selected_ids):
@@ -556,6 +557,14 @@ class NotificationCampaignCreateView(CreateView):
556557
def form_valid(self, form):
557558
form.instance.created_by = self.request.user
558559

560+
if 'manual' in form.cleaned_data['filters']:
561+
if not form.cleaned_data['filters']['manual']['children']:
562+
form.add_error(
563+
'filters',
564+
'Manual filters cannot be empty.'
565+
)
566+
return self.form_invalid(form)
567+
559568
form.instance.metadata = {
560569
'filters': form.cleaned_data['filters'],
561570
'context': form.cleaned_data['context'],
@@ -620,20 +629,16 @@ class NotificationCampaignsRecipientsPreview(PermissionRequiredMixin, ListView):
620629
paginate_by = 25
621630

622631
def get_queryset(self):
623-
filters = {}
632+
query = Q()
624633
raw_filters = self.request.GET.get('filters', None)
625634
if raw_filters:
626635
json_filters = json.loads(raw_filters)
627636
if predefined := json_filters.get('predefined'):
628-
filters = FILTER_PRESETS.get(predefined, {})
637+
query = Q(**FILTER_PRESETS.get(predefined, {}))
629638
else:
630-
for item in json_filters.get('manual', []):
631-
if item['lookup'] != 'in':
632-
filters[f'{item["field"]}__{item["lookup"]}'] = item['value']
633-
else:
634-
filters[f'{item["field"]}__{item["lookup"]}'] = [value.strip() for value in item['value'].split(',')]
639+
query = build_query(json_filters.get('manual'))
635640

636-
qs = OSFUser.objects.filter(**filters)
641+
qs = OSFUser.objects.filter(query)
637642
qs = qs.annotate(
638643
guid=F('guids___id'),
639644
activity_score=Coalesce(Subquery(counter_subquery), 0)
@@ -649,8 +654,10 @@ def get_context_data(self, **kwargs):
649654
users,
650655
page_size,
651656
)
657+
658+
filters = self.request.GET.get('filters')
652659
# append search param to pagination links
653-
kwargs.update({'extra_query_params': f'&filters={self.request.GET.get("filters")}'})
660+
kwargs.update({'extra_query_params': f'&{urlencode({'filters': filters})}'})
654661
return super().get_context_data(
655662
**kwargs,
656663
page=page,
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<div class="filter-group{% if not is_root %} ms-4{% endif %}">
2+
<div class="d-flex align-items-center gap-2 mb-2">
3+
<strong>Match</strong>
4+
<span class="badge bg-secondary">{{ group.operator }}</span>
5+
</div>
6+
7+
<div class="filter-children border-start ps-3">
8+
{% for child in group.children %}
9+
{% if child.children %}
10+
{% include "notifications/campaign_filter_group.html" with group=child is_root=False %}
11+
{% else %}
12+
<div class="mb-2">
13+
<strong>{{ child.field }}</strong>
14+
{{ child.lookup }}
15+
<code>{{ child.value }}</code>
16+
</div>
17+
{% endif %}
18+
{% empty %}
19+
<div class="text-muted">No filters configured.</div>
20+
{% endfor %}
21+
</div>
22+
</div>

admin/templates/notifications/notification_campaigns_detail.html

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77
{% endblock title %}
88

99
{% block content %}
10+
<style>
11+
.filter-group {
12+
padding-left: 24px;
13+
border-left: 2px solid #000000;
14+
margin-left: 8px;
15+
}
16+
</style>
17+
1018
<div>
1119
{% if messages %}
1220
<ul>
@@ -261,30 +269,13 @@ <h4>Recipient Filters</h4>
261269

262270
{% if not "predefined" in metadata.filters %}
263271

264-
<table class="table table-bordered table-striped">
265-
<thead>
266-
<tr>
267-
<th style="width:35%;">Field</th>
268-
<th style="width:25%;">Lookup</th>
269-
<th>Value</th>
270-
</tr>
271-
</thead>
272-
<tbody>
273-
{% for filter in metadata.filters.manual %}
274-
<tr>
275-
<td>{{ filter.field }}</td>
276-
<td>{{ filter.lookup }}</td>
277-
<td><code>{{ filter.value }}</code></td>
278-
</tr>
279-
{% empty %}
280-
<tr>
281-
<td colspan="3" class="text-center">
282-
No filters configured.
283-
</td>
284-
</tr>
285-
{% endfor %}
286-
</tbody>
287-
</table>
272+
{% if not "predefined" in metadata.filters %}
273+
{% if metadata.filters.manual %}
274+
{% include "notifications/campaign_filter_group.html" with group=metadata.filters.manual is_root=True %}
275+
{% else %}
276+
<p class="text-muted">No filters configured.</p>
277+
{% endif %}
278+
{% endif %}
288279

289280
{% elif "predefined" in metadata.filters %}
290281

0 commit comments

Comments
 (0)