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.

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

Jump to bottom

Signup page leftovers #2849

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Signup page leftovers #2849

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
60e8f29
refactor(workflow): remove createCollectionFlowToken references
tomer-shvadron Nov 18, 2024
2e96012
Merge branch 'dev' into bal-3089
tomer-shvadron Nov 26, 2024
4d7664c
refactor(workflow): streamline collection flow state creation
tomer-shvadron Nov 26, 2024
9bfca66
refactor(auth): update module imports for improved path clarity
tomer-shvadron Nov 26, 2024
9426d95
Merge branch 'dev' into bal-3089
tomer-shvadron Nov 26, 2024
01de2c2
refactor(workflow): rename service variable for clarity
tomer-shvadron Nov 26, 2024
0982228
test(collection-flow): enhance integration tests for CollectionFlowNo…
tomer-shvadron Nov 26, 2024
dd2f683
test(collection-flow): enhance integration tests for collection flow …
tomer-shvadron Nov 26, 2024
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
1 change: 0 additions & 1 deletion packages/common/src/schemas/documents/workflow/config-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export const WorkflowConfigSchema = Type.Object({
availableDocuments: Type.Optional(Type.Array(AvailableDocumentSchema)),
callbackResult: Type.Optional(CallbackResultSchema),
childCallbackResults: Type.Optional(Type.Array(ChildCallbackResultSchema)),
createCollectionFlowToken: Type.Optional(Type.Boolean()),
mainRepresentative: Type.Optional(MainRepresentativeSchema),
customerName: Type.Optional(Type.String()),
enableManualCreation: Type.Optional(Type.Boolean()),
Expand Down
4 changes: 1 addition & 3 deletions packages/common/src/utils/collection-flow/build-collection-flow-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ export const buildCollectionFlowState = (inputConfig: TCollectionFlowConfig): TC
const config: TCollectionFlow['config'] = initializeConfig(inputConfig);
const state: TCollectionFlow['state'] = initializeState(inputConfig);

const collectionFlow: TCollectionFlow = {
return {
config,
state,
additionalInformation: inputConfig.additionalInformation || {},
};

return collectionFlow;
};
1 change: 0 additions & 1 deletion services/workflows-service/scripts/workflows/dynamic-ui-workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ export const dynamicUiWorkflowDefinition = {
language: 'en',
supportedLanguages: ['en', 'cn'],
initialEvent: 'START',
createCollectionFlowToken: true,
childCallbackResults: [
{
definitionId: kycEmailSessionDefinition.name,
Expand Down
1 change: 0 additions & 1 deletion services/workflows-service/scripts/workflows/kyb-kyc-workflow-definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ export const kybKycWorkflowDefinition = {
deliverEvent: 'KYC_RESPONDED',
},
],
createCollectionFlowToken: true,
},
contextSchema: {
type: 'json-schema',
Expand Down
1 change: 0 additions & 1 deletion ...n/kyb-with-associated-companies/definition/compose-child-associated-company-definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const composeChildAssociatedCompanyDefinition = ({
apiPlugins: [],
},
config: {
createCollectionFlowToken: true,
workflowLevelResolution: true,
},
projectId: projectId,
Expand Down
1 change: 0 additions & 1 deletion ...-with-associated-companies/definition/compose-kyb-with-associated-companies-definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ export const composeKybWithAssociatedCompaniesDefinition = ({
},
config: {
isExample: true,
createCollectionFlowToken: true,
workflowLevelResolution: true,
childCallbackResults: [
{
Expand Down
1 change: 0 additions & 1 deletion ...inition/kyb-with-associated-companies/definition/compose-kyc-child-workflow-definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const composeKycChildWorkflowDefinition = ({
apiPlugins: [],
},
config: {
createCollectionFlowToken: true,
workflowLevelResolution: true,
isCaseOverviewEnabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion services/workflows-service/src/auth/auth.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AuthService } from './auth.service';
import { BasicStrategy } from './basic/basic.strategy';
import { PasswordService } from './password/password.service';
// eslint-disable-next-line import/no-cycle
import { UserModule } from '../user/user.module';
import { UserModule } from '@/user/user.module';
Comment on lines 6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider extracting authentication-related functionality into a separate module

Based on the analysis of the codebase, here's how to resolve the circular dependency:

  1. Create a new AuthenticationModule that will contain:

    • PasswordService (it's self-contained and only handles password operations)
    • Authentication strategies (local.strategy.ts)
    • Session serializer

  2. Restructure the current modules:

    • AuthModule should focus on authorization and depend on UserModule and AuthenticationModule
    • UserModule should only depend on AuthenticationModule for password operations
    • Both modules can then remove their forwardRef usage

This separation follows the single responsibility principle and creates a cleaner dependency graph:

AuthenticationModule <-- UserModule
                    <-- AuthModule <-- UserModule
🔗 Analysis chain

Consider resolving the circular dependency with UserModule.

While forwardRef works as a temporary solution, circular dependencies can lead to maintenance issues and potential bugs. Consider:

  1. Extracting shared logic into a separate module
  2. Restructuring the dependencies to follow a more unidirectional flow

Let's analyze the dependency cycle:

Let me gather more information about the services and dependencies to better understand the circular dependency structure.

Let me check the specific services and their dependencies to understand what can be extracted.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Analyze the circular dependency between AuthModule and UserModule

# Check imports in UserModule
echo "Checking UserModule imports:"
rg -A 5 "import.*from.*auth" $(fd -t f "user.module.ts$")

# Check imports in AuthModule
echo "Checking AuthModule imports:"
rg -A 5 "import.*from.*user" $(fd -t f "auth.module.ts$")

Length of output: 4294

Script:

#!/bin/bash
# Check the services and dependencies in both modules

# Check AuthModule contents
echo "AuthModule contents:"
rg -A 15 "@Module\({" services/workflows-service/src/auth/auth.module.ts

# Check UserModule contents
echo "UserModule contents:"
rg -A 15 "@Module\({" services/workflows-service/src/user/user.module.ts

# Check what services from auth are used in user
echo "Auth services used in user:"
rg "import.*from.*auth/" services/workflows-service/src/user/

# Check what services from user are used in auth
echo "User services used in auth:"
rg "import.*from.*user/" services/workflows-service/src/auth/

Length of output: 2148

Script:

#!/bin/bash
# Check the specific service implementations

# Check PasswordService usage
echo "PasswordService implementation:"
ast-grep --pattern 'class PasswordService {
  $$$
}'

# Check AuthService implementation
echo "AuthService implementation:"
ast-grep --pattern 'class AuthService {
  $$$
}'

# Check UserService implementation
echo "UserService implementation:"
ast-grep --pattern 'class UserService {
  $$$
}'

# Check module decorators (fixing previous regex)
echo "Module decorators:"
rg "@Module\(\{" -A 10 services/workflows-service/src/auth/auth.module.ts services/workflows-service/src/user/user.module.ts

Length of output: 10268

import { LocalStrategy } from '@/auth/local/local.strategy';
import { SessionSerializer } from '@/auth/session-serializer';
import { UserService } from '@/user/user.service';
Expand Down
12 changes: 10 additions & 2 deletions services/workflows-service/src/auth/workflow-token/workflow-token.repository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Prisma, PrismaClient } from '@prisma/client';
import { Injectable } from '@nestjs/common';
import type { PrismaTransaction, TProjectId } from '@/types';
import { Prisma, PrismaClient } from '@prisma/client';

import { PrismaService } from '@/prisma/prisma.service';
import type { PrismaTransaction, TProjectId } from '@/types';

@Injectable()
export class WorkflowTokenRepository {
Expand All @@ -23,6 +24,13 @@ export class WorkflowTokenRepository {
});
}

async count(args: Prisma.WorkflowRuntimeDataTokenCountArgs, projectId: TProjectId) {
return await this.prismaService.workflowRuntimeDataToken.count({
...args,
where: { ...args.where, projectId },
});
}

async findFirstByWorkflowruntimeDataIdUnscoped(workflowRuntimeDataId: string) {
return await this.prismaService.workflowRuntimeDataToken.findFirst({
select: {
Expand Down
89 changes: 87 additions & 2 deletions services/workflows-service/src/auth/workflow-token/workflow-token.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,94 @@
import { Injectable } from '@nestjs/common';
import { WorkflowTokenRepository } from '@/auth/workflow-token/workflow-token.repository';
import type { PrismaTransaction, TProjectId } from '@/types';
import type { InputJsonValue, PrismaTransaction, TProjectId } from '@/types';
import { Prisma, UiDefinitionContext } from '@prisma/client';
import { buildCollectionFlowState, getOrderedSteps } from '@ballerine/common';
import { env } from '@/env';
import { WORKFLOW_FINAL_STATES } from '@/workflow/consts';
import { UiDefinitionService } from '@/ui-definition/ui-definition.service';
import { WorkflowRuntimeDataRepository } from '@/workflow/workflow-runtime-data.repository';
import { CustomerService } from '@/customer/customer.service';

@Injectable()
export class WorkflowTokenService {
constructor(private readonly workflowTokenRepository: WorkflowTokenRepository) {}
constructor(
private readonly customerService: CustomerService,
private readonly uiDefinitionService: UiDefinitionService,
private readonly workflowTokenRepository: WorkflowTokenRepository,
private readonly workflowRuntimeDataRepository: WorkflowRuntimeDataRepository,
) {}

async create(
projectId: TProjectId,
data: Parameters<typeof this.workflowTokenRepository.create>[1],
transaction?: PrismaTransaction,
) {
const { workflowRuntimeDataId } = data;

const existingTokensForWorkflowRuntime = await this.count(
{ where: { workflowRuntimeDataId } },
projectId,
);

Comment on lines +28 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential race condition when checking existing tokens

The check for existingTokensForWorkflowRuntime may lead to a race condition if multiple requests are processed concurrently, possibly resulting in multiple tokens being created when only one is intended.

Consider implementing a mutex, database-level constraint, or transaction to ensure that only one token is created per workflowRuntimeDataId.

if (existingTokensForWorkflowRuntime === 0) {
const { workflowDefinitionId, context } = await this.workflowRuntimeDataRepository.findById(
workflowRuntimeDataId,
{ select: { workflowDefinitionId: true, context: true } },
[projectId],
);

const [uiDefinition, customer] = await Promise.all([
this.uiDefinitionService.getByWorkflowDefinitionId(
workflowDefinitionId,
UiDefinitionContext.collection_flow,
[projectId],
),
this.customerService.getByProjectId(projectId),
]);

const collectionFlow = buildCollectionFlowState({
apiUrl: env.APP_API_URL,
steps: uiDefinition?.definition
? getOrderedSteps(
(uiDefinition?.definition as Prisma.JsonObject)?.definition as Record<
string,
Record<string, unknown>
>,
{ finalStates: [...WORKFLOW_FINAL_STATES] },
).map(stepName => ({
stateName: stepName,
}))
: [],
additionalInformation: {
customerCompany: customer.displayName,
},
});

const workflowToken = await this.workflowTokenRepository.create(projectId, data, transaction);

await this.workflowRuntimeDataRepository.updateStateById(
workflowRuntimeDataId,
{
data: {
context: {
...context,
collectionFlow,
metadata: {
...(context.metadata ?? {}),
token: workflowToken.token,
collectionFlowUrl: env.COLLECTION_FLOW_URL,
webUiSDKUrl: env.WEB_UI_SDK_URL,
},
} as InputJsonValue,
projectId,
},
},
transaction,
);

return workflowToken;
}

return await this.workflowTokenRepository.create(projectId, data, transaction);
}

Expand All @@ -26,6 +104,13 @@ export class WorkflowTokenService {
return await this.workflowTokenRepository.findByTokenWithExpiredUnscoped(token);
}

async count(
args: Parameters<typeof this.workflowTokenRepository.count>[0],
projectId: TProjectId,
) {
return await this.workflowTokenRepository.count(args, projectId);
}

async deleteByToken(token: string) {
return await this.workflowTokenRepository.deleteByTokenUnscoped(token);
}
Expand Down
22 changes: 20 additions & 2 deletions ...s-service/src/collection-flow/controllers/collection-flow.no-user.controller.intg.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,16 @@ import { WorkflowEventEmitterService } from '@/workflow/workflow-event-emitter.s
import { WorkflowRuntimeDataRepository } from '@/workflow/workflow-runtime-data.repository';
import { WorkflowService } from '@/workflow/workflow.service';
import { CollectionFlowNoUserController } from './collection-flow.no-user.controller';
import { UiDefinitionRepository } from '@/ui-definition/ui-definition.repository';
import { ApiKeyService } from '@/customer/api-key/api-key.service';
import { ApiKeyRepository } from '@/customer/api-key/api-key.repository';

describe('CollectionFlowSignupController', () => {
let app: INestApplication;
let prismaClient: PrismaService;
let workflowTokenService: WorkflowTokenService;
let workflowDefinitionRepository: WorkflowDefinitionRepository;
let uiDefinitionRepository: UiDefinitionRepository;
let workflowRuntimeDataRepository: WorkflowRuntimeDataRepository;
let customerRepository: CustomerRepository;
let endUserRepository: EndUserRepository;
Expand All @@ -55,8 +59,6 @@ describe('CollectionFlowSignupController', () => {
controllers: [CollectionFlowNoUserController],
providers: [
{ provide: BusinessService, useValue: noop },
{ provide: UiDefinitionService, useValue: noop },
{ provide: CustomerService, useValue: noop },
{ provide: FileService, useValue: noop },
{ provide: SalesforceService, useValue: noop },
{ provide: RiskRuleService, useValue: noop },
Expand All @@ -72,6 +74,11 @@ describe('CollectionFlowSignupController', () => {
{ provide: WorkflowEventEmitterService, useValue: { emit: noop } },
WorkflowService,
EndUserService,
UiDefinitionService,
UiDefinitionRepository,
CustomerService,
ApiKeyService,
ApiKeyRepository,
BusinessReportService,
BusinessRepository,
EntityRepository,
Expand All @@ -95,6 +102,7 @@ describe('CollectionFlowSignupController', () => {
workflowDefinitionRepository = module.get<WorkflowDefinitionRepository>(
WorkflowDefinitionRepository,
);
uiDefinitionRepository = module.get<UiDefinitionRepository>(UiDefinitionRepository);
workflowRuntimeDataRepository = module.get<WorkflowRuntimeDataRepository>(
WorkflowRuntimeDataRepository,
);
Expand Down Expand Up @@ -124,6 +132,16 @@ describe('CollectionFlowSignupController', () => {
},
});

await uiDefinitionRepository.create({
data: {
uiSchema: {},
projectId: project.id,
uiContext: 'collection_flow',
name: 'signup-test-ui-definition',
workflowDefinitionId: workflowDefinition.id,
},
});
Comment on lines +135 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

UI definition test coverage is incomplete

The test file creates a UI definition with an empty schema but doesn't verify its integration with the workflow. This is concerning because:

  • The controller implementation shows that uiDefinitionId is used in the workflow context
  • There are no assertions verifying the UI definition's presence or schema in the test responses
  • The empty uiSchema might not represent real-world scenarios

Suggested improvements:

  • Add assertions to verify the UI definition is correctly linked to the workflow
  • Test the workflow context endpoint to ensure UI definition data is properly included
  • Use a realistic UI schema that matches production scenarios
🔗 Analysis chain

Enhance test coverage for UI definition integration

While the UI definition setup is added, consider the following improvements:

  1. Add assertions to verify the UI definition's integration with the workflow
  2. Consider adding a more realistic uiSchema instead of an empty object
  3. Add test cases specifically for UI definition-related functionality

Let's check if there are any existing UI definition test cases:

Example test case to add:

it('should include UI definition in the workflow context', async () => {
  const response = await request(app.getHttpServer())
    .get('/collection-flow/context')
    .set('authorization', `Bearer ${workflowRuntimeDataToken.token}`);

  expect(response.body.context.uiDefinition).toBeDefined();
  expect(response.body.context.uiDefinition.uiSchema).toEqual({});
});
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for UI definition related test cases
rg -l "uiDefinition.*test" --type ts

Length of output: 38

Script:

#!/bin/bash
# Let's try a broader search for UI definition tests and related code
rg -l "uiDefinition" --type ts

# Also search for specific UI definition test patterns
rg "describe.*['\"].*UI.*definition" --type ts -A 5

# Search for UI schema related tests
rg "uiSchema.*test" --type ts -A 5

Length of output: 5914

Script:

#!/bin/bash
# Let's check the collection flow controller test file for context
rg "uiSchema|uiDefinition" services/workflows-service/src/collection-flow/controllers/collection-flow.no-user.controller.intg.test.ts -B 2 -A 2

# Let's also check if there are any UI definition assertions in the test file
rg "expect.*uiDefinition|expect.*uiSchema" services/workflows-service/src/collection-flow/controllers/collection-flow.no-user.controller.intg.test.ts

# Check the actual controller implementation to understand UI definition usage
rg "uiDefinition|uiSchema" services/workflows-service/src/collection-flow/controllers/collection-flow.no-user.controller.ts -B 2 -A 2

Length of output: 1272


const { id: workflowRuntimeDataId } = await workflowRuntimeDataRepository.create({
data: {
workflowDefinitionId: workflowDefinition.id,
Expand Down
29 changes: 25 additions & 4 deletions services/workflows-service/src/common/guards/token-guard/token-auth.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
import { WorkflowTokenRepository } from '@/auth/workflow-token/workflow-token.repository';
import { WorkflowTokenService } from '@/auth/workflow-token/workflow-token.service';
import { TokenAuthGuard } from '@/common/guards/token-guard/token-auth.guard';
import { Module } from '@nestjs/common';

import { CustomerService } from '@/customer/customer.service';
import { ApiKeyService } from '@/customer/api-key/api-key.service';
import { CustomerRepository } from '@/customer/customer.repository';
import { ProjectScopeService } from '@/project/project-scope.service';
import { ApiKeyRepository } from '@/customer/api-key/api-key.repository';
import { UiDefinitionService } from '@/ui-definition/ui-definition.service';
import { TokenAuthGuard } from '@/common/guards/token-guard/token-auth.guard';
import { UiDefinitionRepository } from '@/ui-definition/ui-definition.repository';
import { WorkflowTokenService } from '@/auth/workflow-token/workflow-token.service';
import { WorkflowTokenRepository } from '@/auth/workflow-token/workflow-token.repository';
import { WorkflowRuntimeDataRepository } from '@/workflow/workflow-runtime-data.repository';

@Module({
providers: [WorkflowTokenRepository, WorkflowTokenService, TokenAuthGuard],
providers: [
WorkflowTokenRepository,
WorkflowTokenService,
TokenAuthGuard,
CustomerService,
CustomerRepository,
UiDefinitionService,
ProjectScopeService,
UiDefinitionRepository,
WorkflowRuntimeDataRepository,
ApiKeyService,
ApiKeyRepository,
],
exports: [WorkflowTokenRepository, WorkflowTokenService, TokenAuthGuard],
})
export class TokenAuthModule {}
4 changes: 0 additions & 4 deletions services/workflows-service/src/workflow/schemas/zod-schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ export const ConfigSchema = z
.optional(),
)
.optional(),
createCollectionFlowToken: z
.boolean()
.optional()
.describe('Whether to create a collection flow token as part of the workflow'),
mainRepresentative: z
.object({
fullName: z.string(),
Expand Down
Loading
Loading