feat: opportunity rematch

[capJavert]

ENG-579

[capJavert]

@claude review

[claude]

Claude finished @capJavert's task in 1m 41s —— View job

---

Code Review Complete ✓

Todo List

• Read and understand all changed files
• Review migration structure and formatting
• Review OpportunityMatch entity changes
• Review worker implementations
• Review notification logic
• Review test coverage
• Check adherence to CLAUDE.md guidelines
• Provide comprehensive feedback

---

Summary

This PR implements an "opportunity rematch" feature that allows candidates who previously rejected a job opportunity to be re-matched when the job requirements change. The implementation adds history tracking to OpportunityMatch entities, introduces a new notification worker, and includes comprehensive test coverage.

Overall Assessment: The implementation is solid with good test coverage and proper use of transactions. I found a few issues that should be addressed:

Issues Found

1. 🔴 Missing Notification Settings for ReMatchedOpportunity

File: src/notifications/common.ts:77

The new NotificationType.ReMatchedOpportunity notification type is defined but not included in DEFAULT_NOTIFICATION_SETTINGS. This means users won't have default notification preferences for this notification type.

Fix: Add the notification type to DEFAULT_NOTIFICATION_SETTINGS around line 275:

[NotificationType.NewOpportunityMatch]: {
  email: NotificationPreferenceStatus.Subscribed,
  inApp: NotificationPreferenceStatus.Subscribed,
},
[NotificationType.ReMatchedOpportunity]: {
  email: NotificationPreferenceStatus.Subscribed,
  inApp: NotificationPreferenceStatus.Subscribed,
},

Fix this →

---

2. 🟡 TODO Comment for Email Template ID

File: src/workers/newNotificationV2Mail.ts:126

rematched_opportunity: '', // TODO opportunity-rematch

The email template ID is empty with a TODO comment. This should be tracked to ensure the email template is implemented.

Action: Ensure this is tracked in your project management system (ENG-579) and will be completed before release.

---

3. 🟡 Redundant User Query in reMatchedOpportunityNotification

File: src/workers/notifications/reMatchedOpportunityNotification.ts:33-37

const user = await con.getRepository(User).findOneBy({ id: userId });

if (!user) {
  return;
}

This user query is unnecessary because:

1. The notification system will handle missing users downstream
2. You're not using any user properties in this worker
3. It adds an extra database query

Recommendation: Remove this user check to simplify the code and reduce database queries.

---

4. 🟢 Consider Adding Migration to /format-migration Skill

File: src/migration/1770145297391-OpportunityMatchHistory.ts

The migration is well-formed and follows best practices. However, according to CLAUDE.md:

After generating a migration, use the /format-migration skill to format the SQL code for readability and consistency.

Recommendation: Run /format-migration on this migration to ensure consistent formatting, even though the current formatting looks acceptable.

---

Positive Observations ✅

1. Excellent Transaction Usage: The storeCandidateOpportunityMatch worker correctly wraps all database operations in a transaction (src/workers/opportunity/storeCandidateOpportunityMatch.ts:45-118), ensuring atomicity.

2. Proper JSONB Update Pattern: The history update correctly uses query builder with setParameter for nested JSONB values (src/workers/opportunity/storeCandidateOpportunityMatch.ts:71-83), following the best practice documented in AGENTS.md.

3. Comprehensive Test Coverage: Both new workers have excellent test coverage with edge cases:

   • Tests for missing fields
   • Tests for re-match vs new match scenarios
   • Tests for alert updates
   • Tests for history archiving

4. Good Worker Separation: The separation between candidateOpportunityMatchNotification (new matches) and reMatchedOpportunityNotification (re-matches) is clean and follows single responsibility principle.

5. Documentation Updates: The AGENTS.md updates add valuable guidance about type declarations, comments, and JSONB patterns.

6. Proper Select Statement: The workers correctly use explicit .select() to fetch only needed fields (src/workers/notifications/reMatchedOpportunityNotification.ts:25).

---

Minor Observations

1. Test Consistency: The test file updates for candidateOpportunityMatchNotification properly add fixtures for Organization, Opportunity, and DatasetLocation to ensure proper foreign key relationships.

2. Infrastructure Setup: The new worker is properly registered in .infra/common.ts with the correct topic and subscription.

3. Notification Generation: The notification title and generation logic in src/notifications/generate.ts is well-implemented and consistent with existing patterns.

---

Recommendations

Before merging:

1. 🔴 Add ReMatchedOpportunity to DEFAULT_NOTIFICATION_SETTINGS (critical)
2. 🟡 Remove redundant user query in reMatchedOpportunityNotification (nice-to-have)
3. 🟡 Track the email template TODO (ENG-579)

Optional improvements:

• Consider running /format-migration on the migration file for consistency

---

Files Reviewed

• ✅ .infra/common.ts - Worker registration
• ✅ AGENTS.md - Documentation updates
• ✅ __tests__/workers/notifications/candidateOpportunityMatchNotification.ts - Test updates
• ✅ __tests__/workers/notifications/reMatchedOpportunityNotification.ts - New tests
• ✅ __tests__/workers/opportunity/storeCandidateOpportunityMatch.ts - Test updates
• ✅ src/entity/OpportunityMatch.ts - Entity changes
• ✅ src/migration/1770145297391-OpportunityMatchHistory.ts - Migration
• ✅ src/notifications/common.ts - Notification type enum
• ✅ src/notifications/generate.ts - Notification generation
• ✅ src/workers/newNotificationV2Mail.ts - Email templates
• ✅ src/workers/notifications/candidateOpportunityMatchNotification.ts - Worker updates
• ✅ src/workers/notifications/index.ts - Worker exports
• ✅ src/workers/notifications/reMatchedOpportunityNotification.ts - New worker
• ✅ src/workers/opportunity/storeCandidateOpportunityMatch.ts - Worker updates

---

[pulumi]

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

Resource Changes

    Name                                                       Type                                  Operation
+   vpc-native-api-db-migration-a33c273d                       kubernetes:batch/v1:Job               create
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob           update
-   vpc-native-api-clickhouse-migration-be566344               kubernetes:batch/v1:Job               delete
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob           update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment         update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob           update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment         update
+   vpc-native-api-clickhouse-migration-a33c273d               kubernetes:batch/v1:Job               create
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment         update
~   vpc-native-update-tag-recommendations-cron                 kubernetes:batch/v1:CronJob           update
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob           update
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment         update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob           update
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob           update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob           update
~   vpc-native-personalized-digest-cron                        kubernetes:batch/v1:CronJob           update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob           update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob           update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob           update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment         update
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob           update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob           update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob           update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob           update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob           update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob           update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob           update
-   vpc-native-api-db-migration-be566344                       kubernetes:batch/v1:Job               delete
~   vpc-native-update-source-tag-view-cron                     kubernetes:batch/v1:CronJob           update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob           update
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob           update
~   vpc-native-deployment                                      kubernetes:apps/v1:Deployment         update
+   api-sub-api.rematched-opportunity-notification             gcp:pubsub/subscription:Subscription  create
~   vpc-native-update-tags-str-cron                            kubernetes:batch/v1:CronJob           update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob           update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob           update
~   vpc-native-update-views-cron                               kubernetes:batch/v1:CronJob           update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob           update
... and 2 other changes

[rebelchris]

Should we not only do this if you rejected for the reason if change?
I think its mote important to rematch on gondul with people it skipped?

[capJavert]

@rebelchris for now only rejected, we will expand it further later