diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 079fe93e5642ab..9fcef0a5a5b605 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ replays: 0007_organizationmember_replay_access seer: 0007_add_extras_to_nightshiftrun -sentry: 1070_increase_integration_external_id_length +sentry: 1071_add_broadcast_sync_locked social_auth: 0003_social_auth_json_field diff --git a/src/sentry/api/endpoints/broadcast_details.py b/src/sentry/api/endpoints/broadcast_details.py index cc04ae9429f130..7bc745a440e6e6 100644 --- a/src/sentry/api/endpoints/broadcast_details.py +++ b/src/sentry/api/endpoints/broadcast_details.py @@ -15,6 +15,12 @@ logger = logging.getLogger("sentry") +# Broadcast fields refreshed by the getsentry changelog sync job. Editing any of +# these on a changelog-sourced broadcast locks it from future auto-updates. +SYNC_MANAGED_FIELDS = frozenset( + {"title", "message", "link", "media_url", "category", "is_active", "date_expires"} +) + from rest_framework.request import Request from rest_framework.response import Response @@ -93,8 +99,18 @@ def put(self, request: Request, broadcast: Broadcast) -> Response: update_kwargs["media_url"] = result["mediaUrl"] if result.get("category"): update_kwargs["category"] = result["category"] + if result.get("syncLocked") is not None: + update_kwargs["sync_locked"] = result["syncLocked"] if update_kwargs: + if ( + broadcast.upstream_id + and not broadcast.sync_locked + and "sync_locked" not in update_kwargs + and SYNC_MANAGED_FIELDS & update_kwargs.keys() + ): + update_kwargs["sync_locked"] = True + with transaction.atomic(using=router.db_for_write(Broadcast)): broadcast.update(**update_kwargs) logger.info( diff --git a/src/sentry/api/serializers/models/broadcast.py b/src/sentry/api/serializers/models/broadcast.py index 4c6d5a041c2369..01dd18bac6b002 100644 --- a/src/sentry/api/serializers/models/broadcast.py +++ b/src/sentry/api/serializers/models/broadcast.py @@ -52,4 +52,6 @@ def serialize(self, obj, attrs, user, **kwargs): context = super().serialize(obj, attrs, user) context["userCount"] = attrs["user_count"] context["createdBy"] = obj.created_by_id.email if obj.created_by_id else None + context["upstreamId"] = obj.upstream_id + context["syncLocked"] = obj.sync_locked return context diff --git a/src/sentry/api/validators/broadcast.py b/src/sentry/api/validators/broadcast.py index c464c064f46f7c..ebb6ff675b4546 100644 --- a/src/sentry/api/validators/broadcast.py +++ b/src/sentry/api/validators/broadcast.py @@ -16,3 +16,4 @@ class AdminBroadcastValidator(BroadcastValidator): cta = serializers.CharField(max_length=256, required=False) mediaUrl = serializers.URLField(required=False) category = serializers.ChoiceField(choices=BROADCAST_CATEGORIES, required=False) + syncLocked = serializers.BooleanField(required=False) diff --git a/src/sentry/migrations/1071_add_broadcast_sync_locked.py b/src/sentry/migrations/1071_add_broadcast_sync_locked.py new file mode 100644 index 00000000000000..708b5b915e7106 --- /dev/null +++ b/src/sentry/migrations/1071_add_broadcast_sync_locked.py @@ -0,0 +1,33 @@ +# Generated by Django 5.2.12 on 2026-04-20 00:00 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "1070_increase_integration_external_id_length"), + ] + + operations = [ + migrations.AddField( + model_name="broadcast", + name="sync_locked", + field=models.BooleanField(default=False, db_default=False), + ), + ] diff --git a/src/sentry/models/broadcast.py b/src/sentry/models/broadcast.py index 6aec7ed5032f88..07a4887a236213 100644 --- a/src/sentry/models/broadcast.py +++ b/src/sentry/models/broadcast.py @@ -34,6 +34,10 @@ class Broadcast(Model): media_url = models.URLField(null=True, blank=True) category = models.CharField(choices=BROADCAST_CATEGORIES, max_length=32, null=True, blank=True) created_by_id = FlexibleForeignKey("sentry.User", null=True, on_delete=models.SET_NULL) + # When True, the hourly changelog sync job in getsentry skips updates to this + # broadcast. Flipped automatically when an admin edits a sync-managed field + # on a changelog-sourced broadcast; can be cleared to re-enable sync. + sync_locked = models.BooleanField(default=False, db_default=False) class Meta: app_label = "sentry" diff --git a/static/gsAdmin/views/broadcastDetails.tsx b/static/gsAdmin/views/broadcastDetails.tsx index de5fac6aaed467..d8eb482aa33e6b 100644 --- a/static/gsAdmin/views/broadcastDetails.tsx +++ b/static/gsAdmin/views/broadcastDetails.tsx @@ -1,5 +1,7 @@ +import {useState} from 'react'; import moment from 'moment-timezone'; +import {Alert} from '@sentry/scraps/alert'; import {ExternalLink} from '@sentry/scraps/link'; import { @@ -8,6 +10,11 @@ import { addSuccessMessage, clearIndicators, } from 'sentry/actionCreators/indicator'; +import {ApiForm} from 'sentry/components/forms/apiForm'; +import {BooleanField} from 'sentry/components/forms/fields/booleanField'; +import {DateTimeField} from 'sentry/components/forms/fields/dateTimeField'; +import {SelectField} from 'sentry/components/forms/fields/selectField'; +import {TextField} from 'sentry/components/forms/fields/textField'; import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {ConfigStore} from 'sentry/stores/configStore'; @@ -18,6 +25,7 @@ import {useParams} from 'sentry/utils/useParams'; import {DetailLabel} from 'admin/components/detailLabel'; import {DetailList} from 'admin/components/detailList'; +import type {ActionItem, BadgeItem} from 'admin/components/detailsPage'; import {DetailsPage} from 'admin/components/detailsPage'; import { ALL_PLANCHOICES, @@ -29,10 +37,21 @@ import { TRIALCHOICES, } from 'getsentry/utils/broadcasts'; +const fieldProps = { + stacked: true, + inline: false, + flexibleControlStateSize: true, +} as const; + +// Fields the backend accepts as null to clear the stored value. +// Mirror `allow_null=True` in AdminBroadcastValidator. +const NULLABLE_FIELDS = new Set(['dateExpires']); + export function BroadcastDetails() { const {broadcastId} = useParams<{broadcastId: string}>(); const api = useApi(); const queryClient = useQueryClient(); + const [isEditing, setIsEditing] = useState(false); const {data, isPending, isError, refetch} = useApiQuery( [ @@ -53,6 +72,9 @@ export function BroadcastDetails() { return ; } + const isAdmin = ConfigStore.get('user').permissions.has('broadcasts.admin'); + const fromChangelog = Boolean(data.upstreamId); + const onUpdate = async (params: Record) => { addLoadingMessage('Saving Changes...'); try { @@ -61,9 +83,7 @@ export function BroadcastDetails() { data: params, }); clearIndicators(); - addSuccessMessage( - `Customer account has been updated with ${JSON.stringify(params)}.` - ); + addSuccessMessage('Broadcast updated.'); setApiQueryData>( queryClient, [ @@ -78,10 +98,13 @@ export function BroadcastDetails() { ); } catch { clearIndicators(); - addErrorMessage('There was an internal error with updating the customer account.'); + addErrorMessage('There was an internal error updating this broadcast.'); } }; + const toOptions = (choices: ReadonlyArray) => + choices.map(c => ({value: c[0]!, label: c[1]!})); + const formatData = (item: string[] | string, choices: any) => { if (Array.isArray(item)) { if (item.length === 0) { @@ -99,75 +122,242 @@ export function BroadcastDetails() { const overviewSection = ( {data.title} - {data.message} - {data.link} - {data.mediaUrl ?? '-'} - {formatData(data.category, CATEGORYCHOICES)} - {formatData(data.roles, ROLECHOICES)} - {formatData(data.plans, ALL_PLANCHOICES)} - {formatData(data.trialStatus, TRIALCHOICES)} - {data.earlyAdopter ? 'Yes' : '-'} - {formatData(data.region, REGIONCHOICES)} - {formatData(data.platform, PLATFORMCHOICES)} - {formatData(data.product, PRODUCTCHOICES)} - {data.dateExpires ? moment(data.dateExpires).fromNow() : '∞'} - {data.isActive ? 'Active' : 'Inactive'} + + ); + + const editSection = ( + ) => { + const payload: Record = {}; + for (const [key, value] of Object.entries(formData)) { + if (value === '' || value === undefined) { + continue; + } + if (value === null && !NULLABLE_FIELDS.has(key)) { + continue; + } + payload[key] = value; + } + return payload; + }} + onSubmitSuccess={() => { + addSuccessMessage('Broadcast updated.'); + refetch(); + setIsEditing(false); + }} + onSubmitError={error => { + const detail = + error?.responseJSON?.detail || + JSON.stringify(error?.responseJSON) || + 'Unknown error'; + addErrorMessage(`Save failed: ${detail}`); + }} + onCancel={() => setIsEditing(false)} + submitLabel="Save Changes" + cancelLabel="Cancel" + initialData={{ + title: data.title, + message: data.message, + link: data.link, + mediaUrl: data.mediaUrl ?? undefined, + category: data.category ?? undefined, + isActive: data.isActive, + dateExpires: data.dateExpires ?? null, + roles: data.roles ?? [], + plans: data.plans ?? [], + trialStatus: data.trialStatus ?? undefined, + earlyAdopter: Boolean(data.earlyAdopter), + region: data.region ?? [], + platform: data.platform ?? [], + product: data.product ?? [], + }} + > + {fromChangelog && !data.syncLocked && ( + + + This broadcast was created from the changelog. Saving edits will lock it from + future hourly syncs. + + + )} + {fromChangelog && data.syncLocked && ( + + + Changelog sync is locked for this broadcast. Use “Re-enable changelog sync” to + let the hourly job refresh it again. + + + )} + + + + + + + + + + + + + + + + + ); + + const metadataSection = ( + {data.userCount?.toLocaleString()} user(s) - {data.createdBy && {data.createdBy}} + {data.upstreamId && ( + {data.upstreamId} + )} + {fromChangelog && ( + + {data.syncLocked ? 'Locked (manual edits)' : 'Auto-synced from changelog'} + + )} ); + const actions: ActionItem[] = [ + { + key: 'edit-broadcast', + name: 'Edit Broadcast', + help: fromChangelog + ? 'Edit broadcast content. Saving will lock this broadcast from future changelog syncs.' + : 'Edit broadcast content.', + visible: isAdmin && !isEditing, + skipConfirmModal: true, + onAction: () => setIsEditing(true), + }, + { + key: 'toggle-activation', + name: `${data.isActive ? 'Deactivate' : 'Activate'} Broadcast`, + help: data.isActive + ? 'Hide this broadcast from users.' + : "Show this broadcast to users (if it hasn't expired).", + visible: isAdmin, + onAction: () => onUpdate({isActive: !data.isActive}), + }, + { + key: 'unlock-sync', + name: 'Re-enable changelog sync', + help: 'Allow the hourly changelog job to refresh this broadcast again. Your manual edits will be overwritten on the next sync.', + visible: isAdmin && fromChangelog && data.syncLocked && !isEditing, + onAction: () => onUpdate({syncLocked: false}), + }, + ]; + + const badges: BadgeItem[] = [ + { + name: data.isActive ? 'Enabled' : 'Disabled', + level: data.isActive ? 'success' : 'danger', + }, + ]; + if (fromChangelog) { + badges.push({ + name: data.syncLocked ? 'Sync Locked' : 'From Changelog', + level: data.syncLocked ? 'warning' : 'info', + }); + } + if (isEditing) { + badges.push({name: 'Editing', level: 'warning'}); + } + return ( onUpdate({isActive: !data.isActive}), - }, - ]} - sections={[{content: overviewSection}]} + badges={badges} + actions={actions} + sections={ + isEditing + ? [{content: editSection}, {content: metadataSection}] + : [{content: overviewSection}, {content: metadataSection}] + } /> ); } diff --git a/tests/sentry/api/endpoints/test_broadcast_details.py b/tests/sentry/api/endpoints/test_broadcast_details.py index 7ab769e88210c7..8ce2b5943b0885 100644 --- a/tests/sentry/api/endpoints/test_broadcast_details.py +++ b/tests/sentry/api/endpoints/test_broadcast_details.py @@ -63,3 +63,112 @@ def test_superuser(self) -> None: assert broadcast1.message == "foobar" broadcast2 = Broadcast.objects.get(id=broadcast2.id) assert broadcast2.message == "foo" + + def test_edit_changelog_broadcast_locks_sync(self) -> None: + broadcast = Broadcast.objects.create( + upstream_id="changelog-abc", + title="Orig", + message="Orig message", + link="https://sentry.io/changelog/orig/", + is_active=True, + ) + + self.add_user_permission(user=self.user, permission="broadcasts.admin") + self.login_as(user=self.user, superuser=True) + + response = self.client.put( + f"/api/0/broadcasts/{broadcast.id}/", + { + "title": "Fixed typo", + "message": broadcast.message, + "link": broadcast.link, + }, + ) + assert response.status_code == 200 + + broadcast.refresh_from_db() + assert broadcast.title == "Fixed typo" + assert broadcast.sync_locked is True + + def test_edit_non_changelog_broadcast_does_not_lock(self) -> None: + broadcast = Broadcast.objects.create( + title="Manual", + message="Manual", + link="https://sentry.io/", + is_active=True, + ) + + self.add_user_permission(user=self.user, permission="broadcasts.admin") + self.login_as(user=self.user, superuser=True) + + response = self.client.put( + f"/api/0/broadcasts/{broadcast.id}/", + {"title": "Updated", "message": broadcast.message, "link": broadcast.link}, + ) + assert response.status_code == 200 + + broadcast.refresh_from_db() + assert broadcast.sync_locked is False + + def test_hasseen_only_does_not_lock(self) -> None: + broadcast = Broadcast.objects.create( + upstream_id="changelog-xyz", + title="Orig", + message="Orig", + link="https://sentry.io/changelog/x/", + is_active=True, + ) + + self.add_user_permission(user=self.user, permission="broadcasts.admin") + self.login_as(user=self.user, superuser=True) + + response = self.client.put(f"/api/0/broadcasts/{broadcast.id}/", {"hasSeen": "1"}) + assert response.status_code == 200 + + broadcast.refresh_from_db() + assert broadcast.sync_locked is False + + def test_explicit_unlock_clears_sync_locked(self) -> None: + broadcast = Broadcast.objects.create( + upstream_id="changelog-unlock", + title="Orig", + message="Orig", + link="https://sentry.io/changelog/u/", + is_active=True, + sync_locked=True, + ) + + self.add_user_permission(user=self.user, permission="broadcasts.admin") + self.login_as(user=self.user, superuser=True) + + response = self.client.put( + f"/api/0/broadcasts/{broadcast.id}/", + { + "title": broadcast.title, + "message": broadcast.message, + "link": broadcast.link, + "syncLocked": False, + }, + ) + assert response.status_code == 200 + + broadcast.refresh_from_db() + assert broadcast.sync_locked is False + + def test_serializer_exposes_sync_fields(self) -> None: + broadcast = Broadcast.objects.create( + upstream_id="changelog-serialize", + title="T", + message="M", + link="https://sentry.io/changelog/s/", + is_active=True, + sync_locked=True, + ) + + self.add_user_permission(user=self.user, permission="broadcasts.admin") + self.login_as(user=self.user, superuser=True) + + response = self.client.get(f"/api/0/broadcasts/{broadcast.id}/") + assert response.status_code == 200 + assert response.data["upstreamId"] == "changelog-serialize" + assert response.data["syncLocked"] is True