Merge pull request #10707 from uditijmehta/feature/duplicate-notifica…

…tions-fix

[ENG-5075] Add Admin Screen to Manage Duplicate Notifications - Refactor Duplicate Notifications

admin/base/urls.py

@@ -36,7 +36,6 @@
             re_path(r'^schema_responses/', include('admin.schema_responses.urls', namespace='schema_responses')),
             re_path(r'^registration_schemas/', include('admin.registration_schemas.urls', namespace='registration_schemas')),
             re_path(r'^cedar_metadata_templates/', include('admin.cedar.urls', namespace='cedar_metadata_templates')),
-            re_path(r'^notifications/', include('admin.notifications.urls', namespace='notifications')),
         ]),
     ),
 ]

admin/nodes/urls.py

@@ -37,4 +37,5 @@
         name='recalculate-node-storage'),
     re_path(r'^(?P<guid>[a-z0-9]+)/make_private/$', views.NodeMakePrivate.as_view(), name='make-private'),
     re_path(r'^(?P<guid>[a-z0-9]+)/make_public/$', views.NodeMakePublic.as_view(), name='make-public'),
+    re_path(r'^(?P<guid>[a-z0-9]+)/remove_notifications/$', views.NodeRemoveNotificationView.as_view(), name='node-remove-notifications'),
 ]

admin/nodes/views.py

@@ -21,6 +21,7 @@
 from admin.base.utils import change_embargo_date, validate_embargo_date
 from admin.base.views import GuidView
 from admin.base.forms import GuidForm
+from admin.notifications.views import detect_duplicate_notifications, delete_selected_notifications
 
 from api.share.utils import update_share
 from api.caching.tasks import update_storage_usage_cache
@@ -92,12 +93,30 @@ class NodeView(NodeMixin, GuidView):
     raise_exception = True
 
     def get_context_data(self, **kwargs):
-        return super().get_context_data(**{
+        context = super().get_context_data(**kwargs)
+        node = self.get_object()
+
+        detailed_duplicates = detect_duplicate_notifications(node_id=node.id)
+
+        context.update({
             'SPAM_STATUS': SpamStatus,
             'STORAGE_LIMITS': settings.StorageLimits,
-            'node': kwargs.pop('object', self.get_object()),
-        }, **kwargs)
+            'node': node,
+            'duplicates': detailed_duplicates
+        })
+
+        return context
+
+class NodeRemoveNotificationView(View):
+    def post(self, request, *args, **kwargs):
+        selected_ids = request.POST.getlist('selected_notifications')
+        if selected_ids:
+            delete_selected_notifications(selected_ids)
+            messages.success(request, 'Selected notifications were successfully deleted.')
+        else:
+            messages.error(request, 'No notifications selected for deletion.')
 
+        return redirect('nodes:node', guid=kwargs.get('guid'))
 
 class NodeSearchView(PermissionRequiredMixin, FormView):
     """ Allows authorized users to search for a node by it's guid.

admin/notifications/views.py

@@ -1,54 +1,30 @@
-from django.contrib.auth.decorators import user_passes_test
-from django.shortcuts import render, redirect
-from admin.base.utils import osf_staff_check
 from osf.models.notifications import NotificationSubscription
 from django.db.models import Count
 
 def delete_selected_notifications(selected_ids):
     NotificationSubscription.objects.filter(id__in=selected_ids).delete()
 
-def detect_duplicate_notifications():
-    duplicates = (
-        NotificationSubscription.objects.values('user', 'node', 'event_name')
-        .annotate(count=Count('id'))
-        .filter(count__gt=1)
-    )
+def detect_duplicate_notifications(node_id=None):
+    query = NotificationSubscription.objects.values('_id').annotate(count=Count('_id')).filter(count__gt=1)
+    if node_id:
+        query = query.filter(node_id=node_id)
 
     detailed_duplicates = []
-    for dup in duplicates:
+    for dup in query:
         notifications = NotificationSubscription.objects.filter(
-            user=dup['user'], node=dup['node'], event_name=dup['event_name']
+            _id=dup['_id']
         ).order_by('created')
 
         for notification in notifications:
             detailed_duplicates.append({
                 'id': notification.id,
-                'user': notification.user,
-                'node': notification.node,
+                '_id': notification._id,
                 'event_name': notification.event_name,
                 'created': notification.created,
-                'count': dup['count']
+                'count': dup['count'],
+                'email_transactional': [u._id for u in notification.email_transactional.all()],
+                'email_digest': [u._id for u in notification.email_digest.all()],
+                'none': [u._id for u in notification.none.all()]
             })
 
     return detailed_duplicates
-
-def process_duplicate_notifications(request):
-    detailed_duplicates = detect_duplicate_notifications()
-
-    if request.method == 'POST':
-        selected_ids = request.POST.getlist('selected_notifications')
-        delete_selected_notifications(selected_ids)
-        return detailed_duplicates, 'Selected duplicate notifications have been deleted.', True
-
-    return detailed_duplicates, '', False
-
-@user_passes_test(osf_staff_check)
-def handle_duplicate_notifications(request):
-    detailed_duplicates, message, is_post = process_duplicate_notifications(request)
-
-    context = {'duplicates': detailed_duplicates}
-    if is_post:
-        context['message'] = message
-        return redirect('notifications:handle_duplicate_notifications')
-
-    return render(request, 'notifications/handle_duplicate_notifications.html', context)

admin/templates/base.html

@@ -311,9 +311,6 @@
           {% if perms.osf.change_maintenancestate %}
             <li><a href="{% url 'maintenance:display' %}"><i class='fa fa-link'></i> <span>Maintenance Alerts</span></a></li>
           {% endif %}
-          {% if perms.osf.view_notification %}
-            <li><a href="{% url 'notifications:handle_duplicate_notifications' %}"><i class='fa fa-link'></i><span>Duplicate Notifications</span> </a></li>
-          {% endif %}
           </ul><!-- /.sidebar-menu -->
         </section>
         <!-- /.sidebar -->

admin/templates/nodes/node.html

@@ -104,6 +104,50 @@ <h2>{{ node.type|cut:'osf.'|title }}: <b>{{ node.title }}</b> <a href="{{ node.a
                     {% include "nodes/registration_approval.html" with registration_approval=node.registration_approval %}
                     {% include "nodes/actions.html" with actions=node.actions.all|order_by:"created" %}
                     {% include "nodes/storage_usage.html" with node=node %}
+
+                    <tr>
+                        <td colspan="2">
+                            <h3>Duplicate Notifications</h3>
+                            {% if duplicates %}
+                                <form method="post" action="{% url 'nodes:node-remove-notifications' node.guid %}">
+                                    {% csrf_token %}
+                                    <table class="table table-striped table-hover table-responsive">
+                                        <thead>
+                                            <tr>
+                                                <th>Select</th>
+                                                <th>Event Name</th>
+                                                <th>Created</th>
+                                                <th>Count</th>
+                                                <th>Email Transactional</th>
+                                                <th>Email Digest</th>
+                                                <th>None</th>
+                                            </tr>
+                                        </thead>
+                                        <tbody>
+                                        {% for notification in duplicates %}
+                                            <tr>
+                                                <td><input type="checkbox" name="selected_notifications" value="{{ notification.id }}"></td>
+                                                <td>{{ notification.event_name }}</td>
+                                                <td>{{ notification.created }}</td>
+                                                <td>{{ notification.count }}</td>
+                                                <td>{{ notification.email_transactional|join:", " }}</td>
+                                                <td>{{ notification.email_digest|join:", " }}</td>
+                                                <td>{{ notification.none|join:", " }}</td>
+                                            </tr>
+                                        {% empty %}
+                                            <tr>
+                                                <td colspan="6">No duplicate notifications found!</td>
+                                            </tr>
+                                        {% endfor %}
+                                        </tbody>
+                                    </table>
+                                    <button type="submit" class="btn btn-danger">Delete Selected</button>
+                                </form>
+                            {% else %}
+                                <p>No duplicate notifications found.</p>
+                            {% endif %}
+                        </td>
+                    </tr>
                 </tbody>
             </table>
         </div>

admin_tests/notifications/test_views.py

@@ -4,7 +4,6 @@
 from admin.notifications.views import (
     delete_selected_notifications,
     detect_duplicate_notifications,
-    process_duplicate_notifications
 )
 from tests.base import AdminTestCase
 
@@ -35,35 +34,6 @@ def test_detect_duplicate_notifications(self):
 
         duplicates = detect_duplicate_notifications()
 
-        assert len(duplicates) == 2
-        assert duplicates[0]['user'] == self.user
-        assert duplicates[0]['node'] == self.node
-        assert duplicates[0]['event_name'] == 'event1'
-        assert duplicates[0]['count'] == 2
+        print(f"Detected duplicates: {duplicates}")
 
-        non_duplicate_event = [dup for dup in duplicates if dup['event_name'] == 'event2']
-        assert len(non_duplicate_event) == 0
-
-    def test_process_duplicate_notifications_get(self):
-        request = self.request_factory.get('/fake_path')
-        request.user = self.user
-
-        detailed_duplicates, message, is_post = process_duplicate_notifications(request)
-
-        assert detailed_duplicates == []
-        assert message == ''
-        assert not is_post
-
-    def test_process_duplicate_notifications_post(self):
-        notification1 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
-        notification2 = NotificationSubscription.objects.create(user=self.user, node=self.node, event_name='event1')
-
-        request = self.request_factory.post('/fake_path', {'selected_notifications': [notification1.id]})
-        request.user = self.user
-
-        detailed_duplicates, message, is_post = process_duplicate_notifications(request)
-
-        assert message == 'Selected duplicate notifications have been deleted.'
-        assert is_post
-        assert not NotificationSubscription.objects.filter(id=notification1.id).exists()
-        assert NotificationSubscription.objects.filter(id=notification2.id).exists()
+        assert len(duplicates) == 3, f"Expected 3 duplicates, but found {len(duplicates)}"