Skip to content

Conversation

@omBratteng
Copy link
Member

Adds CDC worker to handle assigning cores to user when referral was accepted.

@omBratteng omBratteng requested review from a team and capJavert as code owners October 22, 2025 12:50
@omBratteng omBratteng requested review from AmarTrebinjac, ilasw, nensidosari, rebelchris and sshanzel and removed request for a team October 22, 2025 12:50
@omBratteng
Copy link
Member Author

@claude review this pl0x as a 1337 h3x0r

@claude
Copy link

claude bot commented Oct 22, 2025

Claude finished @omBratteng's task —— View job


Code Review Complete ✅

Todo List:

  • Read and analyze application.properties changes
  • Review njord.ts modifications
  • Examine UserReferral migration changes
  • Analyze CDC worker implementation in primary.ts
  • Provide comprehensive review feedback

Review Summary

This PR adds CDC (Change Data Capture) worker functionality to automatically award cores to users when their referral is accepted. The implementation is well-structured and follows the existing codebase patterns.

Positive Aspects

  1. Clean Architecture: Follows the existing CDC worker pattern consistently
  2. Proper Database Changes: Migration correctly adds user_referral to the CDC table list
  3. Transaction Safety: Uses database transactions properly in awardReferral function
  4. Error Handling: Includes appropriate try-catch blocks for transfer errors
  5. Consistent Naming: Uses established naming conventions throughout
  6. Type Safety: Proper TypeScript usage with correct enum comparisons

🐛 Issues Found

Critical Issue: Debug Code Left in Production

File: src/workers/cdc/primary.ts:1537

console.log('hello');

This debug statement should be removed before merging to production.

Minor Issues

  1. Hardcoded Values: The $10 USD reward amount is hardcoded in njord.ts:1117 and 1120. Consider moving to configuration:

    // Current
    value: usdToCores(10),
    flags: { note: 'Linkedin recruiter referral' }
    
    // Suggested
    value: usdToCores(remoteConfig.vars.referralReward?.linkedin || 10),
    flags: { note: remoteConfig.vars.referralNote?.linkedin || 'Linkedin recruiter referral' }
  2. Error Handling Enhancement: The referral award could benefit from additional logging for debugging:

    } catch (error) {
      logger.error(
        {
          referralId: id,
          userId: ctx.userId,
          error,
        },
        'Failed to award referral cores',
      );
      throw error;
    }

🔍 Code Analysis

Database Changes

  • Migration: Properly adds user_referral table with correct indexes and constraints
  • CDC Configuration: Correctly adds table to Debezium include list

Core Logic Flow

  1. CDC detects UserReferral status change from PendingAccepted
  2. Triggers awardReferral function with referral ID and user context
  3. Creates transaction record with system user as sender
  4. Transfers 10 USD worth of cores via Njord service
  5. Handles transfer errors appropriately

Security & Validation

  • ✅ Uses system user as sender (prevents user manipulation)
  • ✅ Validates transaction status before proceeding
  • ✅ Uses database transactions for consistency
  • ✅ Proper error handling with rollback on failures

📋 Recommendations

  1. Remove debug code before merging
  2. Consider making reward amount configurable
  3. Add structured logging for referral awards
  4. Consider adding metrics/telemetry for referral tracking

🎯 Verdict

LGTM with minor fixes - The implementation is solid and follows best practices. Only needs the debug statement removed and optionally the suggested improvements for production readiness.


Base automatically changed from feat-linkedin-redirector to feat-linkedin-button October 22, 2025 15:06
@omBratteng omBratteng force-pushed the feat-approved-referral-worker branch from 0c05c57 to a17d7b0 Compare October 22, 2025 15:07
@pulumi
Copy link

pulumi bot commented Oct 22, 2025

🍹 The Update (preview) for dailydotdev/api/prod (at 86be560) was successful.

Resource Changes

    Name                                                   Type                           Operation
~   vpc-native-update-views-cron                           kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                               kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tag-recommendations-cron             kubernetes:batch/v1:CronJob    update
-   vpc-native-api-db-migration-aacc3bb8                   kubernetes:batch/v1:Job        delete
+-  vpc-native-debezium-props                              kubernetes:core/v1:Secret      create-replacement
+-  vpc-native-debezium-deployment                         kubernetes:apps/v1:Deployment  create-replacement
~   vpc-native-ws-deployment                               kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-gifted-plus-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-history-day-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-update-trending-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment                          kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-zombie-images-cron                    kubernetes:batch/v1:CronJob    update
+   vpc-native-api-db-migration-6fba18db                   kubernetes:batch/v1:Job        create
~   vpc-native-update-source-tag-view-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-check-analytics-report-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tags-str-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-deployment                                  kubernetes:apps/v1:Deployment  update
-   vpc-native-api-clickhouse-migration-aacc3bb8           kubernetes:batch/v1:Job        delete
~   vpc-native-clean-stale-user-transactions-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                           kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-deployment              kubernetes:apps/v1:Deployment  update
~   vpc-native-validate-active-users-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron            kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-update-highlighted-views-cron               kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                         kubernetes:apps/v1:Deployment  update
+   vpc-native-api-clickhouse-migration-6fba18db           kubernetes:batch/v1:Job        create

@omBratteng omBratteng merged commit 4783732 into feat-linkedin-button Oct 23, 2025
8 checks passed
@omBratteng omBratteng deleted the feat-approved-referral-worker branch October 23, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants