Skip to content

NUMBER_OF_CHILDREN_IN_YOUR_GROUP fields add in kafka consumer service#32

Open
sourav-dev-hub wants to merge 1 commit into
tekdi:mainfrom
sourav-dev-hub:user_migration_new
Open

NUMBER_OF_CHILDREN_IN_YOUR_GROUP fields add in kafka consumer service#32
sourav-dev-hub wants to merge 1 commit into
tekdi:mainfrom
sourav-dev-hub:user_migration_new

Conversation

@sourav-dev-hub
Copy link
Copy Markdown

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 6, 2025

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for tracking the number of children users work with in user profiles.
  • Chores

    • Updated user data migration processes to improve data consistency and accuracy.

Walkthrough

The PR adds two new optional columns to the User entity (userNumOfChildrenWorkingWith and userCustomField), introduces transformation logic to map custom field data to userNumOfChildrenWorkingWith, and includes a database migration script to update UserPhoneType records from source to destination databases.

Changes

Cohort / File(s) Summary
Database Schema Extension
src/entities/user.entity.ts
Added two new nullable columns: userNumOfChildrenWorkingWith (text) and userCustomField (jsonb) to the User entity.
User Transformation Logic
src/constants/transformation/transform-service.ts
Added userNumOfChildrenWorkingWith field mapping in TransformService.transformUserData, sourced from custom field label NUMBER_OF_CHILDREN_IN_YOUR_GROUP.
User Phone Type Migration
shiksha-migration/user-update-phone-type-migration.js
New Node.js migration script that updates UserPhoneType for destination users with null values by fetching corresponding data from source database, with comprehensive error handling and migration statistics logging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The migration script requires careful review of database connection handling, particularly the try/catch/finally blocks and null-value derivation logic in getFirstValue().
  • Verify that the migration script's fieldId constant correctly references the UserPhoneType field and that error cases (skipped records) are appropriately logged.
  • Confirm the schema additions include proper null-handling in dependent code paths.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author, making it impossible to assess if there is any explanation of the changes. Add a description explaining the purpose of the migration script, new entity fields, and their relationship to the kafka consumer service updates.
Title check ⚠️ Warning The PR title mentions 'NUMBER_OF_CHILDREN_IN_YOUR_GROUP fields' which is covered in the transform-service changes, but omits the significant user phone type migration script and database schema updates, making it incomplete. Update title to capture the main changes comprehensively, e.g., 'Add phone type migration and user custom fields support' or 'Add user phone type migration and child count field mapping'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Nov 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
15.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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/constants/transformation/transform-service.ts (1)

78-131: Consider populating the userCustomField column.

The User entity now has a userCustomField column (JSONB type) that could store the entire custom fields object for future extensibility. Currently, specific fields are extracted individually, but storing the complete custom fields data would:

  1. Preserve all custom field data without requiring schema changes for new fields
  2. Provide a fallback for fields not yet mapped
  3. Enable easier debugging and auditing

Consider adding this mapping after line 125:

        userNumOfChildrenWorkingWith: extractCustomField('NUMBER_OF_CHILDREN_IN_YOUR_GROUP'),
+       userCustomField: data.customFields || null,

        // Boolean fields
shiksha-migration/user-update-phone-type-migration.js (2)

37-83: Consider adding transaction support for atomicity.

The migration updates users one-by-one without transaction boundaries. If the migration fails partway through, it will leave the database in a partially migrated state.

Consider wrapping the update loop in a transaction:

try {
  await destClient.query('BEGIN');
  
  for (const user of usersWithNullPhoneType) {
    const result = await updatePhoneTypeForUser(sourceClient, destClient, user.UserID);
    // ... tracking logic
  }
  
  await destClient.query('COMMIT');
  console.log('[PHONE TYPE UPDATE] Transaction committed successfully');
} catch (error) {
  await destClient.query('ROLLBACK');
  console.error('[PHONE TYPE UPDATE] Transaction rolled back due to error:', error);
  throw error;
}

Alternatively, if you prefer per-user atomicity with resilience to partial failures, the current approach is acceptable—just document that the migration can be safely re-run.


9-9: Consider adding a comment documenting the field mapping for consistency.

The UUID da594b2e-c645-4a96-af15-6e2d24587c9a is verified as consistent across all migration files (user-migration.js and user-migration-c2c.js also use this same value). The field ID is already documented in related migration files as mapping to 'UserPhoneType' / 'phone_type_accessible'. To improve maintainability, add a similar inline comment to line 9 (e.g., // phone_type_accessible) to match the documentation pattern in the other migration files.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8fa955a and 0ab4f47.

📒 Files selected for processing (3)
  • shiksha-migration/user-update-phone-type-migration.js (1 hunks)
  • src/constants/transformation/transform-service.ts (1 hunks)
  • src/entities/user.entity.ts (1 hunks)
🔇 Additional comments (5)
src/entities/user.entity.ts (2)

235-236: LGTM! Field properly mapped in transformation service.

The userNumOfChildrenWorkingWith field is correctly defined and is populated in src/constants/transformation/transform-service.ts at line 125 from the custom field NUMBER_OF_CHILDREN_IN_YOUR_GROUP.


238-239: Confirm the intended use of userCustomField column and populate if needed.

The userCustomField column (entity line 239) is not being populated in the transformation service. The transformUserData method extracts specific custom fields by label and maps them to individual entity properties, but does not store custom fields in the userCustomField JSONB column.

Either:

  1. Confirm this is intentional (reserved for future use) and document it, or
  2. Add the mapping to populate it with custom fields data in the transformation service

If option 2, add to the transformedData object:

userCustomField: data.customFields ? Object.fromEntries(data.customFields.map((f: any) => [f.label, f.selectedValues])) : null,
src/constants/transformation/transform-service.ts (1)

125-125: LGTM! Transformation logic correctly maps the new field.

The userNumOfChildrenWorkingWith field is properly extracted from the NUMBER_OF_CHILDREN_IN_YOUR_GROUP custom field using the established extractCustomField pattern. This aligns with the new column added to the User entity.

shiksha-migration/user-update-phone-type-migration.js (2)

1-32: LGTM! Well-structured helper function and constants.

The module structure, constants, and getFirstValue helper function are well-implemented:

  • Handles multiple input formats (string, array, null) gracefully
  • Returns null for empty/invalid inputs
  • Clear documentation

184-196: LGTM! Proper module pattern for CLI and programmatic use.

The script correctly:

  • Checks if it's run directly vs imported as a module
  • Exports the main function for reuse
  • Handles errors appropriately with process.exit(1)

@sourav-dev-hub sourav-dev-hub changed the title User update phone type fields add in kafka consumer service NUMBER_OF_CHILDREN_IN_YOUR_GROUP fields add in kafka consumer service Nov 7, 2025
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.

1 participant