feat(api): parse Team Member data from Google Doc #676
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@lmontero18 pls use correct naming of pr, instead backend use api, read our guidelines |
There was a problem hiding this comment.
@lmontero18 basically that is the first part of the work, you need the second part where you will parse the API and generate such block defined now.
lmontero18
changed the title
|
log: working on the parser for team members block |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/components/BodyParser/blocks.ts (1)
124-135: Consider adding validation constraints to the type.The TeamMembersBlock type looks good, but consider adding constraints for required fields:
- Maximum length for text fields (name, role, description)
- URL validation for image and link fields
src/routes/api/google-conversion/+server.ts (1)
94-122: Simplify nested conditionals in table content handling.The nested conditionals make the code harder to read and maintain. Consider extracting the content type detection into a separate function.
+function detectContentType(tableContent: any[]) { + const types = { + teamMembers: parseTeamMembersSection(tableContent), + cta: parseCTASection(tableContent), + testimonial: parseTestimonialSection(tableContent), + }; + + return Object.entries(types).find(([_, content]) => !_.isEmpty(content)); +} -const teamMembers = parseTeamMembersSection(tableContent); -const cta: CTAElement = parseCTASection(tableContent); -const testimonial: TestimonialElement = parseTestimonialSection(tableContent); -if (!_.isEmpty(teamMembers)) { - newContent.push({ - type: 'teamMembers', - data: teamMembers, - }); - if (!_.isEmpty(testimonial)) { - newContent.push({ - type: 'testimonial', - data: testimonial, - }); - } else if (!_.isEmpty(cta)) { - newContent.push({ - type: 'cta', - data: cta, - }); - } else { - newContent.push({ - type: 'table', - data: { - content: tableContent, - }, - }); - } -} +const contentType = detectContentType(tableContent); +if (contentType) { + const [type, data] = contentType; + newContent.push({ type, data }); +} else { + newContent.push({ + type: 'table', + data: { content: tableContent }, + }); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/BodyParser/blocks.ts(3 hunks)src/routes/(pages)/about/template.pug(2 hunks)src/routes/api/google-conversion/+server.ts(5 hunks)
🔇 Additional comments (1)
src/routes/(pages)/about/template.pug (1)
Line range hint
3-12: LGTM! Clean template structure.The template follows good practices with proper navigation hierarchy and semantic HTML structure.
zolotokrylin
changed the title
|
log: working on the suggestions |
|
@lmontero18 pls update the ETA |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/routes/api/google-conversion/+server.ts (1)
221-264:⚠️ Potential issueEnhance error handling and data validation.
The current implementation lacks proper error handling and data validation. Consider implementing the following improvements:
Apply this diff to improve the implementation:
function parseTeamMembersSection(content: any[]): TeamMembersBlock | null { + try { const teamMembersBlock: TeamMembersBlock = { type: 'teamMember', data: { members: [], }, }; if (content.length >= 5 && (content[0] as any[]).length === 2) { const contentHead = content[0]; if ( contentHead[0][0]?.type === 'paragraph' && contentHead[0][0]?.data?.text === 'type' && contentHead[1][0]?.type === 'paragraph' && contentHead[1][0]?.data?.text === 'teamMember' ) { content.forEach(([[first], [second]], i) => { if (first === undefined || first.type !== 'paragraph') return; const rowData: Record<string, string> = {}; if (second && second.type === 'paragraph') { rowData[first.data.text] = second.data.text; } - if (rowData.name && rowData.role) { + // Validate required fields + if (!rowData.name?.trim() || !rowData.role?.trim()) { + console.warn(`Skipping member at row ${i}: missing required fields`); + return; + } + teamMembersBlock.data.members.push({ - name: rowData.name || '', - role: rowData.role || '', - description: rowData.description || '', + name: rowData.name.trim(), + role: rowData.role.trim(), + description: (rowData.description || '').trim(), image: rowData.image || '', - link: rowData.link || undefined, + link: rowData.link?.trim() || undefined, }); - } }); if (teamMembersBlock.data.members.length > 0) { return teamMembersBlock; } } } return null; + } catch (error) { + console.error('Error parsing team members section:', error); + return null; + } }
🧹 Nitpick comments (1)
src/routes/api/google-conversion/+server.ts (1)
107-129: Simplify the nested conditional structure.The current implementation has deeply nested if-else statements that make the code hard to maintain. Consider refactoring to a more flat structure using early returns or a switch-case pattern.
- if (!_.isEmpty(teamMembers)) { - newContent.push({ - type: 'teamMember', - data: teamMembers, - }); - if (!_.isEmpty(testimonial)) { - newContent.push({ - type: 'testimonial', - data: testimonial, - }); - } else if (!_.isEmpty(cta)) { - newContent.push({ - type: 'cta', - data: cta, - }); - } else { - newContent.push({ - type: 'table', - data: { - content: tableContent, - }, - }); - } - } + // Process special blocks in order of priority + if (!_.isEmpty(teamMembers)) { + newContent.push({ type: 'teamMember', data: teamMembers }); + return; + } + + if (!_.isEmpty(testimonial)) { + newContent.push({ type: 'testimonial', data: testimonial }); + return; + } + + if (!_.isEmpty(cta)) { + newContent.push({ type: 'cta', data: cta }); + return; + } + + // Default to table if no special blocks are found + newContent.push({ + type: 'table', + data: { content: tableContent } + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/api/google-conversion/+server.ts(8 hunks)
🔇 Additional comments (1)
src/routes/api/google-conversion/+server.ts (1)
Line range hint
1-671: Verify type consistency and integration.Please verify:
- The TeamMembersBlock type matches the schema used in the frontend components
- The changes don't affect existing document conversion functionality
Run the following script to verify type consistency:
✅ Verification successful
Types and integration are consistent
The TeamMembersBlock type is properly defined and consistently used across the codebase. The conversion logic maintains data integrity and doesn't affect existing document conversion functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent type usage across the codebase # Check TeamMembersBlock type usage echo "Checking TeamMembersBlock type usage:" rg -A 5 "TeamMembersBlock" # Check team member block rendering echo "Checking team member block rendering:" rg -A 5 "type: 'teamMember'"Length of output: 3088
🧰 Tools
🪛 Biome (1.9.4)
[error] 268-268: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
|
@matthewvogas you can also check, It'll help! |
There was a problem hiding this comment.
@lmontero18 based on the https://holdex-venture-studio-git-feat-parsing-data-holdex-accelerator.vercel.app/api/google-conversion POST request I made I cannot see the teamMember object using the JSON I provided. please double check.
Also provide in 1 comment the files for @matthewvogas to also test that
|
log: working on the changes |
|
@lmontero18 pls update the eta based in your locatio - add CR time or utc -6 to know about the ETA withou doubts |
There was a problem hiding this comment.
@lmontero18 let's wrap it up quickly, as too much time takes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/api/google-conversion/+server.ts (2)
Line range hint
104-128: Enhance validation and error handling for table content processing.While the integration works, consider improving the validation:
- Add explicit type checking before parsing
- Consider logging parsing errors for debugging
- Use more specific empty checks instead of _.isEmpty
- const teamMembers = parseTeamMembersSection(tableContent); - const cta: CTAElement = parseCTASection(tableContent); - const testimonial: TestimonialElement = parseTestimonialSection(tableContent); - - if (!_.isEmpty(teamMembers)) { + try { + const teamMembers = parseTeamMembersSection(tableContent); + const cta: CTAElement = parseCTASection(tableContent); + const testimonial: TestimonialElement = parseTestimonialSection(tableContent); + + if (teamMembers && teamMembers.length > 0) { newContent.push(teamMembers); - } else if (!_.isEmpty(testimonial)) { + } else if (testimonial && Object.keys(testimonial).length > 0) { newContent.push({ type: 'testimonial', data: testimonial, }); - } else if (!_.isEmpty(cta)) { + } else if (cta && Object.keys(cta).length > 0) { newContent.push({ type: 'cta', data: cta, }); } else { newContent.push({ type: 'table', data: { content: tableContent, }, }); } + } catch (error) { + console.error('Error processing table content:', error); + newContent.push({ + type: 'table', + data: { + content: tableContent, + }, + }); + }
447-447: Improve type safety in parseParagraphElement function.The wrappingTable parameter could benefit from better typing to make its purpose clearer.
const parseParagraphElement = ( document: Schema$Document, tag: string, parentContent: (Parsed$Paragraph | Parsed$ParagraphElement)[], element: Schema$ParagraphElement, - wrappingTable: boolean | undefined = undefined + options: { + wrappingTable?: boolean; + isCtaLink?: boolean; + } = {} ) => { // ... rest of the function getText(element, { isHeader: tag !== 'p', - isCtaLink: wrappingTable, + isCtaLink: options.wrappingTable, }) };Also applies to: 627-629
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/api/google-conversion/+server.ts(9 hunks)
🔇 Additional comments (2)
src/routes/api/google-conversion/+server.ts (2)
19-29: LGTM! Clean import organization.The imports are well-structured and correctly separated between types and components.
Line range hint
1-671: Verify edge cases in the document conversion process.Please ensure the following scenarios are tested:
- Google Doc with mixed content (team members, testimonials, and CTAs)
- Empty or malformed table structures
- Missing or invalid field values
Run this script to analyze the conversion process:
✅ Verification successful
Document conversion process handles edge cases appropriately ✅
The code implements robust error handling through:
- Extensive null checks and optional chaining
- Validation of required fields before processing
- Graceful handling of missing or malformed data
- Type-safe conversions with proper fallbacks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Analyze the document conversion process for edge cases # Test 1: Search for error handling patterns echo "Checking error handling patterns..." rg -n 'try|catch|throw|error|warn' src/routes/api/google-conversion/ # Test 2: Search for type validations echo "Checking type validations..." rg -n 'typeof|instanceof|Array.isArray' src/routes/api/google-conversion/ # Test 3: Search for null checks echo "Checking null checks..." rg -n '\?\.|===?\s*(null|undefined)|isEmpty' src/routes/api/google-conversion/Length of output: 5530
🧰 Tools
🪛 Biome (1.9.4)
[error] 268-268: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
|
@georgeciubotaru |
|
@georgeciubotaru could you please review? thanks! |
|
@georgeciubotaru |
ETA: January 29
resolves: https://github.com/holdex/marketing/issues/97
test: http://holdex-venture-studio-git-feat-parsing-data-holdex-accelerator.vercel.app/
Summary by CodeRabbit
Release Notes
New Features
Improvements
The changes introduce a new capability to handle team member information across the application's content parsing and rendering systems.