Merge branch 'release/2.024.36'

 # Conflicts:
 #	kobo/apps/trash_bin/tasks.py

Author: noliveleger

jsapp/js/components/support/helpBubble.tsx

@@ -74,7 +74,8 @@ class HelpBubble extends React.Component<{}, HelpBubbleState> {
           $targetEl.parents('.help-bubble__popup').length === 0 &&
           $targetEl.parents('.help-bubble__popup-content').length === 0 &&
           $targetEl.parents('.help-bubble__row').length === 0 &&
-          $targetEl.parents('.help-bubble__row-wrapper').length === 0
+          $targetEl.parents('.help-bubble__row-wrapper').length === 0 &&
+          $targetEl.parents('.help-bubble').length === 0
         ) {
           this.close();
         }

kobo/apps/hook/models/service_definition_interface.py

@@ -1,4 +1,3 @@
-# coding: utf-8
 import json
 import os
 import re
@@ -9,15 +8,16 @@
 from ssrf_protect.ssrf_protect import SSRFProtect, SSRFProtectException
 
 from kpi.utils.log import logging
+from kpi.utils.strings import split_lines_to_list
+from .hook import Hook
+from .hook_log import HookLog
 from ..constants import (
     HOOK_LOG_FAILED,
     HOOK_LOG_SUCCESS,
     KOBO_INTERNAL_ERROR_STATUS_CODE,
     RETRIABLE_STATUS_CODES,
 )
 from ..exceptions import HookRemoteServerDownError
-from .hook import Hook
-from .hook_log import HookLog
 
 
 class ServiceDefinitionInterface(metaclass=ABCMeta):
@@ -130,12 +130,16 @@ def send(self) -> bool:
             ssrf_protect_options = {}
             if constance.config.SSRF_ALLOWED_IP_ADDRESS.strip():
                 ssrf_protect_options['allowed_ip_addresses'] = (
-                    constance.config.SSRF_ALLOWED_IP_ADDRESS.strip().split('\r\n')
+                    split_lines_to_list(
+                        constance.config.SSRF_ALLOWED_IP_ADDRESS
+                    )
                 )
 
             if constance.config.SSRF_DENIED_IP_ADDRESS.strip():
                 ssrf_protect_options['denied_ip_addresses'] = (
-                    constance.config.SSRF_DENIED_IP_ADDRESS.strip().split('\r\n')
+                    split_lines_to_list(
+                        constance.config.SSRF_DENIED_IP_ADDRESS
+                    )
                 )
 
             SSRFProtect.validate(self._hook.endpoint, options=ssrf_protect_options)

kobo/apps/hook/tests/test_ssrf.py

@@ -20,19 +20,20 @@ class SSRFHookTestCase(HookTestCase):
     @override_config(SSRF_DENIED_IP_ADDRESS='1.2.3.4')
     @responses.activate
     def test_send_with_ssrf_options(self):
-        # Create first hook
 
+        # Create first hook
         hook = self._create_hook()
 
         ServiceDefinition = hook.get_service_definition()
         submissions = self.asset.deployment.get_submissions(self.asset.owner)
         submission_id = submissions[0]['_id']
         service_definition = ServiceDefinition(hook, submission_id)
-        first_mock_response = {'error': 'not found'}
-
-        responses.add(responses.POST, hook.endpoint,
-                      status=status.HTTP_200_OK,
-                      content_type='application/json')
+        responses.add(
+            responses.POST,
+            hook.endpoint,
+            status=status.HTTP_200_OK,
+            content_type='application/json',
+        )
 
         # Try to send data to external endpoint
         # Note: it should failed because we explicitly deny 1.2.3.4 and

kobo/apps/trash_bin/tasks.py

@@ -6,7 +6,10 @@
 from django.db import transaction
 from django.db.models.signals import post_delete
 from django.utils.timezone import now
-from django_celery_beat.models import ClockedSchedule, PeriodicTask, PeriodicTasks
+from django_celery_beat.models import (
+    ClockedSchedule,
+    PeriodicTask,
+)
 from requests.exceptions import HTTPError
 
 from kobo.apps.audit_log.audit_actions import AuditAction
@@ -22,7 +25,11 @@
 from .models import TrashStatus
 from .models.account import AccountTrash
 from .models.project import ProjectTrash
-from .utils import delete_asset, replace_user_with_placeholder
+from .utils import (
+    delete_asset,
+    replace_user_with_placeholder,
+    temporarily_disconnect_signals,
+)
 
 
 @celery_app.task(
@@ -202,8 +209,6 @@ def empty_project(project_trash_id: int):
 
 @task_failure.connect(sender=empty_account)
 def empty_account_failure(sender=None, **kwargs):
-    # Force scheduler to refresh
-    PeriodicTasks.update_changed()
 
     exception = kwargs['exception']
     account_trash_id = kwargs['args'][0]
@@ -231,8 +236,6 @@ def empty_account_retry(sender=None, **kwargs):
 
 @task_failure.connect(sender=empty_project)
 def empty_project_failure(sender=None, **kwargs):
-    # Force scheduler to refresh
-    PeriodicTasks.update_changed()
 
     exception = kwargs['exception']
     project_trash_id = kwargs['args'][0]
@@ -260,27 +263,29 @@ def empty_project_retry(sender=None, **kwargs):
 
 @celery_app.task
 def garbage_collector():
-    with transaction.atomic():
-        # Remove orphan periodic tasks
-        PeriodicTask.objects.exclude(
-            pk__in=AccountTrash.objects.values_list(
-                'periodic_task_id', flat=True
-            ),
-        ).filter(
-            name__startswith=DELETE_USER_STR_PREFIX, clocked__isnull=False
-        ).delete()
-
-        PeriodicTask.objects.exclude(
-            pk__in=ProjectTrash.objects.values_list(
-                'periodic_task_id', flat=True
-            ),
-        ).filter(
-            name__startswith=DELETE_PROJECT_STR_PREFIX, clocked__isnull=False
-        ).delete()
-
-        # Then, remove clocked schedules
-        ClockedSchedule.objects.exclude(
-            pk__in=PeriodicTask.objects.filter(
-                clocked__isnull=False
-            ).values_list('clocked_id', flat=True),
-        ).delete()
+
+    with temporarily_disconnect_signals(delete=True):
+        with transaction.atomic():
+            # Remove orphan periodic tasks
+            PeriodicTask.objects.exclude(
+                pk__in=AccountTrash.objects.values_list(
+                    'periodic_task_id', flat=True
+                ),
+            ).filter(
+                name__startswith=DELETE_USER_STR_PREFIX, clocked__isnull=False
+            ).delete()
+
+            PeriodicTask.objects.exclude(
+                pk__in=ProjectTrash.objects.values_list(
+                    'periodic_task_id', flat=True
+                ),
+            ).filter(
+                name__startswith=DELETE_PROJECT_STR_PREFIX, clocked__isnull=False
+            ).delete()
+
+            # Then, remove clocked schedules
+            ClockedSchedule.objects.exclude(
+                pk__in=PeriodicTask.objects.filter(
+                    clocked__isnull=False
+                ).values_list('clocked_id', flat=True),
+            ).delete()

kobo/apps/trash_bin/utils.py

@@ -1,16 +1,21 @@
 from __future__ import annotations
 
 import json
+from contextlib import contextmanager
 from copy import deepcopy
 from datetime import timedelta
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.db import IntegrityError, models, transaction
 from django.db.models import F, Q
-from django.db.models.signals import pre_delete
-from django.utils.timezone import now
-from django_celery_beat.models import ClockedSchedule, PeriodicTask, PeriodicTasks
+from django.db.models.signals import post_delete, post_save, pre_delete, pre_save
+from django.utils import timezone
+from django_celery_beat.models import (
+    ClockedSchedule,
+    PeriodicTask,
+    PeriodicTasks,
+)
 
 from kobo.apps.audit_log.audit_actions import AuditAction
 from kobo.apps.audit_log.models import AuditLog, AuditType
@@ -98,9 +103,6 @@ def move_to_trash(
     username and primary key is retained after deleting all other data.
     """
 
-    clocked_time = now() + timedelta(days=grace_period)
-    clocked = ClockedSchedule.objects.create(clocked_time=clocked_time)
-
     (
         trash_model,
         fk_field_name,
@@ -152,25 +154,27 @@ def move_to_trash(
             )
         )
 
-    try:
+    with temporarily_disconnect_signals(save=True):
+        clocked_time = timezone.now() + timedelta(days=grace_period)
+        clocked = ClockedSchedule.objects.create(clocked_time=clocked_time)
         trash_model.objects.bulk_create(trash_objects)
+        try:
+            periodic_tasks = PeriodicTask.objects.bulk_create(
+                [
+                    PeriodicTask(
+                        clocked=clocked,
+                        name=task_name_placeholder.format(**ato.metadata),
+                        task=f'kobo.apps.trash_bin.tasks.{task}',
+                        args=json.dumps([ato.id]),
+                        one_off=True,
+                        enabled=not empty_manually,
+                    )
+                    for ato in trash_objects
+                ],
+            )
 
-        periodic_tasks = PeriodicTask.objects.bulk_create(
-            [
-                PeriodicTask(
-                    clocked=clocked,
-                    name=task_name_placeholder.format(**ato.metadata),
-                    task=f'kobo.apps.trash_bin.tasks.{task}',
-                    args=json.dumps([ato.id]),
-                    one_off=True,
-                    enabled=not empty_manually,
-                )
-                for ato in trash_objects
-            ],
-        )
-
-    except IntegrityError as e:
-        raise TrashIntegrityError
+        except IntegrityError:
+            raise TrashIntegrityError
 
     # Update relationships between periodic task and trash objects
     updated_trash_objects = []
@@ -240,17 +244,9 @@ def put_back(
             for obj_dict in objects_list
         ]
     )
-    try:
-        # Disconnect `PeriodicTasks` (plural) signal, until `PeriodicTask` (singular)
-        # delete query finishes to avoid unnecessary DB queries.
-        # see https://django-celery-beat.readthedocs.io/en/stable/reference/django-celery-beat.models.html#django_celery_beat.models.PeriodicTasks
-        pre_delete.disconnect(PeriodicTasks.changed, sender=PeriodicTask)
-        PeriodicTask.objects.only('pk').filter(pk__in=periodic_task_ids).delete()
-    finally:
-        pre_delete.connect(PeriodicTasks.changed, sender=PeriodicTask)
 
-    # Force celery beat scheduler to refresh
-    PeriodicTasks.update_changed()
+    with temporarily_disconnect_signals(delete=True):
+        PeriodicTask.objects.only('pk').filter(pk__in=periodic_task_ids).delete()
 
 
 def replace_user_with_placeholder(
@@ -293,6 +289,35 @@ def replace_user_with_placeholder(
     return placeholder_user
 
 
+@contextmanager
+def temporarily_disconnect_signals(save=False, delete=False):
+    """
+    Temporarily disconnects `PeriodicTasks` signals to prevent accumulating
+    update queries for Celery Beat while bulk operations are in progress.
+
+    See https://django-celery-beat.readthedocs.io/en/stable/reference/django-celery-beat.models.html#django_celery_beat.models.PeriodicTasks
+    """
+
+    try:
+        if delete:
+            pre_delete.disconnect(PeriodicTasks.changed, sender=PeriodicTask)
+            post_delete.disconnect(PeriodicTasks.update_changed, sender=ClockedSchedule)
+        if save:
+            pre_save.disconnect(PeriodicTasks.changed, sender=PeriodicTask)
+            post_save.disconnect(PeriodicTasks.update_changed, sender=ClockedSchedule)
+        yield
+    finally:
+        if delete:
+            post_delete.connect(PeriodicTasks.update_changed, sender=ClockedSchedule)
+            pre_delete.connect(PeriodicTasks.changed, sender=PeriodicTask)
+        if save:
+            pre_save.connect(PeriodicTasks.changed, sender=PeriodicTask)
+            post_save.connect(PeriodicTasks.update_changed, sender=ClockedSchedule)
+
+    # Force celery beat scheduler to refresh
+    PeriodicTasks.update_changed()
+
+
 def _delete_submissions(request_author: settings.AUTH_USER_MODEL, asset: 'kpi.Asset'):
 
     while True:

kpi/tests/test_utils.py

@@ -20,6 +20,7 @@
 from kpi.utils.pyxform_compatibility import allow_choice_duplicates
 from kpi.utils.query_parser import parse
 from kpi.utils.sluggify import sluggify, sluggify_label
+from kpi.utils.strings import split_lines_to_list
 from kpi.utils.xml import (
     edit_submission_xml,
     fromstring_preserve_root_xmlns,
@@ -437,6 +438,12 @@ def test_allow_choice_duplicates(self):
             == 'no'
         )
 
+    def test_split_lines_to_list(self):
+
+        value = '\r\nfoo\r\nbar\n\n'
+        expected = ['foo', 'bar']
+        assert split_lines_to_list(value) == expected
+
 
 class XmlUtilsTestCase(TestCase):
 

kpi/utils/strings.py

@@ -1,4 +1,3 @@
-# coding: utf-8
 import base64
 
 
@@ -10,3 +9,8 @@ def to_str(obj):
     if isinstance(obj, bytes):
         return obj.decode()
     return obj
+
+
+def split_lines_to_list(value: str) -> list:
+    values = value.strip().split('\n')
+    return [ip.strip() for ip in values if ip.strip()]