send receipt to Institution Material contribution #881
Conversation
WalkthroughThe pull request introduces a minor modification to the Changes
Possibly related PRs
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 (
|
belwalshubham
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionMaterialContributionService.php (1)
Line range hint
134-144: Add error handling for email operations.The email sending process lacks error handling for:
- File operations when reading images
- Email sending failures
private static function sendInstitutionMaterialContributionEmails(array $contacts, string $description, string $deliveredBy, string $deliveredByContact, string $activityDate) { foreach ($contacts as $contact) { + try { $email = $contact['email']; $name = $contact['name']; $phone = $contact['phone']; $subject = 'Acknowledgement for your material contribution to Goonj'; $body = self::generateEmailBody($name); $html = self::generateContributionReceiptHtml($email, $phone, $description, $name, $deliveredBy, $deliveredByContact, $activityDate); $attachments = [\CRM_Utils_Mail::appendPDF('institution_material_contribution.pdf', $html)]; $params = self::prepareEmailParams($subject, $body, $attachments, $email); \CRM_Utils_Mail::send($params); + } catch (\Exception $e) { + \Civi::log()->error('Failed to send material contribution email to ' . $email . ': ' . $e->getMessage()); + } } }
🧹 Nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionMaterialContributionService.php (2)
Line range hint
93-102: Consider adding error handling and type hints.The method could benefit from:
- Error handling for API failures
- Parameter and return type hints
- Input validation
- private static function getInstitutionRelationships($organizationId) { + /** + * Fetches institution POC relationships for a given organization. + * + * @param int $organizationId The ID of the organization + * @return array<string, array{email: string, phone: string, name: string}> Array of contacts indexed by email + * @throws \Civi\API\Exception\UnauthorizedException + */ + private static function getInstitutionRelationships(int $organizationId): array { $contacts = []; + if ($organizationId <= 0) { + return $contacts; + } + + try { $relationship = Relationship::get(FALSE) ->addSelect('contact_id_b') ->addWhere('contact_id_a', '=', $organizationId) ->addWhere('relationship_type_id:name', '=', 'Institution POC of') ->execute() ->first(); + } catch (\Exception $e) { + \Civi::log()->error('Failed to fetch institution relationships: ' . $e->getMessage()); + return $contacts; + }
Line range hint
1-391: Consider splitting the class to follow Single Responsibility Principle.The class currently handles multiple concerns:
- Event subscription
- Data fetching
- Email generation
- PDF generation
Consider extracting these into separate services:
InstitutionRelationshipRepositoryfor data fetchingMaterialContributionEmailServicefor email operationsMaterialContributionPdfGeneratorfor PDF generationThis would improve maintainability and testability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionMaterialContributionService.php(1 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionMaterialContributionService.php (1)
94-94: LGTM! Explicit field selection improves code clarity.The addition of explicit
contact_id_bselection makes the code more maintainable by clearly indicating which fields are required.
Summary by CodeRabbit