Open
Conversation
|
Warning Rate limit exceeded@tarunnjoshi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA new CLI PHP script is introduced to handle material contributions in CiviCRM. The script checks if it runs in a CLI environment, retrieves necessary constants from configuration, reads a CSV file for email addresses, and assigns contributions by interfacing with CiviCRM’s Email API. It includes dedicated functions for reading contacts, assigning contributions by email, and orchestrating the overall process with error handling. Changes
Suggested labels
Suggested reviewers
Poem
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 (
|
![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 (4)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (4)
14-21: Consider improving constants validation and configuration handling.While the basic validation is good, consider these improvements:
- Add type validation for the constants
- Consider moving configuration to a dedicated config class/file
- Add validation for the group ID format
+// Validate types and format +if (!is_string($csvFilePath) || !is_numeric($groupId)) { + exit("Error: Invalid configuration types. CSV path must be string and Group ID must be numeric.\n"); +} + +if ($groupId <= 0) { + exit("Error: Group ID must be a positive number.\n"); +}
26-35: Update PHPDoc to match implementation.The PHPDoc mentions 'Activity Contribution Date' but the function only processes email addresses. Either update the documentation or implement the missing functionality.
86-106: Improve process logging and add batch processing.The main function needs several improvements:
- Update log messages (currently mentions "onHold Process")
- Add progress indication
- Implement batch processing for large files
function main(): void { try { - echo "=== Starting onHold Process ===\n"; + echo "=== Starting Material Contribution Assignment ===\n"; $emails = readContactsFromCsv(MATERIAL_CONTRIBUTION_CSV_FILE_PATH); if (empty($emails)) { + echo "No emails found in CSV file.\n"; return; } + $total = count($emails); + $processed = 0; + $batchSize = 100; + + echo "Processing $total emails...\n"; + foreach ($emails as $email) { assignContributionByEmail($email); + $processed++; + if ($processed % $batchSize === 0) { + echo "Processed $processed/$total emails...\n"; + } } - echo "=== onHold Process Completed ===\n"; + echo "=== Material Contribution Assignment Completed ===\n"; + echo "Total processed: $processed/$total\n"; }
108-110: Add proper exit codes.The script should return appropriate exit codes to indicate success or failure.
-// Run the main function. -main(); +try { + main(); + exit(0); +} catch (Exception $e) { + echo "Fatal error: " . $e->getMessage() . "\n"; + exit(1); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php(1 hunks)
Comment on lines
62
to
84
| /** | ||
| * Improved query to ensure case-insensitive matching. | ||
| */ | ||
| function assignContributionByEmail(string $email): void { | ||
| try { | ||
| // Find contact using Email API. | ||
| $result = Email::get(FALSE) | ||
| ->addSelect('contact_id') | ||
| ->addWhere('email', '=', $email) | ||
| ->execute()->first(); | ||
|
|
||
| // If email is found, process the contact. | ||
| if (isset($result['contact_id'])) { | ||
| $contactId = $result['contact_id']; | ||
| } | ||
| else { | ||
| echo "Contact with email $email not found.\n"; | ||
| } | ||
| } | ||
| catch (Exception $e) { | ||
| echo "An error occurred while processing email $email: " . $e->getMessage() . "\n"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Complete the contribution assignment implementation.
The function name suggests it assigns contributions, but it only retrieves the contact. Additionally:
- Missing PHPDoc for the function
- Retrieved contact_id is not used
- No actual contribution assignment logic
+/**
+ * Assigns material contribution to a contact identified by email.
+ *
+ * @param string $email The contact's email address
+ * @throws Exception When API calls fail
+ */
function assignContributionByEmail(string $email): void {
try {
// Find contact using Email API.
$result = Email::get(FALSE)
->addSelect('contact_id')
->addWhere('email', '=', $email)
->execute()->first();
// If email is found, process the contact.
if (isset($result['contact_id'])) {
$contactId = $result['contact_id'];
+ // TODO: Implement contribution assignment logic here
+ // Example:
+ // Contribution::create()
+ // ->addValue('contact_id', $contactId)
+ // ->addValue('financial_type_id', MATERIAL_CONTRIBUTION_TYPE_ID)
+ // ->addValue('total_amount', 0)
+ // ->execute();
+ echo "Assigned contribution to contact ID: $contactId\n";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Improved query to ensure case-insensitive matching. | |
| */ | |
| function assignContributionByEmail(string $email): void { | |
| try { | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute()->first(); | |
| // If email is found, process the contact. | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "An error occurred while processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } | |
| /** | |
| * Assigns material contribution to a contact identified by email. | |
| * | |
| * @param string $email The contact's email address | |
| * @throws Exception When API calls fail | |
| */ | |
| function assignContributionByEmail(string $email): void { | |
| try { | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute()->first(); | |
| // If email is found, process the contact. | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| // TODO: Implement contribution assignment logic here | |
| // Example: | |
| // Contribution::create() | |
| // ->addValue('contact_id', $contactId) | |
| // ->addValue('financial_type_id', MATERIAL_CONTRIBUTION_TYPE_ID) | |
| // ->addValue('total_amount', 0) | |
| // ->execute(); | |
| echo "Assigned contribution to contact ID: $contactId\n"; | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "An error occurred while processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } |
Comment on lines
36
to
60
| function readContactsFromCsv(string $filePath): array { | ||
| if (!file_exists($filePath) || !is_readable($filePath)) { | ||
| throw new Exception("CSV file not found or not readable: $filePath"); | ||
| } | ||
|
|
||
| $contacts = []; | ||
| if (($handle = fopen($filePath, 'r')) !== FALSE) { | ||
| $header = fgetcsv($handle, 1000, ','); | ||
| if (in_array('email', $header)) { | ||
| while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { | ||
| $row = array_combine($header, $data); | ||
|
|
||
| // Clean email by trimming any extra spaces. | ||
| $email = trim($row['email']); | ||
| $contacts[] = $email; | ||
| } | ||
| } | ||
| else { | ||
| throw new Exception("Error: 'email' column not found in CSV."); | ||
| } | ||
| fclose($handle); | ||
| } | ||
|
|
||
| return $contacts; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add email validation and resource management.
The function needs several improvements:
- Missing email format validation
- No protection against large files
- Resource handle not properly closed in error cases
function readContactsFromCsv(string $filePath): array {
+ // Add file size check
+ $maxFileSize = 10 * 1024 * 1024; // 10MB limit
+ if (filesize($filePath) > $maxFileSize) {
+ throw new Exception("File size exceeds limit of 10MB");
+ }
+
if (!file_exists($filePath) || !is_readable($filePath)) {
throw new Exception("CSV file not found or not readable: $filePath");
}
$contacts = [];
+ $handle = null;
try {
- if (($handle = fopen($filePath, 'r')) !== FALSE) {
+ $handle = fopen($filePath, 'r');
+ if ($handle === FALSE) {
+ throw new Exception("Failed to open file: $filePath");
+ }
$header = fgetcsv($handle, 1000, ',');
if (in_array('email', $header)) {
while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) {
$row = array_combine($header, $data);
// Clean email by trimming any extra spaces.
$email = trim($row['email']);
+ // Validate email format
+ if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
+ echo "Warning: Invalid email format: $email\n";
+ continue;
+ }
$contacts[] = $email;
}
}
else {
throw new Exception("Error: 'email' column not found in CSV.");
}
- fclose($handle);
+ } finally {
+ if ($handle) {
+ fclose($handle);
+ }
}
return $contacts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function readContactsFromCsv(string $filePath): array { | |
| if (!file_exists($filePath) || !is_readable($filePath)) { | |
| throw new Exception("CSV file not found or not readable: $filePath"); | |
| } | |
| $contacts = []; | |
| if (($handle = fopen($filePath, 'r')) !== FALSE) { | |
| $header = fgetcsv($handle, 1000, ','); | |
| if (in_array('email', $header)) { | |
| while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { | |
| $row = array_combine($header, $data); | |
| // Clean email by trimming any extra spaces. | |
| $email = trim($row['email']); | |
| $contacts[] = $email; | |
| } | |
| } | |
| else { | |
| throw new Exception("Error: 'email' column not found in CSV."); | |
| } | |
| fclose($handle); | |
| } | |
| return $contacts; | |
| } | |
| function readContactsFromCsv(string $filePath): array { | |
| // Add file size check | |
| $maxFileSize = 10 * 1024 * 1024; // 10MB limit | |
| if (filesize($filePath) > $maxFileSize) { | |
| throw new Exception("File size exceeds limit of 10MB"); | |
| } | |
| if (!file_exists($filePath) || !is_readable($filePath)) { | |
| throw new Exception("CSV file not found or not readable: $filePath"); | |
| } | |
| $contacts = []; | |
| $handle = null; | |
| try { | |
| $handle = fopen($filePath, 'r'); | |
| if ($handle === FALSE) { | |
| throw new Exception("Failed to open file: $filePath"); | |
| } | |
| $header = fgetcsv($handle, 1000, ','); | |
| if (in_array('email', $header)) { | |
| while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { | |
| $row = array_combine($header, $data); | |
| // Clean email by trimming any extra spaces. | |
| $email = trim($row['email']); | |
| // Validate email format | |
| if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { | |
| echo "Warning: Invalid email format: $email\n"; | |
| continue; | |
| } | |
| $contacts[] = $email; | |
| } | |
| } | |
| else { | |
| throw new Exception("Error: 'email' column not found in CSV."); | |
| } | |
| } finally { | |
| if ($handle) { | |
| fclose($handle); | |
| } | |
| } | |
| return $contacts; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (2)
36-65: 🛠️ Refactor suggestionAdd date validation and improve resource management.
The CSV reading function needs several improvements:
function readContactsFromCsv(string $filePath): array { if (!file_exists($filePath) || !is_readable($filePath)) { throw new Exception("CSV file not found or not readable: $filePath"); } $contacts = []; + $handle = null; try { - if (($handle = fopen($filePath, 'r')) !== FALSE) { + $handle = fopen($filePath, 'r'); + if ($handle === FALSE) { + throw new Exception("Failed to open file: $filePath"); + } $header = fgetcsv($handle, 1000, ','); if (!in_array('email', $header) || !in_array('activity_contribution_date', $header)) { throw new Exception("Error: 'email' or 'activity_contribution_date' column missing in CSV."); } while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { $row = array_combine($header, $data); // Clean values. $email = trim($row['email']); + if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { + echo "Warning: Invalid email format: $email\n"; + continue; + } $contributionDate = trim($row['activity_contribution_date']); + if (!strtotime($contributionDate)) { + echo "Warning: Invalid date format: $contributionDate\n"; + continue; + } - $goonjOffice = trim($row['goonj_office']); $contacts[] = [ 'email' => $email, 'contribution_date' => $contributionDate, ]; } - fclose($handle); } + } finally { + if ($handle) { + fclose($handle); + } } return $contacts; }
102-111:⚠️ Potential issueComplete the contribution processing implementation.
The function is currently a placeholder and needs implementation:
function processContribution(int $contactId, string $contributionDate): void { - echo "Assigning contribution for Contact ID $contactId on $contributionDate.\n"; - // Add logic here to store or process the contribution in CiviCRM. + try { + // Create contribution using CiviCRM API + Contribution::create() + ->addValue('contact_id', $contactId) + ->addValue('financial_type_id', MATERIAL_CONTRIBUTION_TYPE_ID) + ->addValue('receive_date', $contributionDate) + ->addValue('total_amount', 0) + ->execute(); + echo "Successfully assigned contribution for Contact ID $contactId on $contributionDate.\n"; + } catch (Exception $e) { + throw new Exception("Failed to create contribution: " . $e->getMessage()); + } }
🧹 Nitpick comments (3)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (3)
14-21: Add type declarations and existence checks for constants.The configuration validation could be more robust:
// Fetch the CSV file path and Group ID from wp-config.php. -$csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; -$groupId = MATERIAL_CONTRIBUTION_GROUP_ID; +if (!defined('MATERIAL_CONTRIBUTION_CSV_FILE_PATH') || !defined('MATERIAL_CONTRIBUTION_GROUP_ID')) { + exit("Error: Required constants are not defined in wp-config.php\n"); +} + +string $csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; +int $groupId = MATERIAL_CONTRIBUTION_GROUP_ID; // Check if constants are set. if (!$csvFilePath || !$groupId) {
116-135: Add progress tracking and batch processing.The main function could be improved with progress tracking and batch processing:
function main(): void { try { echo "=== Starting Material Contribution Assignment ===\n"; $contacts = readContactsFromCsv(MATERIAL_CONTRIBUTION_CSV_FILE_PATH); if (empty($contacts)) { echo "No valid contacts found in CSV.\n"; return; } + $total = count($contacts); + $processed = 0; + $batchSize = 100; + + echo "Processing $total contacts...\n"; + foreach ($contacts as $contact) { - assignContributionByEmail($contact['email'], $contact['contribution_date'], $contact['goonj_office']); + assignContributionByEmail($contact['email'], $contact['contribution_date']); + $processed++; + + if ($processed % $batchSize === 0) { + echo "Processed $processed/$total contacts...\n"; + } + + // Add small delay to prevent overwhelming the API + usleep(100000); // 100ms delay } - echo "=== Material Contribution Assignment Completed ===\n"; + echo "=== Material Contribution Assignment Completed ($processed contacts processed) ===\n"; }
137-139: Add exit codes for better script integration.The script execution could be improved with proper exit codes:
// Run the main function. -main(); +try { + main(); + exit(0); +} catch (Exception $e) { + echo "Fatal error: " . $e->getMessage() . "\n"; + exit(1); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php(1 hunks)
Comment on lines
67
to
100
| /** | ||
| * Assigns material contribution by email. | ||
| * | ||
| * @param string $email | ||
| * @param string $contributionDate | ||
| */ | ||
| function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice): void { | ||
| try { | ||
|
|
||
| if (empty($email)) { | ||
| echo "No email found in database.\n"; | ||
| return; | ||
| } | ||
| // Find contact using Email API. | ||
| $result = Email::get(FALSE) | ||
| ->addSelect('contact_id') | ||
| ->addWhere('email', '=', $email) | ||
| ->execute() | ||
| ->first(); | ||
|
|
||
| if (isset($result['contact_id'])) { | ||
| $contactId = $result['contact_id']; | ||
|
|
||
| // Assign material contribution. | ||
| processContribution($contactId, $contributionDate); | ||
| } | ||
| else { | ||
| echo "Contact with email $email not found.\n"; | ||
| } | ||
| } | ||
| catch (Exception $e) { | ||
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Complete PHPDoc and remove unused parameter.
The function has several issues:
/**
* Assigns material contribution by email.
*
* @param string $email
* @param string $contributionDate
- */
-function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice): void {
+ * @throws Exception When API calls fail
+ */
+function assignContributionByEmail(string $email, string $contributionDate): void {
try {
- if (empty($email)) {
- echo "No email found in database.\n";
- return;
- }
// Find contact using Email API.
$result = Email::get(FALSE)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Assigns material contribution by email. | |
| * | |
| * @param string $email | |
| * @param string $contributionDate | |
| */ | |
| function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice): void { | |
| try { | |
| if (empty($email)) { | |
| echo "No email found in database.\n"; | |
| return; | |
| } | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute() | |
| ->first(); | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| // Assign material contribution. | |
| processContribution($contactId, $contributionDate); | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } | |
| /** | |
| * Assigns material contribution by email. | |
| * | |
| * @param string $email | |
| * @param string $contributionDate | |
| * @throws Exception When API calls fail | |
| */ | |
| function assignContributionByEmail(string $email, string $contributionDate): void { | |
| try { | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute() | |
| ->first(); | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| // Assign material contribution. | |
| processContribution($contactId, $contributionDate); | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (2)
37-69: 🛠️ Refactor suggestionEnhance CSV reading with validation and resource management.
The function needs several improvements:
- Missing file size limit check
- Resource handle not properly closed in error cases
- No validation for contribution_date format
function readContactsFromCsv(string $filePath): array { + // Add file size check + $maxFileSize = 10 * 1024 * 1024; // 10MB limit + if (filesize($filePath) > $maxFileSize) { + throw new Exception("File size exceeds limit of 10MB"); + } + if (!file_exists($filePath) || !is_readable($filePath)) { throw new Exception("CSV file not found or not readable: $filePath"); } $contacts = []; + $handle = null; try { - if (($handle = fopen($filePath, 'r')) !== FALSE) { + $handle = fopen($filePath, 'r'); + if ($handle === FALSE) { + throw new Exception("Failed to open file: $filePath"); + } $header = fgetcsv($handle, 1000, ','); if (!in_array('email', $header) || !in_array('contribution_date', $header)) { throw new Exception("Error: 'email' or 'contribution_date' column missing in CSV."); } while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { $row = array_combine($header, $data); // Clean values. $email = trim($row['email']); + // Validate email format + if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { + echo "Warning: Invalid email format: $email\n"; + continue; + } $contributionDate = trim($row['contribution_date']); + // Validate date format (assuming Y-m-d format) + if (!DateTime::createFromFormat('Y-m-d', $contributionDate)) { + echo "Warning: Invalid date format for $email: $contributionDate\n"; + continue; + } $goonjOffice = trim($row['goonj_office']); $descriptionOfMaterial = trim($row['description_of_material']); $contacts[] = [ 'email' => $email, 'contribution_date' => $contributionDate, 'goonj_office' => $goonjOffice, 'description_of_material' => $descriptionOfMaterial, ]; } - fclose($handle); + } finally { + if ($handle) { + fclose($handle); + } } return $contacts;
116-125:⚠️ Potential issueComplete the contribution processing implementation.
The function is currently a placeholder and needs implementation:
- Missing actual contribution creation logic
- Incomplete PHPDoc (missing goonjOfficeId parameter)
/** * Processes the contribution assignment. * * @param int $contactId * @param string $contributionDate + * @param string $goonjOfficeId + * @throws Exception When contribution creation fails */ function processContribution(int $contactId, string $contributionDate, string $goonjOfficeId): void { - echo "Assigning contribution for Contact ID $contactId on $contributionDate.\n"; - // Add logic here to store or process the contribution in CiviCRM. + try { + // Create contribution using CiviCRM API + $contribution = \Civi\Api4\Contribution::create() + ->addValue('contact_id', $contactId) + ->addValue('financial_type_id', MATERIAL_CONTRIBUTION_TYPE_ID) + ->addValue('receive_date', $contributionDate) + ->addValue('source', 'Material Contribution') + ->addValue('custom.goonj_office_id', $goonjOfficeId) + ->addValue('total_amount', 0) + ->execute() + ->first(); + + echo "Successfully assigned contribution ID {$contribution['id']} for Contact ID $contactId on $contributionDate.\n"; + } catch (Exception $e) { + throw new Exception("Failed to create contribution: " . $e->getMessage()); + } }
🧹 Nitpick comments (1)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (1)
130-149: Add progress tracking for large files.The main function should provide progress feedback when processing large CSV files.
function main(): void { try { echo "=== Starting Material Contribution Assignment ===\n"; $contacts = readContactsFromCsv(MATERIAL_CONTRIBUTION_CSV_FILE_PATH); if (empty($contacts)) { echo "No valid contacts found in CSV.\n"; return; } + $total = count($contacts); + $processed = 0; + $failed = 0; + echo "Processing $total contacts...\n"; + foreach ($contacts as $contact) { assignContributionByEmail($contact['email'], $contact['contribution_date'], $contact['goonj_office'], $contact['description_of_material']); + $processed++; + if ($processed % 10 === 0) { + echo "Progress: $processed/$total contacts processed\n"; + } } - echo "=== Material Contribution Assignment Completed ===\n"; + echo "=== Material Contribution Assignment Completed ===\n"; + echo "Total processed: $processed\n"; + echo "Failed: $failed\n"; } catch (Exception $e) { echo "Error: " . $e->getMessage() . "\n"; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php(1 hunks)
Comment on lines
+15
to
+22
| // Fetch the CSV file path and Group ID from wp-config.php. | ||
| $csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; | ||
| $groupId = MATERIAL_CONTRIBUTION_GROUP_ID; | ||
|
|
||
| // Check if constants are set. | ||
| if (!$csvFilePath || !$groupId) { | ||
| exit("Error: Both MATERIAL_CONTRIBUTION_CSV_FILE_PATH and MATERIAL_CONTRIBUTION_GROUP_ID must be set.\n"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type validation for configuration constants.
While the existence check is good, the script should also validate the types and format of the configuration values:
MATERIAL_CONTRIBUTION_CSV_FILE_PATHshould be a valid file pathMATERIAL_CONTRIBUTION_GROUP_IDshould be a positive integer
// Fetch the CSV file path and Group ID from wp-config.php.
$csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH;
$groupId = MATERIAL_CONTRIBUTION_GROUP_ID;
// Check if constants are set.
-if (!$csvFilePath || !$groupId) {
+if (!$csvFilePath || !$groupId || !is_string($csvFilePath) || !is_numeric($groupId) || $groupId <= 0) {
exit("Error: Both MATERIAL_CONTRIBUTION_CSV_FILE_PATH and MATERIAL_CONTRIBUTION_GROUP_ID must be set.\n");
}
+
+// Validate file path format
+if (!preg_match('/^[a-zA-Z0-9\/_-]+\.csv$/i', $csvFilePath)) {
+ exit("Error: Invalid CSV file path format.\n");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Fetch the CSV file path and Group ID from wp-config.php. | |
| $csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; | |
| $groupId = MATERIAL_CONTRIBUTION_GROUP_ID; | |
| // Check if constants are set. | |
| if (!$csvFilePath || !$groupId) { | |
| exit("Error: Both MATERIAL_CONTRIBUTION_CSV_FILE_PATH and MATERIAL_CONTRIBUTION_GROUP_ID must be set.\n"); | |
| } | |
| // Fetch the CSV file path and Group ID from wp-config.php. | |
| $csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; | |
| $groupId = MATERIAL_CONTRIBUTION_GROUP_ID; | |
| // Check if constants are set. | |
| if (!$csvFilePath || !$groupId || !is_string($csvFilePath) || !is_numeric($groupId) || $groupId <= 0) { | |
| exit("Error: Both MATERIAL_CONTRIBUTION_CSV_FILE_PATH and MATERIAL_CONTRIBUTION_GROUP_ID must be set.\n"); | |
| } | |
| // Validate file path format | |
| if (!preg_match('/^[a-zA-Z0-9\/_-]+\.csv$/i', $csvFilePath)) { | |
| exit("Error: Invalid CSV file path format.\n"); | |
| } |
Comment on lines
71
to
114
| /** | ||
| * Assigns material contribution by email. | ||
| * | ||
| * @param string $email | ||
| * @param string $contributionDate | ||
| */ | ||
| function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice, string $descriptionOfMaterial): void { | ||
| try { | ||
|
|
||
| if (empty($email)) { | ||
| echo "No email found in database.\n"; | ||
| return; | ||
| } | ||
| // Find contact using Email API. | ||
| $result = Email::get(FALSE) | ||
| ->addSelect('contact_id') | ||
| ->addWhere('email', '=', $email) | ||
| ->execute() | ||
| ->first(); | ||
|
|
||
| if (isset($result['contact_id'])) { | ||
| $contactId = $result['contact_id']; | ||
|
|
||
| $OfficeIds = Contact::get(FALSE) | ||
| ->addSelect('sort_name') | ||
| ->addWhere('contact_type', '=', 'Organization') | ||
| ->addWhere('contact_sub_type', 'CONTAINS', 'Goonj_office') | ||
| ->addWhere('sort_name', '=', $goonjOffice) | ||
| ->execute() | ||
| ->first(); | ||
|
|
||
| $goonjOfficeId = $OfficeIds['id']; | ||
|
|
||
| // Assign material contribution. | ||
| processContribution($contactId, $contributionDate, $goonjOfficeId); | ||
| } | ||
| else { | ||
| echo "Contact with email $email not found.\n"; | ||
| } | ||
| } | ||
| catch (Exception $e) { | ||
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling and documentation.
The function needs several improvements:
- Incomplete PHPDoc (missing parameters and return type)
- Redundant empty email check (already handled in CSV reading)
- Missing error handling for Goonj office lookup
/**
* Assigns material contribution by email.
*
- * @param string $email
- * @param string $contributionDate
+ * @param string $email Contact's email address
+ * @param string $contributionDate Date of contribution (Y-m-d format)
+ * @param string $goonjOffice Name of the Goonj office
+ * @param string $descriptionOfMaterial Description of contributed materials
+ * @throws Exception When API calls fail or office not found
*/
function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice, string $descriptionOfMaterial): void {
try {
- if (empty($email)) {
- echo "No email found in database.\n";
- return;
- }
// Find contact using Email API.
$result = Email::get(FALSE)
->addSelect('contact_id')
->addWhere('email', '=', $email)
->execute()
->first();
if (isset($result['contact_id'])) {
$contactId = $result['contact_id'];
$OfficeIds = Contact::get(FALSE)
->addSelect('sort_name')
->addWhere('contact_type', '=', 'Organization')
->addWhere('contact_sub_type', 'CONTAINS', 'Goonj_office')
->addWhere('sort_name', '=', $goonjOffice)
->execute()
->first();
+ if (!isset($OfficeIds['id'])) {
+ throw new Exception("Goonj office not found: $goonjOffice");
+ }
$goonjOfficeId = $OfficeIds['id'];
// Assign material contribution.
processContribution($contactId, $contributionDate, $goonjOfficeId);
}
else {
echo "Contact with email $email not found.\n";
}
}
catch (Exception $e) {
echo "Error processing email $email: " . $e->getMessage() . "\n";
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Assigns material contribution by email. | |
| * | |
| * @param string $email | |
| * @param string $contributionDate | |
| */ | |
| function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice, string $descriptionOfMaterial): void { | |
| try { | |
| if (empty($email)) { | |
| echo "No email found in database.\n"; | |
| return; | |
| } | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute() | |
| ->first(); | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| $OfficeIds = Contact::get(FALSE) | |
| ->addSelect('sort_name') | |
| ->addWhere('contact_type', '=', 'Organization') | |
| ->addWhere('contact_sub_type', 'CONTAINS', 'Goonj_office') | |
| ->addWhere('sort_name', '=', $goonjOffice) | |
| ->execute() | |
| ->first(); | |
| $goonjOfficeId = $OfficeIds['id']; | |
| // Assign material contribution. | |
| processContribution($contactId, $contributionDate, $goonjOfficeId); | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } | |
| /** | |
| * Assigns material contribution by email. | |
| * | |
| * @param string $email Contact's email address | |
| * @param string $contributionDate Date of contribution (Y-m-d format) | |
| * @param string $goonjOffice Name of the Goonj office | |
| * @param string $descriptionOfMaterial Description of contributed materials | |
| * @throws Exception When API calls fail or office not found | |
| */ | |
| function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice, string $descriptionOfMaterial): void { | |
| try { | |
| // Find contact using Email API. | |
| $result = Email::get(FALSE) | |
| ->addSelect('contact_id') | |
| ->addWhere('email', '=', $email) | |
| ->execute() | |
| ->first(); | |
| if (isset($result['contact_id'])) { | |
| $contactId = $result['contact_id']; | |
| $OfficeIds = Contact::get(FALSE) | |
| ->addSelect('sort_name') | |
| ->addWhere('contact_type', '=', 'Organization') | |
| ->addWhere('contact_sub_type', 'CONTAINS', 'Goonj_office') | |
| ->addWhere('sort_name', '=', $goonjOffice) | |
| ->execute() | |
| ->first(); | |
| if (!isset($OfficeIds['id'])) { | |
| throw new Exception("Goonj office not found: $goonjOffice"); | |
| } | |
| $goonjOfficeId = $OfficeIds['id']; | |
| // Assign material contribution. | |
| processContribution($contactId, $contributionDate, $goonjOfficeId); | |
| } | |
| else { | |
| echo "Contact with email $email not found.\n"; | |
| } | |
| } | |
| catch (Exception $e) { | |
| echo "Error processing email $email: " . $e->getMessage() . "\n"; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (2)
146-168: Improve configuration handling and add progress tracking.The function should accept configuration and provide progress feedback.
/** * Main function to process the CSV and update contacts. + * + * @param string $csvFilePath Path to the CSV file + * @throws Exception When processing fails */ -function main(): void { +function main(string $csvFilePath): void { try { echo "=== Starting Material Contribution Assignment ===\n"; - $contacts = readContactsFromCsv(MATERIAL_CONTRIBUTION_CSV_FILE_PATH); + $contacts = readContactsFromCsv($csvFilePath); if (empty($contacts)) { echo "No valid contacts found in CSV.\n"; return; } + $total = count($contacts); + $processed = 0; foreach ($contacts as $contact) { assignContributionByEmail($contact['email'], $contact['contribution_date'], $contact['goonj_office'], $contact['description_of_material']); + $processed++; + if ($processed % 10 === 0) { + echo "Processed $processed/$total contacts...\n"; + } } echo "=== Material Contribution Assignment Completed ===\n";
170-171: Pass configuration to main function.Avoid using global constants directly in function calls.
// Run the main function. -main(); +main(MATERIAL_CONTRIBUTION_CSV_FILE_PATH);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php(1 hunks)
🔇 Additional comments (3)
wp-content/civi-extensions/goonjcustom/cli/assign-material-contribution-to-contacts.php (3)
16-23: Add type validation for configuration constants.The configuration validation needs improvement to ensure data integrity.
// Fetch the CSV file path and Group ID from wp-config.php. $csvFilePath = MATERIAL_CONTRIBUTION_CSV_FILE_PATH; $groupId = MATERIAL_CONTRIBUTION_GROUP_ID; // Check if constants are set. -if (!$csvFilePath || !$groupId) { +if (!$csvFilePath || !$groupId || !is_string($csvFilePath) || !is_numeric($groupId) || $groupId <= 0) { exit("Error: Both MATERIAL_CONTRIBUTION_CSV_FILE_PATH and MATERIAL_CONTRIBUTION_GROUP_ID must be set.\n"); } + +// Validate file path format +if (!preg_match('/^[a-zA-Z0-9\/_-]+\.csv$/i', $csvFilePath)) { + exit("Error: Invalid CSV file path format.\n"); +}
38-70: Add email validation and resource management.The function needs several improvements for robustness and security.
function readContactsFromCsv(string $filePath): array { + // Add file size check + $maxFileSize = 10 * 1024 * 1024; // 10MB limit + if (filesize($filePath) > $maxFileSize) { + throw new Exception("File size exceeds limit of 10MB"); + } + if (!file_exists($filePath) || !is_readable($filePath)) { throw new Exception("CSV file not found or not readable: $filePath"); } $contacts = []; + $handle = null; try { - if (($handle = fopen($filePath, 'r')) !== FALSE) { + $handle = fopen($filePath, 'r'); + if ($handle === FALSE) { + throw new Exception("Failed to open file: $filePath"); + } $header = fgetcsv($handle, 1000, ','); if (!in_array('email', $header) || !in_array('contribution_date', $header)) { throw new Exception("Error: Required columns missing in CSV."); } while (($data = fgetcsv($handle, 1000, ',')) !== FALSE) { $row = array_combine($header, $data); // Clean values. $email = trim($row['email']); + // Validate email format + if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { + echo "Warning: Invalid email format: $email\n"; + continue; + } + // Validate contribution date format + if (!DateTime::createFromFormat('m/d/Y', trim($row['contribution_date']))) { + echo "Warning: Invalid date format for email $email\n"; + continue; + } $contacts[] = [ 'email' => $email, 'contribution_date' => $contributionDate, 'goonj_office' => $goonjOffice, 'description_of_material' => $descriptionOfMaterial, ]; } - fclose($handle); + } finally { + if ($handle) { + fclose($handle); + } } return $contacts;
72-119: Improve error handling and documentation.The function needs several improvements for robustness and clarity.
/** * Assigns material contribution by email. - * @param string $email - * @param string $contributionDate + * @param string $email Contact's email address + * @param string $contributionDate Date of contribution (Y-m-d format) + * @param string $goonjOffice Name of the Goonj office + * @param string $descriptionOfMaterial Description of contributed materials + * @throws Exception When API calls fail or office not found */ function assignContributionByEmail(string $email, string $contributionDate, string $goonjOffice, string $descriptionOfMaterial): void { try { - if (empty($email)) { - echo "No email found in database.\n"; - return; - } // Find contact using Email API. $result = Email::get(FALSE) ->addSelect('contact_id') ->addWhere('email', '=', $email) ->execute() ->first(); if (isset($result['contact_id'])) { $contactId = $result['contact_id']; $OfficeIds = Contact::get(FALSE) ->addSelect('sort_name') ->addWhere('contact_type', '=', 'Organization') ->addWhere('contact_sub_type', 'CONTAINS', 'Goonj_office') ->addWhere('sort_name', '=', $goonjOffice) ->execute() ->first(); + if (!isset($OfficeIds['id'])) { + throw new Exception("Goonj office not found: $goonjOffice"); + } $goonjOfficeId = $OfficeIds['id'];
Comment on lines
+25
to
+26
| echo "CSV File: $csvFilePath\n"; | ||
| echo "Group ID: $groupId\n"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug output of configuration values.
Echoing configuration values could expose sensitive information in logs.
-echo "CSV File: $csvFilePath\n";
-echo "Group ID: $groupId\n";
+echo "Starting material contribution assignment process...\n";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| echo "CSV File: $csvFilePath\n"; | |
| echo "Group ID: $groupId\n"; | |
| echo "Starting material contribution assignment process...\n"; |
Comment on lines
121
to
144
| /** | ||
| * Processes the contribution assignment. | ||
| * | ||
| * @param int $contactId | ||
| * @param string $contributionDate | ||
| */ | ||
| function processContribution(int $contactId, string $formattedContributionDate, string $goonjOfficeId, string $descriptionOfMaterial): void { | ||
| echo "Assigning contribution for Contact ID $contactId on $formattedContributionDate.\n"; | ||
|
|
||
| try { | ||
| $results = Activity::create(FALSE) | ||
| ->addValue('subject', $descriptionOfMaterial) | ||
| ->addValue('activity_type_id:name', 'Material Contribution') | ||
| ->addValue('status_id:name', 'Completed') | ||
| ->addValue('activity_date_time', $formattedContributionDate) | ||
| ->addValue('source_contact_id', $contactId) | ||
| ->addValue('target_contact_id', $contactId) | ||
| ->execute(); | ||
|
|
||
| } | ||
| catch (\CiviCRM_API4_Exception $ex) { | ||
| \Civi::log()->debug("Exception while creating Organize Collection Camp activity: " . $ex->getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix documentation and error message.
The function has incorrect error message and incomplete documentation.
/**
* Processes the contribution assignment.
*
* @param int $contactId
- * @param string $contributionDate
+ * @param string $formattedContributionDate Formatted date in Y-m-d H:i:s format
+ * @param string $goonjOfficeId ID of the Goonj office
+ * @param string $descriptionOfMaterial Description of the material contributed
+ * @throws \CiviCRM_API4_Exception When activity creation fails
*/
function processContribution(int $contactId, string $formattedContributionDate, string $goonjOfficeId, string $descriptionOfMaterial): void {
try {
$results = Activity::create(FALSE)
->addValue('subject', $descriptionOfMaterial)
->addValue('activity_type_id:name', 'Material Contribution')
->addValue('status_id:name', 'Completed')
->addValue('activity_date_time', $formattedContributionDate)
->addValue('source_contact_id', $contactId)
->addValue('target_contact_id', $contactId)
->execute();
}
catch (\CiviCRM_API4_Exception $ex) {
- \Civi::log()->debug("Exception while creating Organize Collection Camp activity: " . $ex->getMessage());
+ \Civi::log()->debug("Exception while creating Material Contribution activity: " . $ex->getMessage());
+ throw $ex;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Processes the contribution assignment. | |
| * | |
| * @param int $contactId | |
| * @param string $contributionDate | |
| */ | |
| function processContribution(int $contactId, string $formattedContributionDate, string $goonjOfficeId, string $descriptionOfMaterial): void { | |
| echo "Assigning contribution for Contact ID $contactId on $formattedContributionDate.\n"; | |
| try { | |
| $results = Activity::create(FALSE) | |
| ->addValue('subject', $descriptionOfMaterial) | |
| ->addValue('activity_type_id:name', 'Material Contribution') | |
| ->addValue('status_id:name', 'Completed') | |
| ->addValue('activity_date_time', $formattedContributionDate) | |
| ->addValue('source_contact_id', $contactId) | |
| ->addValue('target_contact_id', $contactId) | |
| ->execute(); | |
| } | |
| catch (\CiviCRM_API4_Exception $ex) { | |
| \Civi::log()->debug("Exception while creating Organize Collection Camp activity: " . $ex->getMessage()); | |
| } | |
| } | |
| /** | |
| * Processes the contribution assignment. | |
| * | |
| * @param int $contactId | |
| * @param string $formattedContributionDate Formatted date in Y-m-d H:i:s format | |
| * @param string $goonjOfficeId ID of the Goonj office | |
| * @param string $descriptionOfMaterial Description of the material contributed | |
| * @throws \CiviCRM_API4_Exception When activity creation fails | |
| */ | |
| function processContribution(int $contactId, string $formattedContributionDate, string $goonjOfficeId, string $descriptionOfMaterial): void { | |
| try { | |
| $results = Activity::create(FALSE) | |
| ->addValue('subject', $descriptionOfMaterial) | |
| ->addValue('activity_type_id:name', 'Material Contribution') | |
| ->addValue('status_id:name', 'Completed') | |
| ->addValue('activity_date_time', $formattedContributionDate) | |
| ->addValue('source_contact_id', $contactId) | |
| ->addValue('target_contact_id', $contactId) | |
| ->execute(); | |
| } | |
| catch (\CiviCRM_API4_Exception $ex) { | |
| \Civi::log()->debug("Exception while creating Material Contribution activity: " . $ex->getMessage()); | |
| throw $ex; | |
| } | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add material contribution to contacts
Summary by CodeRabbit