-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix merging with premium #761
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { createScopedLogger } from "@/utils/logger"; | |||||||||||||||||||||||||||||||||||||||||||||||||
| import { getLinkingOAuth2Client } from "@/utils/gmail/client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { GOOGLE_LINKING_STATE_COOKIE_NAME } from "@/utils/gmail/constants"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { withError } from "@/utils/middleware"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { transferPremiumDuringMerge } from "@/utils/user/merge-premium"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const logger = createScopedLogger("google/linking/callback"); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -133,6 +134,13 @@ export const GET = withError(async (request: NextRequest) => { | |||||||||||||||||||||||||||||||||||||||||||||||||
| targetUserId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Transfer premium subscription before deleting the source user | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await transferPremiumDuringMerge({ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceUserId: existingAccount.userId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| targetUserId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+138
to
+143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainRun premium transfer inside the same transaction as reassignment/deletion Calling the transfer before a separate Example: await prisma.$transaction(async (tx) => {
await transferPremiumDuringMerge(
{ sourceUserId: existingAccount.userId, targetUserId },
{ tx },
);
await tx.account.update({
where: { id: existingAccount.id },
data: { userId: targetUserId },
});
await tx.emailAccount.update({
where: { accountId: existingAccount.id },
data: {
userId: targetUserId,
name: existingAccount.user.name,
email: existingAccount.user.email,
},
});
await tx.user.delete({ where: { id: existingAccount.userId } });
});Optional: Apply the same change to the Outlook linking route for parity. 🏁 Script executed: #!/bin/bash
# Verify the Outlook route also wraps merge + premium transfer in one transaction
rg -nC2 -g 'apps/web/app/api/**/linking/**/route.ts' -P 'transferPremiumDuringMerge\(|\$transaction\('Length of output: 1666 Wrap premium transfer and merge steps in a single transaction
- await transferPremiumDuringMerge({ sourceUserId, targetUserId });
- await prisma.$transaction([ …updates…, …deletion… ]);
+ await prisma.$transaction(async (tx) => {
+ await transferPremiumDuringMerge(
+ { sourceUserId, targetUserId },
+ { tx },
+ );
+ await tx.account.update({ … });
+ await tx.emailAccount.update({ … });
+ await tx.user.delete({ … });
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| await prisma.$transaction([ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| prisma.account.update({ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| where: { id: existingAccount.id }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { createScopedLogger } from "@/utils/logger"; | |
| import { OUTLOOK_LINKING_STATE_COOKIE_NAME } from "@/utils/outlook/constants"; | ||
| import { withError } from "@/utils/middleware"; | ||
| import { SafeError } from "@/utils/error"; | ||
| import { transferPremiumDuringMerge } from "@/utils/user/merge-premium"; | ||
|
|
||
| const logger = createScopedLogger("outlook/linking/callback"); | ||
|
|
||
|
|
@@ -218,6 +219,13 @@ export const GET = withError(async (request: NextRequest) => { | |
| email: providerEmail, | ||
| targetUserId, | ||
| }); | ||
|
|
||
| // Transfer premium subscription before deleting the source user | ||
| await transferPremiumDuringMerge({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Premium updates are executed outside the merge transaction, risking partial state if $transaction later fails; consider performing all writes in a single interactive transaction Prompt for AI agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. transferPremiumDuringMerge assumes a user can belong to multiple premiums; schema enforces a single premiumId per user, so connecting here will overwrite the target's current premium rather than "maintain access to both" Prompt for AI agents |
||
| sourceUserId: existingAccount.userId, | ||
| targetUserId, | ||
| }); | ||
|
|
||
|
Comment on lines
+223
to
+228
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainMake premium transfer atomic with the merge transaction Running transferPremiumDuringMerge before the $transaction risks partial state (premium moved but merge fails or vice‑versa). Execute the premium transfer inside the same Prisma transaction using an interactive transaction and pass a TransactionClient to the helper. Apply this refactor: - // Transfer premium subscription before deleting the source user
- await transferPremiumDuringMerge({
- sourceUserId: existingAccount.userId,
- targetUserId,
- });
-
- await prisma.$transaction([
- prisma.account.update({
- where: { id: existingAccount.id },
- data: { userId: targetUserId },
- }),
- prisma.emailAccount.update({
- where: { accountId: existingAccount.id },
- data: {
- userId: targetUserId,
- name: existingAccount.user.name,
- email: existingAccount.user.email,
- },
- }),
- prisma.user.delete({
- where: { id: existingAccount.userId },
- }),
- ]);
+ await prisma.$transaction(async (tx) => {
+ // Transfer premium subscription before deleting the source user
+ await transferPremiumDuringMerge(
+ { sourceUserId: existingAccount.userId, targetUserId },
+ tx,
+ );
+
+ await tx.account.update({
+ where: { id: existingAccount.id },
+ data: { userId: targetUserId },
+ });
+
+ await tx.emailAccount.update({
+ where: { accountId: existingAccount.id },
+ data: {
+ userId: targetUserId,
+ name: existingAccount.user.name,
+ email: existingAccount.user.email,
+ },
+ });
+
+ await tx.user.delete({
+ where: { id: existingAccount.userId },
+ });
+ });Update the helper to accept a transaction client (outside this file): // apps/web/utils/user/merge-premium.ts
import type { Prisma } from "@prisma/client";
import prisma from "@/utils/prisma";
export async function transferPremiumDuringMerge(
params: { sourceUserId: string; targetUserId: string },
db: Prisma.TransactionClient = prisma,
) {
// use `db` instead of `prisma` for all queries
}Optional: consider 🏁 Script executed: #!/bin/bash
rg -n -C3 'transferPremiumDuringMerge\('Length of output: 1677 Make premium transfer atomic in merge transactions
(Optional) consider 🤖 Prompt for AI Agents |
||
| await prisma.$transaction([ | ||
| prisma.account.update({ | ||
| where: { id: existingAccount.id }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Premium transfer runs outside the surrounding DB transaction, risking inconsistent state if later merge operations fail.
Prompt for AI agents