-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: account deletion failing issue #1509
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.
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
The PR looks good overall, but there are a few areas where error handling could be improved.
export async function deleteIntegrations(environmentId) {
const session = await getServerSession(authOptions);
if (!session) throw new AuthorizationError("Not authorized");
try {
return await deleteIntegrationsByEnvironmentId(environmentId);
} catch (error) {
console.error(error);
throw new Error("Failed to delete integrations");
}
}
const deleteAccount = async () => {
try {
setDeleting(true);
await deleteIntegrations(environmentId);
await deleteProfileAction();
await signOut();
await formbricksLogout();
} catch (error) {
toast.error("Something went wrong: " + error.message);
} finally {
setDeleting(false);
setOpen(false);
}
};
export const deleteIntegrationsByEnvironmentId = async (environmentId: string): Promise<void> => {
validateInputs([environmentId, ZId]);
try {
await prisma.integration.deleteMany({
where: {
environmentId,
},
});
} catch (error) {
if (error instanceof Prisma.PrismaClientKnownRequestError) {
throw new DatabaseError(error.message);
}
console.error(error);
throw new Error("Failed to delete integrations");
}
};These changes will help to ensure that any errors that occur during the deletion process are properly handled and communicated to the user. |
mattinannt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dhruwang normally we have prisma relations that cascade the delete. e.g. when environment gets deleted, also the surveys get deleted due to this line in the survey model:
environment Environment @relation(fields: [environmentId], references: [id], onDelete: Cascade)
this cascade delete is missing in the integration model. I think it would be the easiest and most consistent way to add this onDelete: Cascade also to the integration model
|
Got it, made the changes 😊 |
mattinannt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dhruwang thanks a lot for the changes :-) 🙏
* main: (28 commits) chore: Add Table of Contents to README (formbricks#1427) fix: account deletion failing issue (formbricks#1509) fix: remove welcome card from email preview (formbricks#1495) fix(bug): default role implemented (formbricks#1524) fix: changing description of Code Action (formbricks#1522) refactor: Migrate activity service (formbricks#1471) fix: Error in Docs navigation formbricks#1518 (formbricks#1521) feat: dynamic title and description (formbricks#1459) fix: Spelling Errors (formbricks#1517) fix: added scrollbar whenever overflowed in the settings/profile page (formbricks#1498) fix: long url not getting reset after closing modal (formbricks#1502) fix: Unexpected Behavior when Toggling Italics in Text Editor and improve clarity of formatting status (formbricks#1506) fix: zod pin validation failing (formbricks#1507) fix: Error message on Login not shown (formbricks#1508) fix: downgrade nextjs to fix error with react email (formbricks#1516) chore: downgrade next version in formbricks-com (formbricks#1513) feat: picture selection question (formbricks#1388) feat: formtribe leaderboard update as per today (formbricks#1505) fix: Added if statement for preventing use of reserved word in question ID (formbricks#1435) fix: Disabling Welcome Card leads buggy preview (formbricks#1320) ...
What does this PR do?
Account deletion was failing for the ones who were having any of the integrations for which we create a integration object associated with an environment (google sheet or airtable)
So now before deleting an account we, first delete integrations associated with it
Fixes FOR-1462
Type of change
How should this be tested?
Connect an integration (GS or airtable)
Try to delete account
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated