Skip to content

chore(eco): add pending deletion state for sentry app installs #101125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 8, 2025
Merged

chore(eco): add pending deletion state for sentry app installs #101125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a3d0f6d
add pending deletion state for sentry app installs
cathteng Oct 7, 2025
b4fe399
Apply suggestion from @cathteng
cathteng Oct 8, 2025
0f705c6
Apply suggestion from @cathteng
cathteng Oct 8, 2025
ff998c6
add assert
cathteng Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion static/app/actionCreators/sentryAppInstallations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function uninstallSentryApp(
install.app.slug.charAt(0).toUpperCase() + install.app.slug.slice(1);
promise.then(
() => {
addSuccessMessage(t('%s successfully uninstalled.', capitalizedAppSlug));
addSuccessMessage(t('%s successfully queued for deletion.', capitalizedAppSlug));
},
() => clearIndicators()
);
Expand Down
2 changes: 1 addition & 1 deletion static/app/types/integrations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export type SentryAppInstallation = {
organization: {
slug: string;
};
status: 'installed' | 'pending';
status: 'installed' | 'pending' | 'pending_deletion';
uuid: string;
code?: string;
};
Expand Down
5 changes: 4 additions & 1 deletion static/app/utils/integrationUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,12 @@ export const getIntegrationFeatureGate = () => {
};

export const getSentryAppInstallStatus = (install: SentryAppInstallation | undefined) => {
if (install) {
if (install && install.status !== 'pending_deletion') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we updated the APIs to return sentry apps that are pending deletion? Also, is there an enum we can reference for object status? Seems fairly generic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The status looks like this https://github.com/getsentry/sentry/pull/101125/files#diff-b2f09d35e3f0947a8312823f96c548de7361dc682c81b6f9c272b7a97408baf4R270

Everything is returned unless it has been "soft deleted", in which case the object manager will not return it

return capitalize(install.status) as IntegrationInstallationStatus;
}
if (install && install.status === 'pending_deletion') {
return 'Pending Deletion';
}
return 'Not Installed';
};

Expand Down
6 changes: 6 additions & 0 deletions static/app/views/settings/organizationIntegrations/sentryAppDetailedView.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@ describe('SentryAppDetailedView', () => {
await userEvent.click(screen.getByRole('button', {name: 'Uninstall'}));
await userEvent.click(screen.getByRole('button', {name: 'Confirm'}));
expect(deleteRequest).toHaveBeenCalledTimes(1);

expect(await screen.findAllByText('Pending Deletion')).toHaveLength(2);

expect(
await screen.findByRole('button', {name: 'Pending Deletion'})
).toBeInTheDocument();
});
});

Expand Down
18 changes: 14 additions & 4 deletions static/app/views/settings/organizationIntegrations/sentryAppDetailedView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,10 @@ export default function SentryAppDetailedView() {
setApiQueryData<SentryAppInstallation[]>(
queryClient,
makeSentryAppInstallationsQueryKey({orgSlug: organization.slug}),
(existingData = []) => existingData.filter(i => i.app.slug !== sentryApp.slug)
(existingData = []) =>
existingData.map(i =>
i.app.slug === sentryApp.slug ? {...i, status: 'pending_deletion'} : i
)
);
Comment on lines +232 to 236
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: The frontend optimistically sets an installation's status to 'pending_deletion', but the backend immediately deletes the record, causing state inconsistency.

  • Description: When a user uninstalls a Sentry App, the frontend sends a DELETE request and then optimistically updates the local state of the installation to have a status of 'pending_deletion'. However, the backend API endpoint immediately and synchronously deletes the installation record from the database, rather than marking it for asynchronous deletion. The backend does not support a 'pending_deletion' status. This mismatch causes the frontend to display an installation as 'Pending Deletion' when it has already been removed from the backend, leading to state inconsistency and potential errors on subsequent data fetches for that installation.

  • Suggested fix: Align the frontend and backend behavior. Either the backend should be updated to support asynchronous deletion with a pending_deletion status, or the frontend should be changed to immediately remove the uninstalled application from its local state upon a successful DELETE API response, instead of setting the 'pending_deletion' status.
    severity: 0.65, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bestie you don't even knowwwww

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read my PR descriptionnnnnn

} catch (error) {
addErrorMessage(t('Unable to uninstall %s', sentryApp.name));
Expand Down Expand Up @@ -306,6 +309,13 @@ export default function SentryAppDetailedView() {
(disabledFromFeatures: boolean, userHasAccess: boolean) => {
const capitalizedSlug =
integrationSlug.charAt(0).toUpperCase() + integrationSlug.slice(1);
if (install?.status === 'pending_deletion') {
return (
<StyledButton size="sm" disabled>
{t('Pending Deletion')}
</StyledButton>
);
}
if (install) {
return (
<Confirm
Expand All @@ -317,10 +327,10 @@ export default function SentryAppDetailedView() {
onConfirming={recordUninstallClicked} // called when the confirm modal opens
priority="danger"
>
<StyledUninstallButton size="sm" data-test-id="sentry-app-uninstall">
<StyledButton size="sm" data-test-id="sentry-app-uninstall">
<IconSubtract isCircled style={{marginRight: space(0.75)}} />
{t('Uninstall')}
</StyledUninstallButton>
</StyledButton>
</Confirm>
);
}
Expand Down Expand Up @@ -431,7 +441,7 @@ const InstallButton = styled(Button)`
margin-left: ${space(1)};
`;

const StyledUninstallButton = styled(Button)`
const StyledButton = styled(Button)`
color: ${p => p.theme.subText};
background: ${p => p.theme.background};

Expand Down
Loading