Skip to content

Issue #000000 Fix: Elasticsearch code improved #412

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.

Sign up for GitHub

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

Open
Tusharmahajan12 wants to merge 3 commits into
base: aspire-leaders
Choose a base branch
from
Open

Issue #000000 Fix: Elasticsearch code improved #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6a1ea1
Issue #000000 Fix: Elasticsearch code improved
Tusharmahajan12 Jul 24, 2025
3fbc68b
changes pushed aug6
Tusharmahajan12 Aug 6, 2025
d0f58ed
elasticsearch changes with new LMS and assessment
Tusharmahajan12 Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 49 additions & 9 deletions src/adapters/postgres/cohortMembers-adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ import { FormSubmissionSearchDto } from 'src/forms/dto/form-submission-search.dt
import { FormsService } from 'src/forms/forms.service';
import { isElasticsearchEnabled } from 'src/common/utils/elasticsearch.util';
import { UserElasticsearchService } from 'src/elasticsearch/user-elasticsearch.service';
import { ElasticsearchDataFetcherService } from 'src/elasticsearch/elasticsearch-data-fetcher.service';
import axios from 'axios';
import {
ElasticsearchSyncService,
SyncSection,
} from '../../elasticsearch/elasticsearch-sync.service';

@Injectable()
export class PostgresCohortMembersService {
constructor(
Expand All @@ -56,7 +62,9 @@ export class PostgresCohortMembersService {
private readonly userService: PostgresUserService,
private readonly formsService: FormsService,
private readonly formSubmissionService: FormSubmissionService,
private readonly userElasticsearchService: UserElasticsearchService
private readonly userElasticsearchService: UserElasticsearchService,
private readonly elasticsearchDataFetcherService: ElasticsearchDataFetcherService,
private readonly elasticsearchSyncService: ElasticsearchSyncService
) {}

//Get cohort member
Expand Down Expand Up @@ -720,7 +728,20 @@ export class PostgresCohortMembersService {
// Update Elasticsearch with cohort member status
if (isElasticsearchEnabled()) {
try {
// First get the existing user document from Elasticsearch
// Use comprehensive sync to get complete user document including courses and assessment data
const userDocument =
await this.elasticsearchDataFetcherService.comprehensiveUserSync(
cohortMembers.userId
);

if (!userDocument) {
LoggerUtil.warn(
`User document not found for ${cohortMembers.userId}, skipping Elasticsearch update`
);
return;
}
Comment on lines +731 to +742
Copy link

@coderabbitai coderabbitai bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Fix early return issue breaking cohort member creation flow

The code returns early if the user document is not found, but this return statement doesn't provide a response object, which will break the API endpoint. Additionally, the early return prevents the cohort member from being saved in the database.

Apply this fix to handle the missing user document gracefully while allowing the database operation to continue:

 if (!userDocument) {
   LoggerUtil.warn(
     `User document not found for ${cohortMembers.userId}, skipping Elasticsearch update`
   );
-  return;
+  // Continue with the database operation even if Elasticsearch sync fails
 } else {
+  // Get the existing user document from Elasticsearch
+  const userDoc = await this.userElasticsearchService.getUser(
+    cohortMembers.userId
+  );
+  // ... rest of the Elasticsearch update logic
 }
-
-// Get the existing user document from Elasticsearch
-const userDoc = await this.userElasticsearchService.getUser(
-  cohortMembers.userId
-);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/adapters/postgres/cohortMembers-adapter.ts around lines 731 to 742, the
code currently does an early "return" when the Elasticsearch user document is
not found which aborts the API flow and prevents the cohort member from being
saved; remove that early return so the DB save continues, replace it with a
LoggerUtil.warn noting the missing document, and conditionally skip or no-op the
Elasticsearch update logic (or wrap the ES update in a try/catch) so missing
userDocument does not stop the method from completing and returning the proper
response object after the cohort member is persisted.


// Get the existing user document from Elasticsearch
const userDoc = await this.userElasticsearchService.getUser(
cohortMembers.userId
);
Expand All @@ -733,6 +754,7 @@ export class PostgresCohortMembersService {
? [...source.applications]
: [];

// Find existing application for this cohort
const appIndex = applications.findIndex(
(app) => app.cohortId === cohortMembers.cohortId
);
Expand All @@ -754,14 +776,15 @@ export class PostgresCohortMembersService {
});
}

// Now update the user document in Elasticsearch with the merged applications array
// Now update the user document in Elasticsearch with comprehensive data
const baseDoc =
typeof userDoc?._source === 'object' ? userDoc._source : {};
await this.userElasticsearchService.updateUser(
cohortMembers.userId,
{ doc: { ...baseDoc, applications } },
{ doc: userDocument }, // Use comprehensive user document
async (userId: string) => {
return await this.formSubmissionService.buildUserDocumentForElasticsearch(
// Use comprehensive sync to build the full user document for Elasticsearch
return await this.elasticsearchDataFetcherService.comprehensiveUserSync(
userId
);
}
Expand All @@ -775,6 +798,15 @@ export class PostgresCohortMembersService {
);
}
}

// Sync to Elasticsearch using centralized service
if (isElasticsearchEnabled()) {
await this.elasticsearchSyncService.syncUserToElasticsearch(
cohortMembers.userId,
{ section: SyncSection.APPLICATIONS }
);
}

return APIResponse.success(
res,
apiId,
Expand Down Expand Up @@ -1013,7 +1045,7 @@ export class PostgresCohortMembersService {
completionPercentageRanges: { min: number; max: number }[],
formId: string
): { query: string; parameters: any[]; limit: number; offset: number } {
// Build completion percentage filter conditions with proper casting
// Build completion percentage filter conditions with proper numeric casting
const completionConditions = completionPercentageRanges
.map(
(range) =>
Expand Down Expand Up @@ -1176,17 +1208,17 @@ export class PostgresCohortMembersService {
: undefined;

if (!existingApplication) {
// If application is missing, build and upsert the full user document (with progress pages)
// If application is missing, use comprehensive sync to build and upsert the full user document
const fullUserDoc =
await this.formSubmissionService.buildUserDocumentForElasticsearch(
await this.elasticsearchDataFetcherService.comprehensiveUserSync(
cohortMembershipToUpdate.userId
);
if (fullUserDoc) {
await this.userElasticsearchService.updateUser(
cohortMembershipToUpdate.userId,
{ doc: fullUserDoc },
async (userId: string) => {
return await this.formSubmissionService.buildUserDocumentForElasticsearch(
return await this.elasticsearchDataFetcherService.comprehensiveUserSync(
userId
);
}
Expand Down Expand Up @@ -1217,6 +1249,14 @@ export class PostgresCohortMembersService {
}
}

// Sync to Elasticsearch using centralized service
if (isElasticsearchEnabled()) {
await this.elasticsearchSyncService.syncUserToElasticsearch(
cohortMembershipToUpdate.userId,
{ section: SyncSection.APPLICATIONS }
);
}

// Send notification if applicable for this status only
let notifyStatuses: string[] = [];
const { status, statusReason } = cohortMembersUpdateDto;
Expand Down
117 changes: 33 additions & 84 deletions src/adapters/postgres/user-adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ import config from '../../common/config';
import { CalendarField } from 'src/fields/fieldValidators/fieldTypeClasses';
import { UserCreateSsoDto } from 'src/user/dto/user-create-sso.dto';
import { UserElasticsearchService } from '../../elasticsearch/user-elasticsearch.service';
import { ElasticsearchDataFetcherService } from '../../elasticsearch/elasticsearch-data-fetcher.service';
import { IUser } from '../../elasticsearch/interfaces/user.interface';
import { isElasticsearchEnabled } from 'src/common/utils/elasticsearch.util';
import { ElasticsearchSyncService, SyncSection } from '../../elasticsearch/elasticsearch-sync.service';

interface UpdateField {
userId: string; // Required
Expand Down Expand Up @@ -94,7 +96,9 @@ export class PostgresUserService implements IServicelocator {
private postgresAcademicYearService: PostgresAcademicYearService,
private readonly cohortAcademicYearService: CohortAcademicYearService,
private readonly authUtils: AuthUtils,
private readonly userElasticsearchService: UserElasticsearchService
private readonly userElasticsearchService: UserElasticsearchService,
private readonly elasticsearchDataFetcherService: ElasticsearchDataFetcherService,
private readonly elasticsearchSyncService: ElasticsearchSyncService,
) {
this.jwt_secret = this.configService.get<string>('RBAC_JWT_SECRET');
this.jwt_password_reset_expires_In = this.configService.get<string>(
Expand Down Expand Up @@ -1042,8 +1046,27 @@ export class PostgresUserService implements IServicelocator {

const updatedUser = await this.updateBasicUserDetails(userId, userDto);

// Sync to Elasticsearch
await this.syncUserToElasticsearch(updatedUser);
// Sync to Elasticsearch using centralized service
if (isElasticsearchEnabled()) {
// Check if user exists in Elasticsearch
const existingUser = await this.userElasticsearchService.getUser(userId);

if (!existingUser) {
// User doesn't exist in Elasticsearch, fetch all data
LoggerUtil.log(`User ${userId} not found in Elasticsearch, fetching all data from database`, apiId);
await this.elasticsearchSyncService.syncUserToElasticsearch(
updatedUser.userId,
{ section: SyncSection.ALL }
);
} else {
// User exists, only update profile section
LoggerUtil.log(`User ${userId} exists in Elasticsearch, updating profile section only`, apiId);
await this.elasticsearchSyncService.syncUserToElasticsearch(
updatedUser.userId,
{ section: SyncSection.PROFILE }
);
}
}

return await APIResponse.success(
response,
Expand Down Expand Up @@ -1573,7 +1596,7 @@ export class PostgresUserService implements IServicelocator {
customFields: elasticCustomFields,
},
applications: [],
courses: [],
// Removed root-level courses field as requested
createdAt: result.createdAt
? result.createdAt.toISOString()
: new Date().toISOString(),
Expand Down Expand Up @@ -2783,90 +2806,16 @@ export class PostgresUserService implements IServicelocator {
}

/**
* Sync user profile to Elasticsearch.
* This will upsert (update or create) the user document in Elasticsearch.
* If the document is missing, it will fetch the user from the database and create it.
* Sync user profile to Elasticsearch using centralized service.
* This method is now deprecated in favor of the centralized service.
* @deprecated Use elasticsearchSyncService.syncUserToElasticsearch instead
*/
private async syncUserToElasticsearch(user: User) {
try {
const customFields = await this.getFilteredCustomFields(user.userId);

let formattedDob: string | null = null;
if (user.dob instanceof Date) {
formattedDob = user.dob.toISOString();
} else if (typeof user.dob === 'string') {
formattedDob = user.dob;
}
// Prepare the profile data
const profile = {
userId: user.userId,
username: user.username,
firstName: user.firstName,
lastName: user.lastName,
middleName: user.middleName || '',
email: user.email || '',
mobile: user.mobile?.toString() || '',
mobile_country_code: user.mobile_country_code || '',
dob: formattedDob,
country: user.country,
gender: user.gender,
address: user.address || '',
district: user.district || '',
state: user.state || '',
pincode: user.pincode || '',
status: user.status,
customFields, // Now filtered to exclude form schema fields
};

// Upsert (update or create) the user profile in Elasticsearch
if (isElasticsearchEnabled()) {
await this.userElasticsearchService.updateUserProfile(
user.userId,
profile,
async (userId: string) => {
// Fetch the latest user from the database for upsert
const dbUser = await this.usersRepository.findOne({
where: { userId },
});
if (!dbUser) return null;
const customFields = await this.getFilteredCustomFields(userId);
let formattedDob: string | null = null;
if (dbUser.dob instanceof Date) {
formattedDob = dbUser.dob.toISOString();
} else if (typeof dbUser.dob === 'string') {
formattedDob = dbUser.dob;
}
return {
userId: dbUser.userId,
profile: {
userId: dbUser.userId,
username: dbUser.username,
firstName: dbUser.firstName,
lastName: dbUser.lastName,
middleName: dbUser.middleName || '',
email: dbUser.email || '',
mobile: dbUser.mobile?.toString() || '',
mobile_country_code: dbUser.mobile_country_code || '',
dob: formattedDob,
gender: dbUser.gender,
country: dbUser.country || '',
address: dbUser.address || '',
district: dbUser.district || '',
state: dbUser.state || '',
pincode: dbUser.pincode || '',
status: dbUser.status,
customFields,
},
applications: [],
courses: [],
createdAt: dbUser.createdAt
? dbUser.createdAt.toISOString()
: new Date().toISOString(),
updatedAt: dbUser.updatedAt
? dbUser.updatedAt.toISOString()
: new Date().toISOString(),
};
}
await this.elasticsearchSyncService.syncUserToElasticsearch(
user.userId,
{ section: SyncSection.ALL }
);
}
} catch (error) {
Expand Down
9 changes: 9 additions & 0 deletions src/app.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ export class AppController {
return this.appService.getHello();
}

@Get("health")
getHealth(): object {
return {
status: "healthy",
timestamp: new Date().toISOString(),
service: "user-microservice",
};
}

@Get("files/:fileName")
seeUploadedFile(@Param("fileName") fileName: string, @Res() res) {
return res.sendFile(fileName, { root: "./uploads" });
Expand Down
2 changes: 2 additions & 0 deletions src/app.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { RbacModule } from './rbac/rbac.module';
import { AssignTenantModule } from './userTenantMapping/user-tenant-mapping.module';
import { FormsModule } from './forms/forms.module';
import { HttpService } from '@utils/http-service';
import { LMSService } from './common/services/lms.service';
Copy link

@coderabbitai coderabbitai bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use double quotes for import path

Replace single quotes with double quotes to maintain consistency with the project's style guide.

Apply this diff:

-import { LMSService } from './common/services/lms.service';
+import { LMSService } from "./common/services/lms.service";
📝 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.

Suggested change
import { LMSService } from './common/services/lms.service';
import { LMSService } from "./common/services/lms.service";
🧰 Tools
🪛 ESLint

[error] 24-24: Replace './common/services/lms.service' with "./common/services/lms.service"

(prettier/prettier)

🤖 Prompt for AI Agents 
In src/app.module.ts around line 24, the import statement uses single quotes for
the module path; update the import to use double quotes to match the project's
style guide (replace './common/services/lms.service' wrapped in single quotes
with the same path wrapped in double quotes) and ensure the rest of file imports
remain consistent with double-quote convention.

import { TenantModule } from './tenant/tenant.module';
import { AcademicyearsModule } from './academicyears/academicyears.module';
import { CohortAcademicYearModule } from './cohortAcademicYear/cohortAcademicYear.module';
Expand Down Expand Up @@ -84,6 +85,7 @@ import { BulkImportModule } from './bulk-import/bulk-import.module';
providers: [
AppService,
HttpService,
LMSService,
{
provide: 'STORAGE_CONFIG',
useValue: storageConfig,
Expand Down
1 change: 1 addition & 0 deletions src/bulk-import/services/bulk-import.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ export class BulkImportService {
formId: formSubmission.formId,
itemId: userId,
status: formSubmission.status || 'active',
completionPercentage: 100, // Set completion percentage to 100 for bulk import
createdBy: adminId,
updatedBy: adminId,
});
Expand Down
Loading