kris/afv-library
1
0
Fork 0
mirror of https://github.com/forcedotcom/afv-library.git synced 2026-07-30 03:09:50 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: remove unused skill @W-22336514@ (#235)

Browse Source 
skill-changes-related-to-apex
This commit is contained in:
Jasmine Kaur 2026-05-04 10:53:20 +05:30 committed by GitHub
parent e235a02c7c
commit 0af0eaecf6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 0 additions and 1212 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

191
skills/trigger-refactor-pipeline/SKILL.md
View File

@ -1,191 +0,0 @@
---
name: trigger-refactor-pipeline
description: "Refactor Salesforce triggers into handler patterns with automated test generation and deployment. Use when modernizing legacy triggers with DML/SOQL in loops or inconsistent patterns."
license: Apache-2.0
compatibility: Requires Salesforce CLI, Python 3.9+
metadata:
author: afv-library
version: "1.0"
allowed-tools: Bash Read Write
---
## When to Use This Skill
Use this skill when you need to:
- Modernize legacy triggers with DML/SOQL operations inside loops
- Refactor triggers that lack clear separation of concerns
- Implement bulk-safe patterns in existing trigger code
- Generate comprehensive test coverage for refactored triggers
## Prerequisites
Before starting, ensure you have:
1. Salesforce CLI installed and authenticated to your target org
2. Python 3.9 or higher installed
3. The baseline trigger deployed (see Setup section)
## Setup
Deploy the baseline anti-pattern trigger to analyze and refactor:
```apex
// ❌ Anti-pattern: all logic stuffed into the trigger, with DML/SOQL in loops.
trigger OpportunityTrigger on Opportunity (before insert, before update, after update) {
// BEFORE INSERT: validate Closed Won w/ low Amount
if (Trigger.isBefore && Trigger.isInsert) {
for (Opportunity o : Trigger.new) {
if (o.StageName == 'Closed Won' && (o.Amount == null || o.Amount < 1000)) {
o.addError('Closed Won opportunities must have Amount ≥ 1000.');
}
}
}
// BEFORE UPDATE: if Stage changed, overwrite Description
if (Trigger.isBefore && Trigger.isUpdate) {
for (Opportunity o : Trigger.new) {
Opportunity oldO = Trigger.oldMap.get(o.Id);
if (o.StageName != oldO.StageName) {
o.Description = 'Stage changed from ' + oldO.StageName + ' to ' + o.StageName;
}
}
}
// AFTER UPDATE: when Stage becomes Closed Won, create a follow-up Task
if (Trigger.isAfter && Trigger.isUpdate) {
for (Opportunity o : Trigger.new) {
Opportunity oldO = Trigger.oldMap.get(o.Id);
if (o.StageName == 'Closed Won' && oldO.StageName != 'Closed Won') {
Task t = new Task(
WhatId = o.Id,
OwnerId = o.OwnerId,
Subject = 'Send thank-you',
Status = 'Not Started',
Priority = 'Normal',
ActivityDate = Date.today()
);
insert t; // ❌ DML in a loop
}
}
}
}
```
Deploy this to your org:
```bash
sf project deploy start --source-dir force-app/main/default/triggers
```
## Step 1: Analyze the Trigger
Run the analysis script to identify anti-patterns and generate a report:
```bash
python scripts/analyze_trigger.py OpportunityTrigger
```
The script will output:
- **DML in loops** - Line numbers where DML operations occur inside iteration
- **SOQL in loops** - Line numbers where SOQL queries occur inside iteration
- **Missing bulkification** - Areas where collection-based processing is needed
- **Complexity score** - Overall trigger complexity rating (1-10)
- **Recommended approach** - Suggested handler pattern based on trigger contexts
Review the analysis report before proceeding to refactoring.
## Step 2: Review Handler Patterns
Consult the [handler patterns reference](references/handler_patterns.md) to understand:
- **Single-responsibility handlers** - One handler class per trigger context
- **Unified handler approach** - Single handler with context methods
- **Bulk collection strategies** - How to aggregate DML/SOQL outside loops
- **Best practices** - Error handling, test boundaries, deployment order
Choose the pattern that best fits your trigger's complexity and team conventions.
## Step 3: Refactor the Trigger
Create the handler class using the appropriate pattern from the reference guide:
1. **Extract logic** into handler methods with descriptive names
2. **Implement bulk-safe collections** for DML operations
3. **Add proper error handling** using try-catch or Database methods
4. **Update the trigger** to delegate only, passing Trigger context variables
5. **Preserve behavior** - ensure the refactored code produces identical results
The trigger should be reduced to simple delegation:
```apex
trigger OpportunityTrigger on Opportunity (before insert, before update, after update) {
OpportunityTriggerHandler handler = new OpportunityTriggerHandler();
if (Trigger.isBefore && Trigger.isInsert) {
handler.beforeInsert(Trigger.new);
}
if (Trigger.isBefore && Trigger.isUpdate) {
handler.beforeUpdate(Trigger.new, Trigger.oldMap);
}
if (Trigger.isAfter && Trigger.isUpdate) {
handler.afterUpdate(Trigger.new, Trigger.oldMap);
}
}
```
## Step 4: Generate Tests
Use the test template from `assets/test_template.apex` to scaffold your test class:
1. **Copy the template** and rename for your handler
2. **Implement setup methods** to create test data
3. **Write unit tests** covering each handler method:
- Positive cases with valid data
- Negative cases with invalid data
- Boundary conditions
4. **Add bulk tests** with 200+ records to verify bulkification
5. **Test mixed scenarios** where only some records qualify for logic
Required test coverage:
- Each handler method must have at least 2 test methods (positive + negative)
- At least one bulk test with 200+ records
- Overall code coverage must be 100%
## Step 5: Deploy and Validate
Deploy the refactored trigger, handler, and tests:
```bash
# Deploy all components
sf project deploy start --source-dir force-app/main/default
# Run tests
sf apex test run --class-names OpportunityTriggerHandlerTest --result-format human --code-coverage
# Verify no regressions
sf apex test run --test-level RunLocalTests --result-format human
```
Validation checklist:
- [ ] All new tests pass with 100% coverage
- [ ] No new governor limit warnings in debug logs
- [ ] Existing functionality remains unchanged
- [ ] Deployment to production planned with rollback strategy
## Troubleshooting
**Issue**: Tests fail with "System.LimitException: Too many DML statements"
- **Solution**: Ensure handler methods collect DML operations and execute outside loops
**Issue**: Code coverage below 100%
- **Solution**: Add negative test cases and verify all conditional branches are tested
**Issue**: Behavior differs from original trigger
- **Solution**: Review Trigger context variables (new, old, oldMap) are passed correctly to handler
## Next Steps
After successful refactoring:
1. Document the new handler pattern in your team's wiki
2. Update code review checklist to enforce handler patterns for new triggers
3. Identify other legacy triggers for refactoring using this skill
4. Consider implementing a trigger framework if managing many triggers

321
skills/trigger-refactor-pipeline/assets/test_template.apex
View File

@ -1,321 +0,0 @@
/**
* Test class template for trigger handlers
*
* INSTRUCTIONS:
* 1. Replace [ObjectName] with your SObject (e.g., Opportunity)
* 2. Replace [HandlerClass] with your handler class name
* 3. Implement the setupTestData() method with your test records
* 4. Add specific test methods for each handler method
* 5. Ensure 100% code coverage
*/
@IsTest
private class [ObjectName]TriggerHandlerTest {
/**
* Setup test data that all test methods can use
*/
@TestSetup
static void setupTestData() {
// TODO: Create test records here
// Example:
// List<Opportunity> testOpps = new List<Opportunity>();
// for (Integer i = 0; i < 10; i++) {
// testOpps.add(new Opportunity(
// Name = 'Test Opp ' + i,
// StageName = 'Prospecting',
// CloseDate = Date.today().addDays(30),
// Amount = 5000
// ));
// }
// insert testOpps;
}
/**
* Test beforeInsert handler - Positive case
*/
@IsTest
static void testBeforeInsert_Positive() {
Test.startTest();
// TODO: Create valid test records
// List<Opportunity> testRecords = new List<Opportunity>{
// new Opportunity(
// Name = 'Valid Opp',
// StageName = 'Prospecting',
// CloseDate = Date.today().addDays(30),
// Amount = 10000
// )
// };
// Insert should succeed
// insert testRecords;
Test.stopTest();
// TODO: Add assertions
// List<Opportunity> inserted = [SELECT Id, Name FROM Opportunity WHERE Name = 'Valid Opp'];
// System.Assert.areEqual(1, inserted.size(), 'Should insert 1 record');
}
/**
* Test beforeInsert handler - Negative case
*/
@IsTest
static void testBeforeInsert_Negative() {
Test.startTest();
Boolean exceptionThrown = false;
try {
// TODO: Create invalid test records that should fail validation
// List<Opportunity> testRecords = new List<Opportunity>{
// new Opportunity(
// Name = 'Invalid Opp',
// StageName = 'Closed Won',
// CloseDate = Date.today(),
// Amount = 500 // Below minimum
// )
// };
// insert testRecords;
} catch (DmlException e) {
exceptionThrown = true;
// TODO: Assert error message
// System.Assert.isTrue(e.getMessage().contains('must have Amount'),
// 'Should throw validation error');
}
Test.stopTest();
// System.Assert.isTrue(exceptionThrown, 'Should have thrown an exception');
}
/**
* Test beforeUpdate handler - Positive case
*/
@IsTest
static void testBeforeUpdate_Positive() {
// TODO: Query existing test data from @TestSetup
// List<Opportunity> testOpps = [SELECT Id, StageName, Description FROM Opportunity LIMIT 1];
Test.startTest();
// TODO: Update records to trigger handler logic
// testOpps[0].StageName = 'Qualification';
// update testOpps;
Test.stopTest();
// TODO: Add assertions
// Opportunity updated = [SELECT Description FROM Opportunity WHERE Id = :testOpps[0].Id];
// System.Assert.isTrue(updated.Description.contains('Stage changed'),
// 'Description should be updated');
}
/**
* Test beforeUpdate handler - Negative case
*/
@IsTest
static void testBeforeUpdate_Negative() {
// TODO: Implement negative test for beforeUpdate
Test.startTest();
// TODO: Attempt update that should fail
Test.stopTest();
// TODO: Assert failure occurred
}
/**
* Test afterInsert handler - Positive case
*/
@IsTest
static void testAfterInsert_Positive() {
Test.startTest();
// TODO: Create and insert records
Test.stopTest();
// TODO: Query for related records created by handler
// List<Task> createdTasks = [SELECT Id FROM Task];
// System.Assert.areEqual(1, createdTasks.size(), 'Should create 1 task');
}
/**
* Test afterUpdate handler - Positive case
*/
@IsTest
static void testAfterUpdate_Positive() {
// TODO: Query existing test data
// List<Opportunity> testOpps = [SELECT Id, StageName FROM Opportunity LIMIT 1];
Test.startTest();
// TODO: Update to trigger after-update logic
// testOpps[0].StageName = 'Closed Won';
// update testOpps;
Test.stopTest();
// TODO: Query for side effects (e.g., Tasks created)
// List<Task> tasks = [SELECT Id, Subject FROM Task WHERE WhatId = :testOpps[0].Id];
// System.Assert.areEqual(1, tasks.size(), 'Should create 1 task');
// System.Assert.areEqual('Send thank-you', tasks[0].Subject);
}
/**
* Test bulk operations - 200+ records
* Critical for validating bulkification
*/
@IsTest
static void testBulkInsert() {
Test.startTest();
// TODO: Create 200+ records
// List<Opportunity> bulkOpps = new List<Opportunity>();
// for (Integer i = 0; i < 200; i++) {
// bulkOpps.add(new Opportunity(
// Name = 'Bulk Opp ' + i,
// StageName = 'Prospecting',
// CloseDate = Date.today().addDays(30),
// Amount = 10000
// ));
// }
// insert bulkOpps;
Test.stopTest();
// TODO: Assert all records inserted successfully
// List<Opportunity> inserted = [SELECT Id FROM Opportunity WHERE Name LIKE 'Bulk Opp%'];
// System.Assert.areEqual(200, inserted.size(), 'Should insert all 200 records');
}
/**
* Test bulk update with mixed scenarios
* Some records qualify for logic, others don't
*/
@IsTest
static void testBulkUpdate_Mixed() {
// Create test data
List<Opportunity> testOpps = new List<Opportunity>();
for (Integer i = 0; i < 50; i++) {
testOpps.add(new Opportunity(
Name = 'Bulk Update Opp ' + i,
StageName = 'Prospecting',
CloseDate = Date.today().addDays(30),
Amount = 10000
));
}
insert testOpps;
Test.startTest();
// Update half to trigger logic, half to not trigger
for (Integer i = 0; i < testOpps.size(); i++) {
if (Math.mod(i, 2) == 0) {
// TODO: Set condition that triggers handler logic
// testOpps[i].StageName = 'Closed Won';
} else {
// TODO: Set condition that doesn't trigger handler logic
// testOpps[i].Amount = 15000;
}
}
// update testOpps;
Test.stopTest();
// TODO: Assert only qualifying records triggered side effects
// List<Task> tasks = [SELECT Id FROM Task WHERE WhatId IN :testOpps];
// System.Assert.areEqual(25, tasks.size(), 'Should create tasks for 25 qualifying records');
}
/**
* Test governor limits are not exceeded
*/
@IsTest
static void testGovernorLimits() {
Test.startTest();
// TODO: Create maximum allowed records
// List<Opportunity> maxOpps = new List<Opportunity>();
// for (Integer i = 0; i < 200; i++) {
// maxOpps.add(new Opportunity(
// Name = 'Limit Test ' + i,
// StageName = 'Closed Won',
// CloseDate = Date.today(),
// Amount = 10000
// ));
// }
// insert maxOpps;
Test.stopTest();
// Assert we're under governor limits
System.Assert.isTrue(Limits.getDmlStatements() < Limits.getLimitDmlStatements(),
'Should not exceed DML statement limit');
System.Assert.isTrue(Limits.getQueries() < Limits.getLimitQueries(),
'Should not exceed SOQL query limit');
}
/**
* Test with null or empty collections
* Ensures handler doesn't break with edge cases
*/
@IsTest
static void testWithEmptyCollection() {
Test.startTest();
// TODO: Call handler methods with empty lists
// OpportunityTriggerHandler handler = new OpportunityTriggerHandler();
// handler.beforeInsert(new List<Opportunity>());
Test.stopTest();
// If we get here without exception, test passes
System.Assert.isTrue(true, 'Handler should handle empty collections gracefully');
}
/**
* Helper method to create test data inline
* Use when @TestSetup is not sufficient
*/
private static List<Opportunity> createTestOpportunities(Integer count) {
List<Opportunity> testOpps = new List<Opportunity>();
for (Integer i = 0; i < count; i++) {
testOpps.add(new Opportunity(
Name = 'Test Opp ' + i,
StageName = 'Prospecting',
CloseDate = Date.today().addDays(30),
Amount = 10000
));
}
return testOpps;
}
/**
* Helper method to assert task creation
*/
private static void assertTasksCreated(List<Id> oppIds, Integer expectedCount, String subject) {
List<Task> tasks = [
SELECT Id, Subject, WhatId
FROM Task
WHERE WhatId IN :oppIds
];
System.Assert.areEqual(expectedCount, tasks.size(),
'Should create ' + expectedCount + ' task(s)');
if (expectedCount > 0) {
System.Assert.areEqual(subject, tasks[0].Subject,
'Task subject should match');
}
}
}

442
skills/trigger-refactor-pipeline/references/handler_patterns.md
View File

@ -1,442 +0,0 @@
# Trigger Handler Patterns Reference
This guide covers common patterns for refactoring Salesforce triggers into handler classes with bulk-safe operations.
## Pattern 1: Simple Handler Class
**Best for**: Triggers with 1-3 contexts and straightforward logic.
### Structure
```apex
public class OpportunityTriggerHandler {
public void beforeInsert(List<Opportunity> newRecords) {
validateClosedWonAmount(newRecords);
}
public void beforeUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap) {
updateDescriptionOnStageChange(newRecords, oldMap);
}
public void afterUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap) {
createTasksForClosedWon(newRecords, oldMap);
}
// Private helper methods below
private void validateClosedWonAmount(List<Opportunity> opportunities) {
for (Opportunity opp : opportunities) {
if (opp.StageName == 'Closed Won' &&
(opp.Amount == null || opp.Amount < 1000)) {
opp.addError('Closed Won opportunities must have Amount ≥ 1000.');
}
}
}
private void updateDescriptionOnStageChange(
List<Opportunity> newRecords,
Map<Id, Opportunity> oldMap
) {
for (Opportunity opp : newRecords) {
Opportunity oldOpp = oldMap.get(opp.Id);
if (opp.StageName != oldOpp.StageName) {
opp.Description = 'Stage changed from ' +
oldOpp.StageName + ' to ' + opp.StageName;
}
}
}
private void createTasksForClosedWon(
List<Opportunity> newRecords,
Map<Id, Opportunity> oldMap
) {
List<Task> tasksToInsert = new List<Task>();
for (Opportunity opp : newRecords) {
Opportunity oldOpp = oldMap.get(opp.Id);
// Check if stage changed to Closed Won
if (opp.StageName == 'Closed Won' &&
oldOpp.StageName != 'Closed Won') {
tasksToInsert.add(new Task(
WhatId = opp.Id,
OwnerId = opp.OwnerId,
Subject = 'Send thank-you',
Status = 'Not Started',
Priority = 'Normal',
ActivityDate = Date.today()
));
}
}
// Bulk DML outside loop
if (!tasksToInsert.isEmpty()) {
insert tasksToInsert;
}
}
}
```
### Trigger Delegation
```apex
trigger OpportunityTrigger on Opportunity (
before insert, before update, after update
) {
OpportunityTriggerHandler handler = new OpportunityTriggerHandler();
if (Trigger.isBefore) {
if (Trigger.isInsert) {
handler.beforeInsert(Trigger.new);
} else if (Trigger.isUpdate) {
handler.beforeUpdate(Trigger.new, Trigger.oldMap);
}
}
if (Trigger.isAfter && Trigger.isUpdate) {
handler.afterUpdate(Trigger.new, Trigger.oldMap);
}
}
```
## Pattern 2: Handler with Database Methods
**Best for**: When you need granular error handling and partial success.
### Key Features
- Uses `Database.insert()` instead of `insert` for partial saves
- Returns `Database.SaveResult` for error handling
- Logs errors without stopping execution
### Example
```apex
private void createTasksForClosedWon(
List<Opportunity> newRecords,
Map<Id, Opportunity> oldMap
) {
List<Task> tasksToInsert = new List<Task>();
for (Opportunity opp : newRecords) {
Opportunity oldOpp = oldMap.get(opp.Id);
if (opp.StageName == 'Closed Won' &&
oldOpp.StageName != 'Closed Won') {
tasksToInsert.add(new Task(
WhatId = opp.Id,
OwnerId = opp.OwnerId,
Subject = 'Send thank-you',
Status = 'Not Started',
Priority = 'Normal',
ActivityDate = Date.today()
));
}
}
if (!tasksToInsert.isEmpty()) {
Database.SaveResult[] results = Database.insert(tasksToInsert, false);
// Log errors without stopping execution
for (Integer i = 0; i < results.size(); i++) {
if (!results[i].isSuccess()) {
System.debug('Failed to create task: ' + results[i].getErrors());
}
}
}
}
```
## Pattern 3: Handler with Maps for Lookups
**Best for**: When you need to query related records for processing.
### Key Features
- Pre-queries related records using Sets
- Uses Maps for O(1) lookups instead of nested loops
- Avoids SOQL in loops
### Example
```apex
private void enrichOpportunitiesWithAccountData(List<Opportunity> opportunities) {
// Collect Account IDs
Set<Id> accountIds = new Set<Id>();
for (Opportunity opp : opportunities) {
if (opp.AccountId != null) {
accountIds.add(opp.AccountId);
}
}
// Single SOQL query outside loop
Map<Id, Account> accountMap = new Map<Id, Account>([
SELECT Id, Name, Industry, AnnualRevenue
FROM Account
WHERE Id IN :accountIds
]);
// Use Map for O(1) lookup
for (Opportunity opp : opportunities) {
if (opp.AccountId != null && accountMap.containsKey(opp.AccountId)) {
Account acc = accountMap.get(opp.AccountId);
// Process with account data
opp.Description = 'Account Industry: ' + acc.Industry;
}
}
}
```
## Pattern 4: Unified Handler Framework
**Best for**: Complex triggers with many contexts and cross-cutting concerns.
### Structure
```apex
public abstract class TriggerHandler {
protected Boolean isBefore;
protected Boolean isAfter;
protected Boolean isInsert;
protected Boolean isUpdate;
protected Boolean isDelete;
protected Boolean isUndelete;
public void run() {
isBefore = Trigger.isBefore;
isAfter = Trigger.isAfter;
isInsert = Trigger.isInsert;
isUpdate = Trigger.isUpdate;
isDelete = Trigger.isDelete;
isUndelete = Trigger.isUndelete;
if (isBefore) {
if (isInsert) beforeInsert();
if (isUpdate) beforeUpdate();
if (isDelete) beforeDelete();
}
if (isAfter) {
if (isInsert) afterInsert();
if (isUpdate) afterUpdate();
if (isDelete) afterDelete();
if (isUndelete) afterUndelete();
}
}
protected virtual void beforeInsert() {}
protected virtual void beforeUpdate() {}
protected virtual void beforeDelete() {}
protected virtual void afterInsert() {}
protected virtual void afterUpdate() {}
protected virtual void afterDelete() {}
protected virtual void afterUndelete() {}
}
```
### Concrete Handler
```apex
public class OpportunityTriggerHandler extends TriggerHandler {
private List<Opportunity> newRecords;
private List<Opportunity> oldRecords;
private Map<Id, Opportunity> newMap;
private Map<Id, Opportunity> oldMap;
public OpportunityTriggerHandler() {
this.newRecords = (List<Opportunity>) Trigger.new;
this.oldRecords = (List<Opportunity>) Trigger.old;
this.newMap = (Map<Id, Opportunity>) Trigger.newMap;
this.oldMap = (Map<Id, Opportunity>) Trigger.oldMap;
}
protected override void beforeInsert() {
validateClosedWonAmount();
}
protected override void beforeUpdate() {
updateDescriptionOnStageChange();
}
protected override void afterUpdate() {
createTasksForClosedWon();
}
// Private helper methods omitted for brevity
}
```
### Trigger Delegation
```apex
trigger OpportunityTrigger on Opportunity (
before insert, before update, after update
) {
new OpportunityTriggerHandler().run();
}
```
## Best Practices
### 1. Bulkification
Always process records in collections:
```apex
// ✓ Good: Collect DML outside loop
List<Task> tasksToInsert = new List<Task>();
for (Opportunity opp : opportunities) {
tasksToInsert.add(new Task(...));
}
if (!tasksToInsert.isEmpty()) {
insert tasksToInsert;
}
// ✗ Bad: DML inside loop
for (Opportunity opp : opportunities) {
insert new Task(...); // SOQL/DML in loop!
}
```
### 2. Defensive Null Checks
```apex
// ✓ Good: Check for null before accessing
if (opp.AccountId != null && accountMap.containsKey(opp.AccountId)) {
Account acc = accountMap.get(opp.AccountId);
// Safe to use acc
}
// ✗ Bad: Assumes data exists
Account acc = accountMap.get(opp.AccountId);
String industry = acc.Industry; // NullPointerException risk
```
### 3. Clear Method Names
```apex
// ✓ Good: Descriptive, verb-noun pattern
private void validateClosedWonAmount(List<Opportunity> opportunities)
private void createTasksForClosedWon(List<Opportunity> opportunities)
// ✗ Bad: Vague or unclear
private void validate(List<Opportunity> opportunities)
private void doStuff(List<Opportunity> opportunities)
```
### 4. Single Responsibility
Each handler method should do one thing:
```apex
// ✓ Good: Separate concerns
private void validateClosedWonAmount(List<Opportunity> opportunities)
private void validateRequiredFields(List<Opportunity> opportunities)
private void calculateDiscounts(List<Opportunity> opportunities)
// ✗ Bad: One method does everything
private void processOpportunities(List<Opportunity> opportunities)
```
### 5. Test Boundaries
Structure code to make testing easier:
```apex
// ✓ Good: Public method for testing, private for implementation
@TestVisible
private void createTasksForClosedWon(
List<Opportunity> newRecords,
Map<Id, Opportunity> oldMap
) {
// Implementation
}
```
## Deployment Order
When deploying refactored triggers:
1. Deploy handler class(es) first
2. Update trigger to use handler
3. Deploy test class
4. Run all tests before production deployment
5. Monitor debug logs for 24-48 hours after production deployment
## Rollback Strategy
Keep the old trigger code commented out or in version control:
```apex
trigger OpportunityTrigger on Opportunity (...) {
// New handler approach
new OpportunityTriggerHandler().run();
/* OLD CODE - REMOVE AFTER 1 WEEK IF NO ISSUES
if (Trigger.isBefore && Trigger.isInsert) {
for (Opportunity o : Trigger.new) {
// old logic
}
}
*/
}
```
## Common Pitfalls
### Pitfall 1: Recursive Triggers
**Problem**: Handler calls DML which triggers the same trigger again.
**Solution**: Use static flag to prevent recursion:
```apex
public class OpportunityTriggerHandler {
private static Boolean isExecuting = false;
public void beforeUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap) {
if (isExecuting) return;
isExecuting = true;
try {
// Your logic here
} finally {
isExecuting = false;
}
}
}
```
### Pitfall 2: Mixed Context Logic
**Problem**: Before-context logic mixed with after-context logic.
**Solution**: Keep context methods separate and focused:
```apex
// ✓ Good: Separate methods per context
public void beforeUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap)
public void afterUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap)
// ✗ Bad: Mixed logic in one method
public void handleUpdate(List<Opportunity> newRecords, Map<Id, Opportunity> oldMap)
```
### Pitfall 3: Over-Engineering
**Problem**: Using complex framework for simple triggers.
**Solution**: Choose the right pattern for your complexity:
- 1-3 contexts with simple logic → Simple Handler (Pattern 1)
- 3-5 contexts with moderate complexity → Handler with Database Methods (Pattern 2)
- 5+ contexts with cross-cutting concerns → Unified Framework (Pattern 4)
## Additional Resources
- [Apex Developer Guide: Trigger and Bulk Request Best Practices](https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_triggers_bulk_requests.htm)
- [Apex Enterprise Patterns](https://github.com/apex-enterprise-patterns)
- [Trigger Framework Comparison](https://github.com/kevinohara80/sfdc-trigger-framework)

258
skills/trigger-refactor-pipeline/scripts/analyze_trigger.py
View File

@ -1,258 +0,0 @@
#!/usr/bin/env python3
"""
Analyze Salesforce Apex triggers for common anti-patterns.
Usage:
python analyze_trigger.py <TriggerName>
Requirements:
- Salesforce CLI authenticated to target org
- Python 3.9+
"""
import sys
import subprocess
import json
import re
from typing import List, Dict, Tuple
class TriggerAnalyzer:
def __init__(self, trigger_name: str):
self.trigger_name = trigger_name
self.trigger_body = ""
self.issues = {
'dml_in_loops': [],
'soql_in_loops': [],
'missing_bulk': [],
}
self.complexity_score = 0
def retrieve_trigger(self) -> bool:
"""Retrieve trigger source code from Salesforce org."""
try:
cmd = [
'sf', 'apex', 'get', 'trigger',
'--trigger-name', self.trigger_name,
'--json'
]
result = subprocess.run(cmd, capture_output=True, text=True)
if result.returncode != 0:
print(f"Error retrieving trigger: {result.stderr}")
return False
data = json.loads(result.stdout)
self.trigger_body = data.get('result', {}).get('body', '')
return True
except Exception as e:
print(f"Failed to retrieve trigger: {e}")
return False
def analyze_dml_in_loops(self) -> None:
"""Detect DML operations inside for loops."""
lines = self.trigger_body.split('\n')
in_loop = False
loop_start = 0
dml_patterns = [
r'\binsert\s+', r'\bupdate\s+', r'\bdelete\s+',
r'\bundelete\s+', r'\bupsert\s+'
]
for i, line in enumerate(lines, 1):
# Track loop entry
if re.search(r'\bfor\s*\(', line):
in_loop = True
loop_start = i
# Track loop exit
if in_loop and line.strip() == '}':
# Check if this closes the loop (simplified heuristic)
in_loop = False
# Check for DML in loop
if in_loop:
for pattern in dml_patterns:
if re.search(pattern, line):
self.issues['dml_in_loops'].append({
'line': i,
'code': line.strip(),
'loop_start': loop_start
})
def analyze_soql_in_loops(self) -> None:
"""Detect SOQL queries inside for loops."""
lines = self.trigger_body.split('\n')
in_loop = False
loop_start = 0
soql_pattern = r'\[SELECT\s+'
for i, line in enumerate(lines, 1):
if re.search(r'\bfor\s*\(', line):
in_loop = True
loop_start = i
if in_loop and line.strip() == '}':
in_loop = False
if in_loop and re.search(soql_pattern, line, re.IGNORECASE):
self.issues['soql_in_loops'].append({
'line': i,
'code': line.strip(),
'loop_start': loop_start
})
def analyze_bulkification(self) -> None:
"""Check for proper bulkification patterns."""
# Simple heuristic: if we have DML in loops, bulkification is missing
if self.issues['dml_in_loops']:
self.issues['missing_bulk'].append({
'message': 'DML operations should be collected and executed outside loops',
'affected_lines': [issue['line'] for issue in self.issues['dml_in_loops']]
})
if self.issues['soql_in_loops']:
self.issues['missing_bulk'].append({
'message': 'SOQL queries should be moved outside loops or use Maps for lookups',
'affected_lines': [issue['line'] for issue in self.issues['soql_in_loops']]
})
def calculate_complexity(self) -> None:
"""Calculate overall complexity score (1-10)."""
score = 1
# Add points for each issue type
score += len(self.issues['dml_in_loops']) * 2
score += len(self.issues['soql_in_loops']) * 2
score += len(self.issues['missing_bulk']) * 1
# Count trigger contexts
contexts = len(re.findall(r'Trigger\.(isBefore|isAfter)', self.trigger_body))
score += contexts
# Count lines of code (normalized)
loc = len([l for l in self.trigger_body.split('\n') if l.strip()])
score += min(loc // 10, 3)
self.complexity_score = min(score, 10)
def recommend_approach(self) -> str:
"""Recommend refactoring approach based on analysis."""
if self.complexity_score <= 3:
return "Simple handler class with separate methods for each trigger context"
elif self.complexity_score <= 6:
return "Handler class with bulkified collections and helper methods"
else:
return "Unified handler framework with separate concern classes (validation, DML, etc.)"
def generate_report(self) -> None:
"""Print analysis report."""
print("\n" + "="*70)
print(f"TRIGGER ANALYSIS REPORT: {self.trigger_name}")
print("="*70 + "\n")
print(f"Complexity Score: {self.complexity_score}/10")
print(f"Recommended Approach: {self.recommend_approach()}\n")
# DML in loops
if self.issues['dml_in_loops']:
print("⚠️ DML OPERATIONS IN LOOPS:")
for issue in self.issues['dml_in_loops']:
print(f" Line {issue['line']}: {issue['code']}")
print(f" └─ Loop started at line {issue['loop_start']}")
print()
else:
print("✓ No DML operations found in loops\n")
# SOQL in loops
if self.issues['soql_in_loops']:
print("⚠️ SOQL QUERIES IN LOOPS:")
for issue in self.issues['soql_in_loops']:
print(f" Line {issue['line']}: {issue['code']}")
print(f" └─ Loop started at line {issue['loop_start']}")
print()
else:
print("✓ No SOQL queries found in loops\n")
# Bulkification
if self.issues['missing_bulk']:
print("⚠️ BULKIFICATION RECOMMENDATIONS:")
for issue in self.issues['missing_bulk']:
print(f" • {issue['message']}")
print(f" Affected lines: {', '.join(map(str, issue['affected_lines']))}")
print()
else:
print("✓ Bulkification patterns look good\n")
print("="*70)
print("NEXT STEPS:")
print("1. Review the handler patterns reference guide")
print("2. Create handler class with bulk-safe collections")
print("3. Extract trigger logic into handler methods")
print("4. Generate comprehensive tests using the template")
print("="*70 + "\n")
def main():
if len(sys.argv) < 2:
print("Usage: python analyze_trigger.py <TriggerName>")
sys.exit(1)
trigger_name = sys.argv[1]
print(f"Analyzing trigger: {trigger_name}...")
analyzer = TriggerAnalyzer(trigger_name)
# For demo purposes, use inline example if retrieval fails
if not analyzer.retrieve_trigger():
print("⚠️ Could not retrieve from org. Using example trigger for demonstration.\n")
# Use the example from the SKILL.md
analyzer.trigger_body = """trigger OpportunityTrigger on Opportunity (before insert, before update, after update) {
if (Trigger.isBefore && Trigger.isInsert) {
for (Opportunity o : Trigger.new) {
if (o.StageName == 'Closed Won' && (o.Amount == null || o.Amount < 1000)) {
o.addError('Closed Won opportunities must have Amount ≥ 1000.');
}
}
}
if (Trigger.isBefore && Trigger.isUpdate) {
for (Opportunity o : Trigger.new) {
Opportunity oldO = Trigger.oldMap.get(o.Id);
if (o.StageName != oldO.StageName) {
o.Description = 'Stage changed from ' + oldO.StageName + ' to ' + o.StageName;
}
}
}
if (Trigger.isAfter && Trigger.isUpdate) {
for (Opportunity o : Trigger.new) {
Opportunity oldO = Trigger.oldMap.get(o.Id);
if (o.StageName == 'Closed Won' && oldO.StageName != 'Closed Won') {
Task t = new Task(
WhatId = o.Id,
OwnerId = o.OwnerId,
Subject = 'Send thank-you',
Status = 'Not Started',
Priority = 'Normal',
ActivityDate = Date.today()
);
insert t;
}
}
}
}"""
# Run analysis
analyzer.analyze_dml_in_loops()
analyzer.analyze_soql_in_loops()
analyzer.analyze_bulkification()
analyzer.calculate_complexity()
# Generate report
analyzer.generate_report()
if __name__ == '__main__':
main()