Implement business logic hooks for Account, Contact, Campaign, and Activity objects #42
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…tivity Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ases Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…klist Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
github-actions
bot
added
the
documentation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /** | ||
| * Create reminder tasks for upcoming renewal | ||
| */ | ||
| async function createRenewalReminderTasks(accountId: string, renewalDate: string, db: typeof import('@hotcrm/core').db): Promise<void> { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
In general, unused functions should either be (1) removed when they are not needed, or (2) actually used (called) if they represent required behavior. Here, the implementation inside createRenewalReminderTasks is currently unreachable, and the only mention of it is in commented-out code inside updateAccountRenewalDate. To fix the issue without changing existing functionality, we should remove the unused helper function entirely, and also adjust the surrounding comments so they no longer reference a function that does not exist.
Concretely:
- In
packages/crm/src/hooks/account.hook.ts, delete the entirecreateRenewalReminderTasksfunction definition (lines 263–287 in the snippet). - Optionally, update the comment in
updateAccountRenewalDatethat mentionscreateRenewalReminderTasksso it no longer refers to a removed helper; we can rephrase it generically (“create reminder tasks at 90/60/30 days before renewal”) or drop the specific commented-out call line. This does not alter runtime behavior because that block is entirely commented out already.
No new imports, methods, or definitions are required.
| @@ -254,38 +254,12 @@ | ||
| // }); | ||
| // | ||
| // // Create reminder tasks at 90/60/30 days before renewal | ||
| // await createRenewalReminderTasks(accountId, nextRenewalDate, db); | ||
| // // (implementation would go here in a real system) | ||
| // } | ||
|
|
||
| console.log(`✅ Renewal date updated for account ${accountId}`); | ||
| } | ||
|
|
||
| /** | ||
| * Create reminder tasks for upcoming renewal | ||
| */ | ||
| async function createRenewalReminderTasks(accountId: string, renewalDate: string, db: typeof import('@hotcrm/core').db): Promise<void> { | ||
| const reminderDays = [90, 60, 30]; | ||
|
|
||
| for (const days of reminderDays) { | ||
| const reminderDate = new Date(renewalDate); | ||
| reminderDate.setDate(reminderDate.getDate() - days); | ||
|
|
||
| if (reminderDate > new Date()) { | ||
| // In real implementation, would create the task | ||
| console.log(`📅 Would create reminder task ${days} days before renewal`); | ||
| // await db.doc.create('Activity', { | ||
| // Subject: `Renewal reminder: ${days} days until contract renewal`, | ||
| // Type: 'Task', | ||
| // Status: 'Planned', | ||
| // Priority: days <= 30 ? 'High' : 'Medium', | ||
| // WhatId: accountId, | ||
| // ActivityDate: reminderDate.toISOString().split('T')[0], | ||
| // Description: `Contract renewal is coming up in ${days} days. Please contact the customer to discuss renewal.` | ||
| // }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Export all hooks | ||
| export { | ||
| AccountHealthScoreTrigger, |
| } | ||
|
|
||
| // Create next occurrence | ||
| const nextActivity = { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
In general, unused local variables should either be removed or put to use. Here, the surrounding comment // Create next occurrence strongly suggests the intent is to create and persist the next recurring activity. The safest and most meaningful fix is to call the database API with nextActivity, rather than deleting it.
Detailed fix:
- After constructing
nextActivity, immediately use it in adb.doc.create(or similar) call to actually create the next occurrence in the database. - Because we are inside a trigger/hook (and we already have access to
dbin this file), we can assume the samedbinstance is in scope within this function. To avoid guessing API names, we should follow the pattern used elsewhere in the same file: other snippets usedb.doc.update('Activity', ...). Here we’ll usedb.doc.create('Activity', nextActivity)as the minimal, consistent way to “create” a new record. - Await this call (the function around this code is already
asyncper the overall trigger design), and optionally log the creation for traceability.
Concretely, within packages/crm/src/hooks/activity.hook.ts, in the block starting at line 222 where nextActivity is declared, add an await db.doc.create('Activity', nextActivity); call and a log line right after the object definition. No new imports or types are required.
| @@ -237,6 +237,10 @@ | ||
| RecurrenceEndDate: activity.RecurrenceEndDate, | ||
| RecurrenceInstanceId: activity.RecurrenceInstanceId || activity.Id | ||
| }; | ||
|
|
||
| // Persist the next recurring activity occurrence | ||
| await db.doc.create('Activity', nextActivity); | ||
| console.log(`🔁 Created next recurring activity instance for ${activity.Subject} on ${nextActivity.DueDate}`); | ||
|
|
||
| // await ctx.db.doc.create('Activity', nextActivity); | ||
| console.log(`✅ Created next recurrence for ${nextDate.toISOString().split('T')[0]}`); |
…case block Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive business logic hooks for the four core CRM objects (Account, Contact, Campaign, and Activity) following the @objectstack/spec v0.6.1 protocol. The implementation provides 20+ automation scenarios including health scoring, hierarchy management, ROI calculation, relationship tracking, and activity recurrence handling.
Changes:
- Added TypeScript hook files for Account, Contact, Campaign, and Activity objects with automated business logic
- Implemented helper functions for cross-object updates and batch processing jobs
- Created comprehensive documentation in HOOKS_IMPLEMENTATION.md and IMPLEMENTATION_SUMMARY.md
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/crm/src/hooks/account.hook.ts | Account health scoring, hierarchy management, status automation, contract value rollup, and renewal date tracking |
| packages/crm/src/hooks/contact.hook.ts | Contact date tracking, decision chain validation, duplicate detection, and relationship strength management |
| packages/crm/src/hooks/campaign.hook.ts | Campaign ROI calculation, budget tracking, member management, and performance metrics |
| packages/crm/src/hooks/activity.hook.ts | Activity auto-completion, related object updates, overdue notifications, and recurrence management |
| IMPLEMENTATION_SUMMARY.md | Executive summary with implementation metrics, quality checklist, and production deployment guide |
| HOOKS_IMPLEMENTATION.md | Technical implementation guide with architecture patterns, data access examples, and integration points |
| } | ||
|
|
||
| // Auto-set IsDecisionMaker for C-Level | ||
| if (contact.Level === 'C-Level' && !contact.IsDecisionMaker) { |
There was a problem hiding this comment.
The logic for checking if IsDecisionMaker should be set has a potential issue. When contact.IsDecisionMaker is false (explicitly set by user), this condition will still override it to true. The condition should check for undefined/null specifically, not just falsy values.
Consider using: if (contact.Level === 'C-Level' && contact.IsDecisionMaker !== true && contact.IsDecisionMaker !== false) or similar logic to avoid overriding explicit user choices.
| if (contact.Level === 'C-Level' && !contact.IsDecisionMaker) { | |
| if ( | |
| contact.Level === 'C-Level' && | |
| contact.IsDecisionMaker !== true && | |
| contact.IsDecisionMaker !== false | |
| ) { |
| const date = new Date(dateString); | ||
| const now = new Date(); | ||
| const diffTime = now.getTime() - date.getTime(); | ||
| const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)); |
There was a problem hiding this comment.
Using Math.ceil for date difference calculation can produce incorrect results. When the time difference is exactly midnight (0 hours), Math.ceil will round up to 1 day when it should be 0 days. For "days since" calculations, Math.floor would be more appropriate to get the complete days elapsed. Alternatively, consider using Math.round if you want half-day precision.
| const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)); | |
| const diffDays = Math.floor(diffTime / (1000 * 60 * 60 * 24)); |
| // | ||
| // const convertedLeads = leads.filter(l => l.Status === 'Converted'); | ||
|
|
||
| // 2. Count opportunities from this campaign | ||
| // const opportunities = await db.find('Opportunity', { | ||
| // filters: [['CampaignId', '=', campaignId]] | ||
| // }); | ||
| // | ||
| // const wonOpportunities = opportunities.filter(o => o.Stage === 'Closed Won'); | ||
|
|
||
| // 3. Calculate actual revenue from won opportunities | ||
| // const actualRevenue = wonOpportunities.reduce((sum, opp) => sum + (opp.Amount || 0), 0); | ||
|
|
There was a problem hiding this comment.
The commented implementation uses JavaScript array.filter() to count converted leads and won opportunities, which is inefficient for large datasets. Instead, use ObjectQL filters to perform server-side filtering.
For example, replace:
const convertedLeads = leads.filter(l => l.Status === 'Converted');
With:
const convertedLeads = await db.find('Lead', { filters: [['CampaignId', '=', campaignId], ['Status', '=', 'Converted']] });
This applies to both the lead and opportunity queries on lines 73 and 80.
| // | |
| // const convertedLeads = leads.filter(l => l.Status === 'Converted'); | |
| // 2. Count opportunities from this campaign | |
| // const opportunities = await db.find('Opportunity', { | |
| // filters: [['CampaignId', '=', campaignId]] | |
| // }); | |
| // | |
| // const wonOpportunities = opportunities.filter(o => o.Stage === 'Closed Won'); | |
| // 3. Calculate actual revenue from won opportunities | |
| // const actualRevenue = wonOpportunities.reduce((sum, opp) => sum + (opp.Amount || 0), 0); | |
| // | |
| // const convertedLeads = await db.find('Lead', { | |
| // filters: [ | |
| // ['CampaignId', '=', campaignId], | |
| // ['Status', '=', 'Converted'], | |
| // ], | |
| // }); | |
| // | |
| // 2. Count opportunities from this campaign | |
| // const opportunities = await db.find('Opportunity', { | |
| // filters: [['CampaignId', '=', campaignId]] | |
| // }); | |
| // | |
| // const wonOpportunities = await db.find('Opportunity', { | |
| // filters: [ | |
| // ['CampaignId', '=', campaignId], | |
| // ['Stage', '=', 'Closed Won'], | |
| // ], | |
| // }); | |
| // | |
| // 3. Calculate actual revenue from won opportunities | |
| // const actualRevenue = wonOpportunities.reduce((sum, opp) => sum + (opp.Amount || 0), 0); | |
| // |
| // Types for Context | ||
| interface TriggerContext { | ||
| old?: Record<string, any>; | ||
| new: Record<string, any>; | ||
| db: typeof db; | ||
| user: { id: string; name: string; email: string; }; | ||
| } |
There was a problem hiding this comment.
The TriggerContext interface is duplicated across multiple hook files (account, contact, campaign, activity). This creates maintenance burden and potential inconsistencies. The interface should be defined once in a shared location (e.g., packages/core/src/types.ts or packages/crm/src/types.ts) and imported by all hook files.
Looking at opportunity.hook.ts, it exports TriggerContext, while other files define it as a non-exported interface. This inconsistency should be resolved by centralizing the definition.
|
|
||
| // Types for Context | ||
| interface TriggerContext { | ||
| old?: Record<string, any>; | ||
| new: Record<string, any>; | ||
| db: typeof db; | ||
| user: { id: string; name: string; email: string; }; | ||
| } | ||
|
|
There was a problem hiding this comment.
The TriggerContext interface is duplicated across multiple hook files. This creates maintenance burden and potential inconsistencies. The interface should be defined once in a shared location and imported by all hook files to ensure consistency.
| // Types for Context | |
| interface TriggerContext { | |
| old?: Record<string, any>; | |
| new: Record<string, any>; | |
| db: typeof db; | |
| user: { id: string; name: string; email: string; }; | |
| } | |
| import type { TriggerContext } from '@hotcrm/core'; |
| // Types for Context | ||
| interface TriggerContext { | ||
| old?: Record<string, any>; | ||
| new: Record<string, any>; | ||
| db: typeof db; | ||
| user: { id: string; name: string; email: string; }; | ||
| } |
There was a problem hiding this comment.
The TriggerContext interface is duplicated across multiple hook files. This creates maintenance burden and potential inconsistencies. The interface should be defined once in a shared location and imported by all hook files to ensure consistency.
| // Types for Context | ||
| interface TriggerContext { | ||
| old?: Record<string, any>; | ||
| new: Record<string, any>; | ||
| db: typeof db; | ||
| user: { id: string; name: string; email: string; }; | ||
| } |
There was a problem hiding this comment.
The TriggerContext interface is duplicated across multiple hook files. This creates maintenance burden and potential inconsistencies. The interface should be defined once in a shared location and imported by all hook files to ensure consistency.
| // await createRenewalReminderTasks(accountId, nextRenewalDate, db); | ||
| // } | ||
|
|
||
| console.log(`✅ Renewal date updated for account ${accountId}`); | ||
| } | ||
|
|
||
| /** | ||
| * Create reminder tasks for upcoming renewal | ||
| */ | ||
| async function createRenewalReminderTasks(accountId: string, renewalDate: string, db: typeof import('@hotcrm/core').db): Promise<void> { | ||
| const reminderDays = [90, 60, 30]; | ||
|
|
||
| for (const days of reminderDays) { | ||
| const reminderDate = new Date(renewalDate); | ||
| reminderDate.setDate(reminderDate.getDate() - days); | ||
|
|
||
| if (reminderDate > new Date()) { | ||
| // In real implementation, would create the task | ||
| console.log(`📅 Would create reminder task ${days} days before renewal`); | ||
| // await db.doc.create('Activity', { | ||
| // Subject: `Renewal reminder: ${days} days until contract renewal`, | ||
| // Type: 'Task', | ||
| // Status: 'Planned', | ||
| // Priority: days <= 30 ? 'High' : 'Medium', | ||
| // WhatId: accountId, | ||
| // ActivityDate: reminderDate.toISOString().split('T')[0], | ||
| // Description: `Contract renewal is coming up in ${days} days. Please contact the customer to discuss renewal.` | ||
| // }); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Unused function createRenewalReminderTasks.
| // await createRenewalReminderTasks(accountId, nextRenewalDate, db); | |
| // } | |
| console.log(`✅ Renewal date updated for account ${accountId}`); | |
| } | |
| /** | |
| * Create reminder tasks for upcoming renewal | |
| */ | |
| async function createRenewalReminderTasks(accountId: string, renewalDate: string, db: typeof import('@hotcrm/core').db): Promise<void> { | |
| const reminderDays = [90, 60, 30]; | |
| for (const days of reminderDays) { | |
| const reminderDate = new Date(renewalDate); | |
| reminderDate.setDate(reminderDate.getDate() - days); | |
| if (reminderDate > new Date()) { | |
| // In real implementation, would create the task | |
| console.log(`📅 Would create reminder task ${days} days before renewal`); | |
| // await db.doc.create('Activity', { | |
| // Subject: `Renewal reminder: ${days} days until contract renewal`, | |
| // Type: 'Task', | |
| // Status: 'Planned', | |
| // Priority: days <= 30 ? 'High' : 'Medium', | |
| // WhatId: accountId, | |
| // ActivityDate: reminderDate.toISOString().split('T')[0], | |
| // Description: `Contract renewal is coming up in ${days} days. Please contact the customer to discuss renewal.` | |
| // }); | |
| } | |
| } | |
| } | |
| // // This logic can be implemented here in the future if needed. | |
| // } | |
| console.log(`✅ Renewal date updated for account ${accountId}`); | |
| } |
| } | ||
|
|
||
| // Create next occurrence | ||
| const nextActivity = { |
There was a problem hiding this comment.
Unused variable nextActivity.
Description
Implements automated business logic hooks for core CRM objects following @objectstack/spec v0.6.1 protocol. Covers 20+ automation scenarios including health scoring, hierarchy management, relationship strength tracking, ROI calculation, and recurrence handling.
Type of Change
Related Issues
Related to Week 2-3 Business Logic Implementation milestone
Changes Made
Account Hooks (
packages/crm/src/hooks/account.hook.ts)Contact Hooks (
packages/crm/src/hooks/contact.hook.ts)Campaign Hooks (
packages/crm/src/hooks/campaign.hook.ts)(ActualRevenue - ActualCost) / ActualCost * 100with infinite ROI handlingActivity Hooks (
packages/crm/src/hooks/activity.hook.ts)Technical Implementation
Edge Cases Addressed
Documentation
HOOKS_IMPLEMENTATION.md- Technical implementation guide with architecture patternsIMPLEMENTATION_SUMMARY.md- Executive summary with metrics and production checklistTesting
npm test)npm run lint)npm run build)Validation Results:
Screenshots
N/A - Backend business logic implementation
Checklist
Additional Notes
Current State: Database operations are commented out for safety. Hooks log intended actions but don't modify data. Uncomment
db.doc.*anddb.find()calls to enable.Production Deployment:
autoCompletePastDueActivities()andsendOverdueNotifications()LastActivityDatefields to Account/Opportunity objectsCampaign.object.ymlto TypeScript for consistencyMetrics: 1,090 LOC across 4 hook files, 10 triggers, 8 helper functions, 2 batch jobs
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.