Skip to content

Commit

Permalink
fix(workspaces): no default discovery (#3622)
Browse files Browse the repository at this point in the history
* fix(workspaces): do not enable discoverability by default

* chore(workspaces): fix test

* chore(workspaces): more tests fix
  • Loading branch information
cdriesler authored Dec 3, 2024
1 parent 9c97f91 commit 101a0b2
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 167 deletions.
3 changes: 1 addition & 2 deletions packages/frontend-2/lib/settings/composables/management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ export function useAddWorkspaceDomain() {
domain: input.domain
}
],
discoverabilityEnabled:
domains.length === 0 ? true : discoverabilityEnabled,
discoverabilityEnabled,
domainBasedMembershipProtectionEnabled,
hasAccessToSSO
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,6 @@ export = FF_WORKSPACES_MODULE_ENABLED
getWorkspace: getWorkspaceFactory({ db }),
findEmailsByUserId: findEmailsByUserIdFactory({ db }),
storeWorkspaceDomain: storeWorkspaceDomainFactory({ db }),
upsertWorkspace: upsertWorkspaceFactory({ db }),
getDomains: getWorkspaceDomainsFactory({ db }),
emitWorkspaceEvent: getEventBus().emit
})({
Expand Down
8 changes: 0 additions & 8 deletions packages/server/modules/workspaces/services/management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,12 @@ export const addDomainToWorkspaceFactory =
findEmailsByUserId,
storeWorkspaceDomain,
getWorkspace,
upsertWorkspace,
emitWorkspaceEvent,
getDomains
}: {
findEmailsByUserId: FindEmailsByUserId
storeWorkspaceDomain: StoreWorkspaceDomain
getWorkspace: GetWorkspace
upsertWorkspace: UpsertWorkspace
getDomains: GetWorkspaceDomains
emitWorkspaceEvent: EventBus['emit']
}) =>
Expand Down Expand Up @@ -533,12 +531,6 @@ export const addDomainToWorkspaceFactory =

await storeWorkspaceDomain({ workspaceDomain })

if (domains.length === 0) {
await upsertWorkspace({
workspace: { ...workspace, discoverabilityEnabled: true }
})
}

await emitWorkspaceEvent({
eventName: WorkspaceEvents.Updated,
payload: { workspace }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ export const createTestWorkspace = async (
findEmailsByUserId: findEmailsByUserIdFactory({ db }),
storeWorkspaceDomain: storeWorkspaceDomainFactory({ db }),
getWorkspace: getWorkspaceFactory({ db }),
upsertWorkspace: upsertWorkspaceFactory({ db }),
emitWorkspaceEvent: getEventBus().emit,
getDomains: getWorkspaceDomainsFactory({ db })
})({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import {
UpsertWorkspaceArgs
} from '@/modules/workspaces/domain/operations'
import { FindVerifiedEmailsByUserId } from '@/modules/core/domain/userEmails/operations'
import { EventNames } from '@/modules/shared/services/eventBus'

type WorkspaceTestContext = {
storedWorkspaces: UpsertWorkspaceArgs['workspace'][]
Expand Down Expand Up @@ -919,9 +918,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand All @@ -948,9 +944,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand Down Expand Up @@ -978,9 +971,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand Down Expand Up @@ -1008,9 +998,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand Down Expand Up @@ -1038,9 +1025,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand Down Expand Up @@ -1082,9 +1066,6 @@ describe('Workspace role services', () => {
storeWorkspaceDomain: async () => {
return
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
}
Expand Down Expand Up @@ -1134,9 +1115,6 @@ describe('Workspace role services', () => {
getDomains: async () => {
return [{ domain }] as WorkspaceDomain[]
},
upsertWorkspace: async () => {
expect.fail()
},
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -1147,139 +1125,6 @@ describe('Workspace role services', () => {

expect(storedDomains).to.be.undefined
})
it('stores the verified workspace domain, toggles workspace discoverability for first domain, emits update event', async () => {
const userId = createRandomPassword()
const workspaceId = createRandomPassword()
const domain = 'example.org'

const domainRequest = {
userId,
workspaceId,
domain
}

let storedDomains: WorkspaceDomain | undefined = undefined
let storedWorkspace: UpsertWorkspaceArgs['workspace'] | undefined = undefined
let omittedEventName: EventNames | undefined = undefined

const workspace: Workspace = {
id: workspaceId,
name: cryptoRandomString({ length: 10 }),
slug: cryptoRandomString({ length: 10 }),
logo: null,
createdAt: new Date(),
updatedAt: new Date(),
description: null,
discoverabilityEnabled: false,
domainBasedMembershipProtectionEnabled: false,
defaultProjectRole: 'stream:contributor',
defaultLogoIndex: 0
}

await addDomainToWorkspaceFactory({
findEmailsByUserId: async () =>
[{ email: `foo@${domain}`, verified: true }] as UserEmail[],
getWorkspace: async () => {
return {
role: Roles.Workspace.Admin,
userId,
...workspace
}
},

getDomains: async () => {
return []
},
upsertWorkspace: async ({ workspace }) => {
storedWorkspace = workspace
},
emitWorkspaceEvent: async ({ eventName }) => {
omittedEventName = eventName
},
storeWorkspaceDomain: async ({ workspaceDomain }) => {
storedDomains = workspaceDomain
}
})(domainRequest)

expect(storedDomains).to.not.be.undefined
expect(storedDomains!.createdByUserId).to.be.equal(userId)
expect(storedDomains!.domain).to.be.equal(domain)
expect(storedDomains!.workspaceId).to.be.equal(workspaceId)
expect(storedDomains!.verified).to.be.true

expect(storedWorkspace!.discoverabilityEnabled).to.be.true

expect(omittedEventName).to.be.equal(WorkspaceEvents.Updated)
})
it('stores the second verified domain, does NOT toggle workspace discoverability for subsequent domains', async () => {
const userId = createRandomPassword()
const workspaceId = createRandomPassword()
const domain = 'example.org'
const domain2 = 'example2.org'

const domainRequest = {
userId,
workspaceId,
domain
}

const workspaceWithoutDomains = {
id: workspaceId,
name: cryptoRandomString({ length: 10 }),
slug: cryptoRandomString({ length: 10 }),
logo: null,
createdAt: new Date(),
updatedAt: new Date(),
description: null,
discoverabilityEnabled: false,
domainBasedMembershipProtectionEnabled: false,
domains: [],
defaultProjectRole: Roles.Stream.Contributor,
defaultLogoIndex: 0
}

let workspaceData: Workspace = {
...workspaceWithoutDomains
}
const insertedDomains: WorkspaceDomain[] = []
let storedDomains: WorkspaceDomain[] = []

const addDomainToWorkspace = addDomainToWorkspaceFactory({
findEmailsByUserId: async () =>
[
{ email: `foo@${domain}`, verified: true },
{ email: `foo@${domain2}`, verified: true }
] as UserEmail[],
getWorkspace: async () => {
return {
role: Roles.Workspace.Admin,
userId,
...workspaceData
}
},
getDomains: async () => storedDomains,
upsertWorkspace: async ({ workspace }) => {
workspaceData = { ...workspaceData, ...workspace }
},
emitWorkspaceEvent: async () => {},
storeWorkspaceDomain: async ({ workspaceDomain }) => {
insertedDomains.push(workspaceDomain)
}
})
await addDomainToWorkspace(domainRequest)

expect(insertedDomains).to.have.lengthOf(1)
expect(workspaceData.discoverabilityEnabled).to.be.true

// dirty hack, im post fact storing the domain on the test object
storedDomains = insertedDomains

//faking user interaction disabling discoverability
workspaceData.discoverabilityEnabled = false
await addDomainToWorkspace({ ...domainRequest, domain: domain2 })

expect(workspaceData.discoverabilityEnabled).to.be.false
})
})
})
})

0 comments on commit 101a0b2

Please sign in to comment.