Conversation
WalkthroughThis update introduces significant enhancements to the notification system, including expanded notification types, improved type safety, and unified notification creation logic. The user entity is extended with notification preferences and presence tracking. Additional improvements include error handling, timezone normalization, and minor refactoring across modules, controllers, and services for consistency and maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Service
participant NotificationService
participant SSE
participant Database
User->>Service: Perform action (e.g., comment, dispute, job post)
Service->>NotificationService: createNotification({ userId, type, content, ... })
NotificationService->>Database: Store notification
NotificationService->>SSE: Emit notification to user (if immediate)
NotificationService-->>Service: Return notification
Service-->>User: Respond with result
sequenceDiagram
participant User
participant NotificationService
participant Database
User->>NotificationService: markAsRead(notificationId, read)
NotificationService->>Database: Update notification read status
NotificationService-->>User: Confirmation
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 23
🔭 Outside diff range comments (1)
src/notifications/controllers/notifications-polling.controller.ts (1)
14-21: 🛠️ Refactor suggestionFix type inconsistency for userId parameter.
There's a type mismatch: the
userIdparameter is typed asnumberbut converted to string when passed to the service. This suggests the service method expects a string type.Consider updating the parameter type to match the expected string type:
-async pollNotifications(@Query('userId') userId: number, @Query('lastSeen') lastSeen?: string) { +async pollNotifications(@Query('userId') userId: string, @Query('lastSeen') lastSeen?: string) { // Get notifications for the user, potentially filtering by timestamp // if lastSeen is provided if (!userId) { return { error: "User ID is required" } } - const notifications = await this.notificationsService.findByUser(userId.toString()); + const notifications = await this.notificationsService.findByUser(userId);
🧹 Nitpick comments (10)
src/job-fit/job-fit.service.ts (3)
50-54: Consider centralizing boost values into constants for maintainability.
The hardcoded endorsement boost values (5and2) could be moved to well-named constants or configuration, improving readability and making future adjustments easier.
68-72: Simplify null checks with optional chaining.
You can replace:if (contract.jobPosting && contract.jobPosting.description) {with:
if (contract.jobPosting?.description) {to safely handle missing properties and reduce verbosity.
-if (contract.jobPosting && contract.jobPosting.description) { +if (contract.jobPosting?.description) {
94-98: Optimize skill normalization in cosine similarity.
Currently,toLowerCase()and array mapping are executed repeatedly for each skill. Consider normalizinguserSkillsandjobSkillsto lowercase sets once before building vectors:const lowerUserSkills = new Set(userSkills.map(s => s.toLowerCase())); const lowerJobSkills = new Set(jobSkills.map(s => s.toLowerCase())); const allSkills = Array.from(new Set([...lowerUserSkills, ...lowerJobSkills]));This reduces redundant work and improves performance for large skill lists.
src/notifications/strategies/email.strategy.ts (1)
52-52: Remove unnecessary empty line.The extra empty line at the end of the method should be removed for consistency.
-src/notifications/strategies/webhook.strategy.ts (1)
43-43: Remove unnecessary empty line.The extra empty line at the end of the method should be removed for consistency.
-src/auth/guards/permissions.guard.ts (1)
44-49: Consider security implications of detailed logging.The logging includes sensitive information like user ID and role. Ensure this doesn't create security vulnerabilities in production logs.
Consider using different log levels or sanitizing sensitive information:
- this.logger.warn( - `User ${user.id} with role ${user.role} lacks required permissions: ${requiredPermissions}`, - ); + this.logger.warn( + `User lacks required permissions`, + { userId: user.id, role: user.role, requiredPermissions } + );src/user/entities/user.entity.ts (1)
167-168: Consider the performance impact of eager loading.While the eager loading ensures notification preferences are always available, this could impact performance for user queries that don't need preference data. Consider if lazy loading with explicit joins where needed would be more appropriate.
If you decide to change this, you can remove the
eager: trueoption:- @OneToMany(() => NotificationPreference, pref => pref.user, { eager: true }) + @OneToMany(() => NotificationPreference, pref => pref.user)src/notifications/strategies/push.strategy.ts (1)
48-75: Consider the trade-offs of individual sends vs multicast.While sending individual notifications provides better granular error handling, it may be less efficient than multicast sending for large numbers of device tokens. The current approach is good for reliability but consider performance implications.
For better performance with many tokens, you could batch tokens and use multicast with smaller batches:
const batchSize = 100; const batches = []; for (let i = 0; i < deviceTokens.length; i += batchSize) { batches.push(deviceTokens.slice(i, i + batchSize)); }src/scheduler/utils/timezone.helper.ts (1)
47-61: Good defensive programming with environment fallback!The fallback implementation ensures compatibility with environments that don't support
Intl.supportedValuesOf.Consider expanding the fallback timezone list to include more common timezones:
return [ 'UTC', 'America/New_York', + 'America/Chicago', + 'America/Denver', + 'America/Los_Angeles', 'Europe/London', + 'Europe/Paris', + 'Europe/Berlin', 'Asia/Tokyo', + 'Asia/Shanghai', + 'Asia/Kolkata', 'Australia/Sydney', - // add more if needed ];src/notifications/notifications.service.ts (1)
97-97: Consider standardizing userId types across the codebase.The conversion from string to number (
Number(notification.userId)) suggests inconsistent userId types between the notification system and SSE service.Consider standardizing all userId references to use a single type (preferably string for UUID compatibility) throughout the application to avoid these conversions and potential type-related bugs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
.env.development(1 hunks).gitignore(1 hunks)package.json(1 hunks)src/anonymization/controllers/anonymization.controller.ts(1 hunks)src/audit/audit.controller.ts(1 hunks)src/auth/decorators/permissions.decorator.ts(1 hunks)src/auth/enums/permission.enum.ts(1 hunks)src/auth/guards/permissions.guard.ts(1 hunks)src/auth/guards/roles.guard.ts(1 hunks)src/auth/services/permission.service.ts(2 hunks)src/auth/types/permission.type.ts(1 hunks)src/comment/comment.service.ts(1 hunks)src/dispute/dispute.service.ts(7 hunks)src/error-tracking/error-tracking.controller.ts(1 hunks)src/escrow-payment/escrow-payment.service.ts(1 hunks)src/job-fit/job-fit.controller.ts(1 hunks)src/job-fit/job-fit.service.ts(1 hunks)src/job-postings/job-postings.service.ts(7 hunks)src/notification-settings/notification-settings.module.ts(1 hunks)src/notifications/controllers/notifications-polling.controller.ts(1 hunks)src/notifications/entities/notification-preference.entity.ts(1 hunks)src/notifications/entities/notification.entity.ts(3 hunks)src/notifications/notifications.controller.ts(2 hunks)src/notifications/notifications.module.ts(2 hunks)src/notifications/notifications.service.ts(7 hunks)src/notifications/queue/notification.processor.ts(3 hunks)src/notifications/strategies/email.strategy.ts(1 hunks)src/notifications/strategies/push.strategy.ts(4 hunks)src/notifications/strategies/webhook.strategy.ts(1 hunks)src/presence/presence.service.ts(1 hunks)src/recommendations/services/recommendation-requests.service.ts(3 hunks)src/recommendations/services/recommendations.service.ts(3 hunks)src/reporting/reporting.service.ts(1 hunks)src/reports/reports.service.ts(2 hunks)src/reputation/reputation.module.ts(1 hunks)src/scheduler/calendar/ics.generator.ts(2 hunks)src/scheduler/scheduler.controller.ts(3 hunks)src/scheduler/scheduler.module.ts(1 hunks)src/scheduler/scheduler.service.ts(1 hunks)src/scheduler/utils/timezone.helper.ts(3 hunks)src/skills/controllers/skill.controller.ts(1 hunks)src/time-tracking/time-tracking.controller.ts(1 hunks)src/user/entities/user.entity.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/notifications/notifications.controller.ts (1)
src/auth/decorators/roles.decorator.ts (1)
Roles(6-6)
src/notification-settings/notification-settings.module.ts (4)
src/notifications/notifications.module.ts (1)
Module(20-48)src/reputation/reputation.module.ts (1)
Module(12-18)src/app.module.ts (1)
Module(66-164)src/user/user.module.ts (1)
Module(15-24)
src/auth/enums/permission.enum.ts (1)
src/auth/types/permission.type.ts (1)
Permission(1-9)
🪛 Biome (1.9.4)
src/job-fit/job-fit.service.ts
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/notifications/notifications.service.ts
[error] 316-316: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (64)
src/reputation/reputation.module.ts (3)
13-13: LGTM! Module imports are well-structured.The addition of
ProjectModule,EndorsementModule, andRatingsModuleto the imports array properly establishes the dependencies needed for reputation functionality integration.
16-16: Good practice: Exporting ReputationService for module reusability.Exporting the
ReputationServiceallows other modules to use reputation functionality, which follows good modular design practices.
6-10:Details
✅ Verification successful
Verify service availability through imported modules.
Ensure that the imported modules (
ProjectModule,EndorsementModule,RatingsModule) properly export their services if they're needed by theReputationService.
🏁 Script executed:
#!/bin/bash # Description: Verify that the imported modules export their services properly # Expected: Each module should export the services that ReputationService might need echo "Checking ProjectModule exports..." rg -A 10 "exports.*:" src/project/project.module.ts echo -e "\nChecking EndorsementModule exports..." rg -A 10 "exports.*:" src/endorsement/endorsement.module.ts echo -e "\nChecking RatingsModule exports..." rg -A 10 "exports.*:" src/rating/rating.module.ts echo -e "\nChecking if ReputationService injects any of these services..." rg -A 5 -B 5 "constructor.*" src/reputation/provider/reputation.service.tsLength of output: 1292
Services properly exported by modules
All required services (ProjectService, EndorsementService, RatingsService) are indeed exported by their respective modules and correctly injected into ReputationService. No further changes needed.
src/notifications/notifications.controller.ts (2)
9-9: Good improvement for consistency.Adding the Role enum import supports the type-safe refactoring effort across the codebase.
26-26: Excellent type safety improvement.Replacing the hardcoded string
'admin'withRole.ADMINenum value improves type safety and maintains consistency with the broader refactoring effort. This change aligns with theRolesdecorator's expected parameter type as shown in the relevant code snippet.src/escrow-payment/escrow-payment.service.ts (4)
36-44: Good formatting improvement.The multiline arrow function mapping improves readability while maintaining the same calculation logic for split amounts.
52-55: Improved readability.Formatting the query parameters across multiple lines enhances code readability without changing functionality.
79-81: Excellent defensive programming.Adding an explicit null check for the parent payment prevents potential runtime errors and provides clearer error messaging. This is a valuable improvement to error handling.
65-71: Good formatting consistency.The multiline formatting for audit operations and queries improves code readability and maintains consistency with the rest of the refactored methods.
Also applies to: 95-98
src/audit/audit.controller.ts (1)
8-8: Approve import path update forRoleenum.
Aligns with the centralized enum location. Ensure yourtsconfig.jsonhas the corresponding path alias (@src/*) configured for this import to resolve correctly.src/auth/guards/roles.guard.ts (1)
9-9: Approve import adjustment forRoleenum.
The path update to../enums/role.enumis consistent with the refactoring strategy for shared enums.src/skills/controllers/skill.controller.ts (1)
8-8: Approve import path standardization forRoleenum.
The absolute alias import (@src/auth/enums/role.enum) matches patterns used elsewhere. Confirm alias resolution intsconfig.json.src/notifications/entities/notification-preference.entity.ts (1)
3-3: Standardized import path for User entity
The import has been updated to use the absolute alias, aligning with the project’s path conventions and ensuring consistency across modules.src/anonymization/controllers/anonymization.controller.ts (1)
12-12: Standardized import path for Role enum
Switching to the absolute alias import aligns with the project-wide convention for centralizing enums.src/error-tracking/error-tracking.controller.ts (1)
15-15: Standardized import path for Role enum
This update ensures consistency with other controllers by using the absolute alias.src/time-tracking/time-tracking.controller.ts (1)
11-11: Standardized import path for Role enum
The Role enum import now uses the project’s alias, matching other modules.src/presence/presence.service.ts (1)
53-53: Corrected typo in updateUserStatus payload
FixedresenceStatus→presenceStatusto match the entity’s field, ensuring the status persists correctly.package.json (1)
63-63:Details
✅ Verification successful
Verify firebase-admin package version and security status.
The firebase-admin dependency has been added to support push notification functionality. Please ensure this version is current and free from known vulnerabilities.
🌐 Web query:
What is the latest version of firebase-admin npm package and are there any security advisories for version 13.4.0?💡 Result:
Latest Version of firebase-admin NPM Package
The latest version of the
firebase-adminnpm package as of May 28, 2025, is 13.4.0. This version was published on May 12, 2025[2][4].Security Advisories for Version 13.4.0
According to the Snyk Vulnerability Database, there are no direct vulnerabilities found for
firebase-adminversion 13.4.0[9]. The report specifically states:"No direct vulnerabilities have been found for this package in Snyk’s vulnerability database. This does not include vulnerabilities belonging to this package’s dependencies."[9]
No security advisories or critical issues have been reported for this version in the official release notes or security trackers at the time of writing[9][4].
Summary Table
Package Latest Version Release Date Security Advisories (13.4.0) firebase-admin (npm) 13.4.0 2025-05-12 None reported You can confidently use version 13.4.0, as there are no known, direct security vulnerabilities for this release[9].
Citations:
- 1: https://firebase.google.com/support/release-notes/admin/node
- 2: https://www.npmjs.com/package/firebase-admin
- 3: https://firebase.google.com/support/releases
- 4: https://github.com/firebase/firebase-admin-node/releases
- 5: https://www.nuget.org/packages/FirebaseAdmin
- 6: https://firebase.blog/posts/2019/03/firebase-security-rules-admin-sdk-tips/
- 7: firebase/firebase-admin-node#2772
- 8: https://www.mongodb.com/community/forums/t/firebase-admin-support/253761
- 9: https://security.snyk.io/package/npm/firebase-admin/13.4.0
- 10: https://cloud.google.com/identity-platform/docs/install-admin-sdk
firebase-admin dependency is up-to-date and has no known vulnerabilities
- package.json (line 63):
"firebase-admin": "^13.4.0"is the latest release (published May 12, 2025).- Verified via npm registry and Snyk database: no direct security advisories against version 13.4.0.
No further changes required.
.gitignore (1)
41-46: LGTM! Good security practice.Adding environment-specific dotenv files to .gitignore prevents accidental commit of sensitive configuration data like Firebase credentials. This aligns well with the Firebase integration changes.
src/reporting/reporting.service.ts (1)
68-68: Good formatting improvement.The added blank line improves code readability by separating the update operation from the return statement.
src/scheduler/scheduler.controller.ts (3)
29-33: Excellent error handling improvement.The enhanced error handling properly type-checks the error before accessing its message property, preventing potential runtime issues with non-Error objects.
52-56: Consistent and robust error handling.The error handling follows the same robust pattern as other methods, providing better error messages and type safety.
75-78: Good error handling with appropriate fallback message.The error handling maintains consistency with other methods while providing a specific fallback message relevant to ICS file generation.
src/notifications/strategies/email.strategy.ts (1)
45-49: LGTM! Improved error handling with type safety.The enhanced error handling with
instanceof Errorchecking is a good practice that prevents potential runtime errors when accessing error properties.src/auth/services/permission.service.ts (2)
2-3: LGTM! Import path updates look correct.The updated import paths for enums improve the project structure organization.
21-24:Details
✅ Verification successful
Verify new roles are defined in the enum files.
The new roles (USER, SECURITY_AUDITOR, MODERATOR, JUROR) are added with VIEW_PROJECT permission. Please ensure these roles are properly defined in the Role enum.
🏁 Script executed:
#!/bin/bash # Description: Verify that the new roles are defined in the Role enum # Expected: Find definitions for USER, SECURITY_AUDITOR, MODERATOR, JUROR echo "Checking Role enum definitions..." ast-grep --pattern 'enum Role { $$$ }' echo -e "\nChecking for specific role definitions..." rg -A 5 -B 5 "USER|SECURITY_AUDITOR|MODERATOR|JUROR" src/auth/enums/role.enum.tsLength of output: 963
Roles Defined in Role Enum Confirmed
Verified that
USER,SECURITY_AUDITOR,MODERATOR, andJURORare all present insrc/auth/enums/role.enum.ts. No further changes needed.src/recommendations/services/recommendations.service.ts (3)
14-14: LGTM! Good addition of NotificationType import.Adding the NotificationType import supports the type-safe refactoring of notification creation.
49-55: LGTM! Consistent use of typed notification creation.The updated notification creation pattern using
createNotificationwithNotificationType.NEW_RECOMMENDATIONenum and structured content object improves type safety and consistency.
130-146: LGTM! Consistent notification patterns for status updates.Both approval and rejection notification creations follow the same improved pattern with typed enums and structured content, maintaining consistency across the service.
src/notification-settings/notification-settings.module.ts (1)
4-4: Import reordering looks good.The reordering of the
NotificationSettingsServiceimport has no functional impact and maintains consistency.src/notifications/queue/notification.processor.ts (1)
24-29: Good improvement to strategy map initialization.The explicit use of
thisreferences and the generic type annotationMap<string, any>improve code clarity and provide better type information.src/scheduler/scheduler.module.ts (2)
6-7: Good additions for enhanced scheduler functionality.The new imports for
TimezoneHelperandIcsGeneratoralign well with the enhanced timezone and calendar functionality described in the summary.
13-13: Appropriate export of TimezoneHelper.Exporting
TimezoneHelpermakes it available for other modules that might need timezone utilities, which is a good architectural decision.src/reports/reports.service.ts (2)
8-8: Good addition of typed imports.Adding imports for
DeliveryChannelandNotificationTypeenables type-safe notification creation.
101-107: Excellent refactoring to use structured notification creation.The migration from
notificationsService.createtocreateNotificationwith typed enums and structured content represents a significant improvement:
- Type Safety: Using
NotificationType.REPORT_UPDATEandDeliveryChannel.PUSHprevents typos and ensures consistency- Structured Content: The content object with a
messageproperty is more maintainable than passing raw strings- Enhanced Control: Adding
priorityand explicitdeliveryChannelprovides better notification management- Consistency: Aligns with the broader notification system refactoring across the codebase
src/scheduler/scheduler.service.ts (1)
79-85:Details
✅ Verification successful
Consider type safety concerns with object intersection.
The type assertion
InterviewSlot & { timezone: string }may not be accurate if theInterviewSlotentity doesn't have atimezoneproperty, potentially causing runtime issues.Verify that the
InterviewSlotentity includes thetimezoneproperty:
🏁 Script executed:
#!/bin/bash # Description: Check if InterviewSlot entity has timezone property # Expected: Find timezone property definition in the entity ast-grep --pattern $'class InterviewSlot { $$$ timezone: $_; $$$ }'Length of output: 74
🏁 Script executed:
#!/bin/bash # Search for InterviewSlot definition (class, interface, or type) and check for timezone property rg -n -C2 'export (interface|type|class) InterviewSlot' src rg -n -C2 'class InterviewSlot' srcLength of output: 803
🏁 Script executed:
#!/bin/bash # Check for any occurrence of 'timezone' in the InterviewSlot entity rg -n 'timezone' src/scheduler/entities/interview-slot.entity.tsLength of output: 89
InterviewSlot includes
timezone; intersection type is safeThe
InterviewSlotentity insrc/scheduler/entities/interview-slot.entity.tsdefines atimezone: stringproperty (line 37), so theInterviewSlot & { timezone: string }assertion accurately reflects the shape and poses no runtime risk. No changes required.src/recommendations/services/recommendation-requests.service.ts (1)
62-68: LGTM! Notification structure refactoring looks good.The refactoring from flat notification objects to structured content objects improves type safety and consistency. The use of enum values instead of string literals is a good practice.
Also applies to: 121-143
src/auth/guards/permissions.guard.ts (2)
16-16: LGTM! Logger addition improves observability.Adding a logger instance enhances debugging capabilities for permission-related issues.
33-36: Good improvement in error handling with logging.Adding logging when no user is found helps with debugging authentication issues.
src/notifications/notifications.module.ts (2)
22-22: Good use of forwardRef for circular dependency resolution.Using
forwardRefis the correct approach to handle circular dependencies between modules.
23-27: LGTM! Entity registration expansion looks correct.Adding
JobNotificationto the TypeORM feature registration is appropriate for the notification system expansion.src/user/entities/user.entity.ts (2)
37-38: LGTM! Clean import statements with consistent path aliases.The import statements use consistent absolute path aliases and follow TypeScript conventions.
170-174: LGTM! Well-designed activity tracking and presence management.The nullable
lastActivityAttimestamp andpresenceStatusenum with sensible default value follow good database design practices.src/job-postings/job-postings.service.ts (3)
1-3: LGTM! Good imports and logger addition.The additional imports support the type safety improvements and proper logging functionality.
Also applies to: 8-8, 12-12
89-89: Excellent: Type safety improvements with NotificationType enum.Replacing string literals with the
NotificationTypeenum significantly improves type safety and reduces the risk of typos.Also applies to: 96-96, 123-123, 137-137, 172-172, 183-183
100-107: LGTM! Robust error handling with proper logging.The error handling properly distinguishes between
Errorinstances and unknown errors, with appropriate logging and graceful degradation.src/scheduler/calendar/ics.generator.ts (5)
3-3: LGTM! Improved type safety with proper imports.Adding
EventAttributesandEventStatustypes enhances type safety for the ICS generation.
10-27: LGTM! Explicit date array construction improves readability.The explicit construction of start and end date arrays makes the code more maintainable and self-documenting compared to inline date extraction.
30-30: LGTM! Proper TypeScript typing for event attributes.Using the
EventStatustype andEventAttributesinterface ensures compile-time type checking and better IDE support.Also applies to: 32-32, 44-44
58-58: LGTM! Defensive programming with fallback value.The fallback to empty string prevents potential undefined return values while maintaining the Promise contract.
65-72: LGTM! Cleaner and more efficient attendees generation.The simplified logic with early return and direct mapping is more readable and efficient than the previous implementation.
src/notifications/strategies/push.strategy.ts (3)
14-23: LGTM! Robust Firebase configuration with proper validation.The explicit config retrieval and validation prevents runtime failures and the private key processing handles environment variable escaping correctly.
77-84: LGTM! Enhanced error handling with proper logging.The error handling correctly distinguishes between known Error instances and unknown errors, providing appropriate logging for debugging.
39-39:Details
✅ Verification successful
Verify the property comparison logic.
The comparison
pref.notificationType === notification.typeassumes both values use the same type system. Ensure consistency between the notification preference type and notification type properties.
🏁 Script executed:
#!/bin/bash # Check the NotificationPreference entity structure ast-grep --pattern 'class NotificationPreference { $$$ notificationType: $_ $$$ }'Length of output: 90
🏁 Script executed:
#!/usr/bin/env bash set -e echo "Searching for NotificationPreference definitions..." rg -n "class NotificationPreference" . rg -n "interface NotificationPreference" . rg -n "type NotificationPreference" . echo -e "\nLocating notificationType property declarations..." rg -n "notificationType\s*:" . echo -e "\nLocating notification\\.type usage..." rg -n "notification\.type" . echo -e "\nExtracting context from push.strategy.ts around line 39..." push_file=$(rg -l "push.strategy.ts") echo "File: $push_file" rg -n -C 3 "pref.notificationType" "$push_file"Length of output: 310
🏁 Script executed:
#!/usr/bin/env bash set -e # Inspect NotificationPreference entity pref_file="src/notifications/entities/notification-preference.entity.ts" echo "NotificationPreference file: $pref_file" rg -n -C2 "notificationType" "$pref_file" || echo "❗ No matches for notificationType in $pref_file" # Locate and inspect Notification entity notif_file=$(rg -l "export class Notification" src/notifications/entities) echo -e "\nNotification entity file: $notif_file" rg -n -C2 "type" "$notif_file" || echo "❗ No matches for type in $notif_file" # Show comparison context in push.strategy.ts ctx_file="src/notifications/strategies/push.strategy.ts" echo -e "\nContext in $ctx_file around pref.notificationType:" rg -n -C3 "pref.notificationType" "$ctx_file" || echo "❗ No matches for pref.notificationType in $ctx_file"Length of output: 2138
🏁 Script executed:
#!/usr/bin/env bash set -e # Locate Notification entity file (exclude preference) echo "Locating Notification entity file..." notif_file=$(rg -l "export class Notification" src/notifications/entities | grep -Ev "notification-preference") echo "Found Notification file: $notif_file" # Show the 'type' property annotation echo -e "\nProperties around 'type:' in Notification entity:" rg -n -C1 "^\s*type\s*:" "$notif_file" || echo "❗ No 'type:' property found" # Show any NotificationType enum references echo -e "\nNotificationType usages in Notification entity:" rg -n "NotificationType" "$notif_file" || echo "❗ No NotificationType usages found"Length of output: 1139
Comparison logic validated: no issues
Bothpref.notificationTypeandnotification.typeare defined as theNotificationTypeenum, so the strict comparison is safe and consistent.• src/notifications/entities/notification-preference.entity.ts:
notificationType: NotificationType
• src/notifications/entities/notification.entity.ts:type: NotificationTypesrc/notifications/entities/notification.entity.ts (3)
1-1: Good use of absolute imports!Using the absolute path alias improves maintainability and makes imports more consistent across the codebase.
41-46: Good addition of WEEKLY digest frequency!The addition of the weekly digest option provides users with more flexibility in notification delivery preferences.
81-84: Well-implemented notification read tracking!The
readfield with a default value offalseis a standard and effective way to track notification read status.src/scheduler/utils/timezone.helper.ts (2)
7-33: Excellent improvements to type safety and validation!The changes enhance the robustness of timezone handling:
- The
SlotWithTimezonetype ensures timezone is always present- Runtime validation prevents undefined date errors
- Explicit return type clarifies that dates are converted to Date objects
- Proper UTC conversion using Luxon
35-45: Clean refactoring of timezone conversion!The simplified implementation removes unnecessary UTC zone setting and clearly indicates the return type includes
displayTimezone.src/dispute/dispute.service.ts (3)
72-75: Good addition of pessimistic locking!Using pessimistic write lock prevents race conditions when multiple requests try to update the same dispute simultaneously.
92-94: Improved randomization logic!The two-step randomization (first selecting 2x candidates, then shuffling and taking the required number) provides better distribution than the previous approach.
256-261: Good use of query builder for flexible filtering!Using query builder instead of simple find allows for more flexible query construction and potential future extensions.
src/notifications/notifications.service.ts (2)
39-58: Well-structured notification creation method!The new
createNotificationmethod provides excellent type safety with the typed parameters and proper default values.
229-259: Excellent preference-based notification handling!The
triggerNotificationmethod properly checks user preferences before creating notifications and handles digest scheduling appropriately.
| import { RatingsService } from '@src/rating/rating.service'; | ||
| import { ProjectService } from '@src/project/project.service'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant service imports.
Since ProjectModule and RatingsModule are imported (lines 9-10), the direct service imports for RatingsService and ProjectService are likely redundant. These services should be available through their respective modules.
-import { RatingsService } from '@src/rating/rating.service';
-import { ProjectService } from '@src/project/project.service';If these services are needed in the ReputationService, they should be injected through dependency injection rather than imported directly in the module.
📝 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.
| import { RatingsService } from '@src/rating/rating.service'; | |
| import { ProjectService } from '@src/project/project.service'; |
🤖 Prompt for AI Agents
In src/reputation/reputation.module.ts around lines 7 to 8, remove the direct
imports of RatingsService and ProjectService as they are redundant due to the
import of RatingsModule and ProjectModule on lines 9-10. Instead, ensure these
services are injected via dependency injection in the ReputationService or other
consumers, not imported directly in the module file.
| export enum Permission { | ||
| MANAGE_USERS = 'manage:users', | ||
| CREATE_PROJECT = 'create:project', | ||
| EDIT_PROJECT = 'edit:project', | ||
| DELETE_PROJECT = 'delete:project', | ||
| VIEW_PROJECT = 'view:project', | ||
| // Add more as needed | ||
| } |
There was a problem hiding this comment.
Critical: Naming conflict with existing Permission type.
This enum conflicts with the Permission type alias in src/auth/types/permission.type.ts. Both export a Permission identifier but with different structures and values:
- This enum:
MANAGE_USERS = 'manage:users',CREATE_PROJECT = 'create:project' - Type alias:
'read:users' | 'create:users' | 'read:posts'
This naming conflict will cause import ambiguity and potential runtime errors.
Consider these solutions:
- Rename this enum to
ProjectPermissionorResourcePermission - Consolidate both permission systems into a single, comprehensive enum
- Use namespacing with different module structures
-export enum Permission {
+export enum ResourcePermission {
MANAGE_USERS = 'manage:users',
CREATE_PROJECT = 'create:project',
EDIT_PROJECT = 'edit:project',
DELETE_PROJECT = 'delete:project',
VIEW_PROJECT = 'view:project',
// Add more as needed
}📝 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.
| export enum Permission { | |
| MANAGE_USERS = 'manage:users', | |
| CREATE_PROJECT = 'create:project', | |
| EDIT_PROJECT = 'edit:project', | |
| DELETE_PROJECT = 'delete:project', | |
| VIEW_PROJECT = 'view:project', | |
| // Add more as needed | |
| } | |
| export enum ResourcePermission { | |
| MANAGE_USERS = 'manage:users', | |
| CREATE_PROJECT = 'create:project', | |
| EDIT_PROJECT = 'edit:project', | |
| DELETE_PROJECT = 'delete:project', | |
| VIEW_PROJECT = 'view:project', | |
| // Add more as needed | |
| } |
🤖 Prompt for AI Agents
In src/auth/enums/permission.enum.ts lines 1 to 8, the enum named Permission
conflicts with an existing Permission type alias in
src/auth/types/permission.type.ts, causing import ambiguity. To fix this, rename
the enum to a more specific name like ProjectPermission or ResourcePermission to
avoid the naming conflict. Update all references to this enum accordingly to use
the new name.
| export type Permission = | ||
| | 'read:users' | ||
| | 'create:users' | ||
| | 'update:users' | ||
| | 'delete:users' | ||
| | 'read:posts' | ||
| | 'create:posts' | ||
| | 'update:posts' | ||
| | 'delete:posts'; | ||
| // Extend this as needed |
There was a problem hiding this comment.
Critical: Naming conflict with Permission enum.
This type alias conflicts with the Permission enum in src/auth/enums/permission.enum.ts. Both export a Permission identifier with different permission schemes:
- This type: User/post CRUD operations (
'read:users','create:posts', etc.) - Enum: Resource management (
'manage:users','create:project', etc.)
This will cause import conflicts when both are used in the same codebase.
Recommended solutions:
- Rename this type to
CrudPermissionorUserPostPermission - Merge with the enum to create a unified permission system
- Use different namespaces to avoid naming collisions
-export type Permission =
+export type CrudPermission =
| 'read:users'
| 'create:users'
| 'update:users'
| 'delete:users'
| 'read:posts'
| 'create:posts'
| 'update:posts'
| 'delete:posts';📝 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.
| export type Permission = | |
| | 'read:users' | |
| | 'create:users' | |
| | 'update:users' | |
| | 'delete:users' | |
| | 'read:posts' | |
| | 'create:posts' | |
| | 'update:posts' | |
| | 'delete:posts'; | |
| // Extend this as needed | |
| export type CrudPermission = | |
| | 'read:users' | |
| | 'create:users' | |
| | 'update:users' | |
| | 'delete:users' | |
| | 'read:posts' | |
| | 'create:posts' | |
| | 'update:posts' | |
| | 'delete:posts'; | |
| // Extend this as needed |
🤖 Prompt for AI Agents
In src/auth/types/permission.type.ts lines 1 to 10, rename the exported type
alias from Permission to CrudPermission or UserPostPermission to avoid naming
conflicts with the Permission enum in src/auth/enums/permission.enum.ts. This
prevents import collisions by clearly distinguishing the type alias for
user/post CRUD permissions from the enum used for resource management
permissions.
| } catch (error: any) { | ||
| throw new HttpException( | ||
| error?.message || 'Failed to compute job fit score', | ||
| HttpStatus.BAD_REQUEST, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error typing by catching unknown instead of any.
Rather than catch (error: any), use catch (error: unknown) and then narrow:
catch (error: unknown) {
let message = 'Failed to compute job fit score';
if (error instanceof Error) {
message = error.message;
}
throw new HttpException(message, HttpStatus.BAD_REQUEST);
}This improves type safety and avoids suppressing unexpected error structures.
🤖 Prompt for AI Agents
In src/job-fit/job-fit.controller.ts around lines 13 to 17, change the catch
clause from catching `any` to catching `unknown` to improve type safety. Then,
narrow the error type inside the catch block by checking if the error is an
instance of Error before accessing its message. Use a default message if the
error is not an Error instance, and throw the HttpException with the determined
message and HttpStatus.BAD_REQUEST.
| if (mentions && mentions.length > 0) { | ||
| for (const mentionedUserId of mentions) { | ||
| await this.notificationsService.createMentionNotification(mentionedUserId, savedComment.id); | ||
| await this.notificationsService.createMessageNotification(mentionedUserId, savedComment.id); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the new notification method handles mention functionality correctly.
The method call has been updated from createMentionNotification to createMessageNotification. Please ensure that:
- The new method properly handles mention-specific functionality
- The notification type is correctly set for user mentions
- The method signature matches the service interface
🏁 Script executed:
#!/bin/bash
# Description: Verify the createMessageNotification method signature and implementation
# Expected: Method should exist and handle mention notifications appropriately
# Search for the createMessageNotification method definition
ast-grep --pattern 'createMessageNotification($_, $_) {
$$$
}'
# Also search for any mention-related logic in notifications service
rg -A 10 "createMessageNotification" src/notifications/Length of output: 889
Incorrect notification type for mentions
The call to createMessageNotification(mentionedUserId, savedComment.id) will always generate a MESSAGE notification (with content: { message: savedComment.id }) rather than a MENTION notification. Mentions should use NotificationType.MENTION and include the comment ID in the payload.
• Location: src/comment/comment.service.ts:41
• Current behaviour:
await this.notificationsService.createMessageNotification(
mentionedUserId,
savedComment.id,
);• Issues:
createMessageNotificationsignature innotifications.service.tsunconditionally setstype: NotificationType.MESSAGE.- Passing
savedComment.idas the “message” string loses the distinction between a generic message and a mention.
Suggested fix:
Either restore a mention‐specific helper or call the generic createNotification API directly with the correct type:
- await this.notificationsService.createMessageNotification(
- mentionedUserId,
- savedComment.id,
- );
+ await this.notificationsService.createNotification({
+ userId: mentionedUserId,
+ type: NotificationType.MENTION,
+ content: { commentId: savedComment.id },
+ deliveryChannel: DeliveryChannel.PUSH,
+ priority: 0,
+ });If a createMentionNotification helper still exists, swap back to that instead:
- await this.notificationsService.createMessageNotification(mentionedUserId, savedComment.id);
+ await this.notificationsService.createMentionNotification(mentionedUserId, savedComment.id);🤖 Prompt for AI Agents
In src/comment/comment.service.ts at line 41, the call to
createMessageNotification incorrectly generates a MESSAGE type notification for
mentions. To fix this, either revert to using a createMentionNotification helper
if it exists, or replace the call with a direct call to the generic
createNotification method specifying NotificationType.MENTION and including the
comment ID in the payload. This ensures the notification type is correctly set
for user mentions and the method signature matches the service interface.
| export enum NotificationType { | ||
| JOB_MATCH = 'job_match', | ||
| MESSAGE = 'message', | ||
| ENDORSEMENT = 'endorsement', | ||
| GENERAL = "general", | ||
| MENTION = "mention", | ||
| DISPUTE_ASSIGNMENT = 'DISPUTE_ASSIGNMENT', | ||
| DISPUTE_RESOLUTION = 'DISPUTE_RESOLUTION', | ||
| REQUEST_DECLINED = 'request_declined', | ||
| REQUEST_ACCEPTED = 'request_accepted', | ||
| REQUEST_COMPLETED = 'request_completed', | ||
| JOB_POSTED = "job_posted", | ||
| JOB_UPDATED = "job_updated", | ||
| JOB_DELETED = "job_deleted", | ||
| NEW_JOB_POSTING = "new_job_posting", | ||
| JOB_POSTING_UPDATED = "job_posting_updated", | ||
| JOB_POSTING_REMOVED = "job_posting_removed", | ||
| RECOMMENDATION_APPROVED = "recommendation_approved", | ||
| RECOMMENDATION_REJECTED = "recommendation_rejected", | ||
| NEW_RECOMMENDATION = "new_recommendation", | ||
| REPORT_UPDATE = "report_update", | ||
|
|
||
| } |
There was a problem hiding this comment.
Fix inconsistent naming conventions in NotificationType enum.
The enum values follow different naming conventions:
- Snake case:
job_match,request_declined - Upper snake case:
DISPUTE_ASSIGNMENT,DISPUTE_RESOLUTION - Lowercase:
general,mention
This inconsistency could lead to confusion and maintenance issues.
Apply this diff to standardize all enum values to upper snake case:
export enum NotificationType {
- JOB_MATCH = 'job_match',
- MESSAGE = 'message',
- ENDORSEMENT = 'endorsement',
- GENERAL = "general",
- MENTION = "mention",
+ JOB_MATCH = 'JOB_MATCH',
+ MESSAGE = 'MESSAGE',
+ ENDORSEMENT = 'ENDORSEMENT',
+ GENERAL = "GENERAL",
+ MENTION = "MENTION",
DISPUTE_ASSIGNMENT = 'DISPUTE_ASSIGNMENT',
DISPUTE_RESOLUTION = 'DISPUTE_RESOLUTION',
- REQUEST_DECLINED = 'request_declined',
- REQUEST_ACCEPTED = 'request_accepted',
- REQUEST_COMPLETED = 'request_completed',
- JOB_POSTED = "job_posted",
- JOB_UPDATED = "job_updated",
- JOB_DELETED = "job_deleted",
- NEW_JOB_POSTING = "new_job_posting",
- JOB_POSTING_UPDATED = "job_posting_updated",
- JOB_POSTING_REMOVED = "job_posting_removed",
- RECOMMENDATION_APPROVED = "recommendation_approved",
- RECOMMENDATION_REJECTED = "recommendation_rejected",
- NEW_RECOMMENDATION = "new_recommendation",
- REPORT_UPDATE = "report_update",
+ REQUEST_DECLINED = 'REQUEST_DECLINED',
+ REQUEST_ACCEPTED = 'REQUEST_ACCEPTED',
+ REQUEST_COMPLETED = 'REQUEST_COMPLETED',
+ JOB_POSTED = "JOB_POSTED",
+ JOB_UPDATED = "JOB_UPDATED",
+ JOB_DELETED = "JOB_DELETED",
+ NEW_JOB_POSTING = "NEW_JOB_POSTING",
+ JOB_POSTING_UPDATED = "JOB_POSTING_UPDATED",
+ JOB_POSTING_REMOVED = "JOB_POSTING_REMOVED",
+ RECOMMENDATION_APPROVED = "RECOMMENDATION_APPROVED",
+ RECOMMENDATION_REJECTED = "RECOMMENDATION_REJECTED",
+ NEW_RECOMMENDATION = "NEW_RECOMMENDATION",
+ REPORT_UPDATE = "REPORT_UPDATE",
}📝 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.
| export enum NotificationType { | |
| JOB_MATCH = 'job_match', | |
| MESSAGE = 'message', | |
| ENDORSEMENT = 'endorsement', | |
| GENERAL = "general", | |
| MENTION = "mention", | |
| DISPUTE_ASSIGNMENT = 'DISPUTE_ASSIGNMENT', | |
| DISPUTE_RESOLUTION = 'DISPUTE_RESOLUTION', | |
| REQUEST_DECLINED = 'request_declined', | |
| REQUEST_ACCEPTED = 'request_accepted', | |
| REQUEST_COMPLETED = 'request_completed', | |
| JOB_POSTED = "job_posted", | |
| JOB_UPDATED = "job_updated", | |
| JOB_DELETED = "job_deleted", | |
| NEW_JOB_POSTING = "new_job_posting", | |
| JOB_POSTING_UPDATED = "job_posting_updated", | |
| JOB_POSTING_REMOVED = "job_posting_removed", | |
| RECOMMENDATION_APPROVED = "recommendation_approved", | |
| RECOMMENDATION_REJECTED = "recommendation_rejected", | |
| NEW_RECOMMENDATION = "new_recommendation", | |
| REPORT_UPDATE = "report_update", | |
| } | |
| export enum NotificationType { | |
| JOB_MATCH = 'JOB_MATCH', | |
| MESSAGE = 'MESSAGE', | |
| ENDORSEMENT = 'ENDORSEMENT', | |
| GENERAL = "GENERAL", | |
| MENTION = "MENTION", | |
| DISPUTE_ASSIGNMENT = 'DISPUTE_ASSIGNMENT', | |
| DISPUTE_RESOLUTION = 'DISPUTE_RESOLUTION', | |
| REQUEST_DECLINED = 'REQUEST_DECLINED', | |
| REQUEST_ACCEPTED = "REQUEST_ACCEPTED", | |
| REQUEST_COMPLETED = "REQUEST_COMPLETED", | |
| JOB_POSTED = "JOB_POSTED", | |
| JOB_UPDATED = "JOB_UPDATED", | |
| JOB_DELETED = "JOB_DELETED", | |
| NEW_JOB_POSTING = "NEW_JOB_POSTING", | |
| JOB_POSTING_UPDATED = "JOB_POSTING_UPDATED", | |
| JOB_POSTING_REMOVED = "JOB_POSTING_REMOVED", | |
| RECOMMENDATION_APPROVED = "RECOMMENDATION_APPROVED", | |
| RECOMMENDATION_REJECTED = "RECOMMENDATION_REJECTED", | |
| NEW_RECOMMENDATION = "NEW_RECOMMENDATION", | |
| REPORT_UPDATE = "REPORT_UPDATE", | |
| } |
🤖 Prompt for AI Agents
In src/notifications/entities/notification.entity.ts from lines 4 to 26, the
NotificationType enum values use inconsistent naming conventions including snake
case, uppercase snake case, and lowercase. To fix this, convert all enum values
to uppercase snake case format consistently, for example, change 'job_match' to
'JOB_MATCH', 'general' to 'GENERAL', and 'request_declined' to
'REQUEST_DECLINED'. This standardization will improve code clarity and
maintainability.
| import { Injectable, NotFoundException, BadRequestException } from '@nestjs/common'; | ||
| import { InjectRepository } from '@nestjs/typeorm'; | ||
| import { Repository, In, DataSource } from 'typeorm'; | ||
| import { Repository, DataSource } from 'typeorm'; |
There was a problem hiding this comment.
Add back the missing QueryRunner import.
QueryRunner was removed from the imports but is still used throughout the service (e.g., line 61, 62, 191).
Apply this diff to fix the import:
-import { Repository, DataSource } from 'typeorm';
+import { Repository, DataSource, QueryRunner } from 'typeorm';📝 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.
| import { Repository, DataSource } from 'typeorm'; | |
| -import { Repository, DataSource } from 'typeorm'; | |
| +import { Repository, DataSource, QueryRunner } from 'typeorm'; |
🤖 Prompt for AI Agents
In src/dispute/dispute.service.ts at line 3, the import statement is missing
QueryRunner, which is used later in the file. Add QueryRunner to the import list
from 'typeorm' alongside Repository and DataSource to restore the missing import
and prevent runtime errors.
| await this.notificationsService.createNotification({ | ||
| userId: dispute.creatorId, | ||
| type: 'DISPUTE_RESOLVED', | ||
| message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`, | ||
| data: { disputeId: dispute.id, outcome: dispute.outcome }, | ||
| }, queryRunner?.manager); | ||
| type: NotificationType.DISPUTE_ASSIGNMENT, | ||
| content: { | ||
| message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`, | ||
| disputeId: dispute.id, | ||
| outcome: dispute.outcome, | ||
| }, | ||
| priority: 10, | ||
| deliveryChannel: DeliveryChannel.PUSH, | ||
| }); |
There was a problem hiding this comment.
Use the correct notification type for dispute resolution.
The notification is for dispute resolution but uses DISPUTE_ASSIGNMENT type instead of DISPUTE_RESOLUTION.
Apply this diff to fix the notification type:
await this.notificationsService.createNotification({
userId: dispute.creatorId,
- type: NotificationType.DISPUTE_ASSIGNMENT,
+ type: NotificationType.DISPUTE_RESOLUTION,
content: {
message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`,
disputeId: dispute.id,
outcome: dispute.outcome,
},
priority: 10,
deliveryChannel: DeliveryChannel.PUSH,
});📝 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.
| await this.notificationsService.createNotification({ | |
| userId: dispute.creatorId, | |
| type: 'DISPUTE_RESOLVED', | |
| message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`, | |
| data: { disputeId: dispute.id, outcome: dispute.outcome }, | |
| }, queryRunner?.manager); | |
| type: NotificationType.DISPUTE_ASSIGNMENT, | |
| content: { | |
| message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`, | |
| disputeId: dispute.id, | |
| outcome: dispute.outcome, | |
| }, | |
| priority: 10, | |
| deliveryChannel: DeliveryChannel.PUSH, | |
| }); | |
| await this.notificationsService.createNotification({ | |
| userId: dispute.creatorId, | |
| type: NotificationType.DISPUTE_RESOLUTION, | |
| content: { | |
| message: `Dispute Resolution: Your dispute "${dispute.title}" has been resolved with outcome: ${dispute.outcome}`, | |
| disputeId: dispute.id, | |
| outcome: dispute.outcome, | |
| }, | |
| priority: 10, | |
| deliveryChannel: DeliveryChannel.PUSH, | |
| }); |
🤖 Prompt for AI Agents
In src/dispute/dispute.service.ts around lines 231 to 241, the notification type
used is DISPUTE_ASSIGNMENT, which is incorrect for dispute resolution
notifications. Change the notification type to DISPUTE_RESOLUTION to accurately
reflect the nature of the notification.
| public async broadcastNotification( | ||
| type: NotificationType, | ||
| message: string, | ||
| data?: any, | ||
| ) { | ||
| const notification = this.notificationRepository.create({ | ||
| userId: 0, // Special ID for broadcast | ||
| type, | ||
| message, | ||
| }) | ||
| await this.notificationRepository.save(notification) | ||
|
|
||
| // Emit the SSE event without a specific userId (broadcast) | ||
| userId: '0', // special ID for broadcast as string | ||
| type, // make sure this is a valid NotificationType enum value | ||
| content: { | ||
| message, | ||
| ...data, | ||
| }, | ||
| status: NotificationStatus.PENDING, | ||
| deliveryChannel: DeliveryChannel.PUSH, | ||
| read: false, | ||
| priority: 0, | ||
| retryCount: 0, | ||
| }); | ||
|
|
||
| await this.notificationRepository.save(notification); | ||
|
|
||
| this.emitSseNotification(type, { | ||
| id: notification.id, | ||
| message, | ||
| type, | ||
| createdAt: notification.createdAt, | ||
| ...data, | ||
| }) | ||
|
|
||
| return notification | ||
| }); | ||
| return notification; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document or extract the magic broadcast userId.
The hardcoded '0' as a special broadcast userId should be documented or extracted as a constant for better maintainability.
Add a constant at the top of the class:
private static readonly BROADCAST_USER_ID = '0';Then update the method:
const notification = this.notificationRepository.create({
- userId: '0', // special ID for broadcast as string
+ userId: NotificationsService.BROADCAST_USER_ID,
type, // make sure this is a valid NotificationType enum value🤖 Prompt for AI Agents
In src/notifications/notifications.service.ts around lines 141 to 171, the
broadcastNotification method uses a hardcoded string '0' as a special broadcast
userId. Extract this magic value into a clearly named private static constant at
the top of the class, for example BROADCAST_USER_ID, and replace the hardcoded
'0' in the method with this constant. This improves maintainability and clarity.
| private calculateDigestDelay(frequency: DigestFrequency): number { | ||
| const now = new Date(); | ||
| let nextRun: Date; | ||
|
|
||
| let delay = 0; | ||
| switch (frequency) { | ||
| case DigestFrequency.HOURLY: | ||
| nextRun = new Date(now.getFullYear(), now.getMonth(), now.getDate(), now.getHours() + 1); | ||
| delay = 60 * 60 * 1000 - (now.getMinutes() * 60 + now.getSeconds()) * 1000; | ||
| break; | ||
| case DigestFrequency.DAILY: | ||
| nextRun = new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1); | ||
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1).getTime() - now.getTime(); | ||
| break; | ||
| case DigestFrequency.WEEKLY: | ||
| const daysUntilNextWeek = (7 - now.getDay()) || 7; | ||
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + daysUntilNextWeek).getTime() - now.getTime(); | ||
| break; | ||
| default: | ||
| throw new Error(`Unsupported digest frequency: ${frequency}`); | ||
| delay = 0; | ||
| } | ||
|
|
||
| return nextRun.getTime() - now.getTime(); | ||
| return delay; | ||
| } |
There was a problem hiding this comment.
Fix the switch declaration scope issue.
The static analysis correctly identifies that the daysUntilNextWeek variable declaration should be wrapped in a block to prevent access from other switch cases.
Apply this diff to fix the scope issue:
case DigestFrequency.WEEKLY:
+ {
const daysUntilNextWeek = (7 - now.getDay()) || 7;
delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + daysUntilNextWeek).getTime() - now.getTime();
+ }
break;📝 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.
| private calculateDigestDelay(frequency: DigestFrequency): number { | |
| const now = new Date(); | |
| let nextRun: Date; | |
| let delay = 0; | |
| switch (frequency) { | |
| case DigestFrequency.HOURLY: | |
| nextRun = new Date(now.getFullYear(), now.getMonth(), now.getDate(), now.getHours() + 1); | |
| delay = 60 * 60 * 1000 - (now.getMinutes() * 60 + now.getSeconds()) * 1000; | |
| break; | |
| case DigestFrequency.DAILY: | |
| nextRun = new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1); | |
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1).getTime() - now.getTime(); | |
| break; | |
| case DigestFrequency.WEEKLY: | |
| const daysUntilNextWeek = (7 - now.getDay()) || 7; | |
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + daysUntilNextWeek).getTime() - now.getTime(); | |
| break; | |
| default: | |
| throw new Error(`Unsupported digest frequency: ${frequency}`); | |
| delay = 0; | |
| } | |
| return nextRun.getTime() - now.getTime(); | |
| return delay; | |
| } | |
| private calculateDigestDelay(frequency: DigestFrequency): number { | |
| const now = new Date(); | |
| let delay = 0; | |
| switch (frequency) { | |
| case DigestFrequency.HOURLY: | |
| delay = 60 * 60 * 1000 - (now.getMinutes() * 60 + now.getSeconds()) * 1000; | |
| break; | |
| case DigestFrequency.DAILY: | |
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1).getTime() - now.getTime(); | |
| break; | |
| case DigestFrequency.WEEKLY: { | |
| const daysUntilNextWeek = (7 - now.getDay()) || 7; | |
| delay = new Date(now.getFullYear(), now.getMonth(), now.getDate() + daysUntilNextWeek).getTime() - now.getTime(); | |
| } | |
| break; | |
| default: | |
| delay = 0; | |
| } | |
| return delay; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 316-316: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/notifications/notifications.service.ts around lines 305 to 323, the
variable daysUntilNextWeek declared inside the WEEKLY case of the switch
statement is not scoped properly, which can cause issues. To fix this, wrap the
WEEKLY case code block in curly braces {} to create a new block scope, ensuring
daysUntilNextWeek is only accessible within that case.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Refactor