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

Object conversion functionality #10459

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oddieis
Copy link

@oddieis oddieis commented Feb 24, 2025

No description provided.

Copy link
Contributor

github-actions bot commented Feb 24, 2025

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against f3c4d5d

Copy link
Author

@oddieis oddieis left a comment

Choose a reason for hiding this comment

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

working on object to object conversion functionality - see issue #10422.
Some testing taking place still

Copy link
Author

@oddieis oddieis left a comment

Choose a reason for hiding this comment

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

working on object to object conversion functionality - see issue #10422.
Some testing taking place still

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR introduces a comprehensive lead conversion system with templates and field mapping functionality. The implementation spans both frontend and backend, enabling conversion of leads to other object types with customizable templates and field mappings.

  • Added ConversionTemplateEntity in /packages/twenty-server/src/modules/lead/conversion-templates/conversion-template.entity.ts with field mapping rules, conversion settings, and matching rules
  • Implemented LeadConversionService in /packages/twenty-server/src/modules/lead/services/lead-conversion.service.ts with transaction management and field mapping logic, though createRelations method needs implementation
  • Added frontend components in /packages/twenty-front/src/modules/lead/components/ConversionTemplates/ for template management UI with drag-and-drop functionality
  • Introduced GraphQL operations in /packages/twenty-front/src/modules/lead/graphql/ for template CRUD and lead conversion, but lacks proper type definitions for fieldMappingRules
  • Added standard object metadata in /packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/ for lead entity configuration, including icons and field IDs

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

20 file(s) reviewed, 32 comment(s)
Edit PR Review Bot Settings | Greptile

);

const handleDragEnd = useCallback(
(result: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Replace 'any' with proper type definition from react-beautiful-dnd (DragEndResult)

Suggested change
(result: any) => {
(result: DragEndResult) => {

<Droppable droppableId="templates">
{(provided) => (
<div {...provided.droppableProps} ref={provided.innerRef}>
{data?.conversionTemplates.map((template: any, index: number) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Replace 'any' with proper interface for template object


const selectedTemplate = selectedTemplateId
? templatesData?.availableTemplatesForLead.find(
(t: any) => t.id === selectedTemplateId,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: avoid using 'any' type - define an interface for the template object structure

Suggested change
(t: any) => t.id === selectedTemplateId,
(t: { id: string; name: string; targetObjectType: string; description?: string }) => t.id === selectedTemplateId,

Comment on lines +67 to +75
if (data.convertLead.success) {
enqueueSnackBar('Lead converted successfully', {
variant: 'success',
});
onConversionSuccess?.(
data.convertLead.convertedObjectId,
data.convertLead.convertedObjectType,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: should handle case where data.convertLead exists but success is false - currently modal closes without error message

Suggested change
if (data.convertLead.success) {
enqueueSnackBar('Lead converted successfully', {
variant: 'success',
});
onConversionSuccess?.(
data.convertLead.convertedObjectId,
data.convertLead.convertedObjectType,
);
}
if (data.convertLead.success) {
enqueueSnackBar('Lead converted successfully', {
variant: 'success',
});
onConversionSuccess?.(
data.convertLead.convertedObjectId,
data.convertLead.convertedObjectType,
);
handleCloseModal();
} else {
enqueueSnackBar('Failed to convert lead', { variant: 'error' });
}

Comment on lines +122 to +128
<Button
variant="secondary"
onClick={handleOpenModal}
icon={<IconArrowRight />}
>
Convert
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider disabling button when no templates are available

Comment on lines +129 to +142
private async createRelations(
queryRunner: any,
workspaceId: string,
lead: LeadWorkspaceEntity,
convertedObjectId: string,
targetObjectMetadata: any,
) {
// Implementation will depend on the specific relations needed
// For example, creating task relations, note relations, etc.
// This would involve:
// 1. Identifying relevant relations from the lead
// 2. Creating corresponding relations for the converted object
// 3. Updating any reference tables as needed
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: createRelations method is incomplete and needs implementation before production use

Comment on lines +219 to +221
sourceField: leadField.name,
targetField: matchingFields[0].name, // Use the first match
});
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Taking first match without considering field compatibility could lead to incorrect mappings

import { WorkspaceIsNotAuditLogged } from 'src/engine/twenty-orm/decorators/workspace-is-not-audit-logged.decorator';
import { WorkspaceIsNullable } from 'src/engine/twenty-orm/decorators/workspace-is-nullable.decorator';
import { WorkspaceIsSystem } from 'src/engine/twenty-orm/decorators/workspace-is-system.decorator';
import { WorkspaceJoinColumn } from 'src/engine/twenty-orm/decorators/workspace-join-column.decorator';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: WorkspaceJoinColumn is imported but never used in this file

Suggested change
import { WorkspaceJoinColumn } from 'src/engine/twenty-orm/decorators/workspace-join-column.decorator';

getTsVectorColumnExpressionFromFields,
} from 'src/engine/workspace-manager/workspace-sync-metadata/utils/get-ts-vector-column-expression.util';
import { AttachmentWorkspaceEntity } from 'src/modules/attachment/standard-objects/attachment.workspace-entity';
import { CompanyWorkspaceEntity } from 'src/modules/company/standard-objects/company.workspace-entity';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: CompanyWorkspaceEntity is imported but never used in this file

Suggested change
import { CompanyWorkspaceEntity } from 'src/modules/company/standard-objects/company.workspace-entity';

Comment on lines 14 to +15
MessagingModule,
LeadModule,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: LeadModule depends on FavoriteModule (via lead.workspace-entity.ts) but is imported before it. Consider moving LeadModule after FavoriteModule to avoid potential initialization issues.

@FelixMalfait
Copy link
Member

Are you an AI :) ?

Thanks for this. We love contributions, but please work with us before putting too much time into this because we're usually quite opioniated about how we want to build things, so this might end up being closed. Could you please share a screenshot? I couldn't start the project locally on your branch, there were frontend errors.

@oddieis
Copy link
Author

oddieis commented Feb 25, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants