Merge branch 'master' into ryan953/rm-nonbadges

Author: ryan953

src/sentry/integrations/slack/tasks/send_notifications_on_activity.py

@@ -63,7 +63,7 @@ def activity_created_receiver(instance, created, **kwargs) -> None:
     """
     If an activity is created for an issue, this will trigger, and we can kick off an async process
     """
-    log_params = {"activity_id": instance.id, "created": created}
+    log_params = {"activity_id": instance.id, "activity_object_created": created}
     _default_logger.info("receiver for activity event", extra=log_params)
     if not created:
         _default_logger.info("instance is not created, skipping post processing", extra=log_params)

src/sentry/monitors/endpoints/base_monitor_details.py

@@ -256,7 +256,9 @@ def delete_monitor(self, request: Request, project: Project, monitor: Monitor) -
             for monitor_object in monitor_objects_list:
                 # randomize slug on monitor deletion to prevent re-creation side effects
                 if isinstance(monitor_object, Monitor):
-                    monitor_object.update(slug=get_random_string(length=24))
+                    new_slug = get_random_string(length=24)
+                    quotas.backend.update_monitor_slug(monitor.slug, new_slug, monitor.project_id)
+                    monitor_object.update(slug=new_slug)
 
                 schedule = RegionScheduledDeletion.schedule(
                     monitor_object, days=0, actor=request.user

src/sentry/replays/tasks.py

@@ -3,8 +3,10 @@
 import concurrent.futures as cf
 from typing import Any
 
+from google.cloud.exceptions import NotFound
+
 from sentry.replays.lib.kafka import initialize_replays_publisher
-from sentry.replays.lib.storage import filestore, storage
+from sentry.replays.lib.storage import filestore, make_video_filename, storage, storage_kv
 from sentry.replays.models import ReplayRecordingSegment
 from sentry.replays.usecases.events import archive_event
 from sentry.replays.usecases.reader import fetch_segments_metadata
@@ -54,15 +56,18 @@ def delete_replay_recording(project_id: int, replay_id: str) -> None:
     # Filestore and direct storage segments are split into two different delete operations.
     direct_storage_segments = []
     filestore_segments = []
+    video_filenames = []
     for segment in segments_from_metadata:
+        video_filenames.append(make_video_filename(segment))
         if segment.file_id:
             filestore_segments.append(segment)
         else:
             direct_storage_segments.append(segment)
 
     # Issue concurrent delete requests when interacting with a remote service provider.
-    if direct_storage_segments:
-        with cf.ThreadPoolExecutor(max_workers=100) as pool:
+    with cf.ThreadPoolExecutor(max_workers=100) as pool:
+        pool.map(_delete_if_exists, video_filenames)
+        if direct_storage_segments:
             pool.map(storage.delete, direct_storage_segments)
 
     # This will only run if "filestore" was used to store the files. This hasn't been the
@@ -81,3 +86,11 @@ def archive_replay(publisher: KafkaPublisher, project_id: int, replay_id: str) -
     # We publish manually here because we sometimes provide a managed Kafka
     # publisher interface which has its own setup and teardown behavior.
     publisher.publish("ingest-replay-events", message)
+
+
+def _delete_if_exists(filename: str) -> None:
+    """Delete the blob if it exists or silence the 404."""
+    try:
+        storage_kv.delete(filename)
+    except NotFound:
+        pass

src/sentry/services/hybrid_cloud/import_export/impl.py

@@ -5,6 +5,7 @@
 
 import traceback
 
+import sentry_sdk
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.core.serializers import deserialize, serialize
 from django.core.serializers.base import DeserializationError
@@ -325,6 +326,7 @@ def import_by_model(
                 )
 
         except DeserializationError:
+            sentry_sdk.capture_exception()
             return RpcImportError(
                 kind=RpcImportErrorKind.DeserializationFailed,
                 on=InstanceID(model_name),
@@ -349,6 +351,7 @@ def import_by_model(
                             )
                         return existing_import_chunk
                     except Exception:
+                        sentry_sdk.capture_exception()
                         return RpcImportError(
                             kind=RpcImportErrorKind.Unknown,
                             on=InstanceID(model_name),
@@ -358,20 +361,23 @@ def import_by_model(
             # All non-`ImportChunk`-related kinds of `IntegrityError` mean that the user's data was
             # not properly sanitized against collision. This could be the fault of either the import
             # logic, or the user's data itself.
+            sentry_sdk.capture_exception()
             return RpcImportError(
                 kind=RpcImportErrorKind.IntegrityError,
                 on=InstanceID(model_name),
                 reason=str(e),
             )
 
         except DatabaseError as e:
+            sentry_sdk.capture_exception()
             return RpcImportError(
                 kind=RpcImportErrorKind.DatabaseError,
                 on=InstanceID(model_name),
                 reason=str(e),
             )
 
         except Exception:
+            sentry_sdk.capture_exception()
             return RpcImportError(
                 kind=RpcImportErrorKind.Unknown,
                 on=InstanceID(model_name),
@@ -509,6 +515,7 @@ def yield_objects():
             )
 
         except Exception:
+            sentry_sdk.capture_exception()
             return RpcExportError(
                 kind=RpcExportErrorKind.Unknown,
                 on=InstanceID(model_name),

static/app/utils/useUrlParams.spec.tsx

@@ -8,14 +8,15 @@ import useUrlParams from './useUrlParams';
 jest.mock('react-router');
 jest.mock('sentry/utils/useLocation');
 
-type Query = {limit: string; page: string};
+type Query = {array: string[]; limit: string; page: string};
 
 describe('useUrlParams', () => {
   beforeEach(() => {
     jest.mocked(browserHistory.getCurrentLocation).mockReturnValue({
       query: {
         page: '3',
         limit: '50',
+        array: ['first', 'second'],
       },
     } as Location<Query>);
   });
@@ -25,6 +26,7 @@ describe('useUrlParams', () => {
 
     expect(result.current.getParamValue('page')).toBe('3');
     expect(result.current.getParamValue('limit')).toBe('50');
+    expect(result.current.getParamValue('array')).toBe('first');
     expect(result.current.getParamValue('foo')).toBeUndefined();
   });
 
@@ -54,6 +56,7 @@ describe('useUrlParams', () => {
 
     expect(browserHistory.push).toHaveBeenCalledWith({
       query: {
+        array: ['first', 'second'],
         page: '4',
         limit: '50',
       },
@@ -69,6 +72,7 @@ describe('useUrlParams', () => {
 
     expect(browserHistory.push).toHaveBeenCalledWith({
       query: {
+        array: ['first', 'second'],
         page: '4',
         limit: '50',
       },

static/app/utils/useUrlParams.tsx

@@ -20,7 +20,10 @@ function useUrlParams(defaultKey?: string, defaultValue?: string) {
   const getParamValue = useCallback(
     (key: string) => {
       const location = browserHistory.getCurrentLocation();
-      return location.query[key] ?? defaultValue;
+      // location.query.key can return string[] but we expect a singular value from this function, so we return the first string (this is picked arbitrarily) if it's string[]
+      return Array.isArray(location.query[key])
+        ? location.query[key]?.at(0) ?? defaultValue
+        : location.query[key] ?? defaultValue;
     },
     [defaultValue]
   );

tests/sentry/monitors/endpoints/test_base_monitor_details.py

@@ -857,7 +857,8 @@ def setUp(self):
         self.login_as(user=self.user)
         super().setUp()
 
-    def test_simple(self):
+    @patch("sentry.quotas.backend.update_monitor_slug")
+    def test_simple(self, update_monitor_slug):
         monitor = self._create_monitor()
         old_slug = monitor.slug
 
@@ -872,6 +873,7 @@ def test_simple(self):
         assert RegionScheduledDeletion.objects.filter(
             object_id=monitor.id, model_name="Monitor"
         ).exists()
+        update_monitor_slug.assert_called_once()
 
     def test_mismatched_org_slugs(self):
         monitor = self._create_monitor()

tests/sentry/replays/test_project_replay_details.py

@@ -7,7 +7,12 @@
 
 from sentry.models.files.file import File
 from sentry.replays.lib import kafka
-from sentry.replays.lib.storage import RecordingSegmentStorageMeta, storage
+from sentry.replays.lib.storage import (
+    RecordingSegmentStorageMeta,
+    make_video_filename,
+    storage,
+    storage_kv,
+)
 from sentry.replays.models import ReplayRecordingSegment
 from sentry.replays.testutils import assert_expected_response, mock_expected_response, mock_replay
 from sentry.testutils.cases import APITestCase, ReplaysSnubaTestCase
@@ -210,6 +215,7 @@ def test_delete_replay_from_clickhouse_data(self):
             file_id=None,
         )
         storage.set(metadata1, b"hello, world!")
+        storage_kv.set(make_video_filename(metadata1), b"hello, world!")
 
         metadata2 = RecordingSegmentStorageMeta(
             project_id=self.project.id,
@@ -219,6 +225,8 @@ def test_delete_replay_from_clickhouse_data(self):
             file_id=None,
         )
         storage.set(metadata2, b"hello, world!")
+        # Intentionally not written.
+        # storage_kv.set(make_video_filename(metadata2), b"hello, world!")
 
         metadata3 = RecordingSegmentStorageMeta(
             project_id=self.project.id,
@@ -228,6 +236,7 @@ def test_delete_replay_from_clickhouse_data(self):
             file_id=None,
         )
         storage.set(metadata3, b"hello, world!")
+        storage_kv.set(make_video_filename(metadata3), b"hello, world!")
 
         with self.feature(REPLAYS_FEATURES):
             with TaskRunner():
@@ -237,3 +246,6 @@ def test_delete_replay_from_clickhouse_data(self):
         assert storage.get(metadata1) is None
         assert storage.get(metadata2) is None
         assert storage.get(metadata3) is not None
+        assert storage_kv.get(make_video_filename(metadata1)) is None
+        assert storage_kv.get(make_video_filename(metadata2)) is None
+        assert storage_kv.get(make_video_filename(metadata3)) is not None