Skip to content
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

fix: Remove duplicate org users #10198

Merged
merged 3 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 packages/server/__tests__/autoJoin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const signUpVerified = async (email: string) => {
}
}

test.skip('autoJoin on multiple teams does not create duplicate `OrganizationUser`s', async () => {
test('autoJoin on multiple teams does not create duplicate `OrganizationUser`s', async () => {
const domain = `${faker.internet.domainWord()}.parabol.fun`

const email = `${faker.internet.userName()}@${domain}`.toLowerCase()
Expand Down
21 changes: 12 additions & 9 deletions packages/server/billing/helpers/adjustUserCount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,13 @@ const changePause = (inactive: boolean) => async (_orgIds: string[], user: IUser

const addUser = async (orgIds: string[], user: IUser, dataLoader: DataLoaderWorker) => {
const {id: userId} = user
const [rawOrganizations, organizationUsers] = await Promise.all([
dataLoader.get('organizations').loadMany(orgIds),
dataLoader.get('organizationUsersByUserId').load(userId)
])
const rawOrganizations = await dataLoader.get('organizations').loadMany(orgIds)
const organizations = rawOrganizations.filter(isValid)
const docs = orgIds.map((orgId) => {
const oldOrganizationUser = organizationUsers.find(
(organizationUser) => organizationUser.orgId === orgId
)
const organization = organizations.find((organization) => organization.id === orgId)!
// continue the grace period from before, if any OR set to the end of the invoice OR (if it is a free account) no grace period
return {
id: oldOrganizationUser?.id || generateUID(),
id: generateUID(),
orgId,
userId,
tier: organization.tier
Expand All @@ -83,7 +77,16 @@ const addUser = async (orgIds: string[], user: IUser, dataLoader: DataLoaderWork
await getKysely()
.insertInto('OrganizationUser')
.values(docs)
.onConflict((oc) => oc.doNothing())
.onConflict((oc) =>
oc.constraint('unique_org_user').doUpdateSet({
joinedAt: sql`CURRENT_TIMESTAMP`,
removedAt: null,
inactive: false,
role: null,
suggestedTier: null,
tier: (eb) => eb.ref('excluded.tier')
})
)
Comment on lines +80 to +89
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm mostly concerned with this part. Previously we just created a new org user when a new user joined, but now we have to reset all the old fields that should be reset.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM! i don't think we use trialStartDate anymore so we can ignore that column.

.execute()
await Promise.all(
orgIds.map((orgId) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import {Kysely, PostgresDialect, sql} from 'kysely'
import getPg from '../getPg'

export async function up() {
const pg = new Kysely<any>({
dialect: new PostgresDialect({
pool: getPg()
})
})

// get rid of duplicates
// no kysely here, because I tested the SQL directly and don't want to touch it
await sql`
DELETE FROM "OrganizationUser" d USING "OrganizationUser" k
WHERE d."userId" = k."userId"
AND d."orgId" = k."orgId"
AND (
-- keep non-removed over removed
(k."removedAt" IS NULL AND d."removedAt" IS NOT NULL)
-- or removed later
OR (k."removedAt" IS NOT NULL
AND d."removedAt" IS NOT NULL
AND (k."removedAt" > d."removedAt"
OR ((k."removedAt" = d."removedAt") AND k.id > d.id)
)
)
-- or newer non-removed
OR (k."removedAt" IS NULL AND d."removedAt" IS NULL AND k.id > d.id)
);
`.execute(pg)

await pg.schema
.alterTable('OrganizationUser')
.addUniqueConstraint('unique_org_user', ['orgId', 'userId'])
.execute()
}

export async function down() {
const pg = new Kysely<any>({
dialect: new PostgresDialect({
pool: getPg()
})
})
await pg.schema.alterTable('OrganizationUser').dropConstraint('unique_org_user').execute()
}
Loading