-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixing endaoment id #1908
fixing endaoment id #1908
Conversation
WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant CronJob
participant OrganizationService
participant ProjectService
CronJob->>OrganizationService: Find Endaoment organization by label
alt Organization found
OrganizationService-->>CronJob: Return organization ID
CronJob->>ProjectService: Fetch projects for organization
ProjectService-->>CronJob: Return projects
CronJob->>CronJob: Process projects
else Organization not found
CronJob->>CronJob: Log error and exit
end
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/services/cronJobs/checkAndUpdateEndaomentProject.ts (3)
24-31
: Consider using a constant for the organization label.While removing the hardcoded organization ID improves maintainability, consider extracting the 'endaoment' label to a named constant or configuration value for better maintainability and type safety.
+const ENDAOMENT_ORG_LABEL = 'endaoment'; const endaomentOrganization = await Organization.findOne({ - where: { label: 'endaoment' }, + where: { label: ENDAOMENT_ORG_LABEL }, });
33-39
: Remove unnecessary optional chaining and improve type clarity.Since we have a null check for
endaomentOrganization
, the optional chaining onendaomentOrganization?.id
is redundant. Also, consider making the status exclusion more explicit.const projects = await Project.find({ where: { - organizationId: endaomentOrganization?.id, + organizationId: endaomentOrganization.id, statusId: Not(ProjStatus.cancelled), }, });
28-31
: Enhance error logging for organization lookup failure.Consider adding more context to the error log when the Endaoment organization is not found, as this is a critical failure point that should be easily debuggable.
if (!endaomentOrganization) { - logger.error('Endaoment organization not found.'); + logger.error('Failed to find Endaoment organization', { + label: 'endaoment', + cronJobTime, + timestamp: new Date().toISOString() + }); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/cronJobs/checkAndUpdateEndaomentProject.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
src/services/cronJobs/checkAndUpdateEndaomentProject.ts (2)
6-6
: LGTM!Clean import addition for the Organization entity.
24-25
: Verify the existence of the Endaoment organization.Let's ensure that an organization with the label 'endaoment' exists in the database.
✅ Verification successful
The 'endaoment' organization is properly configured in the system
The codebase shows that the 'endaoment' organization:
- Is properly initialized with specific configurations
- Has defined token support (ETH on mainnet, base, and optimism networks)
- Has extensive test coverage validating its functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any references to the 'endaoment' organization label in migration files # or seed data to verify its existence echo "Searching for 'endaoment' organization references in migrations..." rg -l "label.*endaoment" "src/migrations" echo "Searching for 'endaoment' organization references in seed data..." rg -l "label.*endaoment" "src/seeds"Length of output: 479
Script:
#!/bin/bash # Broader search for endaoment organization references echo "Searching for files containing 'endaoment'..." rg -i "endaoment" -A 5 -B 5 echo "Looking for database initialization files..." fd -e ts -e js "(seed|migration|init|setup|config.*db)"Length of output: 101668
Summary by CodeRabbit
Bug Fixes
Refactor